Exposing FpendingBit and SrvAckRequested to the application layer

[faridm23]

In our design we need to have access to the Fpending bit and ACK bit so that we can send an empty TX payload whenever Fpending bit is clear and ACK bit is set so that the server receives the acknowledgement message immediately without waiting till the next uplink message.

To do that, I figured I need to add changes to LoRaMac.c. Could you guys please confirm if it is Ok to go ahead and modify LoRaMac.c file to achieve our needs, or it is not something you recommend.

If it is not recommended, is there any plans that you guys would expose these bits to the application layer in the future code release?

Regards,
Farid

[mluis1]

The FPending bit is already exposed to the application layer and a transmission is scheduled as soon as possible.
Please refer to static void McpsIndication( McpsIndication_t *mcpsIndication ) function implementation on LmHandler.c file.

LoRaMac-node/src/apps/LoRaMac/common/LmHandler/LmHandler.c

Lines 755 to 761 in 05353ff

	if( ( ( mcpsIndication->FramePending == true ) && ( LmHandlerGetCurrentClass( ) == CLASS_A ) ) ||
	( mcpsIndication->ResponseTimeout > 0 ) )
	{
	// The server signals that it has pending data to be sent.
	// We schedule an uplink as soon as possible to flush the server.
	IsUplinkTxPending = true;
	}

The SrvAckRequested status is currently not exposed to the application layer. We will add this possibility as soon as possible.

[mluis1]

Issue #1144 has been added to handle this request

[faridm23]

In our application we can only access this structure LmHandlerAppData_s:

The application data structure is defined as below:

/*!
 * Application data structure
 */
typedef struct LmHandlerAppData_s
{
    uint8_t Port;
    uint8_t BufferSize;
    uint8_t *Buffer;
}LmHandlerAppData_t;

my question is: is it safe/recommended to modify this structure by adding uin8_t FramePending. Then our struct will look as below:

typedef struct LmHandlerAppData_s
{
    uint8_t Port;
    uint8_t BufferSize;
    uint8_t *Buffer;
    uin8_t FramePending; 
}LmHandlerAppData_t;

and inside the function static void McpsIndication( McpsIndication_t *mcpsIndication ), we can retrieve the FramePending value as below:

appData.FramePending = mcpsIndication->FramePending;

is this modification acceptable?

My second part of the question about the SrvAckRequested bit: can we add this else if statement as below so that an empty payload is sent to the cloud when FPending is false and the SrvAckRequested is true.

   if( ( mcpsIndication->FramePending == true ) && ( LmHandlerGetCurrentClass( ) == CLASS_A ) )
    {
        // The server signals that it has pending data to be sent.
        // We schedule an uplink as soon as possible to flush the server.

        // Send an empty message
        LmHandlerAppData_t appData =
        {
            .Buffer = NULL,
            .BufferSize = 0,
            .Port = 0
        };
        LmHandlerSend( &appData, LORAMAC_HANDLER_UNCONFIRMED_MSG );
    }
    else if( ( mcpsIndication->FramePending == false ) &&  ( Nvm.MacGroup1.SrvAckRequested == true ) && ( LmHandlerGetCurrentClass( ) == CLASS_A ) )
    {
  		// The server signals that it has pending data to be sent.
  		// We schedule an uplink as soon as possible to flush the server.        // Send an empty message
  		LmHandlerAppData_t appData =
  		{
  			.Buffer = NULL,
  			.BufferSize = 0,
  			.Port = 0
  		};
  		LmHandlerSend( &appData, LORAMAC_HANDLER_UNCONFIRMED_MSG );
    }

[mluis1]

Hope you don't mind I have edited your post in order to make it more readable.

Your proposition has some issues and most certainly will cause operational problems.

As first comment, I think that there is no need to change the LmHandlerAppData_t structure. The user final application does not have to handle MAC stack operations. The LmHandler API role is to abstract as much as possible the MAC stack handling.

Concerning the second part of your question. The issues are:

• LmHandlerSend API should not be called inside McpsIndication callback implementation.
  When LoRaMacProcess function calls McpsIndication callback it has not yet fully finalized its processing. This means that calling LmHandlerSend function may disrupt the LoRaMacProcess processing finalization.
  We can see the MAC stack callbacks as if they were hardware interrupts. Same care that is taken to handle an interrupt almost applies to the MAC stack callbacks.
  This is the reason why in the current McpsIndication callback implementation we just set a flag to indicate that there is a transmission pending (IsUplinkTxPending = true; ). This flag is then checked when LmHandlerProcess function is executed and if the flag is true an uplink is directly sent (direct transmission may depend on duty-cycle restrictions)

