-
Notifications
You must be signed in to change notification settings - Fork 357
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -315,15 +315,24 @@ func makeTrafficReportMsg(ti TrafficInfo) []byte { | |
// | ||
// Algo example at: https://play.golang.org/p/VXCckSdsvT | ||
// | ||
var alt int16 | ||
if ti.Alt < -1000 || ti.Alt > 101350 { | ||
alt = 0x0FFF | ||
// GDL90 expects barometric altitude in traffic reports | ||
var baroAlt int32 | ||
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 commentThe reason will be displayed to describe this comment to others. Learn more.
simplifies to
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 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 Have you seen many aircraft that come through with There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
} else { | ||
baroAlt = ti.Alt | ||
} | ||
var encodedAlt int16 | ||
if baroAlt < -1000 || baroAlt > 101350 { | ||
encodedAlt = 0x0FFF | ||
} else { | ||
// output guaranteed to be between 0x0000 and 0x0FFE | ||
alt = int16((ti.Alt / 25) + 40) | ||
encodedAlt = int16((baroAlt / 25) + 40) | ||
} | ||
msg[11] = byte((alt & 0xFF0) >> 4) // Altitude. | ||
msg[12] = byte((alt & 0x00F) << 4) | ||
msg[11] = byte((encodedAlt & 0xFF0) >> 4) // Altitude. | ||
msg[12] = byte((encodedAlt & 0x00F) << 4) | ||
|
||
// "m" field. Lower four bits define indicator bits: | ||
// - - 0 0 "tt" (msg[17]) is not valid | ||
|
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.