-
Notifications
You must be signed in to change notification settings - Fork 97
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
Add _state suffix to //state subelements #1455
Conversation
Signed-off-by: Steve Peters <[email protected]>
Include joint_state.sdf from model_state.sdf and state.sdf for //world/joint states. Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, in each of the *_state.sdf
can we add a little more description on what state
is? For example, what is joint_state
vs. normal joint
. Alternatively, adding a more thorough comment in state.sdf
would be helpful.
Also, since this is a breaking change, shouldn't we add a method of conversion in https://github.com/gazebosim/sdformat/blob/sdformat14_14.0.0/sdf/1.11/1_10.convert ?
👍
I tried to add that in #1377, but it was difficult to handle multiple levels of nested models. My last commit in that PR (e9de659) added a failing test showing what wasn't yet working (see jenkins job result). The |
Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
I've updated the description of state elements in cd32f6e |
Yes, like @scpeters said, gz-sim hasn't been using the |
🦟 Bug fix
Part of #632, alternative to #1377
Summary
There is some overlap in the names used for elements in the
//world/state
specification and elsewhere, for example the model_state.sdf file included in state.sdf defines an element named model. This has caused challenges with aliasing in XSD (see #632 and #643), and it would be simpler if some of these elements had different names, so a_state
suffix is appended to several subelements of the//state
element. Additionally, the//joint_state
definition is moved to its own file and included fromstate.sdf
so that a//state/joint_state
element can specify the state of a//world/joint
.I made a similar attempt at renaming these state elements in #1377 trying also to support backwards compatibility with a
1_11.convert
file but only achieving partial functionality. Given that the//state
element hasn't been used in any version of gz-sim, this pull request opts for a breaking change instead of partial support for backwards compatibility.Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.