TC-VALCC-3.1: Allow immediate open of valve (project-chip#35851)

* TC-VALCC-3.1: Allow immediate open of valve

If the valve can be opened before the command is complete or before
the next read is done, the test will fail because it expects
the transitioning phase to happen.

In the spec:
When the movement is complete, the device SHALL set the CurrentState attribute
to the Open value.

^ this can happen before the end of the command.

* fixup for 3.1 test

* another fixup for 3.1

* Add close at start of test

* linter

* Restyled by clang-format

* Restyled by isort

* Add back the feature check

* TC-VALCC-3.3: Allow immediate open

* Linter

* Fix 3.2

* couple little fixes

* Restyled by isort

* linter

---------

Co-authored-by: Restyled.io <commits@restyled.io>

Author: cecille

examples/all-clusters-app/linux/ValveControlDelegate.cpp

@@ -38,7 +38,6 @@ DataModel::Nullable<chip::Percent> ValveControlDelegate::HandleOpenValve(DataMod
     // In this demo application, the transition is considered instant,
     // so current level is set to the requested level and current state is set to kOpen.
     currentLevel = sLevel;
-    Attributes::CurrentState::Set(kValveEndpoint, ValveConfigurationAndControl::ValveStateEnum::kOpen);
 
     return DataModel::Nullable<chip::Percent>(currentLevel);
 }
@@ -48,8 +47,6 @@ CHIP_ERROR ValveControlDelegate::HandleCloseValve()
     sLastOpenDuration = 0;
     sLevel            = 0;
     ReturnErrorOnFailure(ValveConfigurationAndControl::UpdateCurrentLevel(kValveEndpoint, sLevel));
-    ReturnErrorOnFailure(
-        ValveConfigurationAndControl::UpdateCurrentState(kValveEndpoint, ValveConfigurationAndControl::ValveStateEnum::kClosed));
     ChipLogProgress(NotSpecified, "Valve closed");
     return CHIP_NO_ERROR;
 }

src/app/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp

@@ -360,9 +360,9 @@ CHIP_ERROR SetValveLevel(EndpointId ep, DataModel::Nullable<Percent> level, Data
     if (!isDelegateNull(delegate))
     {
         DataModel::Nullable<Percent> cLevel = delegate->HandleOpenValve(level);
-        if (HasFeature(ep, ValveConfigurationAndControl::Feature::kLevel))
+        if (HasFeature(ep, ValveConfigurationAndControl::Feature::kLevel) && !cLevel.IsNull())
         {
-            VerifyOrReturnError(Status::Success == CurrentLevel::Set(ep, cLevel), attribute_error);
+            UpdateCurrentLevel(ep, cLevel.Value());
         }
     }
     // start countdown
@@ -376,14 +376,28 @@ CHIP_ERROR UpdateCurrentLevel(EndpointId ep, Percent currentLevel)
     if (HasFeature(ep, ValveConfigurationAndControl::Feature::kLevel))
     {
         VerifyOrReturnError(Status::Success == CurrentLevel::Set(ep, currentLevel), CHIP_IM_GLOBAL_STATUS(ConstraintError));
-        return CHIP_NO_ERROR;
     }
-    return CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute);
+    DataModel::Nullable<Percent> targetLevel = DataModel::NullNullable;
+    TargetLevel::Get(ep, targetLevel);
+    if (!targetLevel.IsNull() && currentLevel == targetLevel.Value())
+    {
+        targetLevel = DataModel::NullNullable;
+        TargetLevel::Set(ep, targetLevel);
+        UpdateCurrentState(ep, currentLevel == 0 ? ValveStateEnum::kClosed : ValveStateEnum::kOpen);
+    }
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR UpdateCurrentState(EndpointId ep, ValveConfigurationAndControl::ValveStateEnum currentState)
 {
     VerifyOrReturnError(Status::Success == CurrentState::Set(ep, currentState), CHIP_IM_GLOBAL_STATUS(ConstraintError));
+    DataModel::Nullable<ValveStateEnum> targetState = DataModel::NullNullable;
+    TargetState::Get(ep, targetState);
+    if (currentState == targetState.ValueOr(ValveStateEnum::kUnknownEnumValue))
+    {
+        targetState = DataModel::NullNullable;
+        TargetState::Set(ep, targetState);
+    }
     emitValveStateChangedEvent(ep, currentState);
     return CHIP_NO_ERROR;
 }

src/python_testing/TC_VALCC_3_1.py

@@ -32,12 +32,10 @@
 #     quiet: true
 # === END CI TEST ARGUMENTS ===
 
-import time
-
 import chip.clusters as Clusters
 from chip.clusters.Types import NullValue
-from chip.interaction_model import InteractionModelError, Status
-from chip.testing.matter_testing import MatterBaseTest, TestStep, async_test_body, default_matter_test_main
+from chip.testing.matter_testing import (AttributeValue, ClusterAttributeChangeAccumulator, MatterBaseTest, TestStep,
+                                         async_test_body, default_matter_test_main)
 from mobly import asserts
 
 
@@ -52,12 +50,16 @@ def desc_TC_VALCC_3_1(self) -> str:
     def steps_TC_VALCC_3_1(self) -> list[TestStep]:
         steps = [
             TestStep(1, "Commissioning, already done", is_commissioning=True),
-            TestStep(2, "Send Open command"),
-            TestStep(3, "Read TargetState attribute"),
-            TestStep(4, "Read CurrentState attribute"),
-            TestStep(5, "Send Close command"),
-            TestStep(6, "Read TargetState attribute"),
-            TestStep(7, "Read CurrentState attribute"),
+            TestStep(2, "Set up a subscription to all attributes on the DUT"),
+            TestStep(3, "Send a close command to the DUT and wait until the CurrentState is closed", "DUT returns SUCCESS"),
+            TestStep(4, "Send Open command", "DUT returns SUCCESS"),
+            TestStep(5, "Wait until TH receives and data report for TargetState set to NULL and an attribute report for CurrentState set to Open (ordering does not matter)",
+                     "Expected attribute reports are received"),
+            TestStep(6, "Read CurrentState and TargetState attribute", "CurrentState is Open, TargetState is NULL"),
+            TestStep(7, "Send Close command", "DUT returns SUCCESS"),
+            TestStep(8, "Wait until TH receives and data report for TargetState set to NULL and an attribute report for CurrentState set to Closed (ordering does not matter)",
+                     "Expected attribute reports are received"),
+            TestStep(9, "Read CurrentState and TargetState attribute", "CurrentState is Closed, TargetState is NULL"),
         ]
         return steps
 
@@ -72,62 +74,55 @@ async def test_TC_VALCC_3_1(self):
 
         endpoint = self.get_endpoint(default=1)
 
-        self.step(1)
-        attributes = Clusters.ValveConfigurationAndControl.Attributes
+        self.step(1)  # commissioning - already done
 
         self.step(2)
-        try:
-            await self.send_single_cmd(cmd=Clusters.Objects.ValveConfigurationAndControl.Commands.Open(), endpoint=endpoint)
-        except InteractionModelError as e:
-            asserts.assert_equal(e.status, Status.Success, "Unexpected error returned")
-            pass
+        cluster = Clusters.ValveConfigurationAndControl
+        attributes = cluster.Attributes
+        attribute_subscription = ClusterAttributeChangeAccumulator(cluster)
+        await attribute_subscription.start(self.default_controller, self.dut_node_id, endpoint)
 
         self.step(3)
-        target_state_dut = await self.read_valcc_attribute_expect_success(endpoint=endpoint, attribute=attributes.TargetState)
-
-        asserts.assert_true(target_state_dut is not NullValue, "TargetState is null")
-        asserts.assert_equal(target_state_dut, Clusters.Objects.ValveConfigurationAndControl.Enums.ValveStateEnum.kOpen,
-                             "TargetState is not the expected value")
-
-        self.step(4)
+        # Wait for the entire duration of the test because this valve may be slow. The test will time out before this does. That's fine.
+        timeout = self.matter_test_config.timeout if self.matter_test_config.timeout is not None else self.default_timeout
+        await self.send_single_cmd(cmd=cluster.Commands.Close(), endpoint=endpoint)
         current_state_dut = await self.read_valcc_attribute_expect_success(endpoint=endpoint, attribute=attributes.CurrentState)
