Update TestEventTrigger to match SVE2 test plan (project-chip#22695)

* Update TestEventTrigger to match SVE2 test plan

- There was a 3-way mismatch between spec, SDK and SVE2 test plans
  in the TestEventTrigger command.
- Since this command is meant for cert testing and 1.0 spec is loced
  down, it was determined that making behavior match TC-DGEN-2.3
  is the best outcome.

Fixes project-chip#22232

Changes:

- Make all EnableKey errors are ConstraintError
- Add test event trigger support to all Linux examples by default
  with a command that always succeeds in the reserved range
- Add integration test for feature
- Fix Python IM status codes list that had an old name
- Add "commission-only" mode to matter_testing_support.py done while
  testing this feature

Testing done:
- All unit tests still pass
- Integration tests pass, including new TC_TestEventTrigger.py

* Update some comments

* Fix CI YAML config

Author: tcarmelveilleux

.github/workflows/tests.yaml

@@ -285,6 +285,7 @@ jobs:
                   scripts/run_in_build_env.sh './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_RR_1_1.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021"'
                   scripts/run_in_build_env.sh './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_SC_3_6.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021"'
                   scripts/run_in_build_env.sh './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_7.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --bool-arg allow_sdk_dac:true"'
+                  scripts/run_in_build_env.sh './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 --enable-key 000102030405060708090a0b0c0d0e0f" --script "src/python_testing/TC_TestEventTrigger.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --bool-arg allow_sdk_dac:true"'
             - name: Uploading core files
               uses: actions/upload-artifact@v2
               if: ${{ failure() && !env.ACT }}

examples/platform/linux/AppMain.cpp

@@ -70,6 +70,7 @@
 #if CHIP_DEVICE_CONFIG_ENABLE_OTA_REQUESTOR
 #include <app/clusters/ota-requestor/OTATestEventTriggerDelegate.h>
 #endif
+#include <app/TestEventTriggerDelegate.h>
 
 #include <signal.h>
 
@@ -142,6 +143,53 @@ static bool EnsureWiFiIsStarted()
 }
 #endif
 
+class SampleTestEventTriggerDelegate : public TestEventTriggerDelegate
+{
+public:
+    /// NOTE: If you copy this, please use the reserved range FFFF_FFFF_<VID_HEX>_xxxx for your trigger codes.
+    static constexpr uint64_t kSampleTestEventTriggerAlwaysSuccess = static_cast<uint64_t>(0xFFFF'FFFF'FFF1'0000ull);
+
+    SampleTestEventTriggerDelegate() { memset(&mEnableKey[0], 0, sizeof(mEnableKey)); }
+
+    /**
+     * @brief Initialize the delegate with a key and an optional other handler
+     *
+     * The `otherDelegate` will be called if there is no match of the eventTrigger
+     * when HandleEventTrigger is called, if it is non-null.
+     *
+     * @param enableKey - EnableKey to use for this instance.
+     * @param otherDelegate - Other delegate (e.g. OTA delegate) where defer trigger. Can be nullptr
+     * @return CHIP_NO_ERROR on success, CHIP_ERROR_INVALID_ARGUMENT if enableKey is wrong size.
+     */
+    CHIP_ERROR Init(ByteSpan enableKey, TestEventTriggerDelegate * otherDelegate)
+    {
+        VerifyOrReturnError(enableKey.size() == sizeof(mEnableKey), CHIP_ERROR_INVALID_ARGUMENT);
+        mOtherDelegate = otherDelegate;
+        MutableByteSpan ourEnableKeySpan(mEnableKey);
+        return CopySpanToMutableSpan(enableKey, ourEnableKeySpan);
+    }
+
+    bool DoesEnableKeyMatch(const ByteSpan & enableKey) const override { return enableKey.data_equal(ByteSpan(mEnableKey)); }
+
+    CHIP_ERROR HandleEventTrigger(uint64_t eventTrigger) override
+    {
+        ChipLogProgress(Support, "Saw TestEventTrigger: " ChipLogFormatX64, ChipLogValueX64(eventTrigger));
+
+        if (eventTrigger == kSampleTestEventTriggerAlwaysSuccess)
+        {
+            // Do nothing, successfully
+            ChipLogProgress(Support, "Handling \"Always success\" internal test event");
+            return CHIP_NO_ERROR;
+        }
+
+        return (mOtherDelegate != nullptr) ? mOtherDelegate->HandleEventTrigger(eventTrigger) : CHIP_ERROR_INVALID_ARGUMENT;
+    }
+
+private:
+    uint8_t mEnableKey[TestEventTriggerDelegate::kEnableKeyLength];
+    TestEventTriggerDelegate * mOtherDelegate = nullptr;
+};
+
 int ChipLinuxAppInit(int argc, char * const argv[], OptionSet * customOptions)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
@@ -298,11 +346,19 @@ void ChipLinuxAppMainLoop()
         initParams.operationalKeystore = &LinuxDeviceOptions::GetInstance().mCSRResponseOptions.badCsrOperationalKeyStoreForTest;
     }
 
+    TestEventTriggerDelegate * otherDelegate = nullptr;
 #if CHIP_DEVICE_CONFIG_ENABLE_OTA_REQUESTOR
-    static OTATestEventTriggerDelegate testEventTriggerDelegate{ ByteSpan(
+    // We want to allow triggering OTA queries if OTA requestor is enabled
+    static OTATestEventTriggerDelegate otaTestEventTriggerDelegate{ ByteSpan(
         LinuxDeviceOptions::GetInstance().testEventTriggerEnableKey) };
-    initParams.testEventTriggerDelegate = &testEventTriggerDelegate;
+    otherDelegate = &otaTestEventTriggerDelegate;
 #endif
+    // For general testing of TestEventTrigger, we have a common "core" event trigger delegate.
+    static SampleTestEventTriggerDelegate testEventTriggerDelegate;
+    VerifyOrDie(testEventTriggerDelegate.Init(ByteSpan(LinuxDeviceOptions::GetInstance().testEventTriggerEnableKey),
+                                              otherDelegate) == CHIP_NO_ERROR);
+
+    initParams.testEventTriggerDelegate = &testEventTriggerDelegate;
 
     // We need to set DeviceInfoProvider before Server::Init to setup the storage of DeviceInfoProvider properly.
     DeviceLayer::SetDeviceInfoProvider(&gExampleDeviceInfoProvider);

src/app/TestEventTriggerDelegate.h

@@ -44,7 +44,7 @@ class TestEventTriggerDelegate
      *
      * @param[in] eventTrigger Event trigger to handle.
      *
-     * @return CHIP_ERROR_INVALID_ARGUMENT when eventTrigger is not a valid test event trigger.
+     * @return CHIP_NO_ERROR on success or another CHIP_ERROR on failure
      */
     virtual CHIP_ERROR HandleEventTrigger(uint64_t eventTrigger) = 0;
 };

src/app/clusters/general-diagnostics-server/general-diagnostics-server.cpp

@@ -364,23 +364,20 @@ bool emberAfGeneralDiagnosticsClusterTestEventTriggerCallback(CommandHandler * c
 
     auto * triggerDelegate = chip::Server::GetInstance().GetTestEventTriggerDelegate();
 
+    // Spec says "EnableKeyMismatch" but this never existed prior to 1.0 SVE2 and mismatches
+    // test plans as well. ConstraintError is specified for most other errors, so
+    // we keep the behavior as close as possible, except for EnableKeyMismatch which
+    // is going to be a ConstraintError.
     if (triggerDelegate == nullptr || !triggerDelegate->DoesEnableKeyMatch(commandData.enableKey))
     {
-        commandObj->AddStatus(commandPath, Status::UnsupportedAccess);
+        commandObj->AddStatus(commandPath, Status::ConstraintError);
         return true;
     }
 
     CHIP_ERROR handleEventTriggerResult = triggerDelegate->HandleEventTrigger(commandData.eventTrigger);
-    Status returnStatus                 = StatusIB(handleEventTriggerResult).mStatus;
-
-    // When HandleEventTrigger returns INVALID_ARGUMENT we convert that into InvalidCommand to be spec
-    // compliant.
-    if (handleEventTriggerResult == CHIP_ERROR_INVALID_ARGUMENT)
-    {
-        returnStatus = Status::InvalidCommand;
-    }
 
-    commandObj->AddStatus(commandPath, returnStatus);
+    // When HandleEventTrigger fails, we simply convert any error to INVALID_COMMAND
+    commandObj->AddStatus(commandPath, (handleEventTriggerResult != CHIP_NO_ERROR) ? Status::InvalidCommand : Status::Success);
     return true;
 }
 

src/controller/python/chip/interaction_model/__init__.py

@@ -42,7 +42,7 @@ class Status(enum.IntEnum):
     Deprecated84 = 0x84
     InvalidCommand = 0x85
     UnsupportedAttribute = 0x86
-    InvalidValue = 0x87
+    ConstraintError = 0x87
     UnsupportedWrite = 0x88
     ResourceExhausted = 0x89
     Deprecated8a = 0x8a
@@ -62,15 +62,18 @@ class Status(enum.IntEnum):
     Reserved98 = 0x98
     Reserved99 = 0x99
     Reserved9a = 0x9a
-    ConstraintError = 0x9b
     Busy = 0x9c
     Deprecatedc0 = 0xc0
     Deprecatedc1 = 0xc1
     Deprecatedc2 = 0xc2
     UnsupportedCluster = 0xc3
     Deprecatedc4 = 0xc4
     NoUpstreamSubscription = 0xc5
-    InvalidArgument = 0xc6
+    NeedsTimedInteraction = 0xc6
+    UnsupportedEvent = 0xc7
+    PathsExhausted = 0xc8
+    TimedRequestMismatch = 0xc9
+    FailsafeRequired = 0xca
 
 
 class InteractionModelError(ChipStackException):

src/controller/python/test/test_scripts/base.py

@@ -1064,7 +1064,7 @@ class AttributeWriteRequest:
         requests = [
             AttributeWriteRequest("Basic", "NodeLabel", "Test"),
             AttributeWriteRequest("Basic", "Location",
-                                  "a pretty loooooooooooooog string", IM.Status.InvalidValue),
+                                  "a pretty loooooooooooooog string", IM.Status.ConstraintError),
         ]
         failed_zcl = []
         for req in requests:

