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 <[email protected]> * Restyled by prettier-yaml --------- Co-authored-by: volodymyr-zvarun-globallogic <[email protected]> Co-authored-by: Restyled.io <[email protected]>
project-chip · Mar 18, 2024 · f4a6aa5 · f4a6aa5
1 parent 4aa5e67
commit f4a6aa5
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 4 deletions.
diff --git 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 <algorithm>
+
 // this file contains all the common includes for clusters in the util
 #include <app-common/zap-generated/attributes/Accessors.h>
 #include <app-common/zap-generated/cluster-objects.h>
@@ -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<uint8_t>(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<uint8_t>(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<uint8_t>(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<uint8_t>(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
@@ -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