Merging for Composition: publish proposal#74
Conversation
Signed-off-by: Eric Cousineau <eric.cousineau@tri.global>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Copied text from gazebosim/sdformat#659. Left a placeholder for an example of decomposing an existing model into separate model files using merge-include. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Add abridged version of husky with sensors on a pan-tilt gimbal and show how to decompose it. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Also discuss how proxy frame name is implementation detail. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
EricCousineau-TRI
left a comment
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @azeey and @scpeters)
composition/merge_proposal.md line 7 at r2 (raw file):
Addisu Taddese `<addisu@openrobotics.org>` Steve Peters `<scpeters@openrobotics.org>` * **Status**: Draft
nit Should we replace "Draft" with "Accepted" / "Implemented"?
composition/merge_proposal.md line 32 at r2 (raw file):
while maintaining the encapsulation provided by SDFormat 1.8. The cost of this feature is that users must take care to avoid name collisions between the entities of the models to be merged.
nit Can we provide mention of "likeness" to Python? e.g.
It may be helpful to image this as Python module imports.
With a normal include, a model include is similar to an import such as
`import my_model`. If you rename, then it is like `import my_model as renamed_model`.
With a merge include, it's effectively the same as `from my_model import *`.
(from slides on Thurs)
composition/merge_proposal.md line 176 at r2 (raw file):
The following is an abridged version of a Clearpath Husky skid-steer with sensors mounted on a pan-tilt gimbal used in the DARPA Subterranean Challenge ([MARBLE HUSKY SENSOR CONFIG 3](https://app.ignitionrobotics.org/OpenRobotics/fuel/models/MARBLE_HUSKY_SENSOR_CONFIG_3)):
nit Can you ensure that this is pinned to particular version / revision? e.g.
https://app.gazebosim.org/OpenRobotics/fuel/models/MARBLE_HUSKY_SENSOR_CONFIG_3/10
composition/merge_proposal.md line 428 at r2 (raw file):
~~~ ## Appendix
nit Should we put "(Unused)" as the body text here?
* Status: Accepted * Add namespacing analogy for python module imports * Link to revision 10 of model in Fuel * Appendix (Unused) Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters
left a comment
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @azeey and @EricCousineau-TRI)
composition/merge_proposal.md line 7 at r2 (raw file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
nit Should we replace "Draft" with "Accepted" / "Implemented"?
Done
composition/merge_proposal.md line 32 at r2 (raw file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
nit Can we provide mention of "likeness" to Python? e.g.
It may be helpful to image this as Python module imports. With a normal include, a model include is similar to an import such as `import my_model`. If you rename, then it is like `import my_model as renamed_model`. With a merge include, it's effectively the same as `from my_model import *`.(from slides on Thurs)
I paraphrased this slightly in 6c9c8d8
composition/merge_proposal.md line 176 at r2 (raw file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
nit Can you ensure that this is pinned to particular version / revision? e.g.
https://app.gazebosim.org/OpenRobotics/fuel/models/MARBLE_HUSKY_SENSOR_CONFIG_3/10
Done
composition/merge_proposal.md line 428 at r2 (raw file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
nit Should we put "(Unused)" as the body text here?
Done
EricCousineau-TRI
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @azeey)
azeey
left a comment
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @scpeters)
This is a resubmission of #55 by me to take over from @EricCousineau-TRI. I have addressed all feedback from that pull request.
This change is