IMU sensors: alignment real ergoCub/iCub w/ simulated one

Nicogene commented 5 months ago

This PR:

https://github.com/robotology/robots-configuration/pull/632

Added the remapper for the imus that uses sensorNames that are not coherent with the actual sensor names of the exposed imu except for iCubGenova11.

embObjIMU exposes as sensor name the id if sensorName is not specified, and sensorName is present in only the iCubGenova11 imus xml.

Moreover, I noticed that on urdf side, iCub and ergoCub use different names for the imus:

Mandatory

@traversaro Stakeholder need: The sensor name used on simulator and real robot (either for iCub and ergoCub) must be coherent
sensorName must be added in all FT imu xml.

Nice to have

iCub and ergoCub uses the same sensor names

Nicer to have

Every sensor follows this guideline (or we can change it as we please)
- https://icub-tech-iit.github.io/documentation/robot_sensors/ergocub_sensors/

This activity can be done in more steps, the mandatory points are the one more urgent, but they depend on if we want to "cleanup" the sensor name situation

cc @pattacini

traversaro commented 5 months ago

fyi @robotology/iit-ami-scrum-masters when implemented this may affect configuration files on our side.

martinaxgloria commented 5 months ago

As discussed in a private chat with @Nicogene, we decided to add the sensorName tag in all the FT IMUs XML files and the chosen names must be coherent with the simulated ones.

For the ergoCub side, since:
- https://github.com/icub-tech-iit/ergocub-software/pull/217
- https://github.com/icub-tech-iit/ergocub-software/pull/232
the sensorName tag should be <part>_ft_imu (for example, l_arm_ft_imu) to make it different from the FT ones without the suffix _imu:

https://github.com/robotology/robots-configuration/blob/3ca4cb27c8ff0adb3ed1036429bd744bce2820b7/ergoCubSN001/hardware/FT/left_arm-eb2-j0_1-strain.xml#L33-L37
For the iCub side, instead, with:
- https://github.com/robotology/icub-models-generator/pull/259
- https://github.com/robotology/icub-models-generator/pull/264
there were added the alljoints-inertials also for iCub models, but the name used is the same as the FT ones, i.e. <part>_ft. According to the nice-to-have, it could be useful/nice/coherent using the same convention used for ergoCub and fix the simulated models with a PR on icub-models-generator.

cc @Nicogene @pattacini @traversaro

traversaro commented 5 months ago

Great! Do you think it could make sense to document this convention of the sensor names somewhere in the docs? We can just write down that its implementation is still work in progress, but at least to have a reference for the future.

martinaxgloria commented 5 months ago

There are those pages in the doc for the sensors:

For each sensor belonging to a robot part there's a table with a column named Sensor Name. What about using this? Or we can add an extra column in these tables

cc @traversaro @Nicogene @pattacini

traversaro commented 5 months ago

For each sensor belonging to a robot part there's a table with a column named Sensor Name. What about using this? Or we can add an extra column in these tables

I think that "sensor name" in the table is a bit misleading, as what is indicating is the prefix of the port that publish information about the reference sensors, not the sensor name, so I totally agree in using that column.

martinaxgloria commented 5 months ago

cc @mfussi66 since he added this information to the documentation.

martinaxgloria commented 5 months ago

For the time being, I opened this PR:

https://github.com/robotology/robots-configuration/pull/637

to set a starting point for the real robots. The simulated ones will be modified taking this PR as a reference

martinaxgloria commented 5 months ago

Since

https://github.com/robotology/robots-configuration/pull/637

has been merged, could we close this issue? I'll proceed to update iCub and ergoCub models according to that PR.

cc @Nicogene @pattacini

martinaxgloria commented 5 months ago

I'll revive this PR:

https://github.com/icub-tech-iit/ergocub-software/pull/232

and open a new one on icub-models-generator

Nicogene commented 5 months ago

Since
* [ IMU sensors: add sensorName tag in FT IMU xml  #637](https://github.com/robotology/robots-configuration/pull/637)
has been merged, could we close this issue? I'll proceed to update iCub and ergoCub models according to that PR.

cc @Nicogene @pattacini

Let's close it when also the models PR will be merged

I'll revive this PR:

https://github.com/icub-tech-iit/ergocub-software/pull/232

and open a new one on icub-models-generator

Put it ready for review and rebase it, it may be that you will face some conficts, once resolved please let me know when generating the V1_1 models

robotology / robots-configuration