Add model iCubGazeboV2_5_visuomanip

[xEnVrE]

Within this PR:

• a new folder iCub_manual for models that contains parts manually edited by users that gets installed under share
• a new model iCub_manual/robots/iCubGazeboV2_5_visuomanip based on iCub/robots/iCubGazeboV2_5 (44d0c7e) with:
  • hands (kinematics/meshes adapted from the Simox project)
  • eyes (kinematics v 2.5 from iKinFwd)
  • a rgb/depth camera on the left eye with parameters:
    • width = 320
    • height = 240
    • fx = fy = 343.12110728152936 (i.e. 50 degrees of HFOV)
    • cx = 160.0
    • cy = 120.0
  • a rgb camera on the right eye with the same parameters
  • MAIS analog output for all the fingers joints (excluding abduction/adduction as on the real robot)
  • configuration files for eyes and fingers that take into account limits for coupled joints

Note: with respect to iCub/robots/iCubGazeboV2_5:

• all the sensors have been disabled a part from the head IMU
• the legs plugins have been disabled
• the robot is fixed at the root link of the robot

I am not totally sure about the proper placement of the files in root/iCub_manual and whether the solution adopted in the CMakeLists.txt is optimal.

The naming of eyes/fingers joints might not be optimal as well.

I kindly ask to review the PR.

cc @traversaro @pattacini

[xEnVrE]

I opened the PR against master. Is it ok @traversaro? Should I change it to devel ?

[traversaro]

I know that people that manually edited iCub models in the past (@gabrielenava @diegoferigo) will hate me if I start accepting manually edited models in icub-models, when I warned them for years against manually modified models . However, in the case of this model that enables important features and enables us to effectively deprecate the use of iCub_SIM, so I guess it is worth to start to integrate it in icub-models, even if not all of it is automatically generate. In particular, I think this is an important initial step in using Gazebo for vision and manipulation and in the near (for a given definition of near) future this model can be improved by automatically generating it from the iCubGazeboV2_5 model, and just adding the hands and the eyes to the model.

Having said that, I have a few comments for @xEnVrE :

• Can you document exactly which version of iCubGazeboV2_5 you used as a base model to build this iCubGazeboV2_5_vision_manipulation, in particular which commit of icub-models? In this way in the future if we need to update it, it is easier to understand what were the inputs.
• Even if the model is not stored in the iCub directory to avoid interfering with the automatic generation process of https://github.com/robotology/icub-model-generator, I think it is important to still install it in the share/iCub prefix, so that it can be found in YARP, Gazebo and ROS using the existing environment variables (as documented in https://github.com/robotology/icub-models#by-installing-the-models) without the need for all the user to add a value in YARP_DATA_DIRS, change the ROS package name from iCub to iCub_manual or append a new value to GAZEBO_MODEL_PATH. To do so, you need to avoid having conflicts between this model .ini files and the one installed by the "hand-less" or "eye-less" models, so I suggest to move them from the conf directory to another directory with a different name, so they can happily coexist.
• I have some doubts of the name iCubGazeboV2_5_vision_manipulation as all the robot names are meant to be (optionally) used as YARP_ROBOT_NAME to find the model.urdf, and the name is different from the usual camel case stile of YARP_ROBOT_NAMEs, but I won't bikeshed about this.
• Despite being supported in Gazebo since a few releases, .obj are not a format that has been supported explicitly by URDF, that for a long time just supported .stl or .dae (Collada) meshes, and for example iDynTree only support this two formats explicitly. Even if the supported meshes format in URDF are now implementation defined (-_-) the suggested format is still Collada, i.e. .dae, so if you could convert the .obj meshes in .dae it would be great, if you need help with this conversion let me know and I can help you.

[traversaro]

I opened the PR against master. Is it ok @traversaro? Should I change it to devel ?

master is ok.

[xEnVrE]

I have some doubts of the name iCubGazeboV2_5_vision_manipulation as all the robot names are meant to be (optionally) used as YARP_ROBOT_NAME to find the model.urdf, and the name is different from the usual camel case stile of YARP_ROBOT_NAMEs, but I won't bikeshed about this.

I agree with you. I am of course open to suggestions for the name.

Ok for the other suggestions.

What about the meshes? After installing everything under iCub the meshes of the hand will be copied in the target directory together with the other ones. Do you think it is better to have them inside say meshes/hand/ ?

[traversaro]

What about the meshes? After installing everything under iCub the meshes of the hand will be copied in the target directory together with the other ones. Do you think it is better to have them inside say meshes/hand/ ?

As long as they are no conflicts, I think it is ok to keep them under meshes.

[traversaro]

I have some doubts of the name iCubGazeboV2_5_vision_manipulation as all the robot names are meant to be (optionally) used as YARP_ROBOT_NAME to find the model.urdf, and the name is different from the usual camel case stile of YARP_ROBOT_NAMEs, but I won't bikeshed about this.

I agree with you. I am of course open to suggestions for the name.

Same for me, if someone has a good name feel free to suggest, otherwise we will proceed with iCubGazeboV2_5_vision_manipulation .

[pattacini]

iCubGazeboV2_5_visuomanip is perhaps a little bit shorter.

[xEnVrE]

All the suggestions have been addressed.

The name I chose is iCubGazeboV2_5_visuomanip and configs are now in conf_manual/iCubGazeboV2_5_visuomanip. This way they are distinct from the original ones and there is also space in conf_manual/<model_name> for customized configs for other robots in iCub_manual.

so if you could convert the .obj meshes in .dae it would be great, if you need help with this conversion let me know and I can help you.

Thank you so much for the suggestion. Btw, after the conversion to dae, the meshes appearance within the Gazebo GUI has improved a lot!

A final question from my side: I am using some of the config files from iCubGazeboV2_5 inside this model (i.e. those that I did not change while building iCubGazeboV2_5_visuomanip). The idea is to share information, e.g. PID gains, so that if the ones of iCubGazeboV2_5 get updated also this model can benefit from that. On the other hand, maybe it is better to have a copy of the all the configuration files for this model to keep things separated. What do you think?

[traversaro]

A final question from my side: I am using some of the config files from iCubGazeboV2_5 inside this model (i.e. those that I did not change while building iCubGazeboV2_5_visuomanip). The idea is to share information, e.g. PID gains, so that if the ones of iCubGazeboV2_5 get updated also this model can benefit from that. On the other hand, maybe it is better to have a copy of the all the configuration files for this model to keep things separated. What do you think?

I agree on share what we can share.

[traversaro]

Marking as "Requests changes" to track the PR status. As suggested by @xEnVrE , the "share what we can share" policy still needs to be implemented.

[xEnVrE]

Marking as "Requests changes" to track the PR status. As suggested by @xEnVrE , the "share what we can share" policy still needs to be implemented.

Hi @traversaro. At the moment, the URDF is already taking some of the configuration files from iCubGazeboV2_5. Do you mean something different by "share what we can share" policy?

[traversaro]

Marking as "Requests changes" to track the PR status. As suggested by @xEnVrE , the "share what we can share" policy still needs to be implemented.

Hi @traversaro. At the moment, the URDF is already taking some of the configuration files from iCubGazeboV2_5. Do you mean something different by "share what we can share" policy?

Sorry, I thought you still need to implement this sharing, if that is done, I guess then the PR is good to go.