dartsim: fix handling inertia matrix pose rotation

[scpeters]

🦟 Bug fix

Fixes a bug in moment of inertia calculation

Summary

When loading a model from SDF in the dartsim plugin, the moment of inertia matrix is currently applying any rotations in the //inertial/pose two times, since the rotations are applied explicitly in SDFFeatures.cc, but they are already applied in math::Inertial::Moi.

I confirmed the bug with a slightly modified version of the inertia_rotations test world from gazebo classic. That world has multiple 1x4x9 boxes with identical inertia values that are expressed with different inertial pose rotations. The boxes are initially standing up on a ground plane. Setting gravity to [2, 0, -9.8] causes the boxes to tip over, and they should move at the same rate and hit the ground at the same time. This works properly in gazebo-classic with DART but not fortress or citadel.

I'll upload a copy of the example world for use with fortress as well as screen captures of my testing and then mark this ready for review.

• https://github.com/gazebosim/gz-physics/blob/fd4fead203ef5da7743a48e501e575a0c688cbaf/dartsim/worlds/inertia_rotations_ign.world
• 
• 

Checklist

• Signed all commits for DCO
• Added tests
• Updated documentation (as needed)
• Updated migration guide (as needed)
• Consider updating Python bindings (if the library has them)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (ign-physics5@e91c58a). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             ign-physics5     #351   +/-   ##
===============================================
  Coverage                ?   76.84%           
===============================================
  Files                   ?      129           
  Lines                   ?     5960           
  Branches                ?        0           
===============================================
  Hits                    ?     4580           
  Misses                  ?     1380           
  Partials                ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us.

[chapulina]

Thanks for the fix, I was wondering about this. Is there a way we can write a test here to catch that?

Also, would it be possible to target this at Citadel?

[JoanAguilar]

Looks good to me. I second the idea of having an automated test to catch future regressions.

[scpeters]

Also, would it be possible to target this at Citadel?

yes, I'll target this at Citadel

[scpeters]

Thanks for the fix, I was wondering about this. Is there a way we can write a test here to catch that?

I second the idea of having an automated test to catch future regressions.

it will be easiest to write the test using the common test framework, which is now added in Garden, so I'll make a separate PR for garden once this is merged forward

[chapulina]

yes, I'll target this at Citadel

Let's merge this into Fortress so it catches the next bus to Garden. We should backport it to Citadel soon.