eclipse-hono / hono

Eclipse Hono™ Project
https://eclipse.dev/hono
Eclipse Public License 2.0
452 stars 137 forks source link

(Actility) LoRaWAN adapter does not pass through the metadata #1190

Closed BobClaerhout closed 5 years ago

BobClaerhout commented 5 years ago

The actility LoRaWAN adapter does not passtrough the meta data received from the actility LoRaWAN provider. I think it would be nice to have at least the possibility to analyze meta data in the backend application. When it is not provided, you do not have the possibility to analyze it. I suggest adding this data in the headers of the message posted on the AMQP network. We will implement this anyway but I would rather push this to the upstream as well. WDYT?

sophokles73 commented 5 years ago

@DanielMaier-BSI @mbaeuerle any thoughts?

BobClaerhout commented 5 years ago

I had a closer look into this and we do not completely understand why this is done. A lot of interesting data concerning the LoRaWAN network is omitted. This data can be used to:

Besides this, when using the proximus adapter, you can change the stream definition in the following screen: image

When adding other fields or renaming the fields in this stream definition, they are dropped in the adapter. Therefore it undoes the changes done by the Proximus network provider. Our proposal: the deviceId can be retrieved from the message and used to push the message to the backend application but the message should be forwarded completely to the backend application. For backwards compatibility we can keep the Payload field and have a resulting json as follows (example is with message from actility):

{
    "payload": "payloadBase64",
    "originalMessage":
        {
            "DevEUI_uplink": {
                "Time": "2019-02-27T14:48:35.544+01:00",
                "DevEUI": "actility-device",
                "FPort": "1",
                "FCntUp": "548",
                "ADRbit": "1",
                "MType": "2",
                "FCntDn": "43",
                "payload_hex": "00",
                "mic_hex": "922d4454",
                "Lrcid": "00000119",
                "LrrRSSI": "-48.000000",
                "LrrSNR": "3.000000",
                "SpFact": "11",
                "SubBand": "G1",
                "Channel": "LC2",
                "DevLrrCnt": "1",
                "Lrrid": "18035559",
                "Late": "0",
                "LrrLAT": "53.108805",
                "LrrLON": "9.193430",
                "Lrrs": {
                "Lrr": [
                    {
                    "Lrrid": "18035559",
                    "Chain": "0",
                    "LrrRSSI": "-48.000000",
                    "LrrSNR": "3.000000",
                    "LrrESP": "-44.334920"
                    }
                ]
                },
                "CustomerID": "43347311",
                "CustomerData": {
                "alr": {
                    "pro": "LORA/Generic",
                    "ver": "1"
                }
                },
                "ModelCfg": "0",
                "InstantPER": "0.000000",
                "MeanPER": "0.000003",
                "DevAddr": "A53925E2",
                "AckRequested": "0",
                "rawMacCommands": ""
            }
        }
}
DanielMaier-BSI commented 5 years ago

The initial idea was to have the same payload for all providers, i.e. the application needs not to be provider aware. We also discussed to add the original message additionally but no of our users needed it so far. So it would be fine to us to add the original message as long as there is still some unified part among all providers. Perhaps we find also some other fields which are worth to be part of the unified part. However our users only needed payload so far.

BobClaerhout commented 5 years ago

Ok, do you agree with the proposed solution in the PR? Later on we can agree on adding more fields.

DanielMaier-BSI commented 5 years ago

As long as we are sure all providers send us JSON objects (I think so), I agree with the solution. Perhaps one extension would be to introduce a configuration property for the adapter if the original message should be included or not (If somebody wants to save bandwidth or whatever). But I for us this is not a hard requirement as for now.

sophokles73 commented 5 years ago
  1. My understanding is that all devices communicate via LoRa, so I would assume that much of the meta data represents LoRa specific parameters/information. If that is true, then I would also assume that it should be possible to extract this data from the network provider specific messages and normalize it reasonably so that it can be put into the downstream messages (e.g. in AMQP application properties or in a special JSON property of the message body).
  2. In order to give the downstream consumer a chance of knowing how to interpret the message body and which data format and structure to expect, it could be very helpful to use network provider specific content-types which are set by the LoRaWAN adapter.
BobClaerhout commented 5 years ago
  1. My understanding is that all devices communicate via LoRa, so I would assume that much of the meta data represents LoRa specific parameters/information. If that is true, then I would also assume that it should be possible to extract this data from the network provider specific messages and normalize it reasonably so that it can be put into the downstream messages (e.g. in AMQP application properties or in a special JSON property of the message body).

I agree but I think the original message is still required. In the Proximus example, you can add custom fields. Hono will never be able to normalize these fields since they are custom. Nevertheless, we can normalize some fields.

  1. In order to give the downstream consumer a chance of knowing how to interpret the message body and which data format and structure to expect, it could be very helpful to use network provider specific _content-type_s which are set by the LoRaWAN adapter.

Great suggestion!

BobClaerhout commented 5 years ago

I've updated the PR with the content-type addition. I also had a look at which fields might be candidate to normalize the meta-data but I do not know the exact content of all providers.
I had a look at the messages used for the tests and found following kerlink message:

{
  "devEui": "myBumluxDevice",
  "userdata": {
    "payload": "YnVtbHV4"
  }
}

Since this message doesn't have any metadata, I presume we cannot normalize the metadata.
@DanielMaier-BSI, can you provide examples for all providers where meta-data is included? If not, I propose we postpone the creation of normalized meta-data.

DanielMaier-BSI commented 5 years ago

In the tests we only included the fields we needed. For message format of the providers please refer to their docs:

BobClaerhout commented 5 years ago

Thanks @DanielMaier-BSI for the overview of these messages. However, since I do know little about these Kerlink messages I'm afraid we cannot take a decision on the normalization of the data

DanielMaier-BSI commented 5 years ago

Well I do have the kerlink docs as a PDF. Unfortunately it is not a single example json which I can post, the information is distributed across different tables. I am not sure if I am allowed to post the whole PDF here in public. If you like I can send it to you per mail.

BobClaerhout commented 5 years ago

Maybe we should avoid any problems concerning the distribution of those documents? Besides this, I propose to postpone the decision on this as currently nobody (or at least we) needs this. Should we create another issue for this?

DanielMaier-BSI commented 5 years ago

I would be fine with another issue.

sophokles73 commented 5 years ago

I have to admit that I do not feel very comfortable with the approach for the AMQP message body. In the original code, the adapter defined the payload as a JSON structure like

{
  "payload": "......."
}

where the payload was a Base64 encoding of the binary payload that the LoRa device had sent. The other protocol adapters all simply put the payload from the device into the downstream AMQP message body as a byte array. FMPOV, and for consistency reasons, the LoRaWAN adapter should (by default) do the same thing, i.e. put the Base64 decoded payload from the device into the message body. In this case, there should be any need to set a special protocol adapter specific content type as we do the same thing as all other adapters do.

I do understand the desire to be able to take advantage of the meta data provided by the network providers in their messages and I see several options to do so:

  1. Instead of the plain binary data that constitutes the payload sent by the device itself, put the whole message as it was received from the network provider into the downstream AMQP message. This will often be a JSON structure that contains the Base64 or Hex encoded binary payload from the device. In this case, the LoRaWAN adapter should not extract the payload from the message received from the network provider but simply forward the JSON as is. However, in order to indicate this fact to downstream consumers, the LoRaWAN adapter specific content-type could be set. A downstream consumer can then use the lora_provider application property to determine how the body needs to be parsed.
  2. The meta data properties (excluding the binary payload from the device) included by the network provider, are included in the downstream AMQP message as a JSON structure in a single application property (e.g. lora_provider_meta_data). A downstream consumer can retrieve the meta data from this property and determine how to parse it by means of the lora_provider property value.

However, my understanding is that some network providers even allow you to define the properties it should include in its messages, thus resulting in hard to predict message formats for downstream consumers.

In any case, I would like to get the meta data out of the downstream message body. I can also imagine to use a (tenant or device) level property in order to determine, if and how the meta data should be included.

WDYT?

BobClaerhout commented 5 years ago

To be honest, it was a surprise for me when I noticed the message received in Hono is not the one going out. As you know, we also use Dash7 (protocol) with Dash7 gateways besides LoRaWAN. These gateways append metadata (similar to the LoRaWAN network provider). This combined message is sent to the MQTT adapter and this exact message is sent to the business application. FMPOV every gateway/network provider should add this metadata since it is something you want to visualize or even react on. This is really important data in installations. If we would follow the same principles as used by the current LoRaWAN adapters, we would have to decode the Dash7 message in Hono and extract the payload from the device out of it. This is certainly an option but this will result in a lot of extra adapters. that being said, I would go for option 1. I think Hono should not change anything about the received message. The only thing we could expect from Hono to do, is adding more data which could make it easier to decode the message in the business application. Thus, adding the content-type in the headers would be perfect. I think option 2 is indeed a hard one to achieve since e.g. Proximus allows you to completely change the message.

sophokles73 commented 5 years ago

@BobClaerhout thanks for your feedback. I agree that option 1 is probably easier to do. However, my proposal is not to replace the existing behavior with option 1 but instead replace the existing behavior with including the plain (binary) payload from the device in the downstream message. This should be the default behavior of the LoRaWAN adapter.

In addition, the adapter should support a configuration option (at the adapter and/or tenant level) which makes the adapter use option 1 instead (for all messages or the messages from devices of the corresponding tenant only).

Downstream consumers can determine which payload to expect by means of checking for the presence of the content-type indicating the full message as received from the network provider.

Does that make sense?

mhemmeter commented 5 years ago

The current LoRaWAN Protocol Adapter normalizes the messages that are sent from the different network providers to Hono in order to be able to develop the business application in a network provider agnostic manner. For the business application this is very helpful as Hono abstracts the device connectivity not only with respect to the transport layer protocol but also the LoRa network providers.

We cannot provide this feature anymore if we switch to option 1. Even if we only change the existing behavior (as proposed above by @sophokles73) the business application needs to know how a specific network provider encodes the binary payload (Base64 or Hex) or needs to find out how it was encoded processing the message.

From my perspective there is one difference to the HTTP or MQTT Protocol Adapter: In case of the LoRaWAN protocol adapter we know the payload format of the incoming message as it is defined by the LoRa network provider we integrate with. So why don't we use that knowledge and provide a uniform format across LoRa network providers towards the business application and even encode the binary payload in a uniform manner? That was the idea behind the current implementation and I still think this makes sense. We can of course add additional meta information from the network provider in the AMQP message as discussed at the beginning of this thread.

sophokles73 commented 5 years ago

We cannot provide this feature anymore if we switch to option 1. Even if we only change the existing behavior (as proposed above by @sophokles73) the business application needs to know how a specific network provider encodes the binary payload (Base64 or Hex) or needs to find out how it was encoded processing the message.

I agree that the adapter knows about the encoding that the network provider uses for the payload. Thus, the adapter can extract the payload, decode it if necessary (hex or Base64) and put it to the downstream message's body. Downstream consumers can always expect the payload to be exactly the data that the device has sent. I do not see any problem there.

In case of the LoRaWAN protocol adapter we know the payload format of the incoming message as it is defined by the LoRa network provider we integrate with.

That seems to be only partially true as the Proximus provider allows complete customization of the message format as pointed out by @BobClaerhout already.

So why don't we use that knowledge and provide a uniform format across LoRa network providers towards the business application and even encode the binary payload in a uniform manner?

That is what I had proposed earlier wrt normalizing the downstream message's payload format. However, for that we would need to have corresponding documentation of the payload format used by the network providers which doesn't seem to be available in any case. I still sympathize with the idea, though.

BobClaerhout commented 5 years ago

