gazebosim / gz-sim

Open source robotics simulator. The latest version of Gazebo.
https://gazebosim.org
Apache License 2.0
621 stars 251 forks source link

Magnetometer: correct field calculation #2460

Open srmainwaring opened 6 days ago

srmainwaring commented 6 days ago

🦟 Bug fix

Summary

Correct the magnitude and direction of the magnetic field calculation in the Magnetometer sensor.

Details

Context

The issue was found during the development of external sensor support for ArduPilot using DDS to subscribe to ROS based sensor topics (in this case magnetic field strength published from Gazebo via the ros_gz bridge).

ArduPilot SITL and Gazebo use similar calculations to simulate the earth's magnetic field at a given location (lat, lon). There was an inconsistency between the two calculations which can be attributed to a combination of incorrect units and frame convention.

The attached notebook reproduces both calculations in Python for comparison.

earth_mag_field.ipynb.zip

Checklist

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.

azeey commented 6 days ago

There's a stalled PR https://github.com/gazebosim/gz-sim/pull/2336 that was fixing the units. There we decided to add a flag to enable this so as to not break behavior. Can we do the same here?

srmainwaring commented 6 days ago

There's a stalled PR #2336 that was fixing the units. There we decided to add a flag to enable this so as to not break behavior. Can we do the same here?

Thanks for the link to the PR, missed that.

The problem with the approach of continuing with the wrong units and orientation by default is that it creates issues downstream - particularly once the data is pushed into the ROS data space where the assumption is that it will be standards conforming.

Perhaps we could do the following:

Add params, which for garden and harmonic default to current behaviour:

<use_units_gauss>true</use_units_gauss>
<use_earth_frame_ned>true</use_earth_frame_ned>

and mark this behaviour as deprecated. Then switch to standard conventions as default if omitted (i.e. both false) in a future release.

Updated in: e8f16285ff2515f3c060f17731d5eb0acd6cb92c

srmainwaring commented 5 days ago

Shouldn't NED be the default then?

The reasoning is that for Gazebo the world frame is ENU, so to be consistent with the orientation given by other sensors (say NavSat when spherical coordinates are provided) I think we need to use ENU when setting the mag field in the world frame.

I'll need to review REP 103 again for the ROS msgs, if the convention is NED rather than ENU that should probably be handled in the ros_gz bridge.

Update

REP 103 suggests

For short-range Cartesian representations of geographic locations, use the east north up [5] (ENU) convention:

X east Y north Z up

If magnetic field data is regarded as geographic data the default frame (no suffix) is ENU. To use the NED frame (which is what aerial vehicles use in flight controllers) the ROS frame_id would add the _ned suffix.