TheThingsNetwork / lorawan-stack

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

Update gateway location observed by Gateway Server #1815

Closed ecities closed 4 years ago

ecities commented 4 years ago

Summary

Hi. I've got questions with comes up during development.

Kerlink Wirenet +with GPS and GSM sends its position (logs in spf) however it is not automatically updated in the console (map or position) v3.3.2. Can it be automatically updated? Can GPS position be added to the JSON status of the device (for integration)?

Why do we need this?

This is good way to test range the whole system (many "static" LoRaWAN controllers and single "mobile" LoRaWAN gateway which send GPS position). ...

What is already there? What do you see now?

...

What is missing? What do you want to see?

...

Environment

...

How do you propose to implement this?

...

Can you do this yourself and submit a Pull Request?

...

johanstokking commented 4 years ago

Thanks, good suggestion.

The gateway status is observed by the Gateway Server, while the location is stored in Identity Server. To keep things simple, we might want to allow the Gateway Server to update the gateway's location with location source set to SOURCE_GPS. We would need to figure out what rights GS needs to do that. There's already an API key but intended only for linking the gateway.

@htdvisser thoughts?

htdvisser commented 4 years ago

This issue has two components:

  1. The GS automatically updating a gateway (antenna) location in the IS:
    • I think we should only do this for mobile gateways (we don't want to update the locations of thousands of fixed gateways with an inaccurate GPS location every 30 seconds). We also have another issue (#545) that might benefit from a "mobile_gateway" flag in the registration.
    • I think it's not too crazy to allow the LINK to update the gateway status and location.
  2. Propagating the gateway (antenna) location to devices.
    • It seems the GS doesn't actually inject the location of the RxMetadata (which the field comment says it should), let's start with that.
neoaggelos commented 4 years ago

For 1:

KrishnaIyer commented 4 years ago

We would need one more upstream handler (I believe using the existing ns one is not correct, since the NS is not relevant here) with a nil HandleUplink and a HandleStatus that listens for gateway status messages and updates the gateway antenna location accordingly (using SOURCE_GPS as location source).

IMO, this is the responsibility of the frontend and not the upstream handler. We already do a GET on the gateway with the LINK rights. As @htdvisser said, it would be ok to use the same rights to SET the location (in a separate short-lived go routine).

PS: We already assume LINK rights for EUI based gateways so if there needs to be separate right for setting, the GS can just assume that after all the validation passes. ACK @johanstokking

It seems the GS doesn't actually inject the location of the RxMetadata (which the field comment says it should), let's start with that.

Indeed that's a good one. This also is to be done by each of the frontends when parsing the packets.

johanstokking commented 4 years ago

IMO, this is the responsibility of the frontend and not the upstream handler.

I think it should be done by GS and not in UDP, gRPC, MQTT etc independently. However, I don't think we need an upstream handler either, but do this in GS's status message handling.

We already assume LINK rights for EUI based gateways [...]

Actually, GS doesn't have authentication to IS for UDP and unauthenticated Basic Station gateways. Currently, GS users cluster auth to get gateway information. Likewise, we need to allow updating the gateway location with cluster auth too.

neoaggelos commented 4 years ago

However, I don't think we need an upstream handler either, but do this in GS's status message handling.

This seems much more straightforward, but would that not slow down/block the status message handling considerably (updating the gateway data in IS every 30s or so)?

johanstokking commented 4 years ago

This seems much more straightforward, but would that not slow down/block the status message handling considerably (updating the gateway data in IS every 30s or so)?

Yeah it should be done async, with debounce. Status message handling is part of the hot path (traffic upstream and downstream), so we can't do external calls there.

As for the authorization; if we allow updating gateway location with cluster auth, it implies that anyone can change gateway locations by sending UDP status messages to a cluster. This is regardless of unregistered gateways; if you know the EUIs, you can change the map. It is current behavior with V2 afaik. @htdvisser any thoughts on this?

htdvisser commented 4 years ago

We should add a new gateway field called auto_update_location which tells the stack that this gateway reports its location and we want to use that.

I don't like the "auto". Maybe update_location_from_status?

If we allow updating gateway location with cluster auth, it implies that anyone can change gateway locations by sending UDP status messages

Correct. What we did in v2 was to block unauthenticated connections after a gateway connected using an authenticated connection. This didn't work out too well as some people with MultiTechs switched between UDP and MQTT. Perhaps add an extra boolean to the gateway registration (require_auth) that would require the gateway to use auth, and block UDP connections.

johanstokking commented 4 years ago

Perhaps add an extra boolean to the gateway registration (require_auth) that would require the gateway to use auth, and block UDP connections.

Yep, that is a good suggestion. In that case, IS wouldn't accept cluster auth from GS, only forwarded auth.

neoaggelos commented 4 years ago

Regarding location updates, there are two typical use-cases here:

IMO the first case is the most common, and for that we can do much better than updating location every 30 seconds. Maybe add a third option update_location_once (or a more general update_location_mode? And tell users that they should avoid using the first one unless absolutely needed?

johanstokking commented 4 years ago

Let's move the implementation discussion to #1999

But my take here is that this is so obvious to see in GS, that we shouldn't ask users to configure this, let alone add Console support with the different location update modes per gateway.