I think we are slowly moving towards the current solution: including a normalized payload AND the original message. I agree this isn't the prettiest solution but it covers both requirements. We, as a technical user, are very much interested in the communication meta data. An end-user is mostly interested in the actual (normalized) data and is not interested in which LoRa provider is handling the communication and providing the data.
As for the Proximus provider: I suggest we design the adapter so that it can handle the default data from Proximus. If someone changes the payload using the Proximus tool, he/she should know what he/she is doing and is therefore responsible for the changes. I suggest we try to parse the payload using the default one and make it failsafe so that the message is still forwarded even when the payload is not found. To conclude: I propose following format:

{
  "NormalizedMessage": {
    "Payload": "Base64Payload",
    "Rss": "rss",
     ...
  },
  "OriginalMessage": {
    "Payload": "OriginalPayload",
    "RssByAnotherName": "rss",
    "ProximusSpecificTag": "foo",
    ...
  }
}

Although I fully agree with @sophokles73 this feels a bit odd, I think this serves well for both worlds.

mhemmeter commented 5 years ago

Great, looks like we will come to a conclusion short term. @DanielMaier-BSI, @mbaeuerle any other ideas or input? @BobClaerhout, I have another question to you: Currently the LoRaWAN Protocol Adapter does not cover Command & Control for all network providers (downstream communication in LoRa). Do you have that use case in your system at all? If yes do you think you can cover that in the current implementation with one adapter supporting several LoRa network providers? How do you think will the messages look like in that use case?

BobClaerhout commented 5 years ago

I've created an issue for this: https://github.com/eclipse/hono/issues/1226 I saw a downlink implementation for kerlink. I'm currenlty using the actility provider and therefore implementing that one already. I'm starting from the Kerlink adapter. The messages we are sending to our devices are binary messages. As for the other providers: I do not know enough of the others to state whether it is possible to implement it the same way. If I would have to guess, I think it is possible.

Regarding this issue: do we keep the PR like it is now? Or should we change it already and add another level (as proposed in my previous comment). What do you think?

sophokles73 commented 5 years ago

think we are slowly moving towards the current solution: including a normalized payload AND the original message. I agree this isn't the prettiest solution but it covers both requirements.

Is there a particular reason why you bring up this approach again, @BobClaerhout? FMPOV there is no reason why we would need to go that way instead of what I had proposed earlier, or is there?

BobClaerhout commented 5 years ago

If my understanding is correct, we are talking about 3 options now: Option 1: forward message as received from network provider:

{
    "Payload": "OriginalPayload",
    "RssByAnotherName": "rss",
    "ProximusSpecificTag": "foo"
  }

Pro's:

Con's:

Option 2: Payload is normalized and meta-data (excluding payload) is passed in message property

"Base64Payload"
Message property containing:
{
    "RssByAnotherName": "rss",
    "ProximusSpecificTag": "foo"
}

Pro's:

Con's

Option 3: Payload and metadata is normalized where possible and original message is sent as property on json object.

{
  "NormalizedMessage": {
    "Payload": "Base64Payload",
    "Rss": "rss"
  },
  "OriginalMessage": {
    "Payload": "OriginalPayload",
    "RssByAnotherName": "rss",
    "ProximusSpecificTag": "foo"
  }
}

Pro's:

Con's

While I was summing up the 3 approaches, I noticed option 2 and 3 are indeed quite similar. The only difference is where we put the metadata. I'll leave the list here anyway so you can check whether I made any mistakes.

To me, it makes no difference on how the metadata is available, as long as it is available. In option 2 the normalized payload is available as well. So, indeed, option 2 in this list ( equals to option 2 in your proposal) might be the best option after all.

Additionally, we can add a "normalized metadata" message property as well.

mhemmeter commented 5 years ago

Based on the discussion in this issue I vote for option 2.

BobClaerhout commented 5 years ago

We had a discussion on this last Friday in the F2F. A short recap of this: Option 2 is the preferred one. Additionally, we will try to get as much normalized data out of the message as possible. A resulting json of data which can not be normalized will be placed in a message property. To conclude the resulting message:

