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] Ensure Link and Model APIs continue to work after joint creation in DART #227

Merged
merged 9 commits into from
Mar 22, 2021

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Mar 17, 2021

🎉 New feature

Summary

When a DART joint is created between two BodyNodes in DART, the child body node is moved from its original skeleton to the parent skeleton. This breaks APIs such as Model::GetLink(std::string), Model::GetLink(std::size_t) and Link::GetIndex(). This PR fixes this by keeping track of links separately from DART.

Requires

Test it

Run tests

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

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
We do this by keeping track of links separately from DART so that APIs
such as `Model::GetLink()` and `Link::GetIndex` are not affected by
BodyNode's moving from one skeleton to another when a joint is created
between different (nested) models.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

Looks pretty good! I left a few minor comments/questions.

Also, it looks like you forgot to include the issue number this PR would close in the description (the description currently says Closes #). @azeey can you add that in?

dartsim/src/Base.hh Show resolved Hide resolved
dartsim/src/Base_TEST.cc Outdated Show resolved Hide resolved
dartsim/src/JointFeatures_TEST.cc Outdated Show resolved Hide resolved
dartsim/src/EntityManagementFeatures.cc Show resolved Hide resolved
dartsim/src/EntityManagementFeatures.cc Outdated Show resolved Hide resolved
dartsim/src/EntityManagementFeatures.cc Show resolved Hide resolved
dartsim/src/EntityManagementFeatures.cc Show resolved Hide resolved
dartsim/src/SDFFeatures.cc Outdated Show resolved Hide resolved
dartsim/src/SDFFeatures_TEST.cc Outdated Show resolved Hide resolved
dartsim/src/SDFFeatures.cc Outdated Show resolved Hide resolved
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

Looks good to me! Just waiting on CI to come back as green, hopefully 🤞

@azeey azeey merged commit 109de85 into gazebosim:main Mar 22, 2021
@azeey azeey deleted the keep_track_of_links branch March 22, 2021 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection DART DART engine 🏢 edifice Ignition Edifice enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants