Update to TC_FAN_3_2, addition of TC_FAN_3_3, TC_FAN_3_4, TC_FAN_3_5,…

… & TC_FAN_3_6 (#28106)

* Modified TC_FAN_3_2 to be compliant with update test plan. Removed TC_FAN_3_4.yaml as it will be a Python test

* Adding TC_FAN_3_3 and TC_FAN_3_4 as Python tests

* Making the function parameter naming more consistent for API & Delegate functions. Also fixes small bug where a step when SpeedSetting was null would crash

* Command should not send success if no delegate is set

* Adding Python test script for TC_FAN_3_5

* Removing unused import from python

* Added missing case to Step Handler to handle wrapping to speedMax when speed 1 and lowesfOff is false

* Added TC_FAN_3_6

* Fixed copy-paste bug

* Added TC_FAN_3_5 to CI

* Restyled by prettier-yaml

* Restyled by autopep8

* Restyled by isort

* Adding 3_6 to disable list in darwin as airflow direction not in matter framework yet

* regen tests after Darwin diable

* Improving tests with suggestions from PR

* Regen codegen

* removing incorrectly included file

---------

Co-authored-by: Restyled.io <[email protected]>

.github/workflows/tests.yaml

@@ -451,6 +451,9 @@ jobs:
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace_decode 1" --script "src/python_testing/TC_TIMESYNC_3_1.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021"'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace_decode 1" --script "src/python_testing/TC_DA_1_5.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values"'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace_decode 1" --script "src/python_testing/TC_IDM_1_2.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021"'
+                  scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace_decode 1" --script "src/python_testing/TC_FAN_3_3.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021"'
+                  scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace_decode 1" --script "src/python_testing/TC_FAN_3_4.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021"'
+                  scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app  --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace_decode 1" --script "src/python_testing/TC_FAN_3_5.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021"'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --script "src/python_testing/TestMatterTestingSupport.py"'
             - name: Uploading core files
               uses: actions/upload-artifact@v3

examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp

@@ -43,14 +43,14 @@ class FanControlManager : public AttributeAccessInterface, public Delegate
     {}
 
     CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
-    Status HandleStep(StepDirectionEnum direction, bool wrap, bool off) override;
+    Status HandleStep(StepDirectionEnum aDirection, bool aWrap, bool aLowestOff) override;
 
 private:
     CHIP_ERROR ReadPercentCurrent(AttributeValueEncoder & aEncoder);
     CHIP_ERROR ReadSpeedCurrent(AttributeValueEncoder & aEncoder);
 };
 
-static FanControlManager * mFanControlManager;
+static FanControlManager * mFanControlManager = nullptr;
 
 CHIP_ERROR FanControlManager::ReadPercentCurrent(AttributeValueEncoder & aEncoder)
 {
@@ -80,22 +80,22 @@ CHIP_ERROR FanControlManager::ReadSpeedCurrent(AttributeValueEncoder & aEncoder)
     return aEncoder.Encode(ret);
 }
 
