Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 15 additions & 6 deletions main/traffic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Owner

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.

Copy link
Contributor Author

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.

// Convert from GPS geoid height to barometric altitude
baroAlt = ti.Alt - int32(mySituation.GPSGeoidSep)
baroAlt = baroAlt - int32(mySituation.GPSAltitudeMSL) + int32(mySituation.BaroPressureAltitude)
Copy link
Owner

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?

Copy link
Contributor Author

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.

} 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
Expand Down