HelTecAutomation / CubeCell-Arduino

Heltec CubeCell Series (based on ASR6501, ASR6502 chip) Arduino support.
247 stars 138 forks source link

Over size user packet is delivered to the network if a network MAC request response is being delivered to the network #135

Open leroyle opened 3 years ago

leroyle commented 3 years ago

This was previously detailed in the forum.

The issue: I am seeing a user payload whose size is greater than allowed by the currently defined data rate, DR_, that is still being delivered to the network server.

This will occur if the LoRaMac runtime is processing a response to a network server request, in this case a SRV_MAC_LINK_ADR_REQ was returned during the Rx window of a previous uplink message.

The runtime is preparing a response by adding the mac command response data to the message that is to be delivered to the network server during the next uplink.

In this case during the next uplink the “user packet”, which again violates the DR_ maximum, is delivered to the network server along with what is apparently the MAC response.

Previous and follow on uplink messages of the same packet size are flagged as invalid and not delivered, only the one containing the MAC response is sent with the user packet intact.

Is this correct LoRaWan behavior, deliver the user packet even though it violates DR_ maximum?

The scenario (RegionUS915)

Default Data Rate = 3 end node user app overrides Data Rate, DR = 0, (implies max user packet of 11 bytes) end node user app is in a loop sending a packet that is size 24, (yes, too large for DR_0)

-Join attempt – network join attempts alternate between DR_0 and DR_3, In this scenario the join succeeds with DR_3.

- first user uplink attempt – the first packet length error is not caught by the LoRaWan library level’s max length test because DR_3 is used as the compare DR ( this does not look correct either) – LoRWan library layer resets the Data Rate to the correct DR_0 before passing the packet buffer off to lower layers (this is after the length check) – LoRaMac layer catches the user packet excess length error and aborts the send, nothing is sent to the network server.

- Next user uplink attempt – this attempt fails the packet length check in LoRaWan lib layer, as now the data rate used in the compare is DR_0 – user packet length is set to 0 – zero length user packet is sent to the LoRMac layer and on to the network server – network server decodes the fact the packet is zero length – at this point the network server is returning (downlink) a SRV_MAC_LINK_ADR_REQ to the end node during the Rx time. – the response is partially packaged for delivery during the next uplink transmission.

- Next user send attempt – now the response to the SRV_MAC_LINK_ADR_REQ is returned to the network server, – this time the LoRaWan lib packet length test passes because fOptsLen is not zero, (though I’m not sure why it should be passing??)

– now the response to the SRV_MAC_LINK_ADR_REQ is completed, mac data is added to the packet to be sent – the send is successful, mac data plus user packet data – the network server successfully extracts out the user packet.

This does not seem correct, that the user packet data is sent even though it exceeds the max length for the given DR_.

It seems the size check within LoRaMac.c - ValidatePayloadLength() may not be correct

actual data: user payload size lenN: 24, fOptsLen: 4, maxN: 11,

The code: payloadSize = userPayload + fOptsLen;

( payloadSize > maxN ) && (fOptsLen != 0) && (fOptsLen <= maxN)) || ( payloadSize <= maxN )) && ( payloadSize <= LORAMAC_PHY_MAXPAYLOAD )

Should this size check pass in this instance?

( is this part correct?? “payloadSize > maxN”, should not that be “payloadSize < maxN” ?

leroyle commented 3 years ago

Unfortunately just changing “>” to “<” exposes more issues. While that will prevent the delivery of the over sized user packet. The MAC response is then not properly delivered. This MAC response should be tested if the above changes are considered.

leroyle commented 3 years ago

After learning and bit more and thinking a bit more, it's probably acceptable to exceed the DR specified maximum if processing a MAC response because:

  1. we do not wish to lose user data
  2. these will probably not occur all that often in the normal course of operation.

However. there is still the case of when the packet exceeds the maximum DR size and no MAC response is being processed, in this instance the runtime sends a zero length packet each time a send of greater than max size is attempted. That seems unnecessary and unproductive.

Heltec-Aaron-Lee commented 3 years ago

Why do not you set the US915_TX_MIN_DATARATE in RegionUS915.h to an appropriate value that the payload length is bigger than your data

leroyle commented 3 years ago

This would work for a one time app. It would break any continuous integration if multiple apps had different minimum requirements. And it is not customary to be required to perform support library code changes.

It is my wish that this runtime has a bit more robustness, thus this issue and #134 as well as forum comments. Some of these have tripped up some of our community member that are new to CubeCell and LoRaWan in general.

Some wishes:

I'm not trying to pretend I'm a LoraWan expert, obviously that is not the case, Just trying to help improve robustness.

I will present a pull request in the next day or so with suggested changes that seem to solve these with my limited testing. They are small changes and of course may not be implemented totally correctly or completely.

I would encourage you to consider them but in the end the choice to support these is yours. Of course if these suggests are not consistent with the LoRaWan spec (entirely possible) or Heltec implementation intentions then by all means we can close these out as working as intended. That would be acceptable as well.

Do let me know if I can help to further explain these concerns.

leroyle commented 3 years ago

I have just created pull request #142 for consideration.