Generic Level Server Bugs

[david_owtschinnikow]

Hello, 

I'm working with Mesh SDK 3.1.0. 

I made some changes in app_level.c file. 

Could you please confirm that these changes are right?

They seem to work fine, but I would appreciate very much your comments/corrections!

1) g_transition_complete(), line 368

ORIGINAL CODE: return (p_server->state.transition_type != TRANSITION_MOVE_SET && TRANSITION_TIME_COMPLETE(p_server));

MY CORRECTION: return (p_server->state.transition_type != TRANSITION_NONE && TRANSITION_TIME_COMPLETE(p_server));

Otherwise the "move" transition is infinite.

2)  generic_level_state_move_set_cb(), line 555

ORIGINAL CODE: if (p_in_transition->transition_time_ms > TRANSITION_TIME_STEP_100MS_MAX)

MY CORRECTION: if (p_in_transition->transition_time_ms > TRANSITION_TIME_MAX_MS)

3) a_transition_start(), line 257

Before assigning the "required move" to abs_delta, I calculate the target level. Otherwise, it is always 32768.

ORIGINAL CODE:

if (p_server->state.transition_type == TRANSITION_MOVE_SET)
{
abs_delta = abs(p_server->state.params.move.required_move);
}

MY CHANGE:

if (p_server->state.transition_type == TRANSITION_MOVE_SET) {
       target = (int64_t)p_server->state.present_level + (int64_t)p_server->state.params.move.required_move;      
       if (target > INT16_MAX) {
           target = INT16_MAX;
       } else if (target < INT16_MIN) {
           target = INT16_MIN;
       }
       p_server->state.target_level = target; 
       abs_delta = abs(p_server->state.params.move.required_move);
}

4) In models\model_spec\generic_ponoff\include\generic_ponoff_messages.h

Line 54

The  PONOFF opcode definitions seem to be wrong. 

Thank you very much in advance,

David 

[Hung Bui]

Hi again David, 

Regarding your questions:

1. According to spec, Mesh model v1.0:

When the Generic Level state is not bound to another state, the overflow/underflow handling is
implementation-specific. Some Generic Level Servers may stop at their maximum or minimum levels,
and some may wrap around.

 Since this is implementation specific, we choose to do continuous movement showing the wrap around behavior. You can modify this behavior to stop at INT16_MAX or INT16_MIN values.

2. TRANSITION_TIME_STEP_100MS_MAX  is the correct value in milliseconds and not the encoded value. You should not use the TRANSITION_TIME_MAX_MS. The reason is the transition number of steps is limited to 0x00 to 0x3E according to spec. 

3. Target level should be 32767 for positive move values (level increases continuously) and should be -32768 for negative move values. This is according to the spec: 

Upon receiving a Generic Move Set message, the Generic Level Server shall respond with a Generic Level Status message (see Section 3.3.2.2.5). The target Generic Level state is the upper limit of the Generic Level state when the transition speed is positive, or the lower limit of the Generic Level state
when the transition speed is negative.

4. It's a bug and we have reported it. Fix is coming on v3.2
 

[david_owtschinnikow]

Thank you for your response

1) I understand, but in my case (I want to stop the movement when the transition time is completed) I don’t find another way of stopping the movement other than the way I posted, because inside “g_transition_complete()”, if the condition to stop is NOT( TRANSITION_MOVE_SET), then a move set never stops. Besides, the target level is already INT16_MAX or INT16_MIN.

2) In that case, I cannot handle transition times higher than TRANSITION_TIME_STEP_100MS_MAX, which is 6200ms (not encoded). The comparison is done between decoded values, in ms, not between encoded values. According to the spec., the maximum transition time is 10.5 hours. I don’t see why the maximum should be 6200ms?

3)I have undone my change according to what you said, but if I do so, then no matter what parameters I send to the server (transition time and delta level), the “present level” at the end of the transition is always equal to the “target level”, which is INT16_MAX or INT16_MIN; I don’t think this is the correct behaviour. What would then be the correct solution?

Thank you very much

[Hung Bui]

Hi, 

1) Yes you can do what you have done. 

2) After a double check we confirmed that it's a bug. You can use TRANSITION_TIME_MAX_MS

3) I think that's the way it should be for the Move Set command. The present level should by the end of transition time stop at the INT16_MAX or INT16_MIN. Or keep wrapping over like what we have implemented.  My understanding is that there is no target level that the state need to get to for this command. If you want a target level you should use the Level Set. 

[david_owtschinnikow]

OK, thank you very much for your support