"Base64Payload", Message properties [ "Rss": "rss", "Non-normalized-data": { "ProximusSpecificTag": "foo"
} ] I'm not happy with the property name "Non-normalized-data". Any suggestions? @ctron, since you are working on the sigfox adapter and probably need the "non-normalized"-field as well, maybe you have a suggestion.

sophokles73 commented 5 years ago

FWIW, the payload will not be Base64 encoded but will be the plain binary bytes as received from the device.

sophokles73 commented 5 years ago

what about additional_data instead of Non-normaized-data? Also keep in mind to not use the - (dash) character in application property names since they cannot be read by a JMS client (JMS does not allow dashes in property names, don't ask me why) ...

BobClaerhout commented 5 years ago

FWIW, the payload will not be Base64 encoded but will be the plain binary bytes as received from the device.

Correct, this reminds me that content-type is also set as message property in order for the business application to be able to decode the message.

what about _additionaldata instead of Non-normaized-data? Also keep in mind to not use the - (dash) character in application property names since they cannot be read by a JMS client (JMS does not allow dashes in property names, don't ask me why) ...

Fine by me. If nobody has any other suggestions, will do this.

sophokles73 commented 5 years ago

IMHO we should also use a common prefix for the normalized data property names, e.g. lora_rss and lora_eui etc.

BobClaerhout commented 5 years ago

I do not agree with this. If someone writes his own adapter, he can reuse the existing fields to send the same data to the business application. The sigfox adapter for example will also have rss values.

sophokles73 commented 5 years ago

Then use hono_ as the prefix ..

mhemmeter commented 5 years ago

Correct, this reminds me that content-type is also set as message property in order for the business application to be able to decode the message.

I would appreciate if we can avoid this (please see my comments above). It would definetly be of value if the business application must not care which LoRa network provider has sent the message. What are the arguments against using a common encoding (e.g. Base64)?

sophokles73 commented 5 years ago

What are the arguments against using a common encoding (e.g. Base64)?

There simply is no need at all for encoding the binary data when put into a Data section. An AMQP Data section contains raw bytes. That is also what the device sends: raw bytes. You only need to encode the data e.g. using Base64 if you want to put it into a JSON structure ...

BobClaerhout commented 5 years ago

Then use hono_ as the prefix ..

Ok.

mhemmeter commented 5 years ago

There simply is no need at all for encoding the binary data when put into a Data section. An AMQP Data section contains raw bytes. That is also what the device sends: raw bytes. You only need to encode the data e.g. using Base64 if you want to put it into a JSON structure ...

Understood. If that's the preferred way of the community let's go for it. However this breaks the API of our commercial product and prevents us from switching to the upstream version of the LoRaWAN Protocol Adapter short term.

BobClaerhout commented 5 years ago

Then use hono_ as the prefix ..

Ok.

In fact, I still do not agree :) Other properties (e.g.: orig_adapter, orig_address, gatewayid, ...) are not prefixed by `hono` so I would omit the prefix and just have properties like geo_location, rss, ...

sophokles73 commented 5 years ago

@BobClaerhout are you concerned about using a common prefix like hono_ in general or is it because we already have some properties that do not contain the prefix (yet), resulting in inconsistent behavior?

BobClaerhout commented 5 years ago

It is because of the inconsistent behavior

sophokles73 commented 5 years ago

But do you agree that it is helpful to use a common prefix so that we can prevent clashes between Hono defined and custom properties?

BobClaerhout commented 5 years ago

If I would have to choose, I would not use the prefix. But I do not have a strong opinion on this (hence my previous agreement). I just noticed other properties are not prefixed so in that case I wouldn't add the hono prefix for these properties neither. Regarding your comment to prevent the clashes. If somebody is adding other properties, he/she can always prefix this with something else to avoid clashing. I think a location (e.g) is always a location and if somebody has another kind of location, it should be specified. But again, not a strong opinion and willing to adapt because I'm neither a Hono expert nor a naming expert.

BobClaerhout commented 5 years ago

I've updated the PR according to this discussion. Currently the prefix is not there.