-        asserts.assert_true(current_state_dut is not NullValue, "CurrentState is null")
-
-        while current_state_dut is Clusters.Objects.ValveConfigurationAndControl.Enums.ValveStateEnum.kTransitioning:
-            time.sleep(1)
-
-            current_state_dut = await self.read_valcc_attribute_expect_success(endpoint=endpoint, attribute=attributes.CurrentState)
-            asserts.assert_true(current_state_dut is not NullValue, "CurrentState is null")
+        if current_state_dut != cluster.Enums.ValveStateEnum.kClosed:
+            current_state_closed = AttributeValue(
+                endpoint_id=endpoint, attribute=attributes.CurrentState, value=cluster.Enums.ValveStateEnum.kClosed)
+            attribute_subscription.await_all_final_values_reported(
+                expected_final_values=[current_state_closed], timeout_sec=timeout)
 
-        asserts.assert_equal(current_state_dut, Clusters.Objects.ValveConfigurationAndControl.Enums.ValveStateEnum.kOpen,
-                             "CurrentState is not the expected value")
+        self.step(4)
+        attribute_subscription.reset()
+        await self.send_single_cmd(cmd=Clusters.Objects.ValveConfigurationAndControl.Commands.Open(), endpoint=endpoint)
 
         self.step(5)
-        try:
-            await self.send_single_cmd(cmd=Clusters.Objects.ValveConfigurationAndControl.Commands.Close(), endpoint=endpoint)
-        except InteractionModelError as e:
-            asserts.assert_equal(e.status, Status.Success, "Unexpected error returned")
-            pass
+        # Wait until the current state is open and the target state is Null.
+        expected_final_state = [AttributeValue(endpoint_id=endpoint, attribute=attributes.TargetState, value=NullValue), AttributeValue(
+            endpoint_id=endpoint, attribute=attributes.CurrentState, value=cluster.Enums.ValveStateEnum.kOpen)]
+        attribute_subscription.await_all_final_values_reported(expected_final_values=expected_final_state, timeout_sec=timeout)
 
         self.step(6)
         target_state_dut = await self.read_valcc_attribute_expect_success(endpoint=endpoint, attribute=attributes.TargetState)
-
-        asserts.assert_true(target_state_dut is not NullValue, "TargetState is null")
-        asserts.assert_equal(target_state_dut, Clusters.Objects.ValveConfigurationAndControl.Enums.ValveStateEnum.kClosed,
-                             "TargetState is not the expected value")
-
-        self.step(7)
         current_state_dut = await self.read_valcc_attribute_expect_success(endpoint=endpoint, attribute=attributes.CurrentState)
-        asserts.assert_true(current_state_dut is not NullValue, "CurrentState is null")
+        asserts.assert_equal(current_state_dut, cluster.Enums.ValveStateEnum.kOpen, "CurrentState is not open")
+        asserts.assert_equal(target_state_dut, NullValue, "TargetState is not null")
 
-        while current_state_dut is Clusters.Objects.ValveConfigurationAndControl.Enums.ValveStateEnum.kTransitioning:
-            time.sleep(1)
+        self.step(7)
+        attribute_subscription.reset()
+        await self.send_single_cmd(cmd=cluster.Commands.Close(), endpoint=endpoint)
 
-            current_state_dut = await self.read_valcc_attribute_expect_success(endpoint=endpoint, attribute=attributes.CurrentState)
-            asserts.assert_true(current_state_dut is not NullValue, "CurrentState is null")
+        self.step(8)
+        expected_final_state = [AttributeValue(endpoint_id=endpoint, attribute=attributes.TargetState, value=NullValue), AttributeValue(
+            endpoint_id=endpoint, attribute=attributes.CurrentState, value=cluster.Enums.ValveStateEnum.kClosed)]
+        attribute_subscription.await_all_final_values_reported(expected_final_values=expected_final_state, timeout_sec=timeout)
 
-        asserts.assert_equal(current_state_dut, Clusters.Objects.ValveConfigurationAndControl.Enums.ValveStateEnum.kClosed,
-                             "CurrentState is not the expected value")
+        self.step(9)
+        target_state_dut = await self.read_valcc_attribute_expect_success(endpoint=endpoint, attribute=attributes.TargetState)
+        current_state_dut = await self.read_valcc_attribute_expect_success(endpoint=endpoint, attribute=attributes.CurrentState)
+        asserts.assert_equal(current_state_dut, cluster.Enums.ValveStateEnum.kClosed, "CurrentState is not closed")
+        asserts.assert_equal(target_state_dut, NullValue, "TargetState is not null")
 
 
 if __name__ == "__main__":