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] Fix joint construction errors due to link name duplication or BodyNodes moving to other skeletons #220

Merged
merged 8 commits into from
Mar 19, 2021

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Mar 4, 2021

🦟 Bug fix

Summary

This started out as a fix for link name collision error when two links in different models have the same name. b54cd35. However, I also found that when creating joints between links in different models, DART actually moves the BodyNode associated with the child link of the joint from its skeleton to the skeleton of the parent BodyNode. After the BodyNode has been moved, SDFFeatures::FindBodyNode was unable to find the child link because it looked for it in the original skeleton.

For example,

<?xml version="1.0" ?>
<sdf version="1.8">
  <world name="nested_model_world">
    <model name="parent_model">

      <model name="nested_model1">
        <link name="nested_link1"/>
      </model>
      <model name="nested_model2">
        <link name="nested_link2"/>
      </model>
      <model name="nested_model3">
        <link name="nested_link3"/>
      </model>

      <joint name="joint1" type="revolute">
        <parent>nested_model1::nested_link1</parent>
        <child>nested_model2::nested_link2</child>
      </joint>
      <joint name="joint2" type="revolute">
        <parent>nested_model2::nested_link2</parent>
        <child>nested_model3::nested_link3</child>
      </joint>

    </model>
  </world>
</sdf>

When joint1 is created, the link parent_model::nested_model2::nested_link2 is moved from it's original skeleton, parent_model::nested_model2 to parent_model::nested_model1. When it's time to create joint2, it will look for it's parent link nested_link2 in the skeleton parent_model::nested_model2, but it will not be found.

This PR adds a new map that tracks BodyNodes by their fully qualified name. I encountered the errors when testing with ign-gazebo, so I have added an alternative LoadWorld function in SDFFeatures_TEST.cc that loads an SDFormat world in a piecemeal manner like ign-gazebo.

PS: A simpler test would be to load the example world above and check if all the joints have been created. However, since the joints themselves move to a new skeleton, we need more changes in the code for parentModel->GetJointCount() to return the correct number. I'll create a separate issue for that.

Checklist

  • Signed all commits for DCO
  • Added tests
  • 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 added 3 commits March 3, 2021 16:49
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>
@azeey azeey requested a review from mxgrey as a code owner March 4, 2021 19:54
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Mar 4, 2021
@azeey azeey self-assigned this Mar 4, 2021
@azeey azeey changed the title Fix joint construction errors due to link name duplication or BodyNodes moving to other skeletons [dartsim] Fix joint construction errors due to link name duplication or BodyNodes moving to other skeletons Mar 4, 2021
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@adlarkin adlarkin self-requested a review March 10, 2021 20:18
dartsim/src/Base.hh Show resolved Hide resolved
dartsim/src/SDFFeatures.cc Outdated Show resolved Hide resolved
dartsim/src/SDFFeatures_TEST.cc Show resolved Hide resolved
dartsim/src/SDFFeatures_TEST.cc Show resolved Hide resolved
dartsim/src/SDFFeatures_TEST.cc Outdated Show resolved Hide resolved
dartsim/src/SDFFeatures_TEST.cc Outdated Show resolved Hide resolved
dartsim/src/SDFFeatures_TEST.cc Outdated Show resolved Hide resolved
dartsim/src/SDFFeatures_TEST.cc Outdated Show resolved Hide resolved
dartsim/src/SDFFeatures_TEST.cc Outdated Show resolved Hide resolved
dartsim/src/SDFFeatures_TEST.cc Outdated Show resolved Hide resolved
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
azeey added 3 commits March 19, 2021 12:45
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey
Copy link
Contributor Author

azeey commented Mar 19, 2021

Thanks for the review!

@azeey azeey merged commit 8e891e9 into gazebosim:main Mar 19, 2021
@azeey azeey deleted the dup_link_names branch March 19, 2021 19: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 🏢 edifice Ignition Edifice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants