Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mass reducing after conversion #9

Merged
merged 3 commits into from
Nov 7, 2024
Merged

Fix mass reducing after conversion #9

merged 3 commits into from
Nov 7, 2024

Conversation

Giulero
Copy link
Contributor

@Giulero Giulero commented Nov 6, 2024

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.

Copy link
Member

@giotherobot giotherobot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

@Giulero Giulero merged commit 034fc00 into main Nov 7, 2024
@Giulero Giulero deleted the fix-model-generation branch November 7, 2024 13:46
@traversaro
Copy link
Contributor

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 ros/kdl_parser#27 and ros/robot_model#6. Recently that behavior has confused some one (see robotology/idyntree#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants