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

[dartsim] Add support for joint frame semantics #288

Merged
merged 4 commits into from
Sep 2, 2021

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Aug 25, 2021

🎉 New feature

Summary

Since joints in DART are not considered frames, this adds dart::dynamics::SimpleFrame attached to the child body of a joint and offset from the child by the pose of the joint.

Test it

Run UNIT_KinematicsFeatures_TEST

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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

@azeey azeey requested a review from scpeters August 25, 2021 20:56
@azeey azeey self-assigned this Aug 25, 2021
@azeey azeey requested a review from mxgrey as a code owner August 25, 2021 20:56
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Aug 25, 2021
Signed-off-by: Steve Peters <[email protected]>
Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

nit: rename the test KinematicsFeaturs_TEST.cc -> KinematicsFeatures_TEST.cc

otherwise looks good

@codecov
Copy link

codecov bot commented Aug 25, 2021

Codecov Report

Merging #288 (5f2bbe2) into ign-physics2 (347f082) will decrease coverage by 0.05%.
The diff coverage is 66.66%.

Impacted file tree graph

@@               Coverage Diff                @@
##           ign-physics2     #288      +/-   ##
================================================
- Coverage         83.21%   83.16%   -0.06%     
================================================
  Files               106      106              
  Lines              4040     4051      +11     
================================================
+ Hits               3362     3369       +7     
- Misses              678      682       +4     
Impacted Files Coverage Δ
dartsim/src/KinematicsFeatures.cc 72.00% <50.00%> (-11.34%) ⬇️
dartsim/src/Base.hh 99.01% <100.00%> (+0.04%) ⬆️

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 347f082...5f2bbe2. Read the comment docs.

Signed-off-by: Addisu Z. Taddese <[email protected]>
@azeey
Copy link
Contributor Author

azeey commented Aug 25, 2021

nit: rename the test KinematicsFeaturs_TEST.cc -> KinematicsFeatures_TEST.cc

Done in 96298c4

Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

out of curiosity, I ran the test with the changes to Base.hh reverted and got an exception somewhere:

[ RUN      ] KinematicsFeaturesFixture.JointFrameSemantics
unknown file: Failure
C++ exception with description "unordered_map::at: key not found" thrown in the test body.
[  FAILED  ] KinematicsFeaturesFixture.JointFrameSemantics (83 ms)

With --gtest_catch_exceptions=0, I was able to get a backtrace implicating this line in KinematicsFeatures::SelectFrame. If you think it would be easy to improve the console output in this situation and worth fixing now, then go ahead, but otherwise it's fine as is

@azeey
Copy link
Contributor Author

azeey commented Sep 2, 2021

out of curiosity, I ran the test with the changes to Base.hh reverted and got an exception somewhere:

[ RUN      ] KinematicsFeaturesFixture.JointFrameSemantics
unknown file: Failure
C++ exception with description "unordered_map::at: key not found" thrown in the test body.
[  FAILED  ] KinematicsFeaturesFixture.JointFrameSemantics (83 ms)

With --gtest_catch_exceptions=0, I was able to get a backtrace implicating this line in KinematicsFeatures::SelectFrame. If you think it would be easy to improve the console output in this situation and worth fixing now, then go ahead, but otherwise it's fine as is

I added a console message and an assertion in 5f2bbe2 following the pattern that was already there in KinematicsFeatures.cc.

@azeey azeey merged commit f1aee5a into gazebosim:ign-physics2 Sep 2, 2021
@azeey azeey deleted the joint_frames branch September 2, 2021 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants