From 6a1ce3830d40c0de2e858c99496f28560356e254 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Fri, 10 Feb 2023 13:14:10 +0000 Subject: [PATCH 1/5] Allow chip-repl to send group commands With chip-repl now able to send we have updated the yamltests runner that uses chip-repl to send group commands --- .../ChipDeviceController-ScriptBinding.cpp | 3 +- src/controller/python/OpCredsBinding.cpp | 15 +++ src/controller/python/chip/ChipDeviceCtrl.py | 31 +++++++ .../python/chip/clusters/Command.py | 50 +++++++++- .../python/chip/clusters/command.cpp | 93 +++++++++++++++++-- src/controller/python/chip/yaml/runner.py | 21 ++++- 6 files changed, 198 insertions(+), 15 deletions(-) diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index 0be9b561ab0b51..ac7da9416ff19b 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -100,7 +100,7 @@ chip::Controller::CommissioningParameters sCommissioningParameters; chip::Controller::ScriptDevicePairingDelegate sPairingDelegate; chip::Controller::ScriptPairingDeviceDiscoveryDelegate sPairingDeviceDiscoveryDelegate; -chip::Credentials::GroupDataProviderImpl sGroupDataProvider; +chip::Credentials::GroupDataProviderImpl sGroupDataProvider{ 5, 8 }; chip::Credentials::PersistentStorageOpCertStore sPersistentStorageOpCertStore; // NOTE: Remote device ID is in sync with the echo server device id @@ -234,6 +234,7 @@ PyChipError pychip_DeviceController_StackInit(Controller::Python::StorageAdapter sGroupDataProvider.SetStorageDelegate(storageAdapter); PyReturnErrorOnFailure(ToPyChipError(sGroupDataProvider.Init())); + Credentials::SetGroupDataProvider(&sGroupDataProvider); factoryParams.groupDataProvider = &sGroupDataProvider; PyReturnErrorOnFailure(ToPyChipError(sPersistentStorageOpCertStore.Init(storageAdapter))); diff --git a/src/controller/python/OpCredsBinding.cpp b/src/controller/python/OpCredsBinding.cpp index 3b12f8febd859f..c7f3e1dc20497c 100644 --- a/src/controller/python/OpCredsBinding.cpp +++ b/src/controller/python/OpCredsBinding.cpp @@ -465,6 +465,21 @@ PyChipError pychip_OpCreds_AllocateController(OpCredsContext * context, chip::Co return ToPyChipError(CHIP_NO_ERROR); } +PyChipError pychip_OpCreds_InitGroupTestingData(chip::Controller::DeviceCommissioner * devCtrl) +{ + VerifyOrReturnError(devCtrl != nullptr, ToPyChipError(CHIP_ERROR_INVALID_ARGUMENT)); + + uint8_t compressedFabricId[sizeof(uint64_t)] = { 0 }; + chip::MutableByteSpan compressedFabricIdSpan(compressedFabricId); + + CHIP_ERROR err = devCtrl->GetCompressedFabricIdBytes(compressedFabricIdSpan); + VerifyOrReturnError(err == CHIP_NO_ERROR, ToPyChipError(err)); + + err = chip::GroupTesting::InitData(&sGroupDataProvider, devCtrl->GetFabricIndex(), compressedFabricIdSpan); + + return ToPyChipError(err); +} + PyChipError pychip_OpCreds_SetMaximallyLargeCertsUsed(OpCredsContext * context, bool enabled) { VerifyOrReturnError(context != nullptr && context->mAdapter != nullptr, ToPyChipError(CHIP_ERROR_INCORRECT_STATE)); diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py index 96e86adb090177..1486e12c50ec3c 100644 --- a/src/controller/python/chip/ChipDeviceCtrl.py +++ b/src/controller/python/chip/ChipDeviceCtrl.py @@ -894,6 +894,24 @@ async def SendCommand(self, nodeid: int, endpoint: int, payload: ClusterObjects. ), payload, timedRequestTimeoutMs=timedRequestTimeoutMs, interactionTimeoutMs=interactionTimeoutMs, busyWaitMs=busyWaitMs).raise_on_error() return await future + async def SendGroupCommand(self, groupid: int, payload: ClusterObjects.ClusterCommand, timedRequestTimeoutMs: typing.Union[None, int] = None, interactionTimeoutMs: typing.Union[None, int] = None, busyWaitMs: typing.Union[None, int] = None): + ''' + Send a group cluster-object encapsulated command to a group_id and get returned a future that can be awaited upon to get confirmation command was sent. + + timedWriteTimeoutMs: Timeout for a timed invoke request. Omit or set to 'None' to indicate a non-timed request. + interactionTimeoutMs: Overall timeout for the interaction. Omit or set to 'None' to have the SDK automatically compute the right + timeout value based on transport characteristics as well as the responsiveness of the target. + ''' + self.CheckIsActive() + + eventLoop = asyncio.get_running_loop() + future = eventLoop.create_future() + + ClusterCommand.SendGroupCommand( + future, eventLoop, groupid, self.devCtrl, 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: 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. @@ -1289,6 +1307,15 @@ def IssueNOCChain(self, csr: Clusters.OperationalCredentials.Commands.CSRRespons self.devCtrl, py_object(self), csr.NOCSRElements, len(csr.NOCSRElements), nodeId) ) + def InitGroupTestingData(self): + """Populates the Device Controller's GroupDataProvider with known test group info and keys.""" + self.CheckIsActive() + + self._ChipStack.Call( + lambda: self._dmLib.pychip_OpCreds_InitGroupTestingData( + self.devCtrl) + ).raise_on_error() + # ----- Private Members ----- def _InitLib(self): if self._dmLib is None: @@ -1455,6 +1482,10 @@ def _InitLib(self): ] self._dmLib.pychip_DeviceController_IssueNOCChain.restype = PyChipError + self._dmLib.pychip_OpCreds_InitGroupTestingData.argtypes = [ + c_void_p] + self._dmLib.pychip_OpCreds_InitGroupTestingData.restype = PyChipError + self._dmLib.pychip_DeviceController_SetIssueNOCChainCallbackPythonCallback.argtypes = [ _IssueNOCChainCallbackPythonCallbackFunct] self._dmLib.pychip_DeviceController_SetIssueNOCChainCallbackPythonCallback.restype = None diff --git a/src/controller/python/chip/clusters/Command.py b/src/controller/python/chip/clusters/Command.py index 056688f5b572e3..12ee773a49f858 100644 --- a/src/controller/python/chip/clusters/Command.py +++ b/src/controller/python/chip/clusters/Command.py @@ -116,6 +116,13 @@ def handleError(self, status: Status, chipError: PyChipError): self._handleError, status, chipError, None ) + def _handleGroupCommandDone(self): + self._future.set_result(None) + + def handleGroupCommandDone(self): + self._event_loop.call_soon_threadsafe( + self._handleGroupCommandDone) + _OnCommandSenderResponseCallbackFunct = CFUNCTYPE( None, py_object, c_uint16, c_uint32, c_uint32, c_uint16, c_uint8, c_void_p, c_uint32) @@ -123,6 +130,8 @@ def handleError(self, status: Status, chipError: PyChipError): None, py_object, c_uint16, c_uint8, PyChipError) _OnCommandSenderDoneCallbackFunct = CFUNCTYPE( None, py_object) +_OnGroupCommandSenderDoneCallbackFunct = CFUNCTYPE( + None, py_object) @_OnCommandSenderResponseCallbackFunct @@ -142,6 +151,13 @@ def _OnCommandSenderDoneCallback(closure): ctypes.pythonapi.Py_DecRef(ctypes.py_object(closure)) +@_OnGroupCommandSenderDoneCallbackFunct +def _OnGroupCommandSenderDoneCallback(closure): + # Group commands have no response so we leverage the group command done callback + closure.handleGroupCommandDone() + ctypes.pythonapi.Py_DecRef(ctypes.py_object(closure)) + + 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. @@ -175,6 +191,34 @@ def SendCommand(future: Future, eventLoop, responseType: Type, device, commandPa )) +def SendGroupCommand(future: Future, eventLoop, groupId: int, devCtrl: c_void_p, payload: ClusterCommand, timedRequestTimeoutMs: Union[None, int] = None, interactionTimeoutMs: Union[None, int] = None, busyWaitMs: Union[None, int] = None) -> PyChipError: + ''' Send a cluster-object encapsulated group command to a device and does the following: + - None (on a successful response containing no data) + - Raises an exception if any errors are encountered. + + If a valid timedRequestTimeoutMs is provided, a timed interaction will be initiated instead. + If a valid interactionTimeoutMs is provided, the interaction will terminate with a CHIP_ERROR_TIMEOUT if a response + has not been received within that timeout. If it isn't provided, a sensible value will be automatically computed that + accounts for the underlying characteristics of both the transport and the responsiveness of the receiver. + ''' + if payload.must_use_timed_invoke and timedRequestTimeoutMs is None or timedRequestTimeoutMs == 0: + raise chip.interaction_model.InteractionModelError(chip.interaction_model.Status.NeedsTimedInteraction) + + handle = chip.native.GetLibraryHandle() + transaction = AsyncCommandTransaction(future, eventLoop, None) + + payloadTLV = payload.ToTLV() + ctypes.pythonapi.Py_IncRef(ctypes.py_object(transaction)) + return builtins.chipStack.Call( + lambda: handle.pychip_CommandSender_SendGroupCommand( + ctypes.py_object(transaction), c_uint16(groupId), devCtrl, + c_uint16(0 if timedRequestTimeoutMs is None else timedRequestTimeoutMs), + payload.cluster_id, payload.command_id, 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(): handle = chip.native.GetLibraryHandle() @@ -185,8 +229,10 @@ def Init(): setter.Set('pychip_CommandSender_SendCommand', PyChipError, [py_object, c_void_p, c_uint16, c_uint32, c_uint32, c_char_p, c_size_t, c_uint16]) + setter.Set('pychip_CommandSender_SendGroupCommand', + PyChipError, [py_object, c_uint16, c_void_p, c_uint16, c_uint32, c_uint32, c_char_p, c_size_t, c_uint16, c_uint16]) setter.Set('pychip_CommandSender_InitCallbacks', None, [ - _OnCommandSenderResponseCallbackFunct, _OnCommandSenderErrorCallbackFunct, _OnCommandSenderDoneCallbackFunct]) + _OnCommandSenderResponseCallbackFunct, _OnCommandSenderErrorCallbackFunct, _OnCommandSenderDoneCallbackFunct, _OnGroupCommandSenderDoneCallbackFunct]) handle.pychip_CommandSender_InitCallbacks( - _OnCommandSenderResponseCallback, _OnCommandSenderErrorCallback, _OnCommandSenderDoneCallback) + _OnCommandSenderResponseCallback, _OnCommandSenderErrorCallback, _OnCommandSenderDoneCallback, _OnGroupCommandSenderDoneCallback) diff --git a/src/controller/python/chip/clusters/command.cpp b/src/controller/python/chip/clusters/command.cpp index 169b925913c07a..2a7f47fb2a8405 100644 --- a/src/controller/python/chip/clusters/command.cpp +++ b/src/controller/python/chip/clusters/command.cpp @@ -37,23 +37,30 @@ PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * de chip::EndpointId endpointId, chip::ClusterId clusterId, chip::CommandId commandId, const uint8_t * payload, size_t length, uint16_t interactionTimeoutMs, uint16_t busyWaitMs); + +PyChipError pychip_CommandSender_SendGroupCommand(void * appContext, chip::GroupId groupId, + chip::Controller::DeviceCommissioner * devCtrl, uint16_t timedRequestTimeoutMs, + chip::ClusterId clusterId, chip::CommandId commandId, const uint8_t * payload, + size_t length, uint16_t interactionTimeoutMs, uint16_t busyWaitMs); } namespace chip { namespace python { -using OnCommandSenderResponseCallback = void (*)(PyObject appContext, chip::EndpointId endpointId, chip::ClusterId clusterId, +using OnCommandSenderResponseCallback = void (*)(PyObject appContext, chip::EndpointId endpointId, chip::ClusterId clusterId, chip::CommandId commandId, std::underlying_type_t status, chip::ClusterStatus clusterStatus, const uint8_t * payload, uint32_t length); -using OnCommandSenderErrorCallback = void (*)(PyObject appContext, +using OnCommandSenderErrorCallback = void (*)(PyObject appContext, std::underlying_type_t status, chip::ClusterStatus clusterStatus, PyChipError chiperror); -using OnCommandSenderDoneCallback = void (*)(PyObject appContext); +using OnCommandSenderDoneCallback = void (*)(PyObject appContext); +using OnGroupCommandSenderDoneCallback = void (*)(PyObject appContext); OnCommandSenderResponseCallback gOnCommandSenderResponseCallback = nullptr; OnCommandSenderErrorCallback gOnCommandSenderErrorCallback = nullptr; OnCommandSenderDoneCallback gOnCommandSenderDoneCallback = nullptr; +OnCommandSenderDoneCallback gOnGroupCommandSenderDoneCallback = nullptr; class CommandSenderCallback : public CommandSender::Callback { @@ -106,10 +113,29 @@ class CommandSenderCallback : public CommandSender::Callback delete this; }; -private: +protected: PyObject mAppContext = nullptr; }; +class GroupCommandSenderCallback : public CommandSenderCallback +{ +public: + GroupCommandSenderCallback(PyObject appContext) : CommandSenderCallback(appContext) {} + void OnResponse(CommandSender * apCommandSender, const ConcreteCommandPath & aPath, const app::StatusIB & aStatus, + TLV::TLVReader * aData) override + { + // Group messages are not expected to provide a response. + chipDie(); + } + + void OnDone(CommandSender * apCommandSender) override + { + gOnGroupCommandSenderDoneCallback(mAppContext); + delete apCommandSender; + delete this; + }; +}; + } // namespace python } // namespace chip @@ -118,11 +144,13 @@ using namespace chip::python; extern "C" { void pychip_CommandSender_InitCallbacks(OnCommandSenderResponseCallback onCommandSenderResponseCallback, OnCommandSenderErrorCallback onCommandSenderErrorCallback, - OnCommandSenderDoneCallback onCommandSenderDoneCallback) + OnCommandSenderDoneCallback onCommandSenderDoneCallback, + OnGroupCommandSenderDoneCallback onGroupCommandSenderDoneCallback) { - gOnCommandSenderResponseCallback = onCommandSenderResponseCallback; - gOnCommandSenderErrorCallback = onCommandSenderErrorCallback; - gOnCommandSenderDoneCallback = onCommandSenderDoneCallback; + gOnCommandSenderResponseCallback = onCommandSenderResponseCallback; + gOnCommandSenderErrorCallback = onCommandSenderErrorCallback; + gOnCommandSenderDoneCallback = onCommandSenderDoneCallback; + gOnGroupCommandSenderDoneCallback = onGroupCommandSenderDoneCallback; } PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * device, uint16_t timedRequestTimeoutMs, @@ -168,6 +196,55 @@ PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * de usleep(busyWaitMs * 1000); } +exit: + return ToPyChipError(err); +} + +PyChipError pychip_CommandSender_SendGroupCommand(void * appContext, chip::GroupId groupId, + chip::Controller::DeviceCommissioner * devCtrl, uint16_t timedRequestTimeoutMs, + chip::ClusterId clusterId, chip::CommandId commandId, const uint8_t * payload, + size_t length, uint16_t interactionTimeoutMs, uint16_t busyWaitMs) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + + chip::Messaging::ExchangeManager * exchangeManager = chip::app::InteractionModelEngine::GetInstance()->GetExchangeManager(); + VerifyOrReturnError(exchangeManager != nullptr, ToPyChipError(CHIP_ERROR_INCORRECT_STATE)); + + std::unique_ptr callback = std::make_unique(appContext); + std::unique_ptr sender = std::make_unique(callback.get(), exchangeManager, + /* is timed request */ timedRequestTimeoutMs != 0); + + app::CommandPathParams cmdParams = { groupId, clusterId, commandId, (app::CommandPathFlags::kGroupIdValid) }; + + SuccessOrExit(err = sender->PrepareCommand(cmdParams, false)); + + { + auto writer = sender->GetCommandDataIBTLVWriter(); + TLV::TLVReader reader; + VerifyOrExit(writer != nullptr, err = CHIP_ERROR_INCORRECT_STATE); + reader.Init(payload, length); + reader.Next(); + SuccessOrExit(writer->CopyContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kFields)), reader)); + } + + SuccessOrExit(err = sender->FinishCommand(timedRequestTimeoutMs != 0 ? Optional(timedRequestTimeoutMs) + : Optional::Missing())); + + { + auto fabricIndex = devCtrl->GetFabricIndex(); + + chip::Transport::OutgoingGroupSession session(groupId, fabricIndex); + SuccessOrExit(err = sender->SendGroupCommandRequest(chip::SessionHandle(session))); + } + + 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 b3559a1e431956..a0a3ca2baed2bd 100644 --- a/src/controller/python/chip/yaml/runner.py +++ b/src/controller/python/chip/yaml/runner.py @@ -156,6 +156,11 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): self._expected_response_object = None self._endpoint = test_step.endpoint self._node_id = test_step.node_id + self._group_id = test_step.group_id + + if self._node_id is None and self._group_id is None: + raise UnexpectedParsingError( + 'Both node_id and group_id are None, at least one needs to be provided') command = context.data_model_lookup.get_command(self._cluster, self._command_name) @@ -182,10 +187,16 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): def run_action(self, dev_ctrl: ChipDeviceController) -> _ActionResult: try: - resp = asyncio.run(dev_ctrl.SendCommand( - self._node_id, self._endpoint, self._request_object, - timedRequestTimeoutMs=self._interation_timeout_ms, - busyWaitMs=self._busy_wait_ms)) + if self._group_id: + resp = asyncio.run(dev_ctrl.SendGroupCommand( + self._group_id, self._request_object, + timedRequestTimeoutMs=self._interation_timeout_ms, + busyWaitMs=self._busy_wait_ms)) + else: + resp = asyncio.run(dev_ctrl.SendCommand( + 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) @@ -736,6 +747,7 @@ def __init__(self, test_spec_definition, certificate_authority_manager, alpha_de self._certificate_authority_manager = certificate_authority_manager self._dev_ctrls = {} + alpha_dev_ctrl.InitGroupTestingData() self._dev_ctrls['alpha'] = alpha_dev_ctrl def _invoke_action_factory(self, test_step, cluster: str): @@ -1014,6 +1026,7 @@ def _get_dev_ctrl(self, action: BaseAction): fabric = certificate_authority.NewFabricAdmin(vendorId=0xFFF1, fabricId=fabric_id) dev_ctrl = fabric.NewController() + dev_ctrl.InitGroupTestingData() self._dev_ctrls[action.identity] = dev_ctrl else: dev_ctrl = self._dev_ctrls['alpha'] From 0c340cab008f7a8465ad6615ee75c29ed385dd44 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 21 Feb 2023 18:25:40 +0000 Subject: [PATCH 2/5] Address PR comments --- src/controller/python/BUILD.gn | 4 ++++ src/controller/python/ChipDeviceController-ScriptBinding.cpp | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/controller/python/BUILD.gn b/src/controller/python/BUILD.gn index 82bc6732175a0a..73322c513b47ec 100644 --- a/src/controller/python/BUILD.gn +++ b/src/controller/python/BUILD.gn @@ -51,6 +51,8 @@ shared_library("ChipDeviceCtrl") { sources += [ "chip/native/CommonStackInit.cpp" ] + defines = [] + if (chip_controller) { sources += [ "ChipCommissionableNodeController-ScriptBinding.cpp", @@ -76,6 +78,8 @@ shared_library("ChipDeviceCtrl") { "chip/native/PyChipError.cpp", "chip/utils/DeviceProxyUtils.cpp", ] + defines += ["CHIP_CONFIG_MAX_GROUPS_PER_FABRIC=50"] + defines += ["CHIP_CONFIG_MAX_GROUP_KEYS_PER_FABRIC=50"] } else { sources += [ "chip/server/Options.cpp", diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index ac7da9416ff19b..d784c1bd930a2f 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -100,7 +100,7 @@ chip::Controller::CommissioningParameters sCommissioningParameters; chip::Controller::ScriptDevicePairingDelegate sPairingDelegate; chip::Controller::ScriptPairingDeviceDiscoveryDelegate sPairingDeviceDiscoveryDelegate; -chip::Credentials::GroupDataProviderImpl sGroupDataProvider{ 5, 8 }; +chip::Credentials::GroupDataProviderImpl sGroupDataProvider; chip::Credentials::PersistentStorageOpCertStore sPersistentStorageOpCertStore; // NOTE: Remote device ID is in sync with the echo server device id From 712f1f19d77d184e174b93271ebb1955c5685d81 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 21 Feb 2023 18:30:59 +0000 Subject: [PATCH 3/5] Restyle --- src/controller/python/BUILD.gn | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/controller/python/BUILD.gn b/src/controller/python/BUILD.gn index 73322c513b47ec..da9b080e4fcd6d 100644 --- a/src/controller/python/BUILD.gn +++ b/src/controller/python/BUILD.gn @@ -78,8 +78,8 @@ shared_library("ChipDeviceCtrl") { "chip/native/PyChipError.cpp", "chip/utils/DeviceProxyUtils.cpp", ] - defines += ["CHIP_CONFIG_MAX_GROUPS_PER_FABRIC=50"] - defines += ["CHIP_CONFIG_MAX_GROUP_KEYS_PER_FABRIC=50"] + defines += [ "CHIP_CONFIG_MAX_GROUPS_PER_FABRIC=50" ] + defines += [ "CHIP_CONFIG_MAX_GROUP_KEYS_PER_FABRIC=50" ] } else { sources += [ "chip/server/Options.cpp", From ca9af5210f595ecd70357217cb2d58a7eec8ab61 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 22 Feb 2023 15:20:17 +0000 Subject: [PATCH 4/5] Address PR comments --- src/controller/python/chip/ChipDeviceCtrl.py | 16 ++--- .../python/chip/clusters/Command.py | 38 ++---------- .../python/chip/clusters/command.cpp | 61 +++++-------------- src/controller/python/chip/yaml/runner.py | 5 +- 4 files changed, 28 insertions(+), 92 deletions(-) diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py index 1486e12c50ec3c..f37316651effd7 100644 --- a/src/controller/python/chip/ChipDeviceCtrl.py +++ b/src/controller/python/chip/ChipDeviceCtrl.py @@ -894,23 +894,17 @@ async def SendCommand(self, nodeid: int, endpoint: int, payload: ClusterObjects. ), payload, timedRequestTimeoutMs=timedRequestTimeoutMs, interactionTimeoutMs=interactionTimeoutMs, busyWaitMs=busyWaitMs).raise_on_error() return await future - async def SendGroupCommand(self, groupid: int, payload: ClusterObjects.ClusterCommand, timedRequestTimeoutMs: typing.Union[None, int] = None, interactionTimeoutMs: typing.Union[None, int] = None, busyWaitMs: typing.Union[None, int] = None): + def SendGroupCommand(self, groupid: int, payload: ClusterObjects.ClusterCommand, busyWaitMs: typing.Union[None, int] = None): ''' Send a group cluster-object encapsulated command to a group_id and get returned a future that can be awaited upon to get confirmation command was sent. - - timedWriteTimeoutMs: Timeout for a timed invoke request. Omit or set to 'None' to indicate a non-timed request. - interactionTimeoutMs: Overall timeout for the interaction. Omit or set to 'None' to have the SDK automatically compute the right - timeout value based on transport characteristics as well as the responsiveness of the target. ''' self.CheckIsActive() - eventLoop = asyncio.get_running_loop() - future = eventLoop.create_future() - ClusterCommand.SendGroupCommand( - future, eventLoop, groupid, self.devCtrl, payload, timedRequestTimeoutMs=timedRequestTimeoutMs, - interactionTimeoutMs=interactionTimeoutMs, busyWaitMs=busyWaitMs).raise_on_error() - return await future + groupid, self.devCtrl, payload, busyWaitMs=busyWaitMs).raise_on_error() + + # None is the expected return for sending group commands. + return 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): ''' diff --git a/src/controller/python/chip/clusters/Command.py b/src/controller/python/chip/clusters/Command.py index 12ee773a49f858..df2037192e24c3 100644 --- a/src/controller/python/chip/clusters/Command.py +++ b/src/controller/python/chip/clusters/Command.py @@ -116,13 +116,6 @@ def handleError(self, status: Status, chipError: PyChipError): self._handleError, status, chipError, None ) - def _handleGroupCommandDone(self): - self._future.set_result(None) - - def handleGroupCommandDone(self): - self._event_loop.call_soon_threadsafe( - self._handleGroupCommandDone) - _OnCommandSenderResponseCallbackFunct = CFUNCTYPE( None, py_object, c_uint16, c_uint32, c_uint32, c_uint16, c_uint8, c_void_p, c_uint32) @@ -130,8 +123,6 @@ def handleGroupCommandDone(self): None, py_object, c_uint16, c_uint8, PyChipError) _OnCommandSenderDoneCallbackFunct = CFUNCTYPE( None, py_object) -_OnGroupCommandSenderDoneCallbackFunct = CFUNCTYPE( - None, py_object) @_OnCommandSenderResponseCallbackFunct @@ -151,13 +142,6 @@ def _OnCommandSenderDoneCallback(closure): ctypes.pythonapi.Py_DecRef(ctypes.py_object(closure)) -@_OnGroupCommandSenderDoneCallbackFunct -def _OnGroupCommandSenderDoneCallback(closure): - # Group commands have no response so we leverage the group command done callback - closure.handleGroupCommandDone() - ctypes.pythonapi.Py_DecRef(ctypes.py_object(closure)) - - 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. @@ -191,30 +175,18 @@ def SendCommand(future: Future, eventLoop, responseType: Type, device, commandPa )) -def SendGroupCommand(future: Future, eventLoop, groupId: int, devCtrl: c_void_p, payload: ClusterCommand, timedRequestTimeoutMs: Union[None, int] = None, interactionTimeoutMs: Union[None, int] = None, busyWaitMs: Union[None, int] = None) -> PyChipError: +def SendGroupCommand(groupId: int, devCtrl: c_void_p, payload: ClusterCommand, busyWaitMs: Union[None, int] = None) -> PyChipError: ''' Send a cluster-object encapsulated group command to a device and does the following: - None (on a successful response containing no data) - Raises an exception if any errors are encountered. - - If a valid timedRequestTimeoutMs is provided, a timed interaction will be initiated instead. - If a valid interactionTimeoutMs is provided, the interaction will terminate with a CHIP_ERROR_TIMEOUT if a response - has not been received within that timeout. If it isn't provided, a sensible value will be automatically computed that - accounts for the underlying characteristics of both the transport and the responsiveness of the receiver. ''' - if payload.must_use_timed_invoke and timedRequestTimeoutMs is None or timedRequestTimeoutMs == 0: - raise chip.interaction_model.InteractionModelError(chip.interaction_model.Status.NeedsTimedInteraction) - handle = chip.native.GetLibraryHandle() - transaction = AsyncCommandTransaction(future, eventLoop, None) payloadTLV = payload.ToTLV() - ctypes.pythonapi.Py_IncRef(ctypes.py_object(transaction)) return builtins.chipStack.Call( lambda: handle.pychip_CommandSender_SendGroupCommand( - ctypes.py_object(transaction), c_uint16(groupId), devCtrl, - c_uint16(0 if timedRequestTimeoutMs is None else timedRequestTimeoutMs), + c_uint16(groupId), devCtrl, payload.cluster_id, payload.command_id, payloadTLV, len(payloadTLV), - ctypes.c_uint16(0 if interactionTimeoutMs is None else interactionTimeoutMs), ctypes.c_uint16(0 if busyWaitMs is None else busyWaitMs), )) @@ -230,9 +202,9 @@ def Init(): setter.Set('pychip_CommandSender_SendCommand', PyChipError, [py_object, c_void_p, c_uint16, c_uint32, c_uint32, c_char_p, c_size_t, c_uint16]) setter.Set('pychip_CommandSender_SendGroupCommand', - PyChipError, [py_object, c_uint16, c_void_p, c_uint16, c_uint32, c_uint32, c_char_p, c_size_t, c_uint16, c_uint16]) + PyChipError, [c_uint16, c_void_p, c_uint32, c_uint32, c_char_p, c_size_t, c_uint16]) setter.Set('pychip_CommandSender_InitCallbacks', None, [ - _OnCommandSenderResponseCallbackFunct, _OnCommandSenderErrorCallbackFunct, _OnCommandSenderDoneCallbackFunct, _OnGroupCommandSenderDoneCallbackFunct]) + _OnCommandSenderResponseCallbackFunct, _OnCommandSenderErrorCallbackFunct, _OnCommandSenderDoneCallbackFunct]) handle.pychip_CommandSender_InitCallbacks( - _OnCommandSenderResponseCallback, _OnCommandSenderErrorCallback, _OnCommandSenderDoneCallback, _OnGroupCommandSenderDoneCallback) + _OnCommandSenderResponseCallback, _OnCommandSenderErrorCallback, _OnCommandSenderDoneCallback) diff --git a/src/controller/python/chip/clusters/command.cpp b/src/controller/python/chip/clusters/command.cpp index 2a7f47fb2a8405..cb25953ff97c32 100644 --- a/src/controller/python/chip/clusters/command.cpp +++ b/src/controller/python/chip/clusters/command.cpp @@ -38,29 +38,27 @@ PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * de const uint8_t * payload, size_t length, uint16_t interactionTimeoutMs, uint16_t busyWaitMs); -PyChipError pychip_CommandSender_SendGroupCommand(void * appContext, chip::GroupId groupId, - chip::Controller::DeviceCommissioner * devCtrl, uint16_t timedRequestTimeoutMs, +PyChipError pychip_CommandSender_SendGroupCommand(chip::GroupId groupId, + chip::Controller::DeviceCommissioner * devCtrl, chip::ClusterId clusterId, chip::CommandId commandId, const uint8_t * payload, - size_t length, uint16_t interactionTimeoutMs, uint16_t busyWaitMs); + size_t length, uint16_t busyWaitMs); } namespace chip { namespace python { -using OnCommandSenderResponseCallback = void (*)(PyObject appContext, chip::EndpointId endpointId, chip::ClusterId clusterId, +using OnCommandSenderResponseCallback = void (*)(PyObject appContext, chip::EndpointId endpointId, chip::ClusterId clusterId, chip::CommandId commandId, std::underlying_type_t status, chip::ClusterStatus clusterStatus, const uint8_t * payload, uint32_t length); -using OnCommandSenderErrorCallback = void (*)(PyObject appContext, +using OnCommandSenderErrorCallback = void (*)(PyObject appContext, std::underlying_type_t status, chip::ClusterStatus clusterStatus, PyChipError chiperror); -using OnCommandSenderDoneCallback = void (*)(PyObject appContext); -using OnGroupCommandSenderDoneCallback = void (*)(PyObject appContext); +using OnCommandSenderDoneCallback = void (*)(PyObject appContext); OnCommandSenderResponseCallback gOnCommandSenderResponseCallback = nullptr; OnCommandSenderErrorCallback gOnCommandSenderErrorCallback = nullptr; OnCommandSenderDoneCallback gOnCommandSenderDoneCallback = nullptr; -OnCommandSenderDoneCallback gOnGroupCommandSenderDoneCallback = nullptr; class CommandSenderCallback : public CommandSender::Callback { @@ -113,29 +111,10 @@ class CommandSenderCallback : public CommandSender::Callback delete this; }; -protected: +private: PyObject mAppContext = nullptr; }; -class GroupCommandSenderCallback : public CommandSenderCallback -{ -public: - GroupCommandSenderCallback(PyObject appContext) : CommandSenderCallback(appContext) {} - void OnResponse(CommandSender * apCommandSender, const ConcreteCommandPath & aPath, const app::StatusIB & aStatus, - TLV::TLVReader * aData) override - { - // Group messages are not expected to provide a response. - chipDie(); - } - - void OnDone(CommandSender * apCommandSender) override - { - gOnGroupCommandSenderDoneCallback(mAppContext); - delete apCommandSender; - delete this; - }; -}; - } // namespace python } // namespace chip @@ -144,13 +123,11 @@ using namespace chip::python; extern "C" { void pychip_CommandSender_InitCallbacks(OnCommandSenderResponseCallback onCommandSenderResponseCallback, OnCommandSenderErrorCallback onCommandSenderErrorCallback, - OnCommandSenderDoneCallback onCommandSenderDoneCallback, - OnGroupCommandSenderDoneCallback onGroupCommandSenderDoneCallback) + OnCommandSenderDoneCallback onCommandSenderDoneCallback) { - gOnCommandSenderResponseCallback = onCommandSenderResponseCallback; - gOnCommandSenderErrorCallback = onCommandSenderErrorCallback; - gOnCommandSenderDoneCallback = onCommandSenderDoneCallback; - gOnGroupCommandSenderDoneCallback = onGroupCommandSenderDoneCallback; + gOnCommandSenderResponseCallback = onCommandSenderResponseCallback; + gOnCommandSenderErrorCallback = onCommandSenderErrorCallback; + gOnCommandSenderDoneCallback = onCommandSenderDoneCallback; } PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * device, uint16_t timedRequestTimeoutMs, @@ -200,19 +177,17 @@ PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * de return ToPyChipError(err); } -PyChipError pychip_CommandSender_SendGroupCommand(void * appContext, chip::GroupId groupId, - chip::Controller::DeviceCommissioner * devCtrl, uint16_t timedRequestTimeoutMs, +PyChipError pychip_CommandSender_SendGroupCommand(chip::GroupId groupId, + chip::Controller::DeviceCommissioner * devCtrl, chip::ClusterId clusterId, chip::CommandId commandId, const uint8_t * payload, - size_t length, uint16_t interactionTimeoutMs, uint16_t busyWaitMs) + size_t length, uint16_t busyWaitMs) { CHIP_ERROR err = CHIP_NO_ERROR; chip::Messaging::ExchangeManager * exchangeManager = chip::app::InteractionModelEngine::GetInstance()->GetExchangeManager(); VerifyOrReturnError(exchangeManager != nullptr, ToPyChipError(CHIP_ERROR_INCORRECT_STATE)); - std::unique_ptr callback = std::make_unique(appContext); - std::unique_ptr sender = std::make_unique(callback.get(), exchangeManager, - /* is timed request */ timedRequestTimeoutMs != 0); + std::unique_ptr sender = std::make_unique(nullptr /* callback */, exchangeManager); app::CommandPathParams cmdParams = { groupId, clusterId, commandId, (app::CommandPathFlags::kGroupIdValid) }; @@ -227,8 +202,7 @@ PyChipError pychip_CommandSender_SendGroupCommand(void * appContext, chip::Group SuccessOrExit(writer->CopyContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kFields)), reader)); } - SuccessOrExit(err = sender->FinishCommand(timedRequestTimeoutMs != 0 ? Optional(timedRequestTimeoutMs) - : Optional::Missing())); + SuccessOrExit(err = sender->FinishCommand(Optional::Missing())); { auto fabricIndex = devCtrl->GetFabricIndex(); @@ -237,9 +211,6 @@ PyChipError pychip_CommandSender_SendGroupCommand(void * appContext, chip::Group SuccessOrExit(err = sender->SendGroupCommandRequest(chip::SessionHandle(session))); } - sender.release(); - callback.release(); - if (busyWaitMs) { usleep(busyWaitMs * 1000); diff --git a/src/controller/python/chip/yaml/runner.py b/src/controller/python/chip/yaml/runner.py index a0a3ca2baed2bd..8780c2b5c40fcf 100644 --- a/src/controller/python/chip/yaml/runner.py +++ b/src/controller/python/chip/yaml/runner.py @@ -188,10 +188,9 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): def run_action(self, dev_ctrl: ChipDeviceController) -> _ActionResult: try: if self._group_id: - resp = asyncio.run(dev_ctrl.SendGroupCommand( + resp = dev_ctrl.SendGroupCommand( self._group_id, self._request_object, - timedRequestTimeoutMs=self._interation_timeout_ms, - busyWaitMs=self._busy_wait_ms)) + busyWaitMs=self._busy_wait_ms) else: resp = asyncio.run(dev_ctrl.SendCommand( self._node_id, self._endpoint, self._request_object, From 0097ebb762c853b43c5024f6e36c71640037de11 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 22 Feb 2023 15:55:13 +0000 Subject: [PATCH 5/5] Restyle --- src/controller/python/chip/clusters/command.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/controller/python/chip/clusters/command.cpp b/src/controller/python/chip/clusters/command.cpp index cb25953ff97c32..0c812c49b83f72 100644 --- a/src/controller/python/chip/clusters/command.cpp +++ b/src/controller/python/chip/clusters/command.cpp @@ -38,8 +38,7 @@ PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * de const uint8_t * payload, size_t length, uint16_t interactionTimeoutMs, uint16_t busyWaitMs); -PyChipError pychip_CommandSender_SendGroupCommand(chip::GroupId groupId, - chip::Controller::DeviceCommissioner * devCtrl, +PyChipError pychip_CommandSender_SendGroupCommand(chip::GroupId groupId, chip::Controller::DeviceCommissioner * devCtrl, chip::ClusterId clusterId, chip::CommandId commandId, const uint8_t * payload, size_t length, uint16_t busyWaitMs); } @@ -177,8 +176,7 @@ PyChipError pychip_CommandSender_SendCommand(void * appContext, DeviceProxy * de return ToPyChipError(err); } -PyChipError pychip_CommandSender_SendGroupCommand(chip::GroupId groupId, - chip::Controller::DeviceCommissioner * devCtrl, +PyChipError pychip_CommandSender_SendGroupCommand(chip::GroupId groupId, chip::Controller::DeviceCommissioner * devCtrl, chip::ClusterId clusterId, chip::CommandId commandId, const uint8_t * payload, size_t length, uint16_t busyWaitMs) { @@ -187,7 +185,7 @@ PyChipError pychip_CommandSender_SendGroupCommand(chip::GroupId groupId, chip::Messaging::ExchangeManager * exchangeManager = chip::app::InteractionModelEngine::GetInstance()->GetExchangeManager(); VerifyOrReturnError(exchangeManager != nullptr, ToPyChipError(CHIP_ERROR_INCORRECT_STATE)); - std::unique_ptr sender = std::make_unique(nullptr /* callback */, exchangeManager); + std::unique_ptr sender = std::make_unique(nullptr /* callback */, exchangeManager); app::CommandPathParams cmdParams = { groupId, clusterId, commandId, (app::CommandPathFlags::kGroupIdValid) };