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

Make links within nested models modifiable from GUI Client #3031

Merged
merged 5 commits into from
Aug 10, 2021
Merged

Make links within nested models modifiable from GUI Client #3031

merged 5 commits into from
Aug 10, 2021

Conversation

lihui815
Copy link
Contributor

Links within (properly) nested models can't be modified:

  • World::ModelById() calls Base::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 method Base::GetByIdRecursive() in that searches for descendent entities in the tree recursively, and use it in World::ModelById() .

Once ^^ is solved, it will expose that model and link names get reset on a modify erroneously:

  • On receiving a ~/model/modify request, the model name in the request message has StripWorldName() 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 method StripParentScopeName() to properly get the short (unscoped) requesting name.
  • On sending a ~/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
  • ^^ by itself will cause link names to be renamed erroneously on the server side, because the server is resetting short (unscoped) link names to the link name in the ~/model/modify request. Therefore, this change also to applies StripParentScopeName() to the link name in a modify request on the server side.

Also added tests for:

  • link properties for worlds with nested models

@lihui815 lihui815 changed the title Sonia/gazebo11 fn Make links within nested models modifiable from GUI Client Jun 23, 2021
@emersonknapp
Copy link
Contributor

@j-rivero @chapulina friendly ping, can you assign a reviewer for this one? thanks!

@chapulina chapulina requested a review from scpeters July 12, 2021 18:10
@chapulina chapulina requested review from iche033 and removed request for scpeters August 2, 2021 18:21
@iche033
Copy link
Contributor

iche033 commented Aug 3, 2021

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:

  • style fixes
  • moved multilink models to tthe est directory as they are only used for tests
  • fixed flaky PluginProperties() test in ModelListWidget_TEST
  • added unit test for Strip*ScopedName functions

Can you take a look and merge if they look good to you?

@lihui815
Copy link
Contributor Author

lihui815 commented Aug 9, 2021

@iche033 thanks, merged and resubmitted.

Copy link
Contributor

@iche033 iche033 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

@iche033 iche033 merged commit 88afb77 into gazebosim:gazebo11 Aug 10, 2021
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>
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 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

3 participants