From b381ba8f5493232578f8d1274a28c2c0aac60d83 Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Fri, 16 Sep 2022 15:02:51 -0400 Subject: [PATCH] Update TestEventTrigger to match SVE2 test plan (#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 #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 --- .github/workflows/tests.yaml | 1 + examples/platform/linux/AppMain.cpp | 60 ++++++++++++++- src/app/TestEventTriggerDelegate.h | 2 +- .../general-diagnostics-server.cpp | 17 ++--- .../python/chip/interaction_model/__init__.py | 9 ++- .../python/test/test_scripts/base.py | 2 +- .../test/test_scripts/cluster_objects.py | 2 +- .../interaction_model/StatusCodeList.h | 2 + src/python_testing/TC_TestEventTrigger.py | 74 +++++++++++++++++++ src/python_testing/matter_testing_support.py | 10 ++- 10 files changed, 160 insertions(+), 19 deletions(-) create mode 100644 src/python_testing/TC_TestEventTrigger.py diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 0f36999934a58d..ed0f24bad7eec9 100644 --- a/.github/workflows/tests.yaml +++ b/.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 }} diff --git a/examples/platform/linux/AppMain.cpp b/examples/platform/linux/AppMain.cpp index 5a2f22034cab10..876e0da8bc3751 100644 --- a/examples/platform/linux/AppMain.cpp +++ b/examples/platform/linux/AppMain.cpp @@ -70,6 +70,7 @@ #if CHIP_DEVICE_CONFIG_ENABLE_OTA_REQUESTOR #include #endif +#include #include @@ -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__xxxx for your trigger codes. + static constexpr uint64_t kSampleTestEventTriggerAlwaysSuccess = static_cast(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); diff --git a/src/app/TestEventTriggerDelegate.h b/src/app/TestEventTriggerDelegate.h index 5186fc473fd0cc..5f72415d021104 100644 --- a/src/app/TestEventTriggerDelegate.h +++ b/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; }; diff --git a/src/app/clusters/general-diagnostics-server/general-diagnostics-server.cpp b/src/app/clusters/general-diagnostics-server/general-diagnostics-server.cpp index 0ab14a94540115..0d7add50c721ae 100644 --- a/src/app/clusters/general-diagnostics-server/general-diagnostics-server.cpp +++ b/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; } diff --git a/src/controller/python/chip/interaction_model/__init__.py b/src/controller/python/chip/interaction_model/__init__.py index bc1fa35c214865..58ba600e3667e4 100644 --- a/src/controller/python/chip/interaction_model/__init__.py +++ b/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,7 +62,6 @@ class Status(enum.IntEnum): Reserved98 = 0x98 Reserved99 = 0x99 Reserved9a = 0x9a - ConstraintError = 0x9b Busy = 0x9c Deprecatedc0 = 0xc0 Deprecatedc1 = 0xc1 @@ -70,7 +69,11 @@ class Status(enum.IntEnum): UnsupportedCluster = 0xc3 Deprecatedc4 = 0xc4 NoUpstreamSubscription = 0xc5 - InvalidArgument = 0xc6 + NeedsTimedInteraction = 0xc6 + UnsupportedEvent = 0xc7 + PathsExhausted = 0xc8 + TimedRequestMismatch = 0xc9 + FailsafeRequired = 0xca class InteractionModelError(ChipStackException): diff --git a/src/controller/python/test/test_scripts/base.py b/src/controller/python/test/test_scripts/base.py index 03249474d799c9..cab2f665ffe65d 100644 --- a/src/controller/python/test/test_scripts/base.py +++ b/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: diff --git a/src/controller/python/test/test_scripts/cluster_objects.py b/src/controller/python/test/test_scripts/cluster_objects.py index 896d0c79103421..2145893ddf2aea 100644 --- a/src/controller/python/test/test_scripts/cluster_objects.py +++ b/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}") diff --git a/src/protocols/interaction_model/StatusCodeList.h b/src/protocols/interaction_model/StatusCodeList.h index f2b7a96c6770cf..62891c2e7b978d 100644 --- a/src/protocols/interaction_model/StatusCodeList.h +++ b/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) diff --git a/src/python_testing/TC_TestEventTrigger.py b/src/python_testing/TC_TestEventTrigger.py new file mode 100644 index 00000000000000..8834417e41d847 --- /dev/null +++ b/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() diff --git a/src/python_testing/matter_testing_support.py b/src/python_testing/matter_testing_support.py index 98ffa4bf064e0e..e558d5389ba2da 100644 --- a/src/python_testing/matter_testing_support.py +++ b/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