How to understand the functions model_delay_encode() and model_delay_decode() in light_switch example?

[Young]

Hi Nordic Support Team,

uint32_t model_delay_decode(uint8_t enc_delay)
{
    return (enc_delay * DELAY_TIME_STEP_FACTOR_MS);
}

uint8_t model_delay_encode(uint32_t delay)
{
    if (delay > DELAY_TIME_STEP_MAX)
    {
        return DELAY_TIME_STEP_MAX;
    }
    else
    {
        return (delay/DELAY_TIME_STEP_FACTOR_MS);
    }
}

When decode the delay, DELAY_TIME_STEP_FACTOR_MS is used for the decoded result. However, in model_delay_encode(), if (delay > DELAY_TIME_STEP_MAX), it just returns DELAY_TIME_STEP_MAX, without being divided by DELAY_TIME_STEP_FACTOR_MS, why?  how to understand this kind of situation. 

[Edvin]

Hello,

/** Maximum encoded value of the delay field */
#define DELAY_TIME_STEP_MAX                     (0xFF)

Based on this, I would guess that the 0xff value is a special case where you just would want to send the maximum value. 

However, I see that in the light_switch\client example (I don't know which one you are using), there is a check before this function is called, every time message_set_packet_create() is called, to check whether p_transition->delay_ms > DELAY_TIME_MAX_MS, so in that case, it will never happen that the delay is larger than 0xFF.

BR,

Edvin

[Young]

Hi Edvin,

Our project is based on client project.  

Yes， there is a condition p_transition->delay_ms > DELAY_TIME_MAX_MS, but DELAY_TIME_MAX_MS is different from DELAY_TIME_STEP_MAX. 

In model_delay_encode(), when getting comparing, it uses DELAY_TIME_STEP_MAX {if (delay > DELAY_TIME_STEP_MAX).}.  here should be DELAY_TIME_MAX_MS?   

[Edvin]

Ah, thank you! I didn't notice that they were using different definitions.

I see, however, that one is derived from the other. DELAY_TIME_MAX_MS = DELAY_TIME_STEP_MAX * DELAY_TIME_STEP_FACTOR_MS, where the last one is 5.

So don't you agree that it will still always be caught in the first if-statement? 

BR,

Edvin

[Young]

Edvin,

uint8_t model_delay_encode(uint32_t delay)
{
    if (delay > DELAY_TIME_STEP_MAX)
    {
        return DELAY_TIME_STEP_MAX;
    }
    else
    {
        return (delay/DELAY_TIME_STEP_FACTOR_MS);
    }
}

Actually delay is the real delay time in millisecond, and apparently it can be greater than DELAY_TIME_STEP_MAX. The real max value of delay should be DELAY_TIME_MAX_MS, so delay should compare with DELAY_TIME_MAX_MS, not DELAY_TIME_STEP_MAX.  so here DELAY_TIME_STEP_MAX should be replaced with DELAY_TIME_MAX_MS. This is my understanding, is right? 

[Edvin]

Hello,

That looks about right. I agree that model_delay_encode() is called with the delay in ms units, so it should have been compared with DELAY_TIME_STEP_MAX_MS. I would say you can safely change that. 

Unfortunately, there is not planned any future releases of the nRF5 SDK for Mesh, so this will likely not be patched from our side. I am sorry for the inconvenience.

Best regards,

Edvin