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

Translate poses of nested models inside other nested models (sdf6) #596

Merged
merged 4 commits into from
Jun 30, 2021

Conversation

j-rivero
Copy link
Contributor

When addNestedModel function is called, it processes links and joints to translate the pose accordingly to parent pose. This was not done for nested models inside the SDF being processed by the function, hence they were not aware of parent pose.

This PR includes nested models in the same way that is doing for links and add a test that fails without the change.
The change was proposed by @azeey. If we are not wrong, the change is not present in newer versions of sdformat after the implementation of frame semantics.

When addNestedModel function is called, it processes links and joints
to translate the pose accordingly to parent pose. This was not done
for nested models inside the SDF being processed.

The change includes nested models in the same way that is doing for
links and add a test that fails without the change.

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@chapulina chapulina requested a review from azeey June 21, 2021 18:04
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.

this looks good to me

I tried running this test on the sdf9 branch, and it fails since it looks only at the <pose> values without considering the //pose/@relative_to attribute. I believe sdf9 is doing the right thing though, and that the test would pass if the pose values are calculated with the SemanticPose API

I believe an equivalent fix was merged to sdf10 in #399 and then back-ported to sdf9 in #424

Copy link
Collaborator

@azeey azeey 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 too! We should merge this along with #597 so it doesn't break models with joints that reference links in nested models.

test/integration/model/box_with_submodel/model.config Outdated Show resolved Hide resolved
Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>
@chapulina chapulina added this to Inbox in Core development via automation Jun 28, 2021
@chapulina chapulina moved this from Inbox to In review in Core development Jun 28, 2021
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@j-rivero j-rivero merged commit 41a2c3c into sdf6 Jun 30, 2021
@j-rivero j-rivero deleted the fix_pose_nested_models_sdf6 branch June 30, 2021 18:48
Core development automation moved this from In review to Done Jun 30, 2021
@azeey azeey mentioned this pull request Nov 5, 2021
@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-25-fortress-edifice-citadel/1343/1

@j-rivero j-rivero removed this from Done in Core development May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants