TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
973 stars 304 forks source link

Publish location solved event based on decoded payload #4176

Closed johanstokking closed 3 years ago

johanstokking commented 3 years ago

Summary

Publish location solved event based on decoded payload.

Why do we need this?

To channel location solving on a single channel; the location solved application upstream message type.

This is helpful for upstream subscribers to have one source to subscribe to.

Also, this decouples application payload from location data, which is better for privacy. Currently, users have to configure the TTN Mapper integration for all their uplink message data, whereas only location data is interesting for TTN Mapper. cc @jpmeijers

What is already there? What do you see now?

When there's a geolocation integration like LoRa Cloud Geolocation for LoRa Edge devices, the location_solved message type is published by Application Server.

However, when the location coded in the application payload, like GPS coordinates, the location is only made available in a non-unified way through the uplink_message message type.

What is missing? What do you want to see?

Recognize location information from decoded uplink. This is based on predefined fields in the decoded payload.

How do you propose to implement this?

In Application Server, after decoding the payload, recognize fields like latitude, longitude (degrees), altitude, hdop and accuracy (meters). CayenneLPP coordinates should also be recognized. This is to be compatible with what TTN Mapper currently needs. See https://ttnmapper.org/faq/ under "How can I contribute data".

Integrations like TTN Mapper still rely on RX metadata, which is currently not part of location_solved messages. Therefore we cannot remove uplink_message from the TTN Mapper's webhook template (https://github.com/TheThingsNetwork/lorawan-webhook-templates/blob/e03f663772ffd04682f7a3029d1f16fb79ccb628/ttnmapper.yml#L23) but I believe this should be the aim on the longer term.

I'm also curious to hearing @jpmeijers his opinion on this matter.

How do you propose to test this?

Unit testing

Can you do this yourself and submit a Pull Request?

Yes

jpmeijers commented 3 years ago

Integrations like TTN Mapper still rely on RX metadata, which is currently not part of location_solved messages.

Yes this is indeed an issue I faced recently. But currently my location_solved api is just a stub, and TTN Mapper only relies on uplink_message.

Therefore we cannot remove uplink_message from the TTN Mapper's webhook template but I believe this should be the aim on the longer term.

We will have to add the RX metadata to the location_solved object for this to work. But if that is done I am happy with this suggestion.

recognize fields like latitude, longitude (degrees), altitude, hdop and accuracy (meters). CayenneLPP coordinates should also be recognized.

Yes we can do this. TTN Mapper has a long list of keys it currently checks. There are also hdop and satellite fields, as not all systems provide the accuracy in meters.

Fields: https://github.com/ttnmapper/ingress-api/blob/902e54e908d106e370f9289d8a283922ad7ae547/types/ttnmapper_message.go#L41-L47

    Latitude       float64 `json:"latitude,omitempty"`
    Longitude      float64 `json:"longitude,omitempty"`
    Altitude       float64 `json:"altitude,omitempty"`
    AccuracyMeters float64 `json:"accuracy_meters,omitempty"`
    Satellites     int32   `json:"satellites,omitempty"`
    Hdop           float64 `json:"hdop,omitempty"`
    AccuracySource string  `json:"accuracy_source,omitempty"`

Parser for payload fields to above struct: https://github.com/ttnmapper/ingress-api/blob/master/utils/payload_fields_parser.go

There should also be a hierarchy of which location we use: https://github.com/ttnmapper/ingress-api/blob/902e54e908d106e370f9289d8a283922ad7ae547/tts/tts_handlers.go#L124

    // 1. Try to use the location from the metadata first. This is likely the location set on the console.
    // 2. If the packetIn contains a solved location, rather use that - TODO this is likely sent to the /location-solved endpoint
    // 3. If payload fields are available, try getting coordinates from there

Maybe we can make a Go package out of this that both of us use/maintain.

johanstokking commented 3 years ago

Thanks @jpmeijers for your reply. That's very helpful. I can imagine we follow a similar way to get location information from a decoded uplink message, especially for the time being. Using a common go module just for this could be useful.

We'll be adding RX metadata to location_solved; it won't always be available but when location solving is triggered by an uplink message (like most of the time), gateway metadata will be available.

One bit more challenging thing is how we deal with multi-frame LoRa/TDOA geolocation. We provide up to 16 recent uplink frames with metadata, and get one position back. So we might have lots of RX metadata, with varying RSSI and SNR values, assuming that the end device didn't physically move. How would that be made available through location_solved? I don't think we should be including all the raw metadata of 16 recent frames, but just taking an average RSSI or SNR value also doesn't feel right.

jpmeijers commented 3 years ago

assuming that the end device didn't physically move

That's a concern. How do we (and LoRaCloud for that matter) know if a device moved or not? I guess we'll see it from the location accuracy in meters that should be a higher number.

I don't think we should be including all the raw metadata of 16 recent frames, but just taking an average RSSI or SNR value also doesn't feel right.

Taking the average of multiple decibel values is tricky. So rather stay far away from that. I would rather opt for too much metadata that too little. The end service can then decide how to process it.

I'm not sure how useful these multi-frame geolocation messages are for TTN Mapper's coverage map. But for monitoring gateways, frequencies, SFs, etc. the full metadata set can be useful.

htdvisser commented 3 years ago

Pushing this to v3.13.2. I don't think @johanstokking is going to be working on this on his day off, so this definitely won't make it to today's deadline for v3.13.1.