-
Notifications
You must be signed in to change notification settings - Fork 365
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
GDL90: traffic report convert GNSS altitude #755
base: master
Are you sure you want to change the base?
Conversation
GPL90 traffic reports expect barometric altitude. Convert GNSS altitudes in traffic information to barometric altitude. Estimate barometric altitude from reported GPS geoid height, own geoid separation and barometric pressure. Signed-off-by: PepperJo <[email protected]>
ping |
alt = 0x0FFF | ||
// GDL90 expects barometric altitude in traffic reports | ||
var baroAlt int32 | ||
if ti.AltIsGNSS { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&& isGPSValid() && isTempPressValid()
.
Not all Stratux builds have GPS units and/or pressure sensors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. Should be discussed what to do then since the reported traffic altitude could be quite a bit off if we fall back to not adjusting it at all.
if ti.AltIsGNSS { | ||
// Convert from GPS geoid height to barometric altitude | ||
baroAlt = ti.Alt - int32(mySituation.GPSGeoidSep) | ||
baroAlt = baroAlt - int32(mySituation.GPSAltitudeMSL) + int32(mySituation.BaroPressureAltitude) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
baroAlt += -mySituation.GPSGeoidSep - mySituation.GPSAltitudeMSL + mySituation.BaroPressureAltitude
simplifies to
baroAlt += -mySituation.GPSHeightAboveEllipsoid + mySituation.BaroPressureAltitude
Seems like an interesting estimation to convert HAE to pressure altitude using local data.
One thing that is missed here is how data is passed to stratux from dump1090:
https://github.com/stratux/dump1090/blob/master/net_io.c#L569-L582
HAE delta is received and passed over as GnssDiffFromBaroAlt
.
I think this PR could be rolled up into a one line change on stratux/dump1090:
https://github.com/stratux/dump1090/blob/13d61a9a3276de26322b88c99a7426a5625f592b/net_io.c#L575
Removing the Modes.use_hae
condition (seems unexpected to only pass the GNSS-derived altitude when the delta is available).
Have you seen many aircraft that come through with AltIsGNSS
being true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't seen any ADS-B traffic where this is set however the FLARM forks use this since FLARM reports GPS altitude. The flag was already there but was not handled. I propose we either handle this in the FLARM code and remove the flag and make clear that altitude is barometric here or we convert it.
What's the verdict on this pull request? Do we want it or do we rely on every module feeding data to do their own conversion? If so we should at least clarify that the AltIsGNSS does not do any conversion but sends the data as is via GDL90 which expects baro alt. Or remove the flag as suggested above. |
GPL90 traffic reports expect barometric altitude. Convert
GNSS altitudes in traffic information to barometric altitude.
Estimate barometric altitude from reported GPS geoid height,
own geoid separation and barometric pressure.
Signed-off-by: PepperJo [email protected]