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

Support SDFormat 1.8 composition #479

Closed
12 tasks done
azeey opened this issue Dec 9, 2020 · 13 comments · Fixed by #955
Closed
12 tasks done

Support SDFormat 1.8 composition #479

azeey opened this issue Dec 9, 2020 · 13 comments · Fixed by #955
Assignees
Labels
🏢 edifice Ignition Edifice enhancement New feature or request needs upstream release Blocked by a release of an upstream library

Comments

@azeey
Copy link
Contributor

azeey commented Dec 9, 2020

SDFormat 1.8 has introduced new features regarding model composition as detailed in the proposal. These include

  • Nested references to frames can be used in relative_to, attached_to, expressed_in attributes.
  • Frames can be used in the canonical_link attribute. The referenced frame can be nested
  • Frames can be used in the //joint/parent and //joint/child. The referenced frame can be nested
  • Models have a new attribute, placement_frame
  • Models nested via //include are no longer flattened and work as though the included model was directly nested in the parent model.

Sub tasks:

  • Support loading models nested via include
  • Support nested references to frames
  • Support canonical_link attributes that use frames This is currently not supported in SDFormat.
  • Support the use of frames for a joint's parent or child
  • Placement frames
  • SDFormat generation/saving worlds with nested models and references
    • Included nested models end up being expanded even though the option "Expand include tags" is disabled
  • Support Log recording and playback of worlds that contain nested models
  • Support the use of nested references by Sensors
  • Ensure default sensor topics are valid when nested models are used
  • Ensure AABBs take nested models into account
  • Support the use of GUI selection/transform tool with nested models
  • Support the use of nested references by ign-gazebo Systems
  • Ensure that levels work with nested models

Related

@azeey azeey added enhancement New feature or request needs upstream release Blocked by a release of an upstream library 🏢 edifice Ignition Edifice labels Dec 9, 2020
@brawner
Copy link
Contributor

brawner commented Dec 17, 2020

Related branches started by @azeey
https://github.com/ignitionrobotics/ign-gazebo/tree/composition_sdf_18
https://github.com/ignitionrobotics/ign-physics/tree/composition_sdf_18
https://github.com/osrf/sdformat/tree/split_join_name

I added a few more tests to the ign-gazebo branch, which helps make me pretty confident that nested models by <include> is working and nested references to frames is working. Also, there is already test for placement frames that use numerous include tags, so I think that is also working well.

@brawner
Copy link
Contributor

brawner commented Jan 11, 2021

Related PRs (currently WIP right now):

Notes from testing with gazebo:

Working for the most part:

  • Saving, loading worlds works fine, but when saving a world, include tags are expanded in place.
  • Log recording and playback seems to work fine
  • Using frames for parent/child of joint seems to resolve fine, but I noticed the pose element of the frame doesn't seem to be taken into account.

Not working yet:

  • Transform control of nested model. Canonical link will move, but other links will snap back to original location. If models are connected by joints, then everything seems to snap back to the original location. Works as expected for traditional models with a flat hiearchy.
  • Bounding box of child model doesn't follow child model. It is created initially with the appropriate size and position, but follows the root model's canonical link.

Still to investigate:

  • Sensors

@brawner
Copy link
Contributor

brawner commented Jan 28, 2021

After more investigation, I've prepared the above linked PRs for review. Transform control and bounding boxes are working with a few caveats that require separate PRs.

@azeey
Copy link
Contributor Author

azeey commented Mar 9, 2021

I have verified that the following work:

  • Support Log recording and playback of worlds that contain nested models
  • Support the use of nested references by Sensors
    • Not an issue because currently implemented sensors cannot reference nested entities
  • Ensure default sensor topics are valid when nested models are used
  • Ensure that levels work with nested models

@EricCousineau-TRI
Copy link
Contributor

EricCousineau-TRI commented Mar 11, 2021

Should capture workflow (broken or possible) to do SDFormat custom interface formats for Gazebo via global plugins. (gazebosim/sdformat#430)

For current Gazebo, should fail fast on unrecognized.

@azeey
Copy link
Contributor Author

azeey commented Mar 18, 2021

SDFormat generation/saving worlds with nested models and references

  • Included nested models end up being expanded even though the option "Expand include tags" is disabled

The necessary SDFormat functionality to support this is in gazebosim/sdformat#509. The implementation in ign-gazebo to ensure that nested models are not expanded will be handled after Edifice.

Support the use of nested references by ign-gazebo Systems

I think we can wait till after Edifice to handle this

@EricCousineau-TRI
Copy link
Contributor

SGTM. Will this issue remain open after Edifice is released, or will a new issue be made?

And to check, did other Gazebo stakeholders agree with this sentiment?

@chapulina
Copy link
Contributor

It's ok to complete nested model support in a minor release after 5.0.0 as long as that doesn't break API or behaviour. I think it's ok to keep this issue open until all items have been addressed.

Support the use of nested references by ign-gazebo Systems

Can you elaborate on what this task entails, @azeey? Is this about, for example, the LiftDrag plugin's link_name parameter working for links in a nested model?

@azeey
Copy link
Contributor Author

azeey commented Mar 19, 2021

Can you elaborate on what this task entails, @azeey? Is this about, for example, the LiftDrag plugin's link_name parameter working for links in a nested model?

Yeah, or DiffDrive's joint parameters if the joints were inside a nested model. The idea is to provide a convenience function to retrieve the entity from the ECM based on a name that contains ::.

@chapulina
Copy link
Contributor

Got it, sounds good to leave this as a future feature 👍

@azeey
Copy link
Contributor Author

azeey commented Jul 23, 2021

gazebosim/gz-physics#231 has been merged and released, so I've checked off "Support the use of GUI selection/transform tool with nested models"

@chapulina
Copy link
Contributor

Support the use of nested references by ign-gazebo Systems

#917 adds the gazebo::entityFromScopedName function to Util.hh that should help with this remaining task.

@azeey
Copy link
Contributor Author

azeey commented Jul 23, 2021

Support the use of nested references by ign-gazebo Systems

#917 adds the gazebo::entityFromScopedName function to Util.hh that should help with this remaining task.

Wow, I was just writing a function with that exact name. Thanks for the heads up. I'll go review that instead :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏢 edifice Ignition Edifice enhancement New feature or request needs upstream release Blocked by a release of an upstream library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants