Fix level control's currentLevel update when receiving an Off command (…

…#20788) According to the Application Clusters specification, when Level Control cluster is used in association with the On/Off cluster, the action on receipt for an Off command should be to change the current level to: * the minimum level allowed for the device, if OnLevel is defined. * the stored level, if OnLevel is not defined. In the current implementation, when receiving an Off command, the currentLevel attribute is always restored to the stored value. Signed-off-by: Marius Tache <[email protected]> Restyled by clang-format
project-chip · Aug 30, 2023 · 5224161 · 5224161
1 parent 8e04007
commit 5224161
Show file tree

Hide file tree

Showing 5 changed files with 1,612 additions and 30 deletions.
diff --git a/src/app/clusters/level-control/level-control.cpp b/src/app/clusters/level-control/level-control.cpp
@@ -99,7 +99,6 @@ typedef struct
     CommandId commandId;
     uint8_t moveToLevel;
     bool increasing;
-    bool useOnLevel;
     uint8_t onLevel;
     uint8_t minLevel;
     uint8_t maxLevel;
@@ -248,18 +247,6 @@ void emberAfLevelControlClusterServerTickCallback(EndpointId endpoint)
             state->commandId == Commands::StepWithOnOff::Id)
         {
             setOnOffValue(endpoint, (currentLevel != state->minLevel));
-            if (currentLevel == state->minLevel && state->useOnLevel)
-            {
-                status = Attributes::CurrentLevel::Set(endpoint, state->onLevel);
-                if (status != EMBER_ZCL_STATUS_SUCCESS)
-                {
-                    emberAfLevelControlClusterPrintln("ERR: writing current level %x", status);
-                }
-                else
-                {
-                    updateCoupledColorTemp(endpoint);
-                }
-            }
         }
         else
         {
@@ -635,9 +622,6 @@ static EmberAfStatus moveToLevelHandler(EndpointId endpoint, CommandId commandId
     state->eventDurationMs = state->transitionTimeMs / actualStepSize;
     state->elapsedTimeMs   = 0;
 
-    // OnLevel is not used for Move commands.
-    state->useOnLevel = false;
-
     state->storedLevel = storedLevel;
 
     // The setup was successful, so mark the new state as active and return.
@@ -755,9 +739,6 @@ static void moveHandler(EndpointId endpoint, CommandId commandId, uint8_t moveMo
     state->transitionTimeMs = difference * state->eventDurationMs;
     state->elapsedTimeMs    = 0;
 
-    // OnLevel is not used for Move commands.
-    state->useOnLevel = false;
-
     // storedLevel is not used for Move commands.
     state->storedLevel = INVALID_STORED_LEVEL;
 
@@ -876,9 +857,6 @@ static void stepHandler(EndpointId endpoint, CommandId commandId, uint8_t stepMo
     state->eventDurationMs = state->transitionTimeMs / actualStepSize;
     state->elapsedTimeMs   = 0;
 
-    // OnLevel is not used for Step commands.
-    state->useOnLevel = false;
-
     // storedLevel is not used for Step commands
     state->storedLevel = INVALID_STORED_LEVEL;
 
@@ -924,6 +902,7 @@ void emberAfOnOffClusterLevelControlEffectCallback(EndpointId endpoint, bool new
     uint8_t temporaryCurrentLevelCache;
     uint16_t currentOnOffTransitionTime;
     EmberAfStatus status;
+    bool useOnLevel = false;
 
     EmberAfLevelControlState * state = getState(endpoint);
     if (state == nullptr)
@@ -942,14 +921,13 @@ void emberAfOnOffClusterLevelControlEffectCallback(EndpointId endpoint, bool new
         return;
     }
 
-    // Read the OnLevel attribute.
 #ifndef IGNORE_LEVEL_CONTROL_CLUSTER_ON_LEVEL_ATTRIBUTE
     if (emberAfContainsAttribute(endpoint, LevelControl::Id, Attributes::OnLevel::Id))
     {
         status = Attributes::OnLevel::Get(endpoint, resolvedLevel);
         if (status != EMBER_ZCL_STATUS_SUCCESS)
         {
-            emberAfLevelControlClusterPrintln("ERR: reading current level %x", status);
+            emberAfLevelControlClusterPrintln("ERR: reading on level %x", status);
             return;
         }
 
@@ -958,6 +936,10 @@ void emberAfOnOffClusterLevelControlEffectCallback(EndpointId endpoint, bool new
             // OnLevel has undefined value; fall back to CurrentLevel.
             resolvedLevel.SetNonNull(temporaryCurrentLevelCache);
         }
+        else
+        {
+            useOnLevel = true;
+        }
     }
     else
     {
@@ -1007,12 +989,18 @@ void emberAfOnOffClusterLevelControlEffectCallback(EndpointId endpoint, bool new
         // ...else if newValue is OnOff::Commands::Off::Id...
         // "Move CurrentLevel to the minimum level allowed for the device over the
         // time period OnOffTransitionTime."
-        moveToLevelHandler(endpoint, Commands::MoveToLevel::Id, minimumLevelAllowedForTheDevice, currentOnOffTransitionTime, 0xFF,
-                           0xFF, temporaryCurrentLevelCache);
-
-        // "If OnLevel is not defined, set the CurrentLevel to the stored level."
-        // The emberAfLevelControlClusterServerTickCallback implementation handles
-        // this.
+        if (useOnLevel)
+        {
+            // If OnLevel is defined, don't revert to stored level.
+            moveToLevelHandler(endpoint, Commands::MoveToLevel::Id, minimumLevelAllowedForTheDevice, currentOnOffTransitionTime,
+                               0xFF, 0xFF, INVALID_STORED_LEVEL);
+        }
+        else
+        {
+            // If OnLevel is not defined, set the CurrentLevel to the stored level.
+            moveToLevelHandler(endpoint, Commands::MoveToLevel::Id, minimumLevelAllowedForTheDevice, currentOnOffTransitionTime,
+                               0xFF, 0xFF, temporaryCurrentLevelCache);
+        }
     }
 }
 

diff --git a/src/app/tests/suites/TestLevelControlWithOnOffDependency.yaml b/src/app/tests/suites/TestLevelControlWithOnOffDependency.yaml
@@ -0,0 +1,249 @@
+# Copyright (c) 2021 Project CHIP Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: Dependency with the On/Off cluster Verification (DUT as Server)
+
+config:
+    nodeId: 0x12344321
+    cluster: "Level Control"
+    endpoint: 1
+
+tests:
+    - label: "Wait for the commissioned device to be retrieved"
+      cluster: "DelayCommands"
+      command: "WaitForCommissionee"
+      arguments:
+          values:
+              - name: "nodeId"
+                value: nodeId
+
+    - label: "Sends a MoveToLevel command to set current level to min value"
+      command: "MoveToLevel"
+      arguments:
+          values:
+              - name: "level"
+                value: 1
+              - name: "transitionTime"
+                value: 0
+              - name: "optionMask"
+                value: 1
+              - name: "optionOverride"
+                value: 1
+
+    - label: "Wait 100 ms"
+      cluster: "DelayCommands"
+      command: "WaitForMs"
+      arguments:
+          values:
+              - name: "ms"
+                value: 100
+
+    - label: "Reads CurrentLevel attribute from DUT"
+      command: "readAttribute"
+      attribute: "current level"
+      response:
+          value: 1
+
+    - label: "Write OnOffTransitionTime attribute"
+      command: "writeAttribute"
+      attribute: "on off transition time"
+      arguments:
+          value: 0
+
+    - label: "Wait 100 ms"
+      cluster: "DelayCommands"
+      command: "WaitForMs"
+      arguments:
+          values:
+              - name: "ms"
+                value: 100
+
+    - label: "Read OnOffTransitionTime attribute"
+      command: "readAttribute"
+      attribute: "on off transition time"
+      response:
+          value: 0
+
+    - label: "Write OnLevel attribute"
+      command: "writeAttribute"
+      attribute: "on level"
+      arguments:
+          value: 254
+
+    - label: "Wait 100 ms"
+      cluster: "DelayCommands"
+      command: "WaitForMs"
+      arguments:
+          values:
+              - name: "ms"
+                value: 100
+
+    - label: "Read OnLevel attribute"
+      command: "readAttribute"
+      attribute: "on level"
+      response:
+          value: 254
+
+    - label: "Read MinValue attribute"
+      command: "readAttribute"
+      attribute: "min level"
+      response:
+          value: 1
+
+    - label: "Send On Command"
+      cluster: "On/Off"
+      command: "On"
+
+    - label: "Check on/off attribute value is true after on command"
+      cluster: "On/Off"
+      command: "readAttribute"
+      attribute: "OnOff"
+      response:
+          value: 1
+
+    - label: "Wait OnOffTransitionTime"
+      cluster: "DelayCommands"
+      command: "WaitForMs"
+      arguments:
+          values:
+              - name: "ms"
+                value: 100
+
+    - label: "If OnLevel is defined, check CurrentLevel is OnLevel value"
+      command: "readAttribute"
+      attribute: "current level"
+      response:
+          value: 254
+
+    - label: "Send Off Command"
+      cluster: "On/Off"
+      command: "Off"
+
+    - label: "Check on/off attribute value is false after off command"
+      cluster: "On/Off"
+      command: "readAttribute"
+      attribute: "OnOff"
+      response:
+          value: 0
+
+    - label: "Wait OnOffTransitionTime"
+      cluster: "DelayCommands"
+      command: "WaitForMs"
+      arguments:
+          values:
+              - name: "ms"
+                value: 0
+
+    - label: "If OnLevel is defined, check CurrentLevel is min value"
+      command: "readAttribute"
+      attribute: "current level"
+      response:
+          value: 1
+
+    - label: "Sends a MoveToLevel command to set current level to a mid value"
+      command: "MoveToLevel"
+      arguments:
+          values:
+              - name: "level"
+                value: 127
+              - name: "transitionTime"
+                value: 0
+              - name: "optionMask"
+                value: 1
+              - name: "optionOverride"
+                value: 1
+
+    - label: "Wait 100 ms"
+      cluster: "DelayCommands"
+      command: "WaitForMs"
+      arguments:
+          values:
+              - name: "ms"
+                value: 100
+
+    - label: "Reads CurrentLevel attribute from DUT"
+      command: "readAttribute"
+      attribute: "current level"
+      response:
+          value: 127
+
+    - label: "Set OnLevel attribute to null"
+      command: "writeAttribute"
+      attribute: "on level"
+      arguments:
+          value: null
+
+    - label: "Wait 100 ms"
+      cluster: "DelayCommands"
+      command: "WaitForMs"
+      arguments:
+          values:
+              - name: "ms"
+                value: 100
+
+    - label: "Read OnLevel attribute"
+      command: "readAttribute"
+      attribute: "on level"
+      response:
+          value: null
+
+    - label: "Send On Command"
+      cluster: "On/Off"
+      command: "On"
+
+    - label: "Check on/off attribute value is true after on command"
+      cluster: "On/Off"
+      command: "readAttribute"
+      attribute: "OnOff"
+      response:
+          value: 1
+
+    - label: "Wait OnOffTransitionTime"
+      cluster: "DelayCommands"
+      command: "WaitForMs"
+      arguments:
+          values:
+              - name: "ms"
+                value: 100
+
+    - label: "If OnLevel is not defined, check CurrentLevel is restored"
+      command: "readAttribute"
+      attribute: "current level"
+      response:
+          value: 127
+
+    - label: "Send Off Command"
+      cluster: "On/Off"
+      command: "Off"
+
+    - label: "Check on/off attribute value is false after off command"
+      cluster: "On/Off"
+      command: "readAttribute"
+      attribute: "OnOff"
+      response:
+          value: 0
+
+    - label: "Wait OnOffTransitionTime"
+      cluster: "DelayCommands"
+      command: "WaitForMs"
+      arguments:
+          values:
+              - name: "ms"
+                value: 0
+
+    - label: "If OnLevel is not defined, check CurrentLevel is restored"
+      command: "readAttribute"
+      attribute: "current level"
+      response:
+          value: 127
diff --git a/src/app/tests/suites/tests.js b/src/app/tests/suites/tests.js
@@ -843,6 +843,7 @@ function getTests() {
         "TestArmFailSafe",
         "TestFanControl",
         "TestAccessControlConstraints",
+        "TestLevelControlWithOnOffDependency"
     ];
 
     const MultiAdmin = [