src/controller/python/test/test_scripts/cluster_objects.py

@@ -145,7 +145,7 @@ async def TestWriteRequest(cls, devCtrl):
             AttributeStatus(Path=AttributePath(EndpointId=0, ClusterId=40,
                             AttributeId=5), Status=chip.interaction_model.Status.Success),
             AttributeStatus(Path=AttributePath(EndpointId=0, ClusterId=40,
-                            AttributeId=6), Status=chip.interaction_model.Status.InvalidValue)
+                            AttributeId=6), Status=chip.interaction_model.Status.ConstraintError)
         ]
 
         logger.info(f"Received WriteResponse: {res}")

src/protocols/interaction_model/StatusCodeList.h

@@ -22,6 +22,8 @@
  * include this file, then undefine the macro.
  */
 
+/// WARNING: If you touch this list, please also update src/controller/python/chip/interaction_model/__init__.py
+
 // clang-format off
 CHIP_IM_STATUS_CODE(Success               , SUCCESS                 , 0x0)
 CHIP_IM_STATUS_CODE(Failure               , FAILURE                 , 0x01)

src/python_testing/TC_TestEventTrigger.py

@@ -0,0 +1,74 @@
+#
+#    Copyright (c) 2022 Project CHIP Authors
+#    All rights reserved.
+#
+#    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.
+#
+
+from imaplib import Commands
+from matter_testing_support import MatterBaseTest, default_matter_test_main, async_test_body
+from chip.interaction_model import Status, InteractionModelError
+import chip.clusters as Clusters
+import logging
+from mobly import asserts
+
+# Assumes `--enable-key 000102030405060708090a0b0c0d0e0f` on Linux app command line, or a DUT
+# that has that Enable Key
+kExpectedKey = bytes([b for b in range(16)])
+
+kBadKey = bytes([(b + 1) for b in range(16)])
+
+kAllZerosKey = bytes(b"\x00" * 16)
+
+# Assumes `SampleTestEventTriggerDelegate` as it exists in Linux AppMain.cpp
+kValidEventTrigger = 0xFFFF_FFFF_FFF1_0000
+kInvalidEventTrigger = 0  # Per TC-DGEN-2.3
+
+
+class TestEventTrigger(MatterBaseTest):
+    @async_test_body
+    async def test_all_zeros_key(self):
+        dev_ctrl = self.default_controller
+        with asserts.assert_raises_regex(InteractionModelError, "ConstraintError", "All-zero TestEventTrigger key must return ConstraintError"):
+            await dev_ctrl.SendCommand(self.dut_node_id, endpoint=0, payload=Clusters.GeneralDiagnostics.Commands.TestEventTrigger(enableKey=kAllZerosKey, eventTrigger=kValidEventTrigger))
+
+    @async_test_body
+    async def test_incorrect_key(self):
+        dev_ctrl = self.default_controller
+        test_event_triggers_enabled = await self.read_single_attribute(dev_ctrl, self.dut_node_id, endpoint=0, attribute=Clusters.GeneralDiagnostics.Attributes.TestEventTriggersEnabled)
+        asserts.assert_true(test_event_triggers_enabled, "This test expects Test Event Triggers are Enabled")
+
+        with asserts.assert_raises_regex(InteractionModelError, "ConstraintError", "Bad TestEventTrigger key must return ConstraintError"):
+            await dev_ctrl.SendCommand(self.dut_node_id, endpoint=0, payload=Clusters.GeneralDiagnostics.Commands.TestEventTrigger(enableKey=kBadKey, eventTrigger=kValidEventTrigger))
+
+    @async_test_body
+    async def test_correct_key_valid_code(self):
+        dev_ctrl = self.default_controller
+        test_event_triggers_enabled = await self.read_single_attribute(dev_ctrl, self.dut_node_id, endpoint=0, attribute=Clusters.GeneralDiagnostics.Attributes.TestEventTriggersEnabled)
+        asserts.assert_true(test_event_triggers_enabled, "This test expects Test Event Triggers are Enabled")
+
+        # No response to command --> Success yields "None".
+        asserts.assert_is_none(await dev_ctrl.SendCommand(self.dut_node_id, endpoint=0, payload=Clusters.GeneralDiagnostics.Commands.TestEventTrigger(enableKey=kExpectedKey, eventTrigger=kValidEventTrigger)))
+
+    @async_test_body
+    async def test_correct_key_invalid_code(self):
+        dev_ctrl = self.default_controller
+        test_event_triggers_enabled = await self.read_single_attribute(dev_ctrl, self.dut_node_id, endpoint=0, attribute=Clusters.GeneralDiagnostics.Attributes.TestEventTriggersEnabled)
+        asserts.assert_true(test_event_triggers_enabled, "This test expects Test Event Triggers are Enabled")
+
+        with asserts.assert_raises_regex(InteractionModelError, "InvalidCommand", "Unsupported EventTrigger must return InvalidCommand"):
+            await dev_ctrl.SendCommand(self.dut_node_id, endpoint=0, payload=Clusters.GeneralDiagnostics.Commands.TestEventTrigger(enableKey=kExpectedKey, eventTrigger=kInvalidEventTrigger))
+
+
+if __name__ == "__main__":
+    default_matter_test_main()

