CLI model info dump #893

WagnerMarcos · 2021-06-28T18:03:26Z

🎉 New feature

Related to #313

Summary

Adding a model command that is able to print:

Whole model (model pose, model links, model joints, model bounding box)
Just pose.
All links.
All joints.
A given link.
A given joint.

For this, the command can be used the following way:
ign model -m [model_name] to get the full information of the model.
ign model -m [model_name] --pose to get the pose information.
ign model -m [model_name] --links to get the links information.
ign model -m [model_name] --joints to get the joints information.
ign model -m [model_name] --link [link_name] to get a certain link information.
ign model -m [model_name] --joint [joint_name] to get a certain joint information.

Pendings in this PR:

Complete tests.

Description

This features implements a new command allowing the user to request information about a model in the running world. To get this information, a copy of the ECM is created making use of the serialized state message, which is acquired from the /state service, as shown in #859. From there, each desired component is taken from the ECM snapshot and serialized.

This command was going to be initially implemented along the gazebo command, but then it was separated into a new folder following the log structure, in order to separate the responsibilities of each command and also avoid any unwanted conflict between the subcommands.

Using this command you will be able to the following information:

Model name.
Model pose.
Model links.
- Link name.
- Link parent.
- Link mass.
- Link inertial matrix.
- Link pose.
Model joints.
- Joint name.
- Joint parent.
- Joint parent link.
- Joint child link.
Model boundary box.
- Bounding box name.
- Bounding box parent.
- Bounding box size.
- Bounding box center.
- Bounding box minimum corner.
- Bounding box maximum corner.

Test it

Run a gazebo simulation
Try some of the aforementioned commands.

Checklist

Signed all commits for DCO
Added tests
Added example and/or tutorial
Updated documentation (as needed)
Updated migration guide (as needed)
codecheck passed (See contributing)
All tests passed (See test coverage)
While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

francocipollone · 2021-06-28T18:15:21Z

I will review it while the tests are finished.

WagnerMarcos · 2021-06-30T18:17:22Z

Here is a little demo running the diff_drive world. You can see here the usage of the model command, requesting the list of available models in the world, the whole model description, and even the link list and then a link in particular. You can request all the joints and a particular joint the same way.

modelDemo.mp4

I am of course open to any suggestion about the way the information is displayed here.

WagnerMarcos · 2021-06-30T18:31:24Z

The functionality yet to be tested is to display the properties of a bounding box component, even though the implementation for it is ready. The reason for this is that I haven't found a world containing such component. Any suggestions about a world I could use for this?

chapulina · 2021-07-01T19:43:16Z

I haven't found a world containing such component

We don't have any systems or examples using that component. Some system needs to create the AxisAlignedBox component for the entity, like SubT's VisibilityPlugin does.

