Lora-net / SWL2001

LoRa Basics Modem LoRaWAN stack
BSD 3-Clause Clear License
104 stars 60 forks source link

Potential Bug: FMP Unresponsiveness after Receiving multiple FMP_DEV_REBOOT_TIME_REQ msg's #48

Closed MpmTwtg closed 5 months ago

MpmTwtg commented 7 months ago

FMP does not respond after it got a valid FMP_DEV_REBOOT_TIME_REQ. The first time it responds with an answer and after that not.

https://github.com/Lora-net/SWL2001/blob/ffca22633d111ef2874fa7c2f35498a57d87f3f0/lbm_lib/smtc_modem_core/lorawan_packages/firmware_management_protocol/lorawan_fmp_package.c#L307

my proposal is to remove this from line 321:

    case ANS_CMD_TASK_MASK: {
        ctx->fmp_task_ctx_mask &= ~( ANS_CMD_TASK_MASK );
        break;
    } 

And implant it with:

lorawan_api_payload_send(
                FMP_PORT, true, ctx->fmp_tx_payload_ans, ctx->fmp_tx_payload_ans_size, UNCONF_DATA_UP,
                smtc_modem_hal_get_time_in_ms( ) + smtc_modem_hal_get_random_nb_in_range( 0, ctx->fmp_ans_delay ),
                ctx->stack_id );

            ctx->fmp_tx_payload_ans_size = 0;
            ctx->fmp_task_ctx_mask &= ~( ANS_CMD_TASK_MASK );

or

status_lorawan_t status = lorawan_api_payload_send(
                FMP_PORT, true, ctx->fmp_tx_payload_ans, ctx->fmp_tx_payload_ans_size, UNCONF_DATA_UP,
                smtc_modem_hal_get_time_in_ms( ) + smtc_modem_hal_get_random_nb_in_range( 0, ctx->fmp_ans_delay ),
                ctx->stack_id );

            if(status == OKLORAWAN)
            {
                ctx->fmp_tx_payload_ans_size = 0;
                ctx->fmp_task_ctx_mask &= ~( ANS_CMD_TASK_MASK );
            }

This seems to works, would this be a correct fix and are am i missing something?

lbm-team commented 7 months ago

Hi MpmTwtg, Each service is composed of 3 parts:

After received your FMP_DEV_REBOOT_TIME_REQ downlink with a valid time

can you check if the mask is correctly set with ctx->fmp_task_ctx_mask |= ANS_CMD_TASK_MASK;

and that a new task is scheduled with fmp_add_task()

Best

MpmTwtg commented 6 months ago

Hi lbm-team

the ctx->fmp_task_ctx_mask |= ANS_CMD_TASK_MASK; is set but after a second message it is reset by the fmp_add_task before it has a change to send the FMP_DEV_REBOOT_TIME_ANS

see output log out.log

lbm-team commented 6 months ago

Hi MpmTwtg,

I understand your point, the modem received a new downlink that request the same answer the current answering command.

your proposition is correct to remove line 321:

case ANS_CMD_TASK_MASK: {
       // ctx->fmp_task_ctx_mask &= ~( ANS_CMD_TASK_MASK );
        break;
    } 

and to check the output of lorawan_api_send()

if( lorawan_api_payload_send(
                    FMP_PORT, true, ctx->fmp_tx_payload_ans, ctx->fmp_tx_payload_ans_size, UNCONF_DATA_UP,
                    smtc_modem_hal_get_time_in_ms( ) + smtc_modem_hal_get_random_nb_in_range( 0, ctx->fmp_ans_delay ),
                    ctx->stack_id ) == OKLORAWAN )
            {
                ctx->fmp_task_ctx_mask &= ~( ANS_CMD_TASK_MASK );
                ctx->fmp_tx_payload_ans_size = 0;
            }