src/python_testing/matter_testing_support.py

@@ -122,6 +122,7 @@ class MatterTestConfig:
     logs_path: pathlib.Path = None
     paa_trust_store_path: pathlib.Path = None
     ble_interface_id: int = None
+    commission_only: bool = False
 
     admin_vendor_id: int = _DEFAULT_ADMIN_VENDOR_ID
     case_admin_subject: int = None
@@ -428,6 +429,7 @@ def populate_commissioning_args(args: argparse.Namespace, config: MatterTestConf
         return True
 
     config.commissioning_method = args.commissioning_method
+    config.commission_only = args.commission_only
 
     if args.dut_node_id is None:
         print("error: When --commissioning-method present, --dut-node-id is mandatory!")
@@ -585,6 +587,9 @@ def parse_matter_test_args(argv: List[str]) -> MatterTestConfig:
     commission_group.add_argument('--case-admin-subject', action="store", type=int_decimal_or_hex,
                                   metavar="CASE_ADMIN_SUBJECT", help="Set the CASE admin subject to an explicit value (default to commissioner Node ID)")
 
+    commission_group.add_argument('--commission-only', action="store_true", default=False,
+                                  help="If true, test exits after commissioning without running subsequent tests")
+
     code_group = parser.add_mutually_exclusive_group(required=False)
 
     code_group.add_argument('-q', '--qr-code', type=str,
@@ -731,7 +736,10 @@ def default_matter_test_main(argv=None, **kwargs):
         if matter_test_config.commissioning_method is not None:
             runner.add_test_class(test_config, CommissionDeviceTest, None)
 
-        runner.add_test_class(test_config, test_class, tests)
+        # Add the tests selected unless we have a commission-only request
+        if not matter_test_config.commission_only:
+            runner.add_test_class(test_config, test_class, tests)
+
         try:
             runner.run()
             ok = runner.results.is_all_pass and ok