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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add magnetometer plugin parameter to publish units as tesla #2336

Open
wants to merge 3 commits into
base: gz-sim8
Choose a base branch
from

Conversation

gilborty
Copy link

@gilborty gilborty commented Mar 17, 2024

馃 Bug fix

Fixes #2312

Summary

As mentioned in #2312, the magnetic field message is incorrectly using Gauss as the units for the field strength, rather than the SI unit of teslas.

This PR adds a flag, use_tesla_for_magnetic_field, to the MagnetometerSystem to enable this fix as to not break any existing functionality that assumes Gauss. It also adds the Configure step to the MagnetometerSystem.

While I was here, I noticed that the integration test doesn't actually update the MagnetometerSystem because the SphericalCoorindates are not set. I think this might be an oversight. I did not change that existing test in case it was like that on purpose.

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.

@github-actions github-actions bot added the 馃幍 harmonic Gazebo Harmonic label Mar 17, 2024
test/integration/magnetometer_system.cc Show resolved Hide resolved
test/integration/magnetometer_system.cc Show resolved Hide resolved
test/integration/magnetometer_system.cc Show resolved Hide resolved
test/integration/magnetometer_system.cc Outdated Show resolved Hide resolved
test/integration/magnetometer_system.cc Show resolved Hide resolved
test/integration/magnetometer_system.cc Outdated Show resolved Hide resolved
test/integration/magnetometer_system.cc Outdated Show resolved Hide resolved
@gilborty
Copy link
Author

Thank you for the review @ahcorde, I think I updated everything you asked for. Thank you for the helper function point out as well!

@gilborty gilborty requested a review from ahcorde March 18, 2024 17:32
@gilborty
Copy link
Author

Are the INTEGRATION_triggered_publisher tests failing on CI for anyone else? Mine are passing locally, I don't think it is related to this commit.

[       OK ] TriggeredPublisherTest.StringMsgReqBooleanRep (1053 ms)
[----------] 23 tests from TriggeredPublisherTest (13137 ms total)

[----------] Global test environment tear-down
[==========] 23 tests from 1 test suite ran. (13137 ms total)
[  PASSED  ] 23 tests.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
馃幍 harmonic Gazebo Harmonic
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

Magnetometer system publishes field strength in incorrect units
2 participants