AP_GPS: fix SBF undulation usage #31997
Conversation
|
I've tested this on SBF here. We don't get ellipsoid height before, we do after. The reported altitudes are also much closer together after this PR is applied! Same testing as done on the linked PR, same results. Broken before, fixed after. |
Correct handling of ellipsoid and AMSL altitudes from SBF receivers. Previously, undulation was not applied properly, causing altitude values to be reported as elipsoid only.
|
Do you have a sample point? Latitude, longitude, AMSL, alt ellipsoid. Would like to confirm the undulation is being processed correctly. Also looks like lots of other places in the driver which should be migrated to use this function, if you have the hardware to test. |
Happy to supply logs (ping me). See the other PR for graphs made from logs. The general thing being that when this patch is applied all of the wiggly lines get much closer to the uBlox wiggly lines.
Just fixing the thing in front of me here. |
libraries/AP_GPS/AP_GPS_SBF.cpp
Outdated
|
|
||
| static bool is_DNU(double value) | ||
| { | ||
| constexpr double DNU = -2e-10f; |
|
We might need a hex literal and a reinterpret_cast or something to avoid falling afoul of our "floating point consts are singles". |
peterbarker
force-pushed
the
pr/MR-fix
branch
from
c9d65d7 to
afb7acc
Compare
|
Good news, |
tpwrules
left a comment
tpwrules
left a comment
There was a problem hiding this comment.
LGTM, undulation values are processed correctly as confirmed with private data.
Consider the comment too.
3 participants
Correct handling of ellipsoid and AMSL altitudes
from SBF receivers. Previously, undulation
was not applied properly, causing altitude
values to be reported as elipsoid only.
This is a slightly simpler version of #30971 which adds an epsilon check into the mix. Since this is a magic value we're looking for I think it makes sense to be a direct comparison.