Infer location from decoded payload and publish location solved

[johanstokking]

Summary

Closes #4176

cc @jpmeijers

Changes

• Infer location from decoded payload. Took some inspiration from TTN Mapper
• General housekeeping in AS

Testing

Unit testing

Regressions

None expected

Notes for Reviewers

Only functional change is in b6863bb

Checklist

• Scope: The referenced issue is addressed, there are no unrelated changes.
• Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
• Documentation: Relevant documentation is added or updated.
• Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
• Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

[jpmeijers]

If I understand this code correctly, this will only allow these 9 possible sets of keys, and no other combinations of them. Won't it be better to check one field at a time, trying all it's possibilities, before checking the next field?

[jpmeijers]

ie, say I have the fields latitude, long and altitude, then this algorithm won't accept it.

[johanstokking]

Yes indeed. I wanted to make this explicit, to not trigger magic behavior when these keys aren't really consistent (like latitudeDeg and gps_lng)

[jpmeijers]

Yes I see your point. My opinion is slightly different: I try my best to obtain a valid location, no matter how "broken" the payload parser may be.

From TTN Mapper's side we recommend a specific set of keys:
https://docs.ttnmapper.org/FAQ.html#using-a-gps-tracker-device-transmitting-gps-coordinates-in-the-lora-payload

The JSON object should contain the keys “latitude”, “longitude” and “altitude” and one of “hdop”, “accuracy” or “sats”

Something more to note here is that sats, hdop and accuracy are handled as three different things. They are inherently three different ways to indicate how reliable the gps location is. Putting hdop into a field called "accuracy" can cause confusion, as an accuracy of 3 metres is good, but an hdop of 3 is bad.

[jpmeijers]

I still think we should split the parsing of the decoded payload out into a module which we can share, and I can help maintain. There are some edge cases that are currently not handled here.

For example a Digital Matter Yabby or Oyster device sends a fixFailed field. If this field is true, the location that is in the latitude and longitude fields is a cached location which can be very old. We've had issues where this caused invalid coverage to be draw - like a link between Melbourne and Sydney (700km) at -80dBm.

https://github.com/ttnmapper/ingress-api/blob/master/utils/payload_fields_parser.go#L195-L197 and https://github.com/ttnmapper/ingress-api/blob/master/utils/payload_fields_parser.go#L213-216

If we don't handle edge cases like these in the TTI location inference, the location-solved webhook will be useless for TTN Mapper.

[johanstokking]

Right. I see also that this codec is in the device repository; https://github.com/TheThingsNetwork/lorawan-devices/blob/master/vendor/digital-matter/oyster.js. Now, isn't that an issue with this codec? Shouldn't it become cachedLatitudeDeg? I mean, what is the point of GPS coordinates if there's no fix?

[jpmeijers]

Now, isn't that an issue with this codec? Shouldn't it become cachedLatitudeDeg? I mean, what is the point of GPS coordinates if there's no fix?

Yes agreed. But that means we need to lecture codec writers on properly naming fields. Digital Matter isn't the first, and won't be the last that does something like this. The second example was Tabs - one that can be seen as a reference implementation by some.

[johanstokking]

OK, there will be a module that we can both import. I'll change this PR to use that module.

I really want to avoid more magic in there that checks arbitrary fields whether the location is valid or not.

In the particular case of Oyster and Yabby, I would strongly prefer putting coordinates under cached and adding a warning that the fix failed. Filed TheThingsNetwork/lorawan-devices#233

[johanstokking]

@jpmeijers please see https://pkg.go.dev/go.thethings.network/lorawan-application-payload. I welcome any suggestions in https://github.com/TheThingsNetwork/lorawan-application-payload.