void LmHandlerProcess( void )
{
...
    // If a MAC layer scheduled uplink is still pending try to send it.
    if( IsUplinkTxPending == true )
    {
        // Send an empty message
        LmHandlerAppData_t appData =
        {
            .Buffer = NULL,
            .BufferSize = 0,
            .Port = 0,
        };

        if( LmHandlerSend( &appData, LmHandlerParams->IsTxConfirmed ) == LORAMAC_HANDLER_SUCCESS )
        {
            IsUplinkTxPending = false;
        }
    }
}

• The proposed else if is not necessary as it just duplicates the code of the if condition. In addition the condition
  ( ( mcpsIndication->FramePending == false ) && ( Nvm.MacGroup1.SrvAckRequested == true ) && ( LmHandlerGetCurrentClass( ) == CLASS_A ) )
  is not correct. It should look like to instead:
  ( ( ( mcpsIndication->FramePending == false ) && ( LmHandlerGetCurrentClass( ) == CLASS_A ) ) || ( Nvm.MacGroup1.SrvAckRequested == true ) || ( mcpsIndication->ResponseTimeout > 0 ) )
  An uplink is to be sent if FramePending OR SrvAckRequested are true.
• The other problem with else if condition is that you are accessing a private MAC stack variable Nvm. The stack does not expose this variable publicly and will never do it. In case the access to this variable would be required we would need to add an API to access it.

In conclusion the way to implement your feature-request as I already said in my previous post is to:

• Change the MAC stack in order to provide the SrvAckRequested variable status inside McpsIndication_t structure.

• Then the if condition inside McpsIndication callback implementation would become
  ( ( mcpsIndication->FramePending == true ) || ( mcpsIndication->SrvAckRequested== true ) || ( mcpsIndication->ResponseTimeout > 0 ) )
  I have removed the && ( LmHandlerGetCurrentClass( ) == CLASS_A ) as this check should be done inside the MAC stack implementation instead.

  Another potential improvement would be maybe to just change the MAC stack McpsIndication_t struct to only provide IsUplinkTxPending instead of FramePending and SrvAckRequested fields.
  The stack would then update IsUplinkTxPending by doing something similar to the following:

mcpsIndication->IsUplinkTxPending = FramePending || SrvAckRequested || ( ResponseTimeout > 0 );

And then McpsIndication callback implementation would become.

static void McpsIndication( McpsIndication_t *mcpsIndication )
{
    LmHandlerAppData_t appData;

    RxParams.IsMcpsIndication = 1;
    RxParams.Status = mcpsIndication->Status;

    if( RxParams.Status != LORAMAC_EVENT_INFO_STATUS_OK )
    {
        return;
    }

...

    // Call packages RxProcess function
    LmHandlerPackagesNotify( PACKAGE_MCPS_INDICATION, mcpsIndication );

     // The server signals if it has pending data to be sent. 
     // If true we schedule an uplink as soon as possible to flush the server. 
    IsUplinkTxPending = mcpsIndication->IsUplinkTxPending;
}

Its a pretty long post. Hope I have been clear enough.

We will implement the required changes as soon as possible (may take some time).

[faridm23]

Hi Mluis,

Thank you so much for your your feedback; it was comprehensive and helpful. I just want to let you that we are using this LoRaWAN L2 v1.0.3 master repo together with Nothern Mechatronics nmsdk:
https://github.com/NorthernMechatronics/nmsdk

I am saying that just because I noticed in your reply that the void LmHandlerProcess( void ) is not implemented the same way in the release we are using.

I did the changes as you recommended in LmHandler.c below. Could you please confirm if this is all to it:

if( ( mcpsIndication->FramePending == true ) || ( mcpsIndication->SrvAckRequested== true ) || ( mcpsIndication->ResponseTimeout > 0 ) )
{
// The server signals that it has pending data to be sent.
// We schedule an uplink as soon as possible to flush the server.

    // Send an empty message
    LmHandlerAppData_t appData =
    {
        .Buffer = NULL,
        .BufferSize = 0,
        .Port = 0
    };
    LmHandlerSend( &appData, LORAMAC_HANDLER_UNCONFIRMED_MSG );
}

if this is all I need, could you please confirm if this change we are doing is recommended or not? Is it an implementation that violates the standard and might cause performance issues?

Again, what we want to achieve is to send an immediate acknowledgement message to the server as soon as the device receives a confirmed downlink message ( a message that requires confirmation from the device).

Regards,
Farid