From ab986ed1641480c66ec99989d02bc7bccef2c327 Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Tue, 19 Mar 2024 02:14:08 +0000 Subject: [PATCH] Fix LevelControl Move when no motion requested (#32539) * 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. Co-authored-by: volodymyr-zvarun-globallogic * Restyled by prettier-yaml --------- Co-authored-by: volodymyr-zvarun-globallogic Co-authored-by: Restyled.io --- .../clusters/level-control/level-control.cpp | 19 +++++-- .../suites/certification/Test_TC_LVL_4_1.yaml | 57 +++++++++++++++++++ 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/src/app/clusters/level-control/level-control.cpp b/src/app/clusters/level-control/level-control.cpp index b04d32fe69c448..0077c59509edc9 100644 --- a/src/app/clusters/level-control/level-control.cpp +++ b/src/app/clusters/level-control/level-control.cpp @@ -18,6 +18,8 @@ // clusters specific header #include "level-control.h" +#include + // this file contains all the common includes for clusters in the util #include #include @@ -904,7 +906,7 @@ static Status moveToLevelHandler(EndpointId endpoint, CommandId commandId, uint8 // The duration between events will be the transition time divided by the // distance we must move. - state->eventDurationMs = state->transitionTimeMs / actualStepSize; + state->eventDurationMs = state->transitionTimeMs / std::max(static_cast(1u), actualStepSize); state->elapsedTimeMs = 0; state->storedLevel = storedLevel; @@ -958,6 +960,14 @@ static void moveHandler(app::CommandHandler * commandObj, const app::ConcreteCom goto send_default_response; } + if (!rate.IsNull() && (rate.Value() == 0)) + { + // Move at a rate of zero is no move at all. Immediately succeed without touching anything. + ChipLogProgress(Zcl, "Immediate success due to move rate of 0 (would move at no rate)."); + status = Status::Success; + goto send_default_response; + } + // Cancel any currently active command before fiddling with the state. cancelEndpointTimerCallback(endpoint); @@ -1034,12 +1044,13 @@ static void moveHandler(app::CommandHandler * commandObj, const app::ConcreteCom status = Status::Success; goto send_default_response; } + // Already checked that defaultMoveRate.Value() != 0. state->eventDurationMs = MILLISECOND_TICKS_PER_SECOND / defaultMoveRate.Value(); } } else { - state->eventDurationMs = MILLISECOND_TICKS_PER_SECOND / rate.Value(); + state->eventDurationMs = MILLISECOND_TICKS_PER_SECOND / std::max(static_cast(1u), rate.Value()); } #else // 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 // milliseconds to reduce rounding errors in integer division. if (stepSize != actualStepSize) { - state->transitionTimeMs = (state->transitionTimeMs * actualStepSize / stepSize); + state->transitionTimeMs = (state->transitionTimeMs * actualStepSize / std::max(static_cast(1u), stepSize)); } } #else @@ -1187,7 +1198,7 @@ static void stepHandler(app::CommandHandler * commandObj, const app::ConcreteCom // The duration between events will be the transition time divided by the // distance we must move. - state->eventDurationMs = state->transitionTimeMs / actualStepSize; + state->eventDurationMs = state->transitionTimeMs / std::max(static_cast(1u), actualStepSize); state->elapsedTimeMs = 0; // storedLevel is not used for Step commands diff --git a/src/app/tests/suites/certification/Test_TC_LVL_4_1.yaml b/src/app/tests/suites/certification/Test_TC_LVL_4_1.yaml index 737f858b64c468..14ac5ea5a69ffd 100644 --- a/src/app/tests/suites/certification/Test_TC_LVL_4_1.yaml +++ b/src/app/tests/suites/certification/Test_TC_LVL_4_1.yaml @@ -243,6 +243,63 @@ tests: response: value: 254 + - label: "Step 4f: TH reads the MinLevel attribute from the DUT" + PICS: LVL.S.A0002 && LVL.S.F01 + command: "readAttribute" + attribute: "MinLevel" + response: + value: 1 + saveAs: MinLevelValue + constraints: + type: int8u + + - label: "Step 4g: TH sends a MoveToLevel to set the level to MinLevel" + PICS: LVL.S.C01.Rsp && LVL.S.F01 + command: "MoveToLevel" + arguments: + values: + - name: "Level" + value: MinLevelValue + - name: "TransitionTime" + value: 0 + - name: "OptionsMask" + value: 0 + - name: "OptionsOverride" + value: 0 + + - label: + "Step 4h: TH sends a Move command to the DUT with MoveMode =0x00 (up) + and Rate =0 (units/s), expect success" + PICS: LVL.S.C01.Rsp + command: "Move" + arguments: + values: + - name: "MoveMode" + value: 0 + - name: "Rate" + value: 0 + - name: "OptionsMask" + value: 1 + - name: "OptionsOverride" + value: 1 + + - label: "Wait 5s" + cluster: "DelayCommands" + command: "WaitForMs" + arguments: + values: + - name: "ms" + value: 5000 + + - label: + "Step 4i: After another 5 seconds, TH reads CurrentLevel attribute + from DUT, expects mininum level." + PICS: LVL.S.C01.Rsp && LVL.S.A0000 + command: "readAttribute" + attribute: "CurrentLevel" + response: + value: MinLevelValue + - label: "Precondition send Off Command" cluster: "On/Off" PICS: OO.S.C00.Rsp