-Status FanControlManager::HandleStep(StepDirectionEnum direction, bool wrap, bool lowestOff)
+Status FanControlManager::HandleStep(StepDirectionEnum aDirection, bool aWrap, bool aLowestOff)
 {
-    ChipLogProgress(NotSpecified, "FanControlManager::HandleStep direction %d, wrap %d, lowestOff %d", to_underlying(direction),
-                    wrap, lowestOff);
+    ChipLogProgress(NotSpecified, "FanControlManager::HandleStep aDirection %d, aWrap %d, aLowestOff %d", to_underlying(aDirection),
+                    aWrap, aLowestOff);
 
-    VerifyOrReturnError(direction != StepDirectionEnum::kUnknownEnumValue, Status::InvalidCommand);
+    VerifyOrReturnError(aDirection != StepDirectionEnum::kUnknownEnumValue, Status::InvalidCommand);
 
     uint8_t speedMax;
     SpeedMax::Get(mEndpoint, &speedMax);
 
     DataModel::Nullable<uint8_t> speedSetting;
     SpeedSetting::Get(mEndpoint, speedSetting);
 
-    uint8_t newSpeedSetting = speedSetting.Value();
+    uint8_t newSpeedSetting = speedSetting.IsNull() ? 0 : speedSetting.Value();
 
-    if (direction == StepDirectionEnum::kIncrease)
+    if (aDirection == StepDirectionEnum::kIncrease)
     {
         if (speedSetting.IsNull())
         {
@@ -107,36 +107,40 @@ Status FanControlManager::HandleStep(StepDirectionEnum direction, bool wrap, boo
         }
         else if (speedSetting.Value() == speedMax)
         {
-            if (wrap)
+            if (aWrap)
             {
-                newSpeedSetting = lowestOff ? 0 : 1;
+                newSpeedSetting = aLowestOff ? 0 : 1;
             }
         }
     }
-    else if (direction == StepDirectionEnum::kDecrease)
+    else if (aDirection == StepDirectionEnum::kDecrease)
     {
         if (speedSetting.IsNull())
         {
-            newSpeedSetting = lowestOff ? 0 : 1;
+            newSpeedSetting = aLowestOff ? 0 : 1;
         }
         else if ((speedSetting.Value() > 1) && (speedSetting.Value() <= speedMax))
         {
             newSpeedSetting = static_cast<uint8_t>(speedSetting.Value() - 1);
         }
         else if (speedSetting.Value() == 1)
         {
-            if (lowestOff)
+            if (aLowestOff)
             {
                 newSpeedSetting = static_cast<uint8_t>(speedSetting.Value() - 1);
             }
+            else if (aWrap)
+            {
+                newSpeedSetting = speedMax;
+            }
         }
         else if (speedSetting.Value() == 0)
         {
-            if (wrap)
+            if (aWrap)
             {
                 newSpeedSetting = speedMax;
             }
-            else if (!lowestOff)
+            else if (!aLowestOff)
             {
                 newSpeedSetting = 1;
             }
@@ -167,6 +171,7 @@ CHIP_ERROR FanControlManager::Read(const ConcreteReadAttributePath & aPath, Attr
 
 void emberAfFanControlClusterInitCallback(EndpointId endpoint)
 {
+    VerifyOrDie(mFanControlManager == nullptr);
     mFanControlManager = new FanControlManager(endpoint);
     registerAttributeAccessOverride(mFanControlManager);
     FanControl::SetDefaultDelegate(endpoint, mFanControlManager);

examples/all-clusters-app/linux/.kvs

@@ -0,0 +1,28 @@
+[DEFAULT]
+f/1/ac/0/0=FSQBBSQCAjYDBmm2AQAYNAQY
+f/1/g=FSQBACQCACQDACQEACQFACQGASQHABg=
+f/1/i=FTABAQEkAgE3AyQUARgmBIAigScmBYAlTTo3BiQTAhgkBwEkCAEwCUEE3KqJqpPwju/MeqShRDFpusi544wfWj6+gYRG/nor4oHRLfTRz4+zihd9ONWqQ9zwA/CQomn+sjmIvbcRYIYh2DcKNQEpARgkAmAwBBQPVaSOYFdX6pOarWj2mXieOQsSJzAFFGx6kZSTgFw3YQDgKHYjei851xbkGDALQPRuk4wx/ZAQXSmais4GD0JyZl9Jwxq1MM8TUdUQ1nCQ0Q/VuHZqy0pR9HxjgLUDPy2b4YFJIT1rIf8ZYHAeaCMY
+f/1/k/0=FSQBACQCATYDFScEUAIAAOx/AAAlBVRMMAYQ7QtLfz7zOjEY6QUsagDDPRgVJAQAJAUAMAYQAAAAAAAAAAAAAAAAAAAAABgVJAQAJAUAMAYQAAAAAAAAAAAAAAAAAAAAABgYJQf//xg=
+f/1/m=FSUA8f8sAQAY
+f/1/n=FTABAQEkAgE3AyQTAhgmBIAigScmBYAlTTo3BiQVASYRIUM0EhgkBwEkCAEwCUEE7R7vwTlybDjtGBspWXkUMSnID4um6q91pa7EQdCSIioP+AgPAA3zqzXyYunmoZbVijM9UbzOUhvacYvA+2xanzcKNQEoARgkAgE2AwQCBAEYMAQUi5rGWgAt12jH/3fxeswo+bhw+YcwBRQPVaSOYFdX6pOarWj2mXieOQsSJxgwC0AtXja1zqi99vqxUOcDooFbR7g3dR6FHR/DjYJOy2QgtY0m1Krqo9Whb9A//TtnLTpsXj+DKnl4cBJ6Zb1WGdhrGA==
+f/1/o=FSQAATABYQTtHu/BOXJsOO0YGylZeRQxKcgPi6bqr3WlrsRB0JIiKg/4CA8ADfOrNfJi6eahltWKMz1RvM5SG9pxi8D7bFqftpuGwM3zJ0gq/ZXqozNld3ZK32h4PWWNKTlq3zM9wUEY
+f/1/r=FTABAQEkAgE3AyQUARgmBIAigScmBYAlTTo3BiQUARgkBwEkCAEwCUEECT5tju37pQzMYxhc2pumcwtyKq7elKv8mcWFxDqluOWVK6RJYxiytMv4NQA+JFg/rN8AVPzfz2txBhBJKEF0UDcKNQEpARgkAmAwBBRsepGUk4BcN2EA4Ch2I3ovOdcW5DAFFGx6kZSTgFw3YQDgKHYjei851xbkGDALQBPAPvpTcPaJ51tb6/5sPfKuU6QWe2ClszX+T9fCPL4ZSeJZq0ukzeGMqOgnlAh3MVzkugI2IKdO9e/lcAib04QY
+f/1/s/000000000001B669=FTADEO0xJXt8dKE+RAjHrjhJCREwBCAkLjwW/9P1MAh7PKST7kCABWTyP5W6JnAccoRVc8JXPDAFDAAAAAAAAAAAAAAAABg=
+g/a/0/2b/0=BWVuLVVT
+g/a/1/102/7=Gw==
+g/a/1/300/4001=Ag==
+g/a/1/300/7=AAA=
+g/a/1/8/0=/g==
+g/fidx=FSQAAjYBBAEYGA==
+g/gcc=E0Y8Aw==
+g/gdc=PtW/BQ==
+g/gfl=FSQBASQCARg=
+g/im/ec=AAABAAAAAAA=
+g/lkgt=FSYAgwxNLBg=
+g/s/7TEle3x0oT5ECMeuOEkJEQ\x3d\x3d=FSQBASYCabYBABg=
+g/sri=FhUkAQEmAmm2AQAYGA==
+g/sum=MAA=
+g/ts/tz=FhUgAAAkAQAsAkAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAGBg=
+wifi-pass=
+wifi-ssid=
+

src/app/clusters/fan-control-server/fan-control-delegate.h

@@ -38,16 +38,16 @@ class Delegate
      * @brief
      *   This method handles the step command. This will happen as fast as possible.
      *
-     *   @param[in]  direction            direction in which to step
-     *   @param[in]  wrap                 whether the step command wraps or not
-     *   @param[in]  wrap                 whether the step command wraps or not
+     *   @param[in]  aDirection     the direction in which the speed should step
+     *   @param[in]  aWrap          whether the speed should wrap or not
+     *   @param[in]  aLowestOff     whether the device should consider the lowest setting as off
      *
      *   @return Success On success.
      *   @return Other Value indicating it failed to execute the command.
      */
-    virtual Protocols::InteractionModel::Status HandleStep(StepDirectionEnum direction, bool wrap, bool off) = 0;
+    virtual Protocols::InteractionModel::Status HandleStep(StepDirectionEnum aDirection, bool aWrap, bool aLowestOff) = 0;
 
-    Delegate(EndpointId endpoint) : mEndpoint(endpoint) {}
+    Delegate(EndpointId aEndpoint) : mEndpoint(aEndpoint) {}
 
     virtual ~Delegate() = default;
 

src/app/clusters/fan-control-server/fan-control-server.cpp

@@ -58,21 +58,21 @@ namespace app {
 namespace Clusters {
 namespace FanControl {
 
-Delegate * GetDelegate(EndpointId endpoint)
+Delegate * GetDelegate(EndpointId aEndpoint)
 {
     uint16_t ep =
-        emberAfGetClusterServerEndpointIndex(endpoint, FanControl::Id, EMBER_AF_FAN_CONTROL_CLUSTER_SERVER_ENDPOINT_COUNT);
+        emberAfGetClusterServerEndpointIndex(aEndpoint, FanControl::Id, EMBER_AF_FAN_CONTROL_CLUSTER_SERVER_ENDPOINT_COUNT);
     return (ep >= kFanControlDelegateTableSize ? nullptr : gDelegateTable[ep]);
 }
 
-void SetDefaultDelegate(EndpointId endpoint, Delegate * delegate)
+void SetDefaultDelegate(EndpointId aEndpoint, Delegate * aDelegate)
 {
     uint16_t ep =
-        emberAfGetClusterServerEndpointIndex(endpoint, FanControl::Id, EMBER_AF_FAN_CONTROL_CLUSTER_SERVER_ENDPOINT_COUNT);
+        emberAfGetClusterServerEndpointIndex(aEndpoint, FanControl::Id, EMBER_AF_FAN_CONTROL_CLUSTER_SERVER_ENDPOINT_COUNT);
     // if endpoint is found
     if (ep < kFanControlDelegateTableSize)
     {
-        gDelegateTable[ep] = delegate;
+        gDelegateTable[ep] = aDelegate;
     }
 }
 
@@ -460,8 +460,8 @@ bool emberAfFanControlClusterStepCallback(app::CommandHandler * commandObj, cons
         EndpointId endpoint         = commandPath.mEndpointId;
         StepDirectionEnum direction = commandData.direction;
 
-        bool wrapValue      = commandData.wrap.HasValue() ? commandData.wrap.Value() : false;
-        bool lowestOffValue = commandData.lowestOff.HasValue() ? commandData.lowestOff.Value() : false;
+        bool wrapValue      = commandData.wrap.ValueOr(false);
+        bool lowestOffValue = commandData.lowestOff.ValueOr(false);
 
         Delegate * delegate = GetDelegate(endpoint);
         if (delegate)
@@ -471,6 +471,7 @@ bool emberAfFanControlClusterStepCallback(app::CommandHandler * commandObj, cons
         else
         {
             ChipLogProgress(Zcl, "FanControl has no delegate set for endpoint:%u", endpoint);
+            status = Status::Failure;
         }
     }
 

src/app/clusters/fan-control-server/fan-control-server.h

@@ -27,8 +27,8 @@ namespace app {
 namespace Clusters {
 namespace FanControl {
 
-void SetDefaultDelegate(EndpointId endpoint, Delegate * delegate);
-Delegate * GetDelegate(EndpointId endpoint);
+void SetDefaultDelegate(EndpointId aEndpoint, Delegate * aDelegate);
+Delegate * GetDelegate(EndpointId aEndpoint);
 
 } // namespace FanControl
 } // namespace Clusters

src/app/tests/suites/certification/Test_TC_FAN_3_2.yaml

@@ -22,9 +22,6 @@ config:
     nodeId: 0x12344321
     cluster: "Fan Control"
     endpoint: 1
-    ConfigSpeedSetting:
-        type: int8u
-        defaultValue: 50
 
 tests:
     - label: "Step 1: Wait for the commissioned device to be retrieved"
@@ -35,12 +32,21 @@ tests:
               - name: "nodeId"
                 value: nodeId
 
-    - label: "Step 2: TH writes SpeedSetting attribute a valid value to DUT"
+    - label: "Step 2: TH reads from the DUT the SpeedMax attribute"
+      PICS: FAN.S.A0004
+      command: "readAttribute"
+      attribute: "SpeedMax"
+      response:
+          saveAs: rSpeedMax
+
+    - label:
+          "Step 3: TH writes TH writes to the DUT the a value less than or equal
+          to the value read in step 2"
       PICS: FAN.S.A0005
       command: "writeAttribute"
       attribute: "SpeedSetting"
       arguments:
-          value: ConfigSpeedSetting
+          value: rSpeedMax
 
     - label: "Wait 1000ms"
       PICS: FAN.S.A0005
@@ -51,16 +57,16 @@ tests:
               - name: "ms"
                 value: 1000
 
-    - label: "Step 3: TH reads from the DUT the the SpeedSetting attribute"
+    - label: "Step 4: TH reads from the DUT the the SpeedSetting attribute"
       PICS: FAN.S.A0005
       command: "readAttribute"
       attribute: "SpeedSetting"
       response:
-          value: ConfigSpeedSetting
+          value: rSpeedMax
 
-    - label: "Step 4: TH reads from the DUT the the SpeedCurrent attribute"
+    - label: "Step 5: TH reads from the DUT the the SpeedCurrent attribute"
       PICS: FAN.S.A0006
       command: "readAttribute"
       attribute: "SpeedCurrent"
       response:
-          value: ConfigSpeedSetting
+          value: rSpeedMax

src/app/tests/suites/certification/Test_TC_FAN_3_4.yaml → src/app/tests/suites/certification/Test_TC_FAN_3_6.yaml

@@ -12,19 +12,16 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-name: 263.2.9. [TC-FAN-3.4] Optional wind functionality with DUT as Server
+name: 263.2.7. [TC-FAN-3.2] Optional direction functionality with DUT as Server
 
 PICS:
     - FAN.S
-    - FAN.S.F03
+    - FAN.S.F05
 
 config:
     nodeId: 0x12344321
     cluster: "Fan Control"
     endpoint: 1
-    ConfigWindSetting:
-        type: bitmap8
-        defaultValue: 1
 
 tests:
     - label: "Step 1: Wait for the commissioned device to be retrieved"
@@ -35,25 +32,52 @@ tests:
               - name: "nodeId"
                 value: nodeId
 
-    - label: "Step 2: TH writes WindSetting attribute a valid value to DUT"
-      PICS: FAN.S.A0008
+    - label: "Step 2: TH writes a value of Forward to the DUT"
+      PICS: FAN.S.A000B
       command: "writeAttribute"
-      attribute: "WindSetting"
+      attribute: "AirflowDirection"
       arguments:
-          value: ConfigWindSetting
+          value: 0
 
     - label: "Wait 1000ms"
-      PICS: FAN.S.A0008
+      PICS: FAN.S.A000B
       cluster: "DelayCommands"
       command: "WaitForMs"
       arguments:
           values:
               - name: "ms"
                 value: 1000
 
-    - label: "Step 3: TH reads from the DUT the the WindSetting attribute"
-      PICS: FAN.S.A0008
+    - label:
+          "Step 3: TH reads from the DUT the AirflowDirection attribute and
+          check it is forwards"
+      PICS: FAN.S.A000B
       command: "readAttribute"
-      attribute: "WindSetting"
+      attribute: "AirflowDirection"
       response:
-          value: ConfigWindSetting
+          value: 0
+
+    - label: "Step 4: TH writes a value of Reverse to the DUT"
+      PICS: FAN.S.A000B
+      command: "writeAttribute"
+      attribute: "AirflowDirection"
+      arguments:
+          value: 1
+
+    - label: "Wait 1000ms"
+      PICS: FAN.S.A000B
+      cluster: "DelayCommands"
+      command: "WaitForMs"
+      arguments:
+          values:
+              - name: "ms"
+                value: 1000
+
+    - label:
+          "Step 5: TH reads from the DUT the AirflowDirection attribute and
+          check it is reverse"
+      PICS: FAN.S.A000B
+      command: "readAttribute"
+      attribute: "AirflowDirection"
+      response:
+          value: 1

src/app/tests/suites/ciTests.json

@@ -68,7 +68,7 @@
         "Test_TC_FAN_2_5",
         "Test_TC_FAN_3_1",
         "Test_TC_FAN_3_2",
-        "Test_TC_FAN_3_4"
+        "Test_TC_FAN_3_6"
     ],
     "GeneralCommissioning": ["Test_TC_CGEN_1_1", "Test_TC_CGEN_2_1"],
     "GeneralDiagnostics": ["Test_TC_DGGEN_1_1", "Test_TC_DGGEN_2_1"],