My suggestion is to leave this functionality out for now. In the future we could add a simple system that enables bounding box calculation on demand (because it's too expensive to do it all the time).

francocipollone

Nice new feature!
I left a bunch of minor comments. ptal!

model/src/cmd/cmdmodel.rb.in

model/src/cmd/ignmodel.yaml.in

model/src/cmd/cmdmodel.rb.in

model/src/cmd/ModelCommandAPI.cc

model/src/ModelCommandAPI_TEST.cc

francocipollone

Thanks for addressing the comments!. Just added a few nits

model/src/cmd/ModelCommandAPI.cc

model/src/ModelCommandAPI_TEST.cc

francocipollone · 2021-07-12T14:53:56Z

CI is failing when running test because of :

Library error for [/usr/local/lib/libignition-gazebo3-model.so.3.8.0]: can't load /usr/local/lib/libignition-gazebo3-model.so.3.8.0

I haven't faced this error when running it locally though.

francocipollone · 2021-07-12T17:13:52Z

Probably adding a tutorial using the ign model cli at ignition-gazebo/tutorials would be needed, right @chapulina ?

chapulina · 2021-07-12T17:48:24Z

Probably adding a tutorial using the ign model cli at ignition-gazebo/tutorials would be needed, right @chapulina ?

+1, that would help giving the tool some visibility, otherwise it's difficult for users to discover it. It doesn't need to be too detailed, just straight to the point showing that the tool exists and some examples. Thanks!

francocipollone

Thanks for adding the tutorial! I left a couple of comments.

model/src/cmd/ModelCommandAPI.cc

tutorials/model_command.md

codecov · 2021-07-15T13:48:23Z

Codecov Report

Merging #893 (b907c86) into ign-gazebo3 (46425e0) will increase coverage by 0.22%.
The diff coverage is 90.17%.

@@               Coverage Diff               @@
##           ign-gazebo3     #893      +/-   ##
===============================================
+ Coverage        77.92%   78.14%   +0.22%     
===============================================
  Files              215      216       +1     
  Lines            12089    12262     +173     
===============================================
+ Hits              9420     9582     +162     
- Misses            2669     2680      +11

Impacted Files	Coverage Δ
src/cmd/ModelCommandAPI.cc	`90.17% <90.17%> (ø)`
src/ServerPrivate.cc	`84.39% <0.00%> (+2.92%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46425e0...b907c86. Read the comment docs.

francocipollone · 2021-07-15T13:57:11Z

CI is failing when running test because of :

Library error for [/usr/local/lib/libignition-gazebo3-model.so.3.8.0]: can't load /usr/local/lib/libignition-gazebo3-model.so.3.8.0

154b625 fixes this. IGN_CONFIG_PATH wasn't correctly set when running the tests.

francocipollone

Nice job! I left just some nits

tutorials/model_command.md

tutorials.md.in

francocipollone · 2021-07-16T20:27:46Z

@chapulina Ready for the review!

chapulina

Works for me! I did a first pass. Let me know if any comments seem unreasonable 👍

model/src/CMakeLists.txt

model/src/ModelCommandAPI_TEST.cc

chapulina · 2021-07-16T22:07:01Z

CMakeLists.txt

@@ -149,7 +149,9 @@ set(IGNITION_GAZEBO_GUI_PLUGIN_INSTALL_DIR
 #============================================================================
 # Configure the build
 #============================================================================
-ign_configure_build(QUIT_IF_BUILD_ERRORS)
+ign_configure_build(QUIT_IF_BUILD_ERRORS
+  COMPONENTS model


it was separated into a new folder following the log structure

Usually following an existing pattern is a good idea, but I'm not sure that creating a separate component is necessary in this case.

On ign-transport the log feature was kept in a separate component because it brings a new dependency which we didn't want to add to the core of the library (SQLite). But in this case, the model command is not bringing any new dependencies and I think it's not sufficiently separate from the core to justify being its own component.

Would it be possible to move the new command into src/cmd? I think this should also simplify a lot of the current CMake code.

I made a commit with only this change so you can have a look at it. I didn't merge cmdgazebo.rb.in and cmdmodel.rb.in files into one single file because the gazebo command implements many defaults value and sub-commands itself and mixing them would be difficult to follow. Because of this I had to also leave gazebo.yaml.in and ignmodel.yaml.in in different files so the command find different ruby files to process it.

Implementing both commands in the same file was my initial approach when I first started so its doable, but I thought it was really messy and I had to adapt some of the gazebo command implementation so it could coexist with the model one, so thats the reason I decided to look for a way to keep them in different files.

Let me know what you think about it.

See #928 for a possible solution

model/src/ModelCommandAPI_TEST.cc

model/src/cmd/ModelCommandAPI.cc

tutorials/model_command.md

model/src/cmd/ModelCommandAPI.cc

Signed-off-by: Marcos Wagner <[email protected]>

Signed-off-by: Franco Cipollone <[email protected]>

Signed-off-by: Marcos Wagner <[email protected]>

chapulina · 2021-07-23T03:26:31Z

I have various suggestions on #928

Signed-off-by: Louise Poubel <[email protected]> Signed-off-by: Franco Cipollone <[email protected]> Co-authored-by: Franco Cipollone <[email protected]>

chapulina

Let's get it in! Thanks for the new feature! 🎉

github-actions bot added the 🏰 citadel Ignition Citadel label Jun 28, 2021

francocipollone self-requested a review June 28, 2021 18:04

chapulina changed the title ~~Wagnermarcos/cli model info dump~~ CLI model info dump Jun 28, 2021

WagnerMarcos force-pushed the wagnermarcos/CLI_model_info_dump branch 2 times, most recently from f4e5cdd to df9eee1 Compare June 29, 2021 19:50

WagnerMarcos force-pushed the wagnermarcos/CLI_model_info_dump branch from bc3d437 to 5e33380 Compare July 1, 2021 17:18

francocipollone suggested changes Jul 5, 2021

View reviewed changes

francocipollone reviewed Jul 12, 2021

View reviewed changes

francocipollone marked this pull request as ready for review July 12, 2021 14:51

francocipollone requested a review from chapulina as a code owner July 12, 2021 14:51

WagnerMarcos force-pushed the wagnermarcos/CLI_model_info_dump branch 2 times, most recently from 5eb97ed to bc9c373 Compare July 14, 2021 19:06

francocipollone reviewed Jul 14, 2021

View reviewed changes

francocipollone force-pushed the wagnermarcos/CLI_model_info_dump branch from 13bc30a to 154b625 Compare July 15, 2021 12:58

francocipollone approved these changes Jul 16, 2021

View reviewed changes

tutorials/model_command.md Outdated Show resolved Hide resolved

tutorials/model_command.md Outdated Show resolved Hide resolved

tutorials.md.in Outdated Show resolved Hide resolved

chapulina suggested changes Jul 16, 2021

View reviewed changes

WagnerMarcos and others added 5 commits July 19, 2021 16:06

First version of new model command for ign-gazebo

bdf209e

Signed-off-by: Marcos Wagner <[email protected]>

Addressing comments made

38f4e4b

Signed-off-by: Marcos Wagner <[email protected]>

Addressing comments from the PR

6857489

Signed-off-by: Marcos Wagner <[email protected]>

Set IGN_CONFIG_PATH for testing.

bb5e909

Signed-off-by: Franco Cipollone <[email protected]>

Adressing tutorial comments

9796606

Signed-off-by: Marcos Wagner <[email protected]>

WagnerMarcos force-pushed the wagnermarcos/CLI_model_info_dump branch from 084ead9 to 2f8d28c Compare July 20, 2021 13:39

Model command structure update

66bb5cf

Signed-off-by: Marcos Wagner <[email protected]>

WagnerMarcos force-pushed the wagnermarcos/CLI_model_info_dump branch from 2f8d28c to 66bb5cf Compare July 20, 2021 17:56

Addressing PR comments

25c21d0

Signed-off-by: Marcos Wagner <[email protected]>

WagnerMarcos force-pushed the wagnermarcos/CLI_model_info_dump branch from 55018ee to 25c21d0 Compare July 22, 2021 19:02

WagnerMarcos requested a review from chapulina July 22, 2021 19:07

Merge branch 'ign-gazebo3' into wagnermarcos/CLI_model_info_dump

937bb61

chapulina mentioned this pull request Jul 23, 2021

Suggestions to model CLI (#893) #928

Merged

Suggestions to model CLI (#893) (#928)

b907c86

Signed-off-by: Louise Poubel <[email protected]> Signed-off-by: Franco Cipollone <[email protected]> Co-authored-by: Franco Cipollone <[email protected]>

chapulina approved these changes Jul 23, 2021

View reviewed changes

chapulina merged commit 2cd5994 into ign-gazebo3 Jul 23, 2021

chapulina deleted the wagnermarcos/CLI_model_info_dump branch July 23, 2021 23:28

chapulina mentioned this pull request Jul 29, 2021

Model info CLI #313

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI model info dump #893

CLI model info dump #893

WagnerMarcos commented Jun 28, 2021 •

edited

Loading

francocipollone commented Jun 28, 2021

WagnerMarcos commented Jun 30, 2021

WagnerMarcos commented Jun 30, 2021

chapulina commented Jul 1, 2021

francocipollone left a comment

francocipollone left a comment

francocipollone commented Jul 12, 2021

francocipollone commented Jul 12, 2021

chapulina commented Jul 12, 2021

francocipollone left a comment

codecov bot commented Jul 15, 2021 •

edited

Loading

francocipollone commented Jul 15, 2021

francocipollone left a comment

francocipollone commented Jul 16, 2021

chapulina left a comment

chapulina Jul 16, 2021

WagnerMarcos Jul 20, 2021 •

edited

Loading

chapulina Jul 23, 2021

chapulina commented Jul 23, 2021

chapulina left a comment

CLI model info dump #893

CLI model info dump #893

Conversation

WagnerMarcos commented Jun 28, 2021 • edited Loading

🎉 New feature

Summary

Description

Test it

Checklist

francocipollone commented Jun 28, 2021

WagnerMarcos commented Jun 30, 2021

WagnerMarcos commented Jun 30, 2021

chapulina commented Jul 1, 2021

francocipollone left a comment

Choose a reason for hiding this comment

francocipollone left a comment

Choose a reason for hiding this comment

francocipollone commented Jul 12, 2021

francocipollone commented Jul 12, 2021

chapulina commented Jul 12, 2021

francocipollone left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 15, 2021 • edited Loading

Codecov Report

francocipollone commented Jul 15, 2021

francocipollone left a comment

Choose a reason for hiding this comment

francocipollone commented Jul 16, 2021

chapulina left a comment

Choose a reason for hiding this comment

chapulina Jul 16, 2021

Choose a reason for hiding this comment

WagnerMarcos Jul 20, 2021 • edited Loading

Choose a reason for hiding this comment

chapulina Jul 23, 2021

Choose a reason for hiding this comment

chapulina commented Jul 23, 2021

chapulina left a comment

Choose a reason for hiding this comment

WagnerMarcos commented Jun 28, 2021 •

edited

Loading

codecov bot commented Jul 15, 2021 •

edited

Loading

WagnerMarcos Jul 20, 2021 •

edited

Loading