From 08e268e34690d05547c98eadd32eeca32d9394cb Mon Sep 17 00:00:00 2001 From: cecille Date: Fri, 9 Feb 2024 14:38:28 -0500 Subject: [PATCH 01/11] TC-IDM-10.1: Support write-only attributes Write only attributes are not returned in the wildcard, but will return the UNSUPPORTED_READ error if we attempt to read them in a concrete path. We can detect their presence by probing for this error via a read. --- .../python/chip/clusters/ClusterObjects.py | 6 +- .../TC_DeviceBasicComposition.py | 65 ++++++++++++++++--- .../basic_composition_support.py | 2 +- 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/controller/python/chip/clusters/ClusterObjects.py b/src/controller/python/chip/clusters/ClusterObjects.py index 0aef317257014f..111274c884a8dd 100644 --- a/src/controller/python/chip/clusters/ClusterObjects.py +++ b/src/controller/python/chip/clusters/ClusterObjects.py @@ -301,7 +301,7 @@ def __init_subclass__(cls, *args, **kwargs) -> None: """Register a subclass.""" super().__init_subclass__(*args, **kwargs) try: - if cls.cluster_id not in ALL_ATTRIBUTES: + if cls.standard_attribute and cls.cluster_id not in ALL_ATTRIBUTES: ALL_ATTRIBUTES[cls.cluster_id] = {} # register this clusterattribute in the ALL_ATTRIBUTES dict for quick lookups ALL_ATTRIBUTES[cls.cluster_id][cls.attribute_id] = cls @@ -345,6 +345,10 @@ def attribute_type(cls) -> ClusterObjectFieldDescriptor: def must_use_timed_write(cls) -> bool: return False + @ChipUtility.classproperty + def standard_attribute(cls) -> bool: + return True + @ChipUtility.classproperty def _cluster_object(cls) -> ClusterObject: return make_dataclass('InternalClass', diff --git a/src/python_testing/TC_DeviceBasicComposition.py b/src/python_testing/TC_DeviceBasicComposition.py index 446907fe585dbc..06c18442984538 100644 --- a/src/python_testing/TC_DeviceBasicComposition.py +++ b/src/python_testing/TC_DeviceBasicComposition.py @@ -23,7 +23,11 @@ import chip.clusters.ClusterObjects import chip.tlv from basic_composition_support import BasicCompositionTests +from chip import ChipUtility from chip.clusters.Attribute import ValueDecodeFailure +from chip.clusters.ClusterObjects import ClusterAttributeDescriptor, ClusterObjectFieldDescriptor +from chip.interaction_model import InteractionModelError, Status +from chip.tlv import uint from global_attribute_ids import GlobalAttributeIds from matter_testing_support import (AttributePathLocation, ClusterPathLocation, CommandPathLocation, MatterBaseTest, async_test_body, default_matter_test_main) @@ -32,6 +36,9 @@ find_tree_roots, get_all_children, get_direct_children_of_root, parts_list_cycles, separate_endpoint_types) +# The above code is importing various classes from the "ClusterObjects" module and assigning them to +# variables. These classes are likely used for creating and managing clusters of objects in a program. + def check_int_in_range(min_value: int, max_value: int, allow_null: bool = False) -> Callable: """Returns a checker for whether `obj` is an int that fits in a range.""" @@ -165,7 +172,43 @@ def test_TC_DT_1_1(self): if not success: self.fail_current_test("At least one endpoint was missing the descriptor cluster.") - def test_TC_IDM_10_1(self): + async def _read_non_standard_attribute_check_unsupported_read(self, endpoint_id, cluster_id, attribute_id) -> bool: + @dataclass + class TempAttribute(ClusterAttributeDescriptor): + @ChipUtility.classproperty + def cluster_id(cls) -> int: + return cluster_id + + @ChipUtility.classproperty + def attribute_id(cls) -> int: + return attribute_id + + @ChipUtility.classproperty + def attribute_type(cls) -> ClusterObjectFieldDescriptor: + return ClusterObjectFieldDescriptor(Type=uint) + + @ChipUtility.classproperty + def standard_attribute(cls) -> bool: + return False + + value: 'uint' = 0 + + result = await self.default_controller.Read(nodeid=self.dut_node_id, attributes=[(endpoint_id, TempAttribute)]) + try: + attr_ret = result.tlvAttributes[endpoint_id][cluster_id][attribute_id] + except KeyError: + attr_ret = None + + print(attr_ret) + + error_type_ok = attr_ret is not None and isinstance( + attr_ret, Clusters.Attribute.ValueDecodeFailure) and isinstance(attr_ret.Reason, InteractionModelError) + + got_expected_error = error_type_ok and attr_ret.Reason.status == Status.UnsupportedRead + return got_expected_error + + @async_test_body + async def test_TC_IDM_10_1(self): self.print_step(1, "Perform a wildcard read of attributes on all endpoints - already done") @dataclass @@ -236,15 +279,19 @@ class RequiredMandatoryAttribute: logging.debug( f"Checking presence of claimed supported {attribute_string} on {location.as_cluster_string(self.cluster_mapper)}: {'found' if has_attribute else 'not_found'}") - # Check attribute is actually present. if not has_attribute: - # TODO: Handle detecting write-only attributes from schema. - if "WriteOnly" in attribute_string: - continue - - self.record_error(self.get_test_name(), location=location, - problem=f"Did not find {attribute_string} on {location.as_cluster_string(self.cluster_mapper)} when it was claimed in AttributeList ({attribute_list})", spec_location="AttributeList Attribute") - success = False + # Check if this is a write-only attribute by trying to read it. + # If it's present and write-only it should return an UNSUPPORTED_READ error. All other errors are a failure. + # Because these can be MEI attributes, we need to build the ClusterAttributeDescriptor manually since it's + # not guaranteed to be generated. Since we expect an error back anyway, the type doesn't matter. + + write_only_attribute = await self._read_non_standard_attribute_check_unsupported_read( + endpoint_id=endpoint_id, cluster_id=cluster_id, attribute_id=attribute_id) + + if not write_only_attribute: + self.record_error(self.get_test_name(), location=location, + problem=f"Did not find {attribute_string} on {location.as_cluster_string(self.cluster_mapper)} when it was claimed in AttributeList ({attribute_list})", spec_location="AttributeList Attribute") + success = False continue attribute_value = cluster[attribute_id] diff --git a/src/python_testing/basic_composition_support.py b/src/python_testing/basic_composition_support.py index 8a6494eccea42f..72c58f9d82a82d 100644 --- a/src/python_testing/basic_composition_support.py +++ b/src/python_testing/basic_composition_support.py @@ -119,7 +119,7 @@ async def setup_class_helper(self, default_to_pase: bool = True): address = f"{commissionable_node.addresses[0]}" logging.info(f"Found instance {instance_name}, VID={vid}, PID={pid}, Address={address}") - node_id = 1 + node_id = self.dut_node_id dev_ctrl.EstablishPASESessionIP(address, info.passcode, node_id) else: asserts.fail("Failed to find the DUT according to command line arguments.") From 2a38fada2e277eef4af709b85572b28953a545ba Mon Sep 17 00:00:00 2001 From: cecille Date: Fri, 9 Feb 2024 14:55:26 -0500 Subject: [PATCH 02/11] remove random comment --- src/python_testing/TC_DeviceBasicComposition.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/python_testing/TC_DeviceBasicComposition.py b/src/python_testing/TC_DeviceBasicComposition.py index 06c18442984538..30bfdb93519897 100644 --- a/src/python_testing/TC_DeviceBasicComposition.py +++ b/src/python_testing/TC_DeviceBasicComposition.py @@ -36,9 +36,6 @@ find_tree_roots, get_all_children, get_direct_children_of_root, parts_list_cycles, separate_endpoint_types) -# The above code is importing various classes from the "ClusterObjects" module and assigning them to -# variables. These classes are likely used for creating and managing clusters of objects in a program. - def check_int_in_range(min_value: int, max_value: int, allow_null: bool = False) -> Callable: """Returns a checker for whether `obj` is an int that fits in a range.""" From 9745323a58f6c3786949a5d3305ec9f84b884c4f Mon Sep 17 00:00:00 2001 From: cecille Date: Mon, 12 Feb 2024 17:38:25 -0500 Subject: [PATCH 03/11] Establish PASE session from code --- .../ChipDeviceController-ScriptBinding.cpp | 8 +++++++ src/controller/python/chip/ChipDeviceCtrl.py | 12 +++++++++++ .../basic_composition_support.py | 21 +++---------------- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index 2576a360307611..49241c88f3c6b2 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -153,6 +153,8 @@ PyChipError pychip_DeviceController_EstablishPASESessionIP(chip::Controller::Dev uint32_t setupPINCode, chip::NodeId nodeid, uint16_t port); PyChipError pychip_DeviceController_EstablishPASESessionBLE(chip::Controller::DeviceCommissioner * devCtrl, uint32_t setupPINCode, uint16_t discriminator, chip::NodeId nodeid); +PyChipError pychip_DeviceController_EstablishPASESession(chip::Controller::DeviceCommissioner * devCtrl, const char* setUpCode, + chip::NodeId nodeid); PyChipError pychip_DeviceController_Commission(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid); PyChipError pychip_DeviceController_DiscoverCommissionableNodesLongDiscriminator(chip::Controller::DeviceCommissioner * devCtrl, @@ -620,6 +622,12 @@ PyChipError pychip_DeviceController_EstablishPASESessionBLE(chip::Controller::De return ToPyChipError(devCtrl->EstablishPASEConnection(nodeid, params)); } +PyChipError pychip_DeviceController_EstablishPASESession(chip::Controller::DeviceCommissioner * devCtrl, const char* setUpCode, + chip::NodeId nodeid) { + sPairingDelegate.SetExpectingPairingComplete(true); + return ToPyChipError(devCtrl->EstablishPASEConnection(nodeid, setUpCode)); +} + PyChipError pychip_DeviceController_Commission(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid) { CommissioningParameters params; diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py index 2297dfff3049ab..228f528e65dbff 100644 --- a/src/controller/python/chip/ChipDeviceCtrl.py +++ b/src/controller/python/chip/ChipDeviceCtrl.py @@ -500,6 +500,15 @@ def EstablishPASESessionIP(self, ipaddr: str, setupPinCode: int, nodeid: int, po self.devCtrl, ipaddr.encode("utf-8"), setupPinCode, nodeid, port) ) + def EstablishPASESession(self, setUpCode: str, nodeid: int): + self.CheckIsActive() + + self.state = DCState.RENDEZVOUS_ONGOING + return self._ChipStack.CallAsync( + lambda: self._dmLib.pychip_DeviceController_EstablishPASESession( + self.devCtrl, setUpCode.encode("utf-8"), nodeid) + ) + def GetTestCommissionerUsed(self): return self._ChipStack.Call( lambda: self._dmLib.pychip_TestCommissionerUsed() @@ -1605,6 +1614,9 @@ def _InitLib(self): self._dmLib.pychip_DeviceController_EstablishPASESessionBLE.argtypes = [ c_void_p, c_uint32, c_uint16, c_uint64] self._dmLib.pychip_DeviceController_EstablishPASESessionBLE.restype = PyChipError + self._dmLib.pychip_DeviceController_EstablishPASESession.argtypes = [ + c_void_p, c_char_p, c_uint64] + self._dmLib.pychip_DeviceController_EstablishPASESession.restype = PyChipError self._dmLib.pychip_DeviceController_DiscoverAllCommissionableNodes.argtypes = [ c_void_p] diff --git a/src/python_testing/basic_composition_support.py b/src/python_testing/basic_composition_support.py index 72c58f9d82a82d..d3dbc28384ca93 100644 --- a/src/python_testing/basic_composition_support.py +++ b/src/python_testing/basic_composition_support.py @@ -105,24 +105,9 @@ async def setup_class_helper(self, default_to_pase: bool = True): dump_device_composition_path: Optional[str] = self.user_params.get("dump_device_composition_path", None) if do_test_over_pase: - info = self.get_setup_payload_info() - - commissionable_nodes = dev_ctrl.DiscoverCommissionableNodes( - info.filter_type, info.filter_value, stopOnFirst=True, timeoutSecond=15) - logging.info(f"Commissionable nodes: {commissionable_nodes}") - # TODO: Support BLE - if commissionable_nodes is not None and len(commissionable_nodes) > 0: - commissionable_node = commissionable_nodes[0] - instance_name = f"{commissionable_node.instanceName}._matterc._udp.local" - vid = f"{commissionable_node.vendorId}" - pid = f"{commissionable_node.productId}" - address = f"{commissionable_node.addresses[0]}" - logging.info(f"Found instance {instance_name}, VID={vid}, PID={pid}, Address={address}") - - node_id = self.dut_node_id - dev_ctrl.EstablishPASESessionIP(address, info.passcode, node_id) - else: - asserts.fail("Failed to find the DUT according to command line arguments.") + setupCode = self.matter_test_config.qr_code_content if self.matter_test_config.qr_code_content is not None else self.matter_test_config.manual_code + asserts.assert_true(setupCode, "Require either --qr-code or --manual-code.") + dev_ctrl.EstablishPASESession(setupCode, self.dut_node_id) else: # Using the already commissioned node node_id = self.dut_node_id From 8f6bc293c9acd0cc927de445cccf40967915f21e Mon Sep 17 00:00:00 2001 From: Cecille Freeman Date: Mon, 12 Feb 2024 17:58:02 -0500 Subject: [PATCH 04/11] fix node id --- src/python_testing/basic_composition_support.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/python_testing/basic_composition_support.py b/src/python_testing/basic_composition_support.py index d3dbc28384ca93..4a516f67ecbc97 100644 --- a/src/python_testing/basic_composition_support.py +++ b/src/python_testing/basic_composition_support.py @@ -107,7 +107,8 @@ async def setup_class_helper(self, default_to_pase: bool = True): if do_test_over_pase: setupCode = self.matter_test_config.qr_code_content if self.matter_test_config.qr_code_content is not None else self.matter_test_config.manual_code asserts.assert_true(setupCode, "Require either --qr-code or --manual-code.") - dev_ctrl.EstablishPASESession(setupCode, self.dut_node_id) + node_id = self.dut_node_id + dev_ctrl.EstablishPASESession(setupCode, node_id) else: # Using the already commissioned node node_id = self.dut_node_id From 50f040c87f435fd5f64c305522472da59231c8f4 Mon Sep 17 00:00:00 2001 From: Cecille Freeman Date: Mon, 12 Feb 2024 17:58:39 -0500 Subject: [PATCH 05/11] Catch KeyError on missing global This would have already been validated in the last step, so catch and release this error and move on. --- src/python_testing/TC_DeviceBasicComposition.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/python_testing/TC_DeviceBasicComposition.py b/src/python_testing/TC_DeviceBasicComposition.py index 30bfdb93519897..55f0167a653128 100644 --- a/src/python_testing/TC_DeviceBasicComposition.py +++ b/src/python_testing/TC_DeviceBasicComposition.py @@ -262,6 +262,9 @@ class RequiredMandatoryAttribute: problem=f"Failed validation of value on {location.as_string(self.cluster_mapper)}: {str(e)}", spec_location="Global Elements") success = False continue + except KeyError: + # This would have been caught already in the previous step + continue self.print_step(4, "Validate the attribute list exactly matches the set of reported attributes") if success: From 76dfd179c77209fe2116d537f25690175005b019 Mon Sep 17 00:00:00 2001 From: Cecille Freeman Date: Mon, 12 Feb 2024 17:59:25 -0500 Subject: [PATCH 06/11] Fix global attribute names --- src/python_testing/matter_testing_support.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/python_testing/matter_testing_support.py b/src/python_testing/matter_testing_support.py index 2f3ca96d12038d..ea1cf3e5257fdc 100644 --- a/src/python_testing/matter_testing_support.py +++ b/src/python_testing/matter_testing_support.py @@ -60,6 +60,7 @@ from chip.setup_payload import SetupPayload from chip.storage import PersistentStorage from chip.tracing import TracingContext +from global_attribute_ids import GlobalAttributeIds from mobly import asserts, base_test, signals, utils from mobly.config_parser import ENV_MOBLY_LOGPATH, TestRunConfig from mobly.test_runner import TestRunner @@ -412,6 +413,8 @@ def get_cluster_string(self, cluster_id: int) -> str: return f"Cluster {name} ({cluster_id}, 0x{cluster_id:04X})" def get_attribute_string(self, cluster_id: int, attribute_id) -> str: + if attribute_id in GlobalAttributeIds: + return f"Attribute {GlobalAttributeIds(attribute_id).name} {attribute_id}, 0x{attribute_id:04X}" mapping = self._mapping._CLUSTER_ID_DICT.get(cluster_id, None) if not mapping: return f"Attribute Unknown ({attribute_id}, 0x{attribute_id:08X})" From 3c1a09295eb6cec0ab39f844aaa449df4ef9eb61 Mon Sep 17 00:00:00 2001 From: cecille Date: Tue, 13 Feb 2024 11:15:01 -0500 Subject: [PATCH 07/11] Fix attribute string --- src/python_testing/matter_testing_support.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/python_testing/matter_testing_support.py b/src/python_testing/matter_testing_support.py index ea1cf3e5257fdc..891043f555f5b9 100644 --- a/src/python_testing/matter_testing_support.py +++ b/src/python_testing/matter_testing_support.py @@ -413,8 +413,9 @@ def get_cluster_string(self, cluster_id: int) -> str: return f"Cluster {name} ({cluster_id}, 0x{cluster_id:04X})" def get_attribute_string(self, cluster_id: int, attribute_id) -> str: - if attribute_id in GlobalAttributeIds: - return f"Attribute {GlobalAttributeIds(attribute_id).name} {attribute_id}, 0x{attribute_id:04X}" + global_attrs = [item.value for item in GlobalAttributeIds] + if attribute_id in global_attrs: + return f"Attribute {GlobalAttributeIds(attribute_id).to_name()} {attribute_id}, 0x{attribute_id:04X}" mapping = self._mapping._CLUSTER_ID_DICT.get(cluster_id, None) if not mapping: return f"Attribute Unknown ({attribute_id}, 0x{attribute_id:08X})" From d3b282e8d8e54523e4c3f81714ec04d6aa9ff9ff Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 13 Feb 2024 16:15:47 +0000 Subject: [PATCH 08/11] Restyled by clang-format --- .../python/ChipDeviceController-ScriptBinding.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index 49241c88f3c6b2..7444d6648b7667 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -153,7 +153,7 @@ PyChipError pychip_DeviceController_EstablishPASESessionIP(chip::Controller::Dev uint32_t setupPINCode, chip::NodeId nodeid, uint16_t port); PyChipError pychip_DeviceController_EstablishPASESessionBLE(chip::Controller::DeviceCommissioner * devCtrl, uint32_t setupPINCode, uint16_t discriminator, chip::NodeId nodeid); -PyChipError pychip_DeviceController_EstablishPASESession(chip::Controller::DeviceCommissioner * devCtrl, const char* setUpCode, +PyChipError pychip_DeviceController_EstablishPASESession(chip::Controller::DeviceCommissioner * devCtrl, const char * setUpCode, chip::NodeId nodeid); PyChipError pychip_DeviceController_Commission(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid); @@ -622,8 +622,9 @@ PyChipError pychip_DeviceController_EstablishPASESessionBLE(chip::Controller::De return ToPyChipError(devCtrl->EstablishPASEConnection(nodeid, params)); } -PyChipError pychip_DeviceController_EstablishPASESession(chip::Controller::DeviceCommissioner * devCtrl, const char* setUpCode, - chip::NodeId nodeid) { +PyChipError pychip_DeviceController_EstablishPASESession(chip::Controller::DeviceCommissioner * devCtrl, const char * setUpCode, + chip::NodeId nodeid) +{ sPairingDelegate.SetExpectingPairingComplete(true); return ToPyChipError(devCtrl->EstablishPASEConnection(nodeid, setUpCode)); } From cbade5a3b9f28b101f88cada637d36095aa50cb7 Mon Sep 17 00:00:00 2001 From: C Freeman Date: Thu, 15 Feb 2024 11:44:19 -0500 Subject: [PATCH 09/11] Update src/python_testing/TC_DeviceBasicComposition.py --- src/python_testing/TC_DeviceBasicComposition.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/python_testing/TC_DeviceBasicComposition.py b/src/python_testing/TC_DeviceBasicComposition.py index 55f0167a653128..d6101695ef5c93 100644 --- a/src/python_testing/TC_DeviceBasicComposition.py +++ b/src/python_testing/TC_DeviceBasicComposition.py @@ -196,7 +196,6 @@ def standard_attribute(cls) -> bool: except KeyError: attr_ret = None - print(attr_ret) error_type_ok = attr_ret is not None and isinstance( attr_ret, Clusters.Attribute.ValueDecodeFailure) and isinstance(attr_ret.Reason, InteractionModelError) From 2a1cf53af2c871d3c6a8f71f3fac913cfba555bf Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 15 Feb 2024 16:44:42 +0000 Subject: [PATCH 10/11] Restyled by autopep8 --- src/python_testing/TC_DeviceBasicComposition.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/python_testing/TC_DeviceBasicComposition.py b/src/python_testing/TC_DeviceBasicComposition.py index d6101695ef5c93..dd5a3b6aa6e921 100644 --- a/src/python_testing/TC_DeviceBasicComposition.py +++ b/src/python_testing/TC_DeviceBasicComposition.py @@ -196,7 +196,6 @@ def standard_attribute(cls) -> bool: except KeyError: attr_ret = None - error_type_ok = attr_ret is not None and isinstance( attr_ret, Clusters.Attribute.ValueDecodeFailure) and isinstance(attr_ret.Reason, InteractionModelError) From 114bff398d77eebd8cc57724bea5639796489457 Mon Sep 17 00:00:00 2001 From: cecille Date: Thu, 15 Feb 2024 16:51:56 -0500 Subject: [PATCH 11/11] Make comment more verbose. --- src/python_testing/TC_DeviceBasicComposition.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/python_testing/TC_DeviceBasicComposition.py b/src/python_testing/TC_DeviceBasicComposition.py index dd5a3b6aa6e921..87bffe6f7c5420 100644 --- a/src/python_testing/TC_DeviceBasicComposition.py +++ b/src/python_testing/TC_DeviceBasicComposition.py @@ -261,7 +261,8 @@ class RequiredMandatoryAttribute: success = False continue except KeyError: - # This would have been caught already in the previous step + # A KeyError here means the attribute does not exist. This problem was already recorded in step 2, + # but we don't assert until the end of the test, so ignore this and don't re-record the error. continue self.print_step(4, "Validate the attribute list exactly matches the set of reported attributes")