Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Joint: rename parent/child *LinkName APIs #1053

Merged
merged 4 commits into from
Jun 21, 2022
Merged

Conversation

scpeters
Copy link
Member

🦟 Bug fix

Fixes #979

Summary

Remove Link from the names of APIs for getting and setting the name of parent/child frames, since any frame can be specified, not just links.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Remove Link from the names of APIs for getting and
setting the name of parent/child frames, since any
frame can be specified, not just links.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@codecov
Copy link

codecov bot commented Jun 20, 2022

Codecov Report

Merging #1053 (a1429f7) into main (39a3f87) will decrease coverage by 0.01%.
The diff coverage is 79.66%.

❗ Current head a1429f7 differs from pull request most recent head 70023d6. Consider uploading reports for the commit 70023d6 to get more accurate results

@@            Coverage Diff             @@
##             main    #1053      +/-   ##
==========================================
- Coverage   83.47%   83.45%   -0.02%     
==========================================
  Files         148      148              
  Lines       18393    18413      +20     
==========================================
+ Hits        15353    15367      +14     
- Misses       3040     3046       +6     
Impacted Files Coverage Δ
python/src/sdf/pyJoint.cc 66.29% <50.00%> (-1.61%) ⬇️
usd/src/usd_parser/USDJoints.cc 64.17% <57.14%> (ø)
usd/src/sdf_parser/Joint.cc 58.10% <60.00%> (ø)
src/parser.cc 87.17% <75.00%> (-0.16%) ⬇️
src/FrameSemantics.cc 84.43% <100.00%> (ø)
src/Joint.cc 90.94% <100.00%> (+0.37%) ⬆️
src/ign.cc 81.29% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39a3f87...70023d6. Read the comment docs.

@scpeters
Copy link
Member Author

this will introduce some deprecation warnings in gz-physics and gz-sim after this is merged, but I'll fix that in follow-up pull requests

@@ -34,6 +36,7 @@ namespace python
void defineJoint(pybind11::object module)
{
pybind11::class_<sdf::Joint> jointModule(module, "Joint");
GZ_UTILS_WARN_IGNORE__DEPRECATED_DECLARATION
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we even include the deprecated functions in the python API since we don't have a stable release of the bindings yet?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gz-mujoco is already using the deprecated functions, so I was going to leave them in until gz-mujoco can be updated to avoid breakage. Should I just remove it and then quickly fix it in a gz-mujoco PR?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that sounds good to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed deprecated API from python binding in 70023d6

scpeters added a commit to gazebosim/gz-mujoco that referenced this pull request Jun 20, 2022
Follow-up to gazebosim/sdformat#1053.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters scpeters merged commit 03bef50 into main Jun 21, 2022
@scpeters scpeters deleted the scpeters/rename_joint_apis branch June 21, 2022 00:05
scpeters added a commit to gazebosim/gz-physics that referenced this pull request Jun 21, 2022
Follow-up to gazebosim/sdformat#1053.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters added a commit to gazebosim/gz-physics that referenced this pull request Jun 21, 2022
Follow-up to gazebosim/sdformat#1053.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters added a commit to gazebosim/gz-mujoco that referenced this pull request Jun 21, 2022
Follow-up to gazebosim/sdformat#1053.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters added a commit to gazebosim/gz-physics that referenced this pull request Jun 21, 2022
Follow-up to gazebosim/sdformat#1053.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters added a commit to gazebosim/gz-sim that referenced this pull request Jun 21, 2022
Follow-up to gazebosim/sdformat#1053.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters added a commit to gazebosim/gz-sim that referenced this pull request Jun 21, 2022
Follow-up to gazebosim/sdformat#1053.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants