ami-iit / mujoco-urdf-loader

BSD 3-Clause "New" or "Revised" License
8 stars 0 forks source link

Fix mass reducing after conversion #9

Closed Giulero closed 2 weeks ago

Giulero commented 2 weeks ago

This PR should fix an issue that arose after #8. I was doing some sim2sim test, and I just felt that something was odd - just a feeling. I inspected the generated xml and I figured out that the mass of the root link had vanished. I then compared the total mass of the robot loading the urdf model with idyntree and the total mass of the robot imported in mujoco: it was different. In #8, I added an additional frame connecting the world to the root with a floating joint. In the conversion from urdf to mjcf, it seems that the root link is lumped with this frame, losing the mass property. In this PR, I simplify this logic, exploiting the fact that the idyntree model reducer adds, before the root link, a fixed joint that is then converted to a floating one.

I added an additional test that checks that the mass of the idytree model and of the mujoco model coincide. Just to be sure a test is added also with a different robot, anymal_c from robots_description.py.

traversaro commented 2 weeks ago

In this PR, I simplify this logic, exploiting the fact that the idyntree model reducer adds, before the root link, a fixed joint that is then converted to a floating one.

Just a cross-reference. That behavior is there as a workaround to a really old bug in the default URDF parser of ROS see https://github.com/ros/kdl_parser/issues/27 and https://github.com/ros/robot_model/issues/6. Recently that behavior has confused some one (see https://github.com/robotology/idyntree/issues/1201) so I was thinking (before reading this PR) of changing at least the default behavior of the exporter. Just to be safe from future iDynTree changes, if it is easy to do we may think of setting explicitly exportFirstBaseLinkAdditionalFrameAsFakeURDFBase to true instead of relying on its default value being true.