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

Port stickbot to the usage of the YARP nws/nwc infrastructure #11

Merged
merged 12 commits into from
Nov 23, 2021

Conversation

Nicogene
Copy link
Member

This porting has been possible after robotology/gazebo-yarp-plugins#583 and robotology/gazebo-yarp-plugins#584.

Now all the devices (included wholeBodyDynamics) are loaded by the gazebo_robotinterface plugin.

The hold_box.sh work smoothly.

@@ -19,6 +19,8 @@ This repo contains a `gazebo` model, starting from iCub3, and some scripts that
- [`urdfpy`](https://github.com/mmatl/urdfpy)
- [`dataclasses`](https://pypi.org/project/dataclasses/)

:warning: **Since `stickBot` has been now ported to the new nws/nwc architecture, it requires `YARP 3.5.0` or greater and latest `gazebo-yarp-plugins` devel branch. Moreover before compiling `gazebo-yarp-plugins` the CMake flag `GAZEBO_YARP_PLUGINS_DISABLE_IMPLICIT_NETWORK_WRAPPERS` has to be set to `ON`**
Copy link
Member

@traversaro traversaro Nov 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite problematic for integration to be honest. Different models (including the existing iCub ones) require GAZEBO_YARP_PLUGINS_DISABLE_IMPLICIT_NETWORK_WRAPPERS to be set to ON, and the fact that instead this models require this option to be ON may complicate a lot its use. Furthermore, it prevent to use any pre-compiled version of gazebo-yarp-plugins for quite a long time. Just to understand, why is this required?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is required because unfortunately with that flag set to OFF, the opening of the devices that have ini without wrappers will fail. This is for the gazebo_imu and gazebo_forcetorque, but the same thing for the gazebo_depthcamera that has been ported by @mfussi66. I don't know how to make it work with both the flag set to ON and set to OFF.
Are you aware of a way to do it?
We wanted just to uniform to one standard (the new one)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! actually we need also the latest master of whole-body-estimators, since without the bugfix wbd get stuck

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is required because unfortunately with that flag set to OFF, the opening of the devices that have ini without wrappers will fail.

I think this is a problem in gazebo-yarp-plugins, in a post-gazebo_yarp_controlboard I think it is perfectly fine for a implicitly launched wrapped to not start without preventing the rest of the plugin to start fine. I am not asking you to fix all the plugins to permit this, I wanted to make clear that we identify the problem (that in this case is on the gazebo-yarp-plugins side).
The GAZEBO_YARP_PLUGINS_DISABLE_IMPLICIT_NETWORK_WRAPPERS option was just meant to be a way for people to make sure that they stopped depending on the implicitly launched network wrappers (see robotology/gazebo-yarp-plugins#565) i.e. to check that models that are migrated to the use of libgazebo_yarp_robotinterface completed the migration, not meant to have models that just work if this option is enabled (as this would complexify the situation.

Are you aware of a way to do it?

Yes, I think we should just transform https://github.com/robotology/gazebo-yarp-plugins/blob/6bf70a582ef81200a072065606048b47e8b963b6/plugins/forcetorque/src/ForceTorque.cc#L89 in a warning, and remove the return. To clarify, this is not blocking in any way this PR, I just want to make sure that we are aligned on the fact that having models that do not work with the default configuration of gazebo-yarp-plugins is quite problematic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nicogene
Copy link
Member Author

As we discussed T2T and here robotology/gazebo-yarp-plugins#586, we decided to add to all the plugins the flag disableImplicitNetworkWrapper in order to allow to not specify the network wrapper in the configuration phase without setting the define that might break the usage of other models that use the "old" convention.

Until this new flag is not added we can merge this PR with this hard constraint and then relax it changing the configuration files of the plugins

cc @GrmanRodriguez @traversaro @AlexAntn @mfussi66

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.

5 participants