Implement McpsRetransmit primitive to notify the application of uplink retransmissions

[janakj]

I am working on an application that needs to be notified of uplink retransmissions. Since I haven't found a way to obtain retransmission notifications from vanilla LoRaMac-node, I have created a new primitive called McpsRetransmit that is invoked by the MAC on each retransmission.

This is a draft PR to solicit feedback from LoRaMac-node authors. If you would like to include such functionality in LoRaMac-node, please feel free to merge the PR, or let me know if you would like to have this implemented in some other way. I am not overly attached to the current implementation.

[mluis1]

Thanks for the proposition.

I think that we should provide this information using McpsIndication instead of creating a new callback.

To the McpsIndication_t we would add bool IsUplinkTxRetransmit a shown below:

Click to expand!

/*!
 * LoRaMAC MCPS-Indication primitive
 */
typedef struct sMcpsIndication
{
    /*!
     * MCPS-Indication type
     */
    Mcps_t McpsIndication;
    /*!
     * Status of the operation
     */
    LoRaMacEventInfoStatus_t Status;
    /*!
     * Multicast
     */
    uint8_t Multicast;
    /*!
     * Application port
     */
    uint8_t Port;
    /*!
     * Downlink datarate
     */
    uint8_t RxDatarate;
    /*!
     * Frame pending status
     */
    uint8_t IsUplinkTxPending;
    /*!
     * Indicate if the uplink was a re-transmission
     */
    bool IsUplinkTxRetransmit;
    /*!
     * Pointer to the received data stream
     */
    uint8_t* Buffer;
    /*!
     * Size of the received data stream
     */
    uint8_t BufferSize;
    /*!
     * Indicates, if data is available
     */
    bool RxData;
    /*!
     * Rssi of the received packet
     */
    int16_t Rssi;
    /*!
     * Snr of the received packet
     */
    int8_t Snr;
    /*!
     * Receive window
     */
    LoRaMacRxSlot_t RxSlot;
    /*!
     * Set if an acknowledgement was received
     */
    bool AckReceived;
    /*!
     * The downlink counter value for the received frame
     */
    uint32_t DownLinkCounter;
    /*!
     * The device address of the frame
     */
    uint32_t DevAddress;
    /*!
     * Set if a DeviceTimeAns MAC command was received.
     */
    bool DeviceTimeAnsReceived;
    /*!
     * Response timeout for a class b or c device when a
     * confirmed downlink has been received. In all other
     * cases this variable is 0.
     */
    TimerTime_t ResponseTimeout;
}McpsIndication_t;

@djaeckle what do you think?

[janakj]

Hi @mluis1, IsUplinkTxRetransmit sounds like a good idea to me. Please let me know if you would like me to update the PR.

[mluis1]

@janakj I forgot to provide an answer here. Sorry.

Yes, please.

[janakj]

Hi @mluis1, so I was looking into this and I am wondering if invoking the McpsIndication primitive for uplink retransmissions is the right thing to do. When I look at the documentation and the various fields in McpsIndication_t, it seems to me that this primitive has been primarily intended to notify the application of downlinks. There are fields like Rssi, Snr, and RxData, none of which could be populated with a value for uplink retransmissions.

Do we really want to overload the McpsIndication primitive for uplink retransmission notifications? Conceptually, this might be hard to understand for application developers if they assume that McpsIndication is invoked whenever something has arrived from the network.

It may also break existing applications. Every application using the McpsIndication primitive would need to be updated to check IsUplinkTxRetransmit flag in the callback function. Without the update, the callback might erroneously assume that it's being called for a downlink, while it is in fact being notified of an uplink retransmission.

What do you think?

[mluis1]

Hi @janakj Thanks for the feedback.

With @djaeckle we have discussed you proposition and we would like to avoid adding a new callback as originally proposed.
However we agree that using McpsIndication would potentially bring confusion.

As we would like to keep the MAC layer handling as close as possible to the original specification LoRaMac-node-doc-v4.6.0

We would like to propose instead to use MlmeIndication callback. As per original specification the MLME primitives are meant to manage the LoRaWAN network. While MCPS requests are meant to handle transmissions/receptions.

Reporting the re-transmissions states/events fall under the MLME category.

Therefore our proposition would be to add a MLME_RETRANSMISSION to Mlme_t type and NbTransCounter to MlmeIndication_t type.

Would you be OK with this compromise?

diff --git a/src/mac/LoRaMac.h b/src/mac/LoRaMac.h
index de52e9a2..4a0684c4 100644
--- a/src/mac/LoRaMac.h
+++ b/src/mac/LoRaMac.h
@@ -1096,6 +1096,7 @@ typedef struct sMcpsIndication
  * \ref MLME_DERIVE_MC_KE_KEY   | YES     | NO         | NO       | YES
  * \ref MLME_DERIVE_MC_KEY_PAIR | YES     | NO         | NO       | YES
  * \ref MLME_REVERT_JOIN        | NO      | YES        | NO       | NO
+ * \ref MLME_RETRANSMISSION     | NO      | YES        | NO       | NO
  *
  * The following table provides links to the function implementations of the
  * related MLME primitives.
@@ -1203,6 +1204,10 @@ typedef enum eMlme
      * \remark The upper layer is required to trigger the Join process again.
      */
     MLME_REVERT_JOIN,
+    /*!
+     * Indicates that an uplink retransmission has been executed
+     */
+    MLME_RETRANSMISSION,
 }Mlme_t;
 
 /*!
@@ -1366,6 +1371,7 @@ typedef struct sMlmeConfirm
 /*!
  * LoRaMAC MLME-Indication primitive
  */
+// TODO: In future major release use an union around the indication parameters \ref BeaconInfo and \ref NbTransCounter.
 typedef struct sMlmeIndication
 {
     /*!
@@ -1381,6 +1387,11 @@ typedef struct sMlmeIndication
      * status \ref LORAMAC_EVENT_INFO_STATUS_BEACON_LOCKED
      */
     BeaconInfo_t BeaconInfo;
+    /*!
+     * Provides the current number of retransmissions that have been executed
+     * Only valid for \ref MLME_RETRANSMISSION
+     */
+    uint8_t NbTransCounter;
 }MlmeIndication_t;
 
 /*!

[janakj]

Hi @mluis1, I like this idea! Conceptually, I think it fits nicely with all the other uses of MlmeIndication. I will update the PR. Thanks.