This repository has been archived by the owner on Feb 3, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 498
Make links within nested models modifiable from GUI Client #3031
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@j-rivero @chapulina friendly ping, can you assign a reviewer for this one? thanks! |
I verified that modifying nested model/link properties though the ModelListWidget on the left hand pane now works as expected. I made a few changes in the iche033/gazebo11-fn branch:
Can you take a look and merge if they look good to you? |
@iche033 thanks, merged and resubmitted. |
…e setting on modify consistent
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
iche033
approved these changes
Aug 10, 2021
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.
looks good to me
iche033
added a commit
that referenced
this pull request
Aug 10, 2021
* get ids recursively * make namespacing of ~/model/modify, ~/model/info, and server-side name setting on modify consistent * add tests for included and nested models * get scoped name by stripping parent name * move test models, add strip name test, style changes Signed-off-by: Ian Chen <ichen@osrfoundation.org> Co-authored-by: Ian Chen <ichen@osrfoundation.org>
This was referenced Aug 10, 2021
iche033
added a commit
that referenced
this pull request
Aug 16, 2021
Backport #3031 - Make links within nested models modifiable from GUI Client
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Links within (properly) nested models can't be modified:
World::ModelById()
callsBase::getById()
, which is not recursive (one layer deep only), so it can't find nested models to modify on a~/model/modify
request. Create a new methodBase::GetByIdRecursive()
in that searches for descendent entities in the tree recursively, and use it inWorld::ModelById()
.Once ^^ is solved, it will expose that model and link names get reset on a modify erroneously:
~/model/modify
request, the model name in the request message hasStripWorldName()
applied to it, and then the result is used to reset the model name. This is assuming that the model is a top-element of the world and cannot be nested. Changed this to use new methodStripParentScopeName()
to properly get the short (unscoped) requesting name.~/model/modify
request, the client sends the short (unscoped) link name and the server expects the short (unscoped) link name to set the link name. However, the server then publishes the request message back as the~/model/info
for the response to the client. The client is actually expecting full (scoped) names for everything, thus this change also includes sending the full (scoped) name for the link~/model/modify
request. Therefore, this change also to appliesStripParentScopeName()
to the link name in a modify request on the server side.Also added tests for: