robotology / icub-models-generator

Resources and programs to generated models (URDF, SDF) of the iCub robot
14 stars 23 forks source link

ICUB_2-5_BB_simmechanics_options.yaml: Use the child link of the FT sensor fixed joint for linkName of l_arm_ft_sensor and r_arm_ft_sensor #260

Closed traversaro closed 6 months ago

traversaro commented 7 months ago

Attempt to fix https://github.com/robotology/icub-models-generator/issues/233, or at least debug it more easily. In URDF, when an FT sensor is modeled by a fixed joint, the origin frame of the FT sensor must be coincident with the frame of the child link of the fixed joint.

For the arm FT sensor, the joint is defined as in the following (see https://icub-tech-iit.github.io/documentation/icub_kinematics/icub-model-naming-conventions/icub-model-naming-conventions/#links):

before this PR, for iCub 2.* models the used to refer to l_shoulder_3 and r_shoulder_3 for the definitions related to l_arm_ft_sensor and r_arm_ft_sensor . I am not sure if this is the source of the error in https://github.com/robotology/icub-models-generator/issues/233 (in that case, probably if there is a bug in https://github.com/robotology/simmechanics-to-urdf), but for sure using the childLink make the model easier to debug as you can directly check that the translation between l_upper_arm and l_arm_ft_sensor is indeed 0 0 0.

traversaro commented 7 months ago

In https://github.com/robotology/icub-models-generator/pull/260/commits/29d480afbdec1e2f259a57bb49b797d58336f5c6 I did the same modifications also for the IMUs, for the same reason (so that the IMU pose should be 0 0 0, and it is easier to debug).

traversaro commented 7 months ago

In 29d480a I did the same modifications also for the IMUs, for the same reason (so that the IMU pose should be 0 0 0, and it is easier to debug).

And this is making the CI fail, great:

[  2%] Generating iCubDarmstadt01.urdf
cd /home/runner/work/icub-models-generator/icub-models-generator/build/simmechanics && simmechanics_to_urdf /home/runner/work/icub-models-generator/icub-models-generator/simmechanics/data/icub2_5/ICUB_2-5_BB_SIM_MODEL.xml --output xml --yaml /home/runner/work/icub-models-generator/icub-models-generator/build/simmechanics/iCubDarmstadt01.yaml --csv-joints /home/runner/work/icub-models-generator/icub-models-generator/build/simmechanics/iCubDarmstadt01.csv --outputfile iCubDarmstadt01.urdf
/usr/local/lib/python3.8/dist-packages/simmechanics_to_urdf-0.4.2-py3.8.egg/simmechanics_to_urdf/firstgen.py:717: SyntaxWarning: "is" with a literal. Did you mean "=="?
/usr/local/lib/python3.8/dist-packages/simmechanics_to_urdf-0.4.2-py3.8.egg/simmechanics_to_urdf/firstgen.py:1631: SyntaxWarning: "is" with a literal. Did you mean "=="?
/usr/local/lib/python3.8/dist-packages/simmechanics_to_urdf-0.4.2-py3.8.egg/simmechanics_to_urdf/firstgen.py:717: SyntaxWarning: "is" with a literal. Did you mean "=="?
/usr/local/lib/python3.8/dist-packages/simmechanics_to_urdf-0.4.2-py3.8.egg/simmechanics_to_urdf/firstgen.py:1631: SyntaxWarning: "is" with a literal. Did you mean "=="?
/usr/local/lib/python3.8/dist-packages/simmechanics_to_urdf-0.4.2-py3.8.egg/simmechanics_to_urdf/firstgen.py:369: YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated, as the default Loader is unsafe. Please read https://msg.pyyaml.org/load for full details.
Traceback (most recent call last):
  File "/usr/local/bin/simmechanics_to_urdf", line 11, in <module>
    load_entry_point('simmechanics-to-urdf==0.4.2', 'console_scripts', 'simmechanics_to_urdf')()
  File "/usr/local/lib/python3.8/dist-packages/simmechanics_to_urdf-0.4.2-py3.8.egg/simmechanics_to_urdf/firstgen.py", line 2000, in main
  File "/usr/local/lib/python3.8/dist-packages/simmechanics_to_urdf-0.4.2-py3.8.egg/simmechanics_to_urdf/firstgen.py", line 239, in convert
  File "/usr/local/lib/python3.8/dist-packages/simmechanics_to_urdf-0.4.2-py3.8.egg/simmechanics_to_urdf/firstgen.py", line 328, in addSensors
KeyError: ('l_upper_arm', 'SCSYS_L_UPPER_ARM_FT45')
make[2]: *** [simmechanics/CMakeFiles/generate-models-simmechanics.dir/build.make:149: simmechanics/iCubDarmstadt01.urdf] Error 1
make[2]: Leaving directory '/home/runner/work/icub-models-generator/icub-models-generator/build'
make[1]: *** [CMakeFiles/Makefile2:972: simmechanics/CMakeFiles/generate-models-simmechanics.dir/all] Error 2
make: *** [Makefile:101: all] Error 2
traversaro commented 7 months ago

Just the first commit instead is not changing anything, not sure why. @Nicogene @martinaxgloria I am out of time to work on this today unfortunately, if you want to debug feel free to continue, my hunch that we should be able to express the ft sensors w.r.t. to the child links and see that their pose is 0 0 0 and 0 0 0, if this does not work there is a problem.

Nicogene commented 7 months ago

And this is making the CI fail, great:

Now I remember in the past I already tried your changes, it can be an issue in simmechanics_to_urdf or in the csys definition

Just the first commit instead is not changing anything, not sure why. @Nicogene @martinaxgloria I am out of time to work on this today unfortunately, if you want to debug feel free to continue, my hunch that we should be able to express the ft sensors w.r.t. to the child links and see that their pose is 0 0 0 and 0 0 0, if this does not work there is a problem.

@martinaxgloria and I can take over, if there is a problem in the definition of CSYS we should think of porting icub-models-generator to creo2urdf because I suppose the simulation model has been saved in creo9. The drawback is that we lose the automatism of the generation because I didn't have time to work on this:

Nicogene commented 7 months ago

And this is making the CI fail, great:

I think this is due to the fact that in the simulation model cad unfortunately the csys is not defined in the child link

cc @fiorisi

traversaro commented 7 months ago

And this is making the CI fail, great:

I think this is due to the fact that in the simulation model cad unfortunately the csys is not defined in the child link

cc @fiorisi

Ah, ok, good point. So either we fix some logic in simmechanics-to-urdf in this case, or we add the csys in the child link in the CAD?

fiorisi commented 7 months ago

And this is making the CI fail, great:

I think this is due to the fact that in the simulation model cad unfortunately the csys is not defined in the child link cc @fiorisi

Ah, ok, good point. So either we fix some logic in simmechanics-to-urdf in this case, or we add the csys in the child link in the CAD?

Yes, this could be a problem. In the procedure we wrote "... FT sensor frame origin must be centred with the origin of the child link...".

Nicogene commented 7 months ago

Ah, ok, good point. So either we fix some logic in simmechanics-to-urdf in this case, or we add the csys in the child link in the CAD?

Adding the csys in the child link in the cad would allow to export the ft sensor referring to child link and maybe fixes the problem, because the frame is exported fine but there is somehow an issue in the definition of the ft sensor:

The problem on chainging the CAD is the CREO9 incompatibility with the first gen of simscape multibody link, for doing a quick test we could:

If it works we can commit that xml.

Another possibility is to do this change in CREO9 and use creo2urdf, but unitl we do not automatize the generation via creo2urdf it is tough add also icub to the models generated by hand (and by me XD)

traversaro commented 6 months ago

Superseded by https://github.com/robotology/icub-models-generator/pull/265 .