From 2743ebb492689cdf729c9cd713a74e164607a339 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Fri, 20 Jan 2023 09:19:47 -0500 Subject: [PATCH] Fix issue with chip-repl yamltest running TestClusterComplexTypes.yaml (#24502) * Fix issue with chip-repl yaml test running TestClusterComplexTypes.yaml * Address PR comments * Restyle * Quick fix * Quick Fix * Address PR comments --- scripts/tests/chiptest/__init__.py | 5 ++++- src/controller/python/chip/ChipDeviceCtrl.py | 8 ++++---- .../python/chip/clusters/Attribute.py | 20 +++++++++++++------ .../python/chip/clusters/Command.py | 16 +++++++++------ .../python/chip/clusters/attribute.cpp | 9 +++++++-- .../python/chip/clusters/command.cpp | 11 ++++++++-- src/controller/python/chip/yaml/runner.py | 15 +++++++++----- .../test/test_scripts/cluster_objects.py | 10 ++++++---- 8 files changed, 64 insertions(+), 30 deletions(-) diff --git a/scripts/tests/chiptest/__init__.py b/scripts/tests/chiptest/__init__.py index 6d14e28012d710..a8f7450db437ea 100644 --- a/scripts/tests/chiptest/__init__.py +++ b/scripts/tests/chiptest/__init__.py @@ -76,7 +76,10 @@ def tests_with_command(chip_tool: str, is_manual: bool): # TODO We will move away from hardcoded list of yamltests to run all file when yamltests # parser/runner reaches parity with the code gen version. def _hardcoded_python_yaml_tests(): - currently_supported_yaml_tests = ["TestConstraints.yaml"] + currently_supported_yaml_tests = [ + "TestClusterComplexTypes.yaml", + "TestConstraints.yaml", + ] for name in currently_supported_yaml_tests: yaml_test_path = _FindYamlTestPath(name) diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py index 903d6b59218492..8668a09a3d7091 100644 --- a/src/controller/python/chip/ChipDeviceCtrl.py +++ b/src/controller/python/chip/ChipDeviceCtrl.py @@ -846,7 +846,7 @@ def ComputeRoundTripTimeout(self, nodeid, upperLayerProcessingTimeoutMs: int = 0 device.deviceProxy, upperLayerProcessingTimeoutMs)) return res - async def SendCommand(self, nodeid: int, endpoint: int, payload: ClusterObjects.ClusterCommand, responseType=None, timedRequestTimeoutMs: int = None, interactionTimeoutMs: int = None): + async def SendCommand(self, nodeid: int, endpoint: int, payload: ClusterObjects.ClusterCommand, responseType=None, timedRequestTimeoutMs: typing.Union[None, int] = None, interactionTimeoutMs: typing.Union[None, int] = None, busyWaitMs: typing.Union[None, int] = None): ''' Send a cluster-object encapsulated command to a node and get returned a future that can be awaited upon to receive the response. If a valid responseType is passed in, that will be used to deserialize the object. If not, the type will be automatically deduced @@ -867,10 +867,10 @@ async def SendCommand(self, nodeid: int, endpoint: int, payload: ClusterObjects. EndpointId=endpoint, ClusterId=payload.cluster_id, CommandId=payload.command_id, - ), payload, timedRequestTimeoutMs=timedRequestTimeoutMs, interactionTimeoutMs=interactionTimeoutMs).raise_on_error() + ), payload, timedRequestTimeoutMs=timedRequestTimeoutMs, interactionTimeoutMs=interactionTimeoutMs, busyWaitMs=busyWaitMs).raise_on_error() return await future - async def WriteAttribute(self, nodeid: int, attributes: typing.List[typing.Tuple[int, ClusterObjects.ClusterAttributeDescriptor, int]], timedRequestTimeoutMs: int = None, interactionTimeoutMs: int = None): + async def WriteAttribute(self, nodeid: int, attributes: typing.List[typing.Tuple[int, ClusterObjects.ClusterAttributeDescriptor, int]], timedRequestTimeoutMs: typing.Union[None, int] = None, interactionTimeoutMs: typing.Union[None, int] = None, busyWaitMs: typing.Union[None, int] = None): ''' Write a list of attributes on a target node. @@ -900,7 +900,7 @@ async def WriteAttribute(self, nodeid: int, attributes: typing.List[typing.Tuple v[0], v[1], v[2], 1, v[1].value)) ClusterAttribute.WriteAttributes( - future, eventLoop, device.deviceProxy, attrs, timedRequestTimeoutMs=timedRequestTimeoutMs, interactionTimeoutMs=interactionTimeoutMs).raise_on_error() + future, eventLoop, device.deviceProxy, attrs, timedRequestTimeoutMs=timedRequestTimeoutMs, interactionTimeoutMs=interactionTimeoutMs, busyWaitMs=busyWaitMs).raise_on_error() return await future def _parseAttributePathTuple(self, pathTuple: typing.Union[ diff --git a/src/controller/python/chip/clusters/Attribute.py b/src/controller/python/chip/clusters/Attribute.py index d83ad04cdb0169..6a72469b4cf43e 100644 --- a/src/controller/python/chip/clusters/Attribute.py +++ b/src/controller/python/chip/clusters/Attribute.py @@ -33,7 +33,7 @@ import chip.interaction_model import chip.tlv import construct -from chip.native import PyChipError +from chip.native import ErrorSDKPart, PyChipError from rich.pretty import pprint from .ClusterObjects import Cluster, ClusterAttributeDescriptor, ClusterEvent @@ -810,7 +810,11 @@ def _handleDone(self): # move on, possibly invalidating the provided _event_loop. # if self._resultError is not None: - self._future.set_exception(self._resultError.to_exception()) + if self._resultError.sdk_part is ErrorSDKPart.IM_GLOBAL_STATUS: + im_status = chip.interaction_model.Status(self._resultError.sdk_code) + self._future.set_exception(chip.interaction_model.InteractionModelError(im_status)) + else: + self._future.set_exception(self._resultError.to_exception()) else: self._future.set_result(self._resultData) @@ -910,14 +914,13 @@ def _OnWriteDoneCallback(closure): closure.handleDone() -def WriteAttributes(future: Future, eventLoop, device, attributes: List[AttributeWriteRequest], timedRequestTimeoutMs: int = None, interactionTimeoutMs: int = None) -> PyChipError: +def WriteAttributes(future: Future, eventLoop, device, attributes: List[AttributeWriteRequest], timedRequestTimeoutMs: Union[None, int] = None, interactionTimeoutMs: Union[None, int] = None, busyWaitMs: Union[None, int] = None) -> PyChipError: handle = chip.native.GetLibraryHandle() writeargs = [] for attr in attributes: if attr.Attribute.must_use_timed_write and timedRequestTimeoutMs is None or timedRequestTimeoutMs == 0: - raise ValueError( - f"Attribute {attr.__class__} must use timed write, please specify a valid timedRequestTimeoutMs value.") + raise chip.interaction_model.InteractionModelError(chip.interaction_model.Status.NeedsTimedInteraction) path = chip.interaction_model.AttributePathIBstruct.parse( b'\x00' * chip.interaction_model.AttributePathIBstruct.sizeof()) path.EndpointId = attr.EndpointId @@ -935,7 +938,12 @@ def WriteAttributes(future: Future, eventLoop, device, attributes: List[Attribut ctypes.pythonapi.Py_IncRef(ctypes.py_object(transaction)) res = builtins.chipStack.Call( lambda: handle.pychip_WriteClient_WriteAttributes( - ctypes.py_object(transaction), device, ctypes.c_uint16(0 if timedRequestTimeoutMs is None else timedRequestTimeoutMs), ctypes.c_uint16(0 if interactionTimeoutMs is None else interactionTimeoutMs), ctypes.c_size_t(len(attributes)), *writeargs)) + ctypes.py_object(transaction), device, + ctypes.c_uint16(0 if timedRequestTimeoutMs is None else timedRequestTimeoutMs), + ctypes.c_uint16(0 if interactionTimeoutMs is None else interactionTimeoutMs), + ctypes.c_uint16(0 if busyWaitMs is None else busyWaitMs), + ctypes.c_size_t(len(attributes)), *writeargs) + ) if not res.is_success: ctypes.pythonapi.Py_DecRef(ctypes.py_object(transaction)) return res diff --git a/src/controller/python/chip/clusters/Command.py b/src/controller/python/chip/clusters/Command.py index 0bc8fc473ffd91..056688f5b572e3 100644 --- a/src/controller/python/chip/clusters/Command.py +++ b/src/controller/python/chip/clusters/Command.py @@ -23,7 +23,7 @@ from asyncio.futures import Future from ctypes import CFUNCTYPE, c_char_p, c_size_t, c_uint8, c_uint16, c_uint32, c_void_p, py_object from dataclasses import dataclass -from typing import Type +from typing import Type, Union import chip.exceptions import chip.interaction_model @@ -142,7 +142,7 @@ def _OnCommandSenderDoneCallback(closure): ctypes.pythonapi.Py_DecRef(ctypes.py_object(closure)) -def SendCommand(future: Future, eventLoop, responseType: Type, device, commandPath: CommandPath, payload: ClusterCommand, timedRequestTimeoutMs: int = None, interactionTimeoutMs: int = None) -> PyChipError: +def SendCommand(future: Future, eventLoop, responseType: Type, device, commandPath: CommandPath, payload: ClusterCommand, timedRequestTimeoutMs: Union[None, int] = None, interactionTimeoutMs: Union[None, int] = None, busyWaitMs: Union[None, int] = None) -> PyChipError: ''' Send a cluster-object encapsulated command to a device and does the following: - On receipt of a successful data response, returns the cluster-object equivalent through the provided future. - None (on a successful response containing no data) @@ -158,8 +158,7 @@ def SendCommand(future: Future, eventLoop, responseType: Type, device, commandPa if (responseType is not None) and (not issubclass(responseType, ClusterCommand)): raise ValueError("responseType must be a ClusterCommand or None") if payload.must_use_timed_invoke and timedRequestTimeoutMs is None or timedRequestTimeoutMs == 0: - raise ValueError( - f"Command {payload.__class__} must use timed invoke, please specify a valid timedRequestTimeoutMs value") + raise chip.interaction_model.InteractionModelError(chip.interaction_model.Status.NeedsTimedInteraction) handle = chip.native.GetLibraryHandle() transaction = AsyncCommandTransaction(future, eventLoop, responseType) @@ -167,8 +166,13 @@ def SendCommand(future: Future, eventLoop, responseType: Type, device, commandPa payloadTLV = payload.ToTLV() ctypes.pythonapi.Py_IncRef(ctypes.py_object(transaction)) return builtins.chipStack.Call( - lambda: handle.pychip_CommandSender_SendCommand(ctypes.py_object( - transaction), device, c_uint16(0 if timedRequestTimeoutMs is None else timedRequestTimeoutMs), commandPath.EndpointId, commandPath.ClusterId, commandPath.CommandId, payloadTLV, len(payloadTLV), ctypes.c_uint16(0 if interactionTimeoutMs is None else interactionTimeoutMs))) + lambda: handle.pychip_CommandSender_SendCommand( + ctypes.py_object(transaction), device, + c_uint16(0 if timedRequestTimeoutMs is None else timedRequestTimeoutMs), commandPath.EndpointId, + commandPath.ClusterId, commandPath.CommandId, payloadTLV, len(payloadTLV), + ctypes.c_uint16(0 if interactionTimeoutMs is None else interactionTimeoutMs), + ctypes.c_uint16(0 if busyWaitMs is None else busyWaitMs), + )) def Init(): diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp index be533e796b55f0..212e784be4ff74 100644 --- a/src/controller/python/chip/clusters/attribute.cpp +++ b/src/controller/python/chip/clusters/attribute.cpp @@ -246,7 +246,7 @@ struct __attribute__((packed)) PyReadAttributeParams // Encodes n attribute write requests, follows 3 * n arguments, in the (AttributeWritePath*=void *, uint8_t*, size_t) order. PyChipError pychip_WriteClient_WriteAttributes(void * appContext, DeviceProxy * device, uint16_t timedWriteTimeoutMs, - uint16_t interactionTimeoutMs, size_t n, ...); + uint16_t interactionTimeoutMs, uint16_t busyWaitMs, size_t n, ...); PyChipError pychip_ReadClient_ReadAttributes(void * appContext, ReadClient ** pReadClient, ReadClientCallback ** pCallback, DeviceProxy * device, uint8_t * readParamsBuf, size_t n, size_t total, ...); } @@ -323,7 +323,7 @@ void pychip_ReadClient_InitCallbacks(OnReadAttributeDataCallback onReadAttribute } PyChipError pychip_WriteClient_WriteAttributes(void * appContext, DeviceProxy * device, uint16_t timedWriteTimeoutMs, - uint16_t interactionTimeoutMs, size_t n, ...) + uint16_t interactionTimeoutMs, uint16_t busyWaitMs, size_t n, ...) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -370,6 +370,11 @@ PyChipError pychip_WriteClient_WriteAttributes(void * appContext, DeviceProxy * client.release(); callback.release(); + if (busyWaitMs) + { + usleep(busyWaitMs * 1000); + } + exit: va_end(args); return ToPyChipError(err); diff --git a/src/controller/python/chip/clusters/command.cpp b/src/controller/python/chip/clusters/command.cpp index 9c4686b9000317..169b925913c07a 100644 --- a/src/controller/python/chip/clusters/command.cpp +++ b/src/controller/python/chip/clusters/command.cpp @@ -35,7 +35,8 @@ using PyObject = void *; extern "C" { PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * device, uint16_t timedRequestTimeoutMs, chip::EndpointId endpointId, chip::ClusterId clusterId, chip::CommandId commandId, - const uint8_t * payload, size_t length, uint16_t interactionTimeoutMs); + const uint8_t * payload, size_t length, uint16_t interactionTimeoutMs, + uint16_t busyWaitMs); } namespace chip { @@ -126,7 +127,8 @@ void pychip_CommandSender_InitCallbacks(OnCommandSenderResponseCallback onComman PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * device, uint16_t timedRequestTimeoutMs, chip::EndpointId endpointId, chip::ClusterId clusterId, chip::CommandId commandId, - const uint8_t * payload, size_t length, uint16_t interactionTimeoutMs) + const uint8_t * payload, size_t length, uint16_t interactionTimeoutMs, + uint16_t busyWaitMs) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -161,6 +163,11 @@ PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * de sender.release(); callback.release(); + if (busyWaitMs) + { + usleep(busyWaitMs * 1000); + } + exit: return ToPyChipError(err); } diff --git a/src/controller/python/chip/yaml/runner.py b/src/controller/python/chip/yaml/runner.py index 7bb59178ad7789..f56f08d740dbf1 100644 --- a/src/controller/python/chip/yaml/runner.py +++ b/src/controller/python/chip/yaml/runner.py @@ -107,6 +107,7 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): UnexpectedParsingError: Raised if there is an unexpected parsing error. ''' super().__init__(test_step.label, test_step.identity) + self._busy_wait_ms = test_step.busy_wait_ms self._command_name = stringcase.pascalcase(test_step.command) self._cluster = cluster self._interation_timeout_ms = test_step.timed_interaction_timeout_ms @@ -142,7 +143,8 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl) -> _ActionResult: try: resp = asyncio.run(dev_ctrl.SendCommand( self._node_id, self._endpoint, self._request_object, - timedRequestTimeoutMs=self._interation_timeout_ms)) + timedRequestTimeoutMs=self._interation_timeout_ms, + busyWaitMs=self._busy_wait_ms)) except chip.interaction_model.InteractionModelError as error: return _ActionResult(status=_ActionStatus.ERROR, response=error) @@ -357,8 +359,10 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): ''' super().__init__(test_step.label, test_step.identity) self._attribute_name = stringcase.pascalcase(test_step.attribute) + self._busy_wait_ms = test_step.busy_wait_ms self._cluster = cluster self._endpoint = test_step.endpoint + self._interation_timeout_ms = test_step.timed_interaction_timeout_ms self._node_id = test_step.node_id self._request_object = None @@ -389,10 +393,11 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): def run_action(self, dev_ctrl: ChipDeviceCtrl) -> _ActionResult: try: resp = asyncio.run( - dev_ctrl.WriteAttribute(self._node_id, [(self._endpoint, self._request_object)])) - except chip.interaction_model.InteractionModelError: - # TODO Should we be doing the same thing as InvokeAction on InteractionModelError? - raise + dev_ctrl.WriteAttribute(self._node_id, [(self._endpoint, self._request_object)], + timedRequestTimeoutMs=self._interation_timeout_ms, + busyWaitMs=self._busy_wait_ms)) + except chip.interaction_model.InteractionModelError as error: + return _ActionResult(status=_ActionStatus.ERROR, response=error) if len(resp) == 1 and isinstance(resp[0], AttributeStatus): if resp[0].Status == chip.interaction_model.Status.Success: return _ActionResult(status=_ActionStatus.SUCCESS, response=None) diff --git a/src/controller/python/test/test_scripts/cluster_objects.py b/src/controller/python/test/test_scripts/cluster_objects.py index eadcc413a1462a..59b55a6a13bf86 100644 --- a/src/controller/python/test/test_scripts/cluster_objects.py +++ b/src/controller/python/test/test_scripts/cluster_objects.py @@ -443,8 +443,9 @@ async def TestTimedRequest(cls, devCtrl): req = Clusters.UnitTesting.Commands.TimedInvokeRequest() await devCtrl.SendCommand(nodeid=NODE_ID, endpoint=1, payload=req) raise AssertionError("The command invoke should be rejected.") - except ValueError: - pass + except chip.interaction_model.InteractionModelError as ex: + if ex.status != chip.interaction_model.Status.NeedsTimedInteraction: + raise AssertionError("The command invoke was expected to error with NeedsTimedInteraction.") logger.info( "4: Writing TestCluster-TimedWriteBoolean without timedRequestTimeoutMs should be rejected") @@ -455,8 +456,9 @@ async def TestTimedRequest(cls, devCtrl): True)), ]) raise AssertionError("The write request should be rejected.") - except ValueError: - pass + except chip.interaction_model.InteractionModelError as ex: + if ex.status != chip.interaction_model.Status.NeedsTimedInteraction: + raise AssertionError("The write attribute was expected to error with NeedsTimedInteraction.") @ classmethod @ base.test_case