From d5e994071429bb4ea6c769fc380a9ced8148d41a Mon Sep 17 00:00:00 2001 From: Jerry Johns Date: Sat, 9 Jul 2022 07:59:50 -0700 Subject: [PATCH] Python: Support for specifying absolute timeouts for Invoke/Write This adds support for specifying an absolute timeout for ChipDeviceController.SendCommand and ChipDeviceController.WriteAttribute methods. This timeout is used to bound both mDNS discovery and the subsequent IM interaction. --- src/controller/python/chip/ChipDeviceCtrl.py | 30 ++++++++++++------- src/controller/python/chip/ChipStack.py | 12 +++++--- .../python/chip/clusters/Attribute.py | 4 +-- .../python/chip/clusters/Command.py | 9 ++++-- .../python/chip/clusters/attribute.cpp | 14 ++++++--- .../python/chip/clusters/command.cpp | 13 ++++++-- 6 files changed, 57 insertions(+), 25 deletions(-) diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py index 0cfa53f3bd8002..7112d1da5d03ae 100644 --- a/src/controller/python/chip/ChipDeviceCtrl.py +++ b/src/controller/python/chip/ChipDeviceCtrl.py @@ -548,7 +548,7 @@ def GetClusterHandler(self): return self._Cluster - def GetConnectedDeviceSync(self, nodeid, allowPASE=True): + def GetConnectedDeviceSync(self, nodeid, allowPASE=True, timeoutMs: int = None): self.CheckIsActive() returnDevice = c_void_p(None) @@ -567,13 +567,13 @@ def DeviceAvailableCallback(device, err): if allowPASE: res = self._ChipStack.Call(lambda: self._dmLib.pychip_GetDeviceBeingCommissioned( - self.devCtrl, nodeid, byref(returnDevice))) + self.devCtrl, nodeid, byref(returnDevice)), timeoutMs) if res == 0: print('Using PASE connection') return returnDevice res = self._ChipStack.Call(lambda: self._dmLib.pychip_GetConnectedDeviceByNodeId( - self.devCtrl, nodeid, DeviceAvailableCallback)) + self.devCtrl, nodeid, DeviceAvailableCallback), timeoutMs) if res != 0: raise self._ChipStack.ErrorToException(res) @@ -581,43 +581,53 @@ def DeviceAvailableCallback(device, err): # Check if the device is already set before waiting for the callback. if returnDevice.value is None: with deviceAvailableCV: - deviceAvailableCV.wait() + timeout = None + if (timeoutMs): + timeout = float(timeoutMs) / 1000 + + ret = deviceAvailableCV.wait(timeout) + if ret is False: + raise TimeoutError("Timed out waiting for DNS-SD resolution") if returnDevice.value is None: raise self._ChipStack.ErrorToException(returnErr) return returnDevice - async def SendCommand(self, nodeid: int, endpoint: int, payload: ClusterObjects.ClusterCommand, responseType=None, timedRequestTimeoutMs: int = None): + async def SendCommand(self, nodeid: int, endpoint: int, payload: ClusterObjects.ClusterCommand, responseType=None, timedRequestTimeoutMs: int = None, interactionTimeoutMs: 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 from the metadata received over the wire. 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() - device = self.GetConnectedDeviceSync(nodeid) + device = self.GetConnectedDeviceSync(nodeid, timeoutMs=interactionTimeoutMs) res = ClusterCommand.SendCommand( future, eventLoop, responseType, device, ClusterCommand.CommandPath( EndpointId=endpoint, ClusterId=payload.cluster_id, CommandId=payload.command_id, - ), payload, timedRequestTimeoutMs=timedRequestTimeoutMs) + ), payload, timedRequestTimeoutMs=timedRequestTimeoutMs, interactionTimeoutMs=interactionTimeoutMs) if res != 0: future.set_exception(self._ChipStack.ErrorToException(res)) return await future - async def WriteAttribute(self, nodeid: int, attributes: typing.List[typing.Tuple[int, ClusterObjects.ClusterAttributeDescriptor, int]], timedRequestTimeoutMs: int = None): + async def WriteAttribute(self, nodeid: int, attributes: typing.List[typing.Tuple[int, ClusterObjects.ClusterAttributeDescriptor, int]], timedRequestTimeoutMs: int = None, interactionTimeoutMs: int = None): ''' Write a list of attributes on a target node. nodeId: Target's Node ID timedWriteTimeoutMs: Timeout for a timed write request. Omit or set to 'None' to indicate a non-timed request. attributes: A list of tuples of type (endpoint, cluster-object): + 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. E.g (1, Clusters.TestCluster.Attributes.XYZAttribute('hello')) -- Write 'hello' to the XYZ attribute on the test cluster to endpoint 1 @@ -627,7 +637,7 @@ async def WriteAttribute(self, nodeid: int, attributes: typing.List[typing.Tuple eventLoop = asyncio.get_running_loop() future = eventLoop.create_future() - device = self.GetConnectedDeviceSync(nodeid) + device = self.GetConnectedDeviceSync(nodeid, timeoutMs=interactionTimeoutMs) attrs = [] for v in attributes: @@ -639,7 +649,7 @@ async def WriteAttribute(self, nodeid: int, attributes: typing.List[typing.Tuple v[0], v[1], v[2], 1, v[1].value)) res = ClusterAttribute.WriteAttributes( - future, eventLoop, device, attrs, timedRequestTimeoutMs=timedRequestTimeoutMs) + future, eventLoop, device, attrs, timedRequestTimeoutMs=timedRequestTimeoutMs, interactionTimeoutMs=interactionTimeoutMs) if res != 0: raise self._ChipStack.ErrorToException(res) return await future diff --git a/src/controller/python/chip/ChipStack.py b/src/controller/python/chip/ChipStack.py index ef1cd597d9ef9e..a4683cafca4b3b 100644 --- a/src/controller/python/chip/ChipStack.py +++ b/src/controller/python/chip/ChipStack.py @@ -151,10 +151,14 @@ def __call__(self): self._cv.notify_all() pythonapi.Py_DecRef(py_object(self)) - def Wait(self): + def Wait(self, timeoutMs: int = None): + timeout = None + if timeoutMs is not None: + timeout = float(timeoutMs) / 1000 + with self._cv: while self._finish is False: - self._cv.wait() + self._cv.wait(timeout) if self._exc is not None: raise self._exc return self._res @@ -335,7 +339,7 @@ def Shutdown(self): self.devMgr = None self.callbackRes = None - def Call(self, callFunct): + def Call(self, callFunct, timeoutMs: int = None): '''Run a Python function on CHIP stack, and wait for the response. This function is a wrapper of PostTaskOnChipThread, which includes some handling of application specific logics. Calling this function on CHIP on CHIP mainloop thread will cause deadlock. @@ -344,7 +348,7 @@ def Call(self, callFunct): self.callbackRes = None self.completeEvent.clear() with self.networkLock: - res = self.PostTaskOnChipThread(callFunct).Wait() + res = self.PostTaskOnChipThread(callFunct).Wait(timeoutMs) self.completeEvent.set() if res == 0 and self.callbackRes != None: return self.callbackRes diff --git a/src/controller/python/chip/clusters/Attribute.py b/src/controller/python/chip/clusters/Attribute.py index a11d324d5890a4..87fd371cf963c3 100644 --- a/src/controller/python/chip/clusters/Attribute.py +++ b/src/controller/python/chip/clusters/Attribute.py @@ -873,7 +873,7 @@ def _OnWriteDoneCallback(closure): closure.handleDone() -def WriteAttributes(future: Future, eventLoop, device, attributes: List[AttributeWriteRequest], timedRequestTimeoutMs: int = None) -> int: +def WriteAttributes(future: Future, eventLoop, device, attributes: List[AttributeWriteRequest], timedRequestTimeoutMs: int = None, interactionTimeoutMs: int = None) -> int: handle = chip.native.GetLibraryHandle() writeargs = [] @@ -898,7 +898,7 @@ 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_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_size_t(len(attributes)), *writeargs)) if res != 0: 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 21c9b476d21000..96edd9c8b8299c 100644 --- a/src/controller/python/chip/clusters/Command.py +++ b/src/controller/python/chip/clusters/Command.py @@ -139,13 +139,18 @@ 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) -> int: +def SendCommand(future: Future, eventLoop, responseType: Type, device, commandPath: CommandPath, payload: ClusterCommand, timedRequestTimeoutMs: int = None, interactionTimeoutMs: int = None) -> int: ''' 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) - Raises an exception if any errors are encountered. If no response type is provided above, the type will be automatically deduced. + + 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 (responseType is not None) and (not issubclass(responseType, ClusterCommand)): raise ValueError("responseType must be a ClusterCommand or None") @@ -160,7 +165,7 @@ def SendCommand(future: Future, eventLoop, responseType: Type, device, commandPa 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))) + 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))) def Init(): diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp index e4480a3bb1e059..59de72d01ffef5 100644 --- a/src/controller/python/chip/clusters/attribute.cpp +++ b/src/controller/python/chip/clusters/attribute.cpp @@ -29,6 +29,8 @@ #include #include +#include + using namespace chip; using namespace chip::app; @@ -241,7 +243,8 @@ struct __attribute__((packed)) PyReadAttributeParams // Encodes n attribute write requests, follows 3 * n arguments, in the (AttributeWritePath*=void *, uint8_t*, size_t) order. chip::ChipError::StorageType pychip_WriteClient_WriteAttributes(void * appContext, DeviceProxy * device, - uint16_t timedWriteTimeoutMs, size_t n, ...); + uint16_t timedWriteTimeoutMs, uint16_t interactionTimeoutMs, + size_t n, ...); chip::ChipError::StorageType pychip_ReadClient_ReadAttributes(void * appContext, ReadClient ** pReadClient, ReadClientCallback ** pCallback, DeviceProxy * device, uint8_t * readParamsBuf, size_t n, size_t total, ...); @@ -319,7 +322,8 @@ void pychip_ReadClient_InitCallbacks(OnReadAttributeDataCallback onReadAttribute } chip::ChipError::StorageType pychip_WriteClient_WriteAttributes(void * appContext, DeviceProxy * device, - uint16_t timedWriteTimeoutMs, size_t n, ...) + uint16_t timedWriteTimeoutMs, uint16_t interactionTimeoutMs, + size_t n, ...) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -331,7 +335,7 @@ chip::ChipError::StorageType pychip_WriteClient_WriteAttributes(void * appContex va_list args; va_start(args, n); - VerifyOrExit(device != nullptr && device->GetSecureSession().HasValue(), err = CHIP_ERROR_INCORRECT_STATE); + VerifyOrExit(device != nullptr && device->GetSecureSession().HasValue(), err = CHIP_ERROR_MISSING_SECURE_SESSION); { for (size_t i = 0; i < n; i++) @@ -359,7 +363,9 @@ chip::ChipError::StorageType pychip_WriteClient_WriteAttributes(void * appContex } } - SuccessOrExit(err = client->SendWriteRequest(device->GetSecureSession().Value())); + SuccessOrExit(err = client->SendWriteRequest(device->GetSecureSession().Value(), + interactionTimeoutMs != 0 ? System::Clock::Milliseconds32(interactionTimeoutMs) + : System::Clock::kZero)); client.release(); callback.release(); diff --git a/src/controller/python/chip/clusters/command.cpp b/src/controller/python/chip/clusters/command.cpp index e87a2252048b26..342e624698ed18 100644 --- a/src/controller/python/chip/clusters/command.cpp +++ b/src/controller/python/chip/clusters/command.cpp @@ -35,7 +35,8 @@ extern "C" { chip::ChipError::StorageType 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); + const uint8_t * payload, size_t length, + uint16_t interactionTimeoutMs); } namespace chip { @@ -127,10 +128,12 @@ void pychip_CommandSender_InitCallbacks(OnCommandSenderResponseCallback onComman chip::ChipError::StorageType 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) + const uint8_t * payload, size_t length, uint16_t interactionTimeoutMs) { CHIP_ERROR err = CHIP_NO_ERROR; + VerifyOrReturnError(device->GetSecureSession().HasValue(), CHIP_ERROR_MISSING_SECURE_SESSION.AsInteger()); + std::unique_ptr callback = std::make_unique(appContext); std::unique_ptr sender = std::make_unique(callback.get(), device->GetExchangeManager(), /* is timed request */ timedRequestTimeoutMs != 0); @@ -151,7 +154,11 @@ chip::ChipError::StorageType pychip_CommandSender_SendCommand(void * appContext, SuccessOrExit(err = sender->FinishCommand(timedRequestTimeoutMs != 0 ? Optional(timedRequestTimeoutMs) : Optional::Missing())); - SuccessOrExit(err = device->SendCommands(sender.get())); + + SuccessOrExit(err = sender->SendCommandRequest(device->GetSecureSession().Value(), + interactionTimeoutMs != 0 + ? MakeOptional(System::Clock::Milliseconds32(interactionTimeoutMs)) + : Optional::Missing())); sender.release(); callback.release();