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

iCubGenova09 wrist yaw is a continuous joint #93

Closed lrapetti closed 2 years ago

lrapetti commented 3 years ago

The right wrist yaw of iCubGenova09 model is defined as continuous joint (see https://github.com/robotology/icub-models/blob/master/iCub/robots/iCubGenova09/model.urdf#L358-L364), instead of being rotational joint with jount limits as the left one

cc @S-Dafarra

S-Dafarra commented 3 years ago

cc @traversaro @Nicogene

traversaro commented 3 years ago

The joint limits are specified in https://github.com/robotology/icub-models-generator/blob/master/simmechanics/data/icub3/ICUB_3_joint_all_parameters.csv#L29 , so something is going wrong in another point.

traversaro commented 3 years ago

@lrapetti please use the complete joint names when opening issues, it simplifies searching them in the future. In particular:

Nicogene commented 3 years ago

The problem should be on the definition of the simrep in Creo, we never noticed probably because nobody used the hands of iCub3 until now

traversaro commented 3 years ago

The problem should be on the definition of the simrep in Creo,

@Nicogene are you sure? r_wrist_yaw corresponds to SIM_ICUB3_R_WRIST_1--SIM_ICUB3_R_HAND (see https://github.com/robotology/icub-models-generator/blob/921c4cbd5eefd94e95437ad3bb01793aff3ace68/simmechanics/data/icub3/ICUB_3_all_options.yaml#L103) and that joint seems to be correctly revolute in the Simmechanics (see https://github.com/robotology/icub-models-generator/blob/921c4cbd5eefd94e95437ad3bb01793aff3ace68/simmechanics/data/icub3/ICUB3_ALL_SIM_MODEL.xml#L4636) exactly like SIM_ICUB3_L_WRIST_1--SIM_ICUB3_L_HAND https://github.com/robotology/icub-models-generator/blob/921c4cbd5eefd94e95437ad3bb01793aff3ace68/simmechanics/data/icub3/ICUB3_ALL_SIM_MODEL.xml#L4694 . Unless we catch any difference, probably the issue could be in the simmechanics-to-urdf script?

Nicogene commented 3 years ago

You are right, I thought it was like https://github.com/robotology/icub-models/issues/72, probably is more fishy

lrapetti commented 2 years ago

@traversaro @Nicogene what do you think of opening a PR to fix those limits while debugging the problem with the autogeneration? I can proceed with that

Nicogene commented 2 years ago

@traversaro @Nicogene what do you think of opening a PR to fix those limits while debugging the problem with the autogeneration? I can proceed with that

Do you mean committing the fix here? Or in icub-models-generator? (or simmechanics-to-urdf)

lrapetti commented 2 years ago

@traversaro @Nicogene what do you think of opening a PR to fix those limits while debugging the problem with the autogeneration? I can proceed with that

Do you mean committing the fix here? Or in icub-models-generator? (or simmechanics-to-urdf)

I was thinking here, directly in the urdf, but I am open if you think another place is better. Just wanted to avoid having issues when using this urdf for s.o. not aware of this bug.

Nicogene commented 2 years ago

You can for sure commit the fix here, the only problem is that, as soon as anyone changes something in the icub3 urdf (even a mesh) your fix will be overwritten.

The best solution would be to find the source of the problem but if it is blocking/affecting your workflow you can commit the fix here

lrapetti commented 2 years ago

I have opened a PR for fixing the problem in the current models https://github.com/robotology/icub-models/pull/111. Since the source of the problem, however, has not been found and fixed yet, we can leave this issue open

Nicogene commented 2 years ago

I was looking in simmechanics_to_urdf and it seems that by default it treat all the joints as continuous:

https://github.com/robotology/simmechanics-to-urdf/blob/f6055ab8d627c45138ddde46532ea42a74b48caa/simmechanics_to_urdf/firstgen.py#L759-L774

But then if it finds limits, it switch it to revolute:

https://github.com/robotology/simmechanics-to-urdf/blob/f6055ab8d627c45138ddde46532ea42a74b48caa/simmechanics_to_urdf/firstgen.py#L1404-L1409

Nicogene commented 2 years ago

This has been fixed by https://github.com/robotology/icub-models-generator/commit/d7dff26541d8796037c26fc776bb34396f3e6643. There was a typo in the csv

lrapetti commented 2 years ago

Great! Thanks @Nicogene @traversaro