robotology / icub-models

Official URDF and SDF models of the iCub humanoid robot.
Creative Commons Attribution Share Alike 4.0 International
33 stars 34 forks source link

iCubGazebo2_5_visuomanip discrepancies #44

Closed pattacini closed 4 years ago

pattacini commented 4 years ago

Hi @xEnVrE

First off, thanks a lot for the new model added in #42 ✨

Regarding this, I've noticed the following points:

cc @traversaro

xEnVrE commented 4 years ago

Hi @pattacini,

regarding the error on the Sensor, I think it is due to the following rows

https://github.com/robotology/icub-models/blob/68719a8a77a10219844fe8f7d3b039ed58af4ffa/iCub_manual/robots/iCubGazeboV2_5_visuomanip/model.urdf#L2740-L2752

where the first sensor is the IMU of the head equipped with its own plugin from gazebo-yarp-plugins. The second one, that I copied from (i.e. from the standard iCubGazeboV2_5)

https://github.com/robotology/icub-models/blob/68719a8a77a10219844fe8f7d3b039ed58af4ffa/iCub/robots/iCubGazeboV2_5/model.urdf#L2526

seems to be wrong at this point and probably not required. Gazebo does not know the sensor type at all.

Therefore, either we use /icubSim/cam/right/rgbImage:o or /icubSim/cam/left, I'd say.

Regarding port names, the first two ports are opened by the depth camera plugin on the left eye while the third one is opened by the rgb-only plugin for the right eye.

The configuration file of the depth camera plugin doesn't allow specifying the trailing part of the port name, i.e. the rgbImage:o. Then, as you suggested, we can use /icubSim/cam/right/rgbImage:o for the right as well by changing the associated entry in the rgb only configuration file

pattacini commented 4 years ago

Ok then, I'll open up a dedicated PR to address this. Thanks 👍🏻

xEnVrE commented 4 years ago

Thank you @pattacini for pointing out the inconsistencies.

traversaro commented 4 years ago

At startup, I can see these errors popping up:

That seems to be a bug of the non-visuomanip model, see https://github.com/robotology/icub-models/issues/46 .

traversaro commented 4 years ago

I would harmonize the name of the cameras' ports. As of now, the model will open (aside from the rpc service ports):

How are the port published on the real robot named? I think it is important (at least for the RGB cameras, and eventually for the depth stream being published by stereo-vision) to try to be aligned between simulation and real robot.

xEnVrE commented 4 years ago

How are the port published on the real robot named? I think it is important (at least for the RGB cameras, and eventually for the depth stream being published by stereo-vision) to try to be aligned between simulation and real robot.

I totally agree with you. However It seems to me that matching the name on the real robot would require a modification of the depth camera plugin. If I am not wrong, the name of the rgb output on the real robot is <robot_name>/cam/{left, right}. However, since we adopted a depth camera for the left eye, the trailing part of the port name seems to be hardcoded to rgbImage:o.

Another solution would be to have two rgb cameras for both eyes, so that we can match easily the names as the output port name is fully configurable in the configuration file of the rgb only plugin. However, I discarded this option since, by using a depth camera on the left eye, we are already extracting the rgb frames for that eye.

Regarding the depth stream from stereo-vision, the original SFM module did not have any depth output port. Recently, a module called DispModule has been added (https://github.com/robotology/stereo-vision/pull/22) that also provides a depth output. In this case, the name seems to be configurable (see here).

traversaro commented 4 years ago

I totally agree with you. However It seems to me that matching the name on the real robot would require a modification of the depth camera plugin.

Ack, I opened a new issue for this (note that opening a new issue does not mean that we need to work on this now, just that is something that we may work on in the future): https://github.com/robotology/icub-models/issues/47

Regarding the depth stream from stereo-vision, the original SFM module did not have any depth output port. Recently, a module called DispModule has been added (robotology/stereo-vision#22) that also provides a depth output. In this case, the name seems to be configurable (see here).

Even if the port output name is configurable, I think the default value of it in simulatio should match the default value of it in the real robot.

xEnVrE commented 4 years ago

Btw, the harcoded part seems to be coming from the YARP RGBDSensorWrapper that is instantiated inside the gazebo-yarp-plugins depthCamera.

Indeed one can find here:

    if(use_YARP)
    {
        std::string rootName;
        rootName = config.check("name",Value("/"), "starting '/' if needed.").asString();

        if (!config.check("name", "Prefix name of the ports opened by the RGBD wrapper."))
        {
            yCError(RGBDSENSORWRAPPER) << "RGBDSensorWrapper: missing 'name' parameter. Check you configuration file; it must be like:";
            yCError(RGBDSENSORWRAPPER) << "   name:         Prefix name of the ports opened by the RGBD wrapper, e.g. /robotName/RGBD";
            return false;
        }
        else
        {
            rootName = config.find("name").asString();
            rpcPort_Name  = rootName + "/rpc:i";
            colorFrame_StreamingPort_Name = rootName + "/rgbImage:o";
            depthFrame_StreamingPort_Name = rootName + "/depthImage:o";
        }
    }