Skip to content

Commit 11f94c3

Browse files
cecilletcarmelveilleuxvolodymyr-zvarun-globallogicrestyled-commits
authored
Fix LevelControl Move when no motion requested (#32539) (#32993)
* Fix LevelControl Move when no motion requested - rate == 0 means "do not move", so handle it more efficiently without moving. Testing done: - Added an integration test for the behavior. * Restyled by prettier-yaml --------- Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com> Co-authored-by: volodymyr-zvarun-globallogic <Zvarun.V@vizio.com> Co-authored-by: Restyled.io <commits@restyled.io>
1 parent 28b2a03 commit 11f94c3

File tree

2 files changed

+72
-4
lines changed

2 files changed

+72
-4
lines changed

src/app/clusters/level-control/level-control.cpp

+15-4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
// clusters specific header
1919
#include "level-control.h"
2020

21+
#include <algorithm>
22+
2123
// this file contains all the common includes for clusters in the util
2224
#include <app-common/zap-generated/attributes/Accessors.h>
2325
#include <app-common/zap-generated/cluster-objects.h>
@@ -904,7 +906,7 @@ static Status moveToLevelHandler(EndpointId endpoint, CommandId commandId, uint8
904906

905907
// The duration between events will be the transition time divided by the
906908
// distance we must move.
907-
state->eventDurationMs = state->transitionTimeMs / actualStepSize;
909+
state->eventDurationMs = state->transitionTimeMs / std::max(static_cast<uint8_t>(1u), actualStepSize);
908910
state->elapsedTimeMs = 0;
909911

910912
state->storedLevel = storedLevel;
@@ -958,6 +960,14 @@ static void moveHandler(app::CommandHandler * commandObj, const app::ConcreteCom
958960
goto send_default_response;
959961
}
960962

963+
if (!rate.IsNull() && (rate.Value() == 0))
964+
{
965+
// Move at a rate of zero is no move at all. Immediately succeed without touching anything.
966+
ChipLogProgress(Zcl, "Immediate success due to move rate of 0 (would move at no rate).");
967+
status = Status::Success;
968+
goto send_default_response;
969+
}
970+
961971
// Cancel any currently active command before fiddling with the state.
962972
cancelEndpointTimerCallback(endpoint);
963973

@@ -1034,12 +1044,13 @@ static void moveHandler(app::CommandHandler * commandObj, const app::ConcreteCom
10341044
status = Status::Success;
10351045
goto send_default_response;
10361046
}
1047+
// Already checked that defaultMoveRate.Value() != 0.
10371048
state->eventDurationMs = MILLISECOND_TICKS_PER_SECOND / defaultMoveRate.Value();
10381049
}
10391050
}
10401051
else
10411052
{
1042-
state->eventDurationMs = MILLISECOND_TICKS_PER_SECOND / rate.Value();
1053+
state->eventDurationMs = MILLISECOND_TICKS_PER_SECOND / std::max(static_cast<uint8_t>(1u), rate.Value());
10431054
}
10441055
#else
10451056
// Transition/rate is not supported so always use fastest transition time and ignore
@@ -1175,7 +1186,7 @@ static void stepHandler(app::CommandHandler * commandObj, const app::ConcreteCom
11751186
// milliseconds to reduce rounding errors in integer division.
11761187
if (stepSize != actualStepSize)
11771188
{
1178-
state->transitionTimeMs = (state->transitionTimeMs * actualStepSize / stepSize);
1189+
state->transitionTimeMs = (state->transitionTimeMs * actualStepSize / std::max(static_cast<uint8_t>(1u), stepSize));
11791190
}
11801191
}
11811192
#else
@@ -1187,7 +1198,7 @@ static void stepHandler(app::CommandHandler * commandObj, const app::ConcreteCom
11871198

11881199
// The duration between events will be the transition time divided by the
11891200
// distance we must move.
1190-
state->eventDurationMs = state->transitionTimeMs / actualStepSize;
1201+
state->eventDurationMs = state->transitionTimeMs / std::max(static_cast<uint8_t>(1u), actualStepSize);
11911202
state->elapsedTimeMs = 0;
11921203

11931204
// storedLevel is not used for Step commands

src/app/tests/suites/certification/Test_TC_LVL_4_1.yaml

+57
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,63 @@ tests:
243243
response:
244244
value: 254
245245

246+
- label: "Step 4f: TH reads the MinLevel attribute from the DUT"
247+
PICS: LVL.S.A0002 && LVL.S.F01
248+
command: "readAttribute"
249+
attribute: "MinLevel"
250+
response:
251+
value: 1
252+
saveAs: MinLevelValue
253+
constraints:
254+
type: int8u
255+
256+
- label: "Step 4g: TH sends a MoveToLevel to set the level to MinLevel"
257+
PICS: LVL.S.C01.Rsp && LVL.S.F01
258+
command: "MoveToLevel"
259+
arguments:
260+
values:
261+
- name: "Level"
262+
value: MinLevelValue
263+
- name: "TransitionTime"
264+
value: 0
265+
- name: "OptionsMask"
266+
value: 0
267+
- name: "OptionsOverride"
268+
value: 0
269+
270+
- label:
271+
"Step 4h: TH sends a Move command to the DUT with MoveMode =0x00 (up)
272+
and Rate =0 (units/s), expect success"
273+
PICS: LVL.S.C01.Rsp
274+
command: "Move"
275+
arguments:
276+
values:
277+
- name: "MoveMode"
278+
value: 0
279+
- name: "Rate"
280+
value: 0
281+
- name: "OptionsMask"
282+
value: 1
283+
- name: "OptionsOverride"
284+
value: 1
285+
286+
- label: "Wait 5s"
287+
cluster: "DelayCommands"
288+
command: "WaitForMs"
289+
arguments:
290+
values:
291+
- name: "ms"
292+
value: 5000
293+
294+
- label:
295+
"Step 4i: After another 5 seconds, TH reads CurrentLevel attribute
296+
from DUT, expects mininum level."
297+
PICS: LVL.S.C01.Rsp && LVL.S.A0000
298+
command: "readAttribute"
299+
attribute: "CurrentLevel"
300+
response:
301+
value: MinLevelValue
302+
246303
- label: "Precondition send Off Command"
247304
cluster: "On/Off"
248305
PICS: OO.S.C00.Rsp

0 commit comments

Comments
 (0)