Fixed Odometry Publisher Angular Velocity Singularity in 3D

[tomcreutz]

🦟 Bug fix

Fixes #2347

Summary

The angular velocity was calculated using euler angles which led to singularities (see #2347 for a more detailed description). In addition to the fix, I also removed the usage of the rolling mean accumulator for angular velocity. While I see its usefulness for outlier rejections, I still didn't really see why this would be necessary. If the maintainers want to keep this mean accumulator, I'll be happy to reimplement it. Also the commit changes seem a bit messy, I was using clang format with the Google C++ Style guide as configuration, which leaded to the changed formatting that you can see in the pull request.

Checklist

• [x] Signed all commits for DCO
• [x] codecheck passed (See contributing)
• [x] All tests passed (See test coverage)

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.

[mjcarroll]

Also the commit changes seem a bit messy, I was using clang format with the Google C++ Style guide as configuration, which leaded to the changed formatting that you can see in the pull request.

Can you roll back the style changes please? We need to just update the logic without altering the style of the entire file.

[tomcreutz]

Hi @mjcarroll, is there anything more that needs to be changed to merge this pull request ?

[tomcreutz]

Hello @mjcarroll, are there any news or requests from your side regarding this pull request ? Are there any plans to address this issue ?

[azeey]

I've merged https://github.com/tomcreutz/gz-sim/pull/1

[shameekganguly]

Please consider adding a test case in https://github.com/gazebosim/gz-sim/blob/gz-sim8/test/integration/odometry_publisher.cc to prevent a regression in computing 3d angular velocity close to singularity. Thank you!

Added test case in 5b877ca

[azeey]

@osrf-jenkins run tests please

[shameekganguly]

Looks like Windows CI is failing as M_PI cannot be found. I guess we should GZ_PI instead in test/integration/odometry_publisher.cc

[tomcreutz]

Thank you very much for your efforts @azeey and @shameekganguly . Just returned back from holidays and wanted to take care of this in the following days.

[azeey]

Not sure why CI on Jammy failed. The test seems completely unrelated to this PR, so I'm rerunning it to see if it's a flake. The previous CI job was green https://build.osrfoundation.org/job/gz_sim-ci-pr_any-jammy-amd64/744/.