Skip to content

Commit

Permalink
Update TestEventTrigger to match SVE2 test plan
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
tcarmelveilleux committed Sep 16, 2022
1 parent 822993c commit e45c879
Show file tree
Hide file tree
Showing 10 changed files with 159 additions and 19 deletions.
1 change: 1 addition & 0 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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" --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 }}
Expand Down
59 changes: 57 additions & 2 deletions examples/platform/linux/AppMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -142,6 +143,52 @@ static bool EnsureWiFiIsStarted()
}
#endif

class SampleTestEventTriggerDelegate : public TestEventTriggerDelegate
{
public:
static constexpr uint64_t kSampleTestEventTriggerAlwaysSuccess = static_cast<uint64_t>(0xFFFF'FFFF'FFFF'FFFEull);

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;
Expand Down Expand Up @@ -298,11 +345,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);
Expand Down
2 changes: 1 addition & 1 deletion src/app/TestEventTriggerDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
9 changes: 6 additions & 3 deletions src/controller/python/chip/interaction_model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class Status(enum.IntEnum):
Deprecated84 = 0x84
InvalidCommand = 0x85
UnsupportedAttribute = 0x86
InvalidValue = 0x87
ConstraintError = 0x87
UnsupportedWrite = 0x88
ResourceExhausted = 0x89
Deprecated8a = 0x8a
Expand All @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion src/controller/python/test/test_scripts/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/controller/python/test/test_scripts/cluster_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,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}")
Expand Down
2 changes: 2 additions & 0 deletions src/protocols/interaction_model/StatusCodeList.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
74 changes: 74 additions & 0 deletions src/python_testing/TC_TestEventTrigger.py
Original file line number Diff line number Diff line change
@@ -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_FFFF_FFFE
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()
10 changes: 9 additions & 1 deletion src/python_testing/matter_testing_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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!")
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e45c879

Please sign in to comment.