Skip to content

AP_GPS: log altitude above ellipsoid instead of undulation #29904

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

Merged
merged 1 commit into from
Apr 29, 2025

Conversation

tpwrules
Copy link
Contributor

ArduPilot's internal definition of undulation is reversed in sign compared to the accepted convention and this creates confusion: #29199 .

Instead of the internal definition, log the altitude above ellipsoid, which ArduPilot already consistently and correctly calculates and broadcasts over e.g. DroneCAN and MAVLink. Switching the quantity logged allows users to accommodate the problem in old logs (i.e. always flip GPA.Und sign to match the accepted convention) and reduces opportunity for future confusion.

Future work will clean up or replace the internal representation, but this should be a complete fix for the issue from a user perspective.

Tested using u-blox, u-blox over AP_Periph over DroneCAN, and NovAtel.

Geoid information using EGM96 from my (approximate) back yard showing an ellipsoid height of 60.57m, an undulation of -28.35m, and a mean sea level height of 88.92m:
Screenshot from 2025-04-26 15-43-09

Dataflash log from a u-blox NEO-M9N, values match well (receiver-calculated undulation is -30.07m):
Screenshot from 2025-04-26 15-42-24

Corresponding telemetry log:
Screenshot from 2025-04-26 15-42-20

@tpwrules
Copy link
Contributor Author

Ran into minor confusion over why I had to change the datatype to include * 100 instead of having just a multiplier of 1e-2.

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

LGTM

ArduPilot's internal definition of undulation is reversed in sign
compared to the accepted convention and this creates confusion:
ArduPilot#29199 .

Instead of the internal definition, log the altitude above ellipsoid,
which ArduPilot already consistently and correctly calculates and
broadcasts over e.g. DroneCAN and MAVLink. Switching the quantity logged
allows users to accommodate the problem in old logs (i.e. always flip
GPA.Und sign to match the accepted convention) and reduces opportunity
for future confusion.

Future work will clean up or replace the internal representation, but
this should be a complete fix for the issue from a user perspective.
@rmackay9 rmackay9 force-pushed the pr/log-alt-ellipsoid branch from caba97d to ca6dc6a Compare April 29, 2025 06:45
@rmackay9
Copy link
Contributor

I've rebased this on master so hopefully it will pass CI now

@rmackay9 rmackay9 merged commit 9cfa8a1 into ArduPilot:master Apr 29, 2025
102 checks passed
@rmackay9
Copy link
Contributor

this is included in 4.6.0-beta6

@tpwrules tpwrules deleted the pr/log-alt-ellipsoid branch April 29, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Pending
Development

Successfully merging this pull request may close these issues.

5 participants