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

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 * 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]>
project-chip · Jul 22, 2022 · 34d0436 · 34d0436
1 parent 83deb47
commit 34d0436
Show file tree

Hide file tree

Showing 5 changed files with 1,592 additions and 96 deletions.
diff --git a/src/app/clusters/level-control/level-control.cpp b/src/app/clusters/level-control/level-control.cpp
@@ -263,7 +263,7 @@ void emberAfLevelControlClusterServerTickCallback(EndpointId endpoint)
         }
         else
         {
-            if (state->storedLevel != INVALID_STORED_LEVEL)
+            if (!state->useOnLevel && state->storedLevel != INVALID_STORED_LEVEL)
             {
                 uint8_t storedLevel8u = (uint8_t) state->storedLevel;
                 status                = Attributes::CurrentLevel::Set(endpoint, storedLevel8u);
@@ -635,9 +635,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.
@@ -942,14 +939,15 @@ void emberAfOnOffClusterLevelControlEffectCallback(EndpointId endpoint, bool new
         return;
     }
 
-    // Read the OnLevel attribute.
+    state->useOnLevel = false;
+
 #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 +956,10 @@ void emberAfOnOffClusterLevelControlEffectCallback(EndpointId endpoint, bool new
             // OnLevel has undefined value; fall back to CurrentLevel.
             resolvedLevel.SetNonNull(temporaryCurrentLevelCache);
         }
+        else
+        {
+            state->useOnLevel = true;
+        }
     }
     else
     {

diff --git a/src/app/tests/suites/TestLevelControlWithOnOffDependency.yaml b/src/app/tests/suites/TestLevelControlWithOnOffDependency.yaml
@@ -0,0 +1,225 @@
+# 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: "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: "Read OnLevel attribute"
+      command: "readAttribute"
+      attribute: "on level"
+      response:
+          value: 254
+
+    - label: "Read MinValue attribute"
+      command: "readAttribute"
+      attribute: "on off transition time"
+      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: "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
@@ -15,15 +15,14 @@
  *    limitations under the License.
  */
 
-function disable(testName)
-{
-  const index = this.indexOf(testName);
-  if (index == -1) {
-    const errStr = `Test ${testName}  does not exists.`;
-    throw new Error(errStr);
-  }
-
-  this.splice(index, 1);
+function disable(testName) {
+    const index = this.indexOf(testName);
+    if (index == -1) {
+        const errStr = `Test ${testName}  does not exists.`;
+        throw new Error(errStr);
+    }
+
+    this.splice(index, 1);
 }
 
 // clang-format off
@@ -843,6 +842,7 @@ function getTests() {
         "TestArmFailSafe",
         "TestFanControl",
         "TestAccessControlConstraints",
+        "TestLevelControlWithOnOffDependency"
     ];
 
     const MultiAdmin = [
@@ -929,5 +929,5 @@ function getTests() {
 //
 // Module exports
 //
-exports.getTests       = getTests;
+exports.getTests = getTests;
 exports.getManualTests = getManualTests;