From 3298077e5e6b5cfc16c127ad882e9b15184c89a5 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 7 Mar 2023 17:16:11 -0500 Subject: [PATCH] Fix chip-repl write attribute for ARM64 Apple Platform devices (#25524) --- .../python/chip/clusters/Attribute.py | 22 +++++++++++---- .../python/chip/clusters/attribute.cpp | 28 +++++++++++++------ 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/controller/python/chip/clusters/Attribute.py b/src/controller/python/chip/clusters/Attribute.py index 5cad75c444a871..1bd60cbf23501d 100644 --- a/src/controller/python/chip/clusters/Attribute.py +++ b/src/controller/python/chip/clusters/Attribute.py @@ -939,9 +939,9 @@ def WriteAttributes(future: Future, eventLoop, device, attributes: List[Attribut 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_uint16(0 if busyWaitMs is None else busyWaitMs), + ctypes.c_size_t(0 if timedRequestTimeoutMs is None else timedRequestTimeoutMs), + ctypes.c_size_t(0 if interactionTimeoutMs is None else interactionTimeoutMs), + ctypes.c_size_t(0 if busyWaitMs is None else busyWaitMs), ctypes.c_size_t(len(attributes)), *writeargs) ) if not res.is_success: @@ -969,8 +969,8 @@ def WriteGroupAttributes(groupId: int, devCtrl: c_void_p, attributes: List[Attri return builtins.chipStack.Call( lambda: handle.pychip_WriteClient_WriteGroupAttributes( - ctypes.c_uint16(groupId), devCtrl, - ctypes.c_uint16(0 if busyWaitMs is None else busyWaitMs), + ctypes.c_size_t(groupId), devCtrl, + ctypes.c_size_t(0 if busyWaitMs is None else busyWaitMs), ctypes.c_size_t(len(attributes)), *writeargs) ) @@ -1108,6 +1108,18 @@ def Init(): handle.pychip_WriteClient_WriteAttributes.restype = PyChipError handle.pychip_WriteClient_WriteGroupAttributes.restype = PyChipError + + # Both WriteAttributes and WriteGroupAttributes are variadic functions. As per ctype documentation + # https://docs.python.org/3/library/ctypes.html#calling-varadic-functions, it is critical that we + # specify the argtypes attribute for the regular, non-variadic, function arguments for this to work + # on ARM64 for Apple Platforms. + # TODO We could move away from a variadic function to one where we provide a vector of the + # attribute information we want written using a vector. This possibility was not implemented at the + # time where simply specified the argtypes, because of time constraints. This solution was quicker + # to fix the crash on ARM64 Apple platforms without a refactor. + handle.pychip_WriteClient_WriteAttributes.argtypes = [py_object, c_void_p, c_size_t, c_size_t, c_size_t, c_size_t] + handle.pychip_WriteClient_WriteGroupAttributes.argtypes = [c_size_t, c_void_p, c_size_t, c_size_t] + setter.Set('pychip_WriteClient_InitCallbacks', None, [ _OnWriteResponseCallbackFunct, _OnWriteErrorCallbackFunct, _OnWriteDoneCallbackFunct]) handle.pychip_ReadClient_Read.restype = PyChipError diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp index 5e7ffcdc679008..d97faf5cdaaad8 100644 --- a/src/controller/python/chip/clusters/attribute.cpp +++ b/src/controller/python/chip/clusters/attribute.cpp @@ -246,10 +246,10 @@ 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, uint16_t busyWaitMs, size_t n, ...); -PyChipError pychip_WriteClient_WriteGroupAttributes(chip::GroupId groupId, chip::Controller::DeviceCommissioner * devCtrl, - uint16_t busyWaitMs, size_t n, ...); +PyChipError pychip_WriteClient_WriteAttributes(void * appContext, DeviceProxy * device, size_t timedWriteTimeoutMsSizeT, + size_t interactionTimeoutMsSizeT, size_t busyWaitMsSizeT, size_t n, ...); +PyChipError pychip_WriteClient_WriteGroupAttributes(size_t groupIdSizeT, chip::Controller::DeviceCommissioner * devCtrl, + size_t busyWaitMsSizeT, size_t n, ...); PyChipError pychip_ReadClient_ReadAttributes(void * appContext, ReadClient ** pReadClient, ReadClientCallback ** pCallback, DeviceProxy * device, uint8_t * readParamsBuf, size_t n, size_t total, ...); } @@ -325,11 +325,17 @@ void pychip_ReadClient_InitCallbacks(OnReadAttributeDataCallback onReadAttribute gOnReportEndCallback = onReportEndCallback; } -PyChipError pychip_WriteClient_WriteAttributes(void * appContext, DeviceProxy * device, uint16_t timedWriteTimeoutMs, - uint16_t interactionTimeoutMs, uint16_t busyWaitMs, size_t n, ...) +PyChipError pychip_WriteClient_WriteAttributes(void * appContext, DeviceProxy * device, size_t timedWriteTimeoutMsSizeT, + size_t interactionTimeoutMsSizeT, size_t busyWaitMsSizeT, size_t n, ...) { CHIP_ERROR err = CHIP_NO_ERROR; + // The FFI from Python to C when calling a variadic function has issues when the regular, non-variadic, function + // arguments are unit16_t. As a result we pass these arguments as size_t and cast them to the expected uint16_t. + uint16_t timedWriteTimeoutMs = static_cast(timedWriteTimeoutMsSizeT); + uint16_t interactionTimeoutMs = static_cast(interactionTimeoutMsSizeT); + uint16_t busyWaitMs = static_cast(busyWaitMsSizeT); + std::unique_ptr callback = std::make_unique(appContext); std::unique_ptr client = std::make_unique( app::InteractionModelEngine::GetInstance()->GetExchangeManager(), callback->GetChunkedCallback(), @@ -383,11 +389,17 @@ PyChipError pychip_WriteClient_WriteAttributes(void * appContext, DeviceProxy * return ToPyChipError(err); } -PyChipError pychip_WriteClient_WriteGroupAttributes(chip::GroupId groupId, chip::Controller::DeviceCommissioner * devCtrl, - uint16_t busyWaitMs, size_t n, ...) +PyChipError pychip_WriteClient_WriteGroupAttributes(size_t groupIdSizeT, chip::Controller::DeviceCommissioner * devCtrl, + size_t busyWaitMsSizeT, size_t n, ...) { CHIP_ERROR err = CHIP_NO_ERROR; + // The FFI from Python to C when calling a variadic function has issues when the regular, non-variadic, function + // arguments are unit16_t (which is the type for chip::GroupId). As a result we pass these arguments as size_t + // and cast them to the expected type here. + chip::GroupId groupId = static_cast(groupIdSizeT); + uint16_t busyWaitMs = static_cast(busyWaitMsSizeT); + chip::Messaging::ExchangeManager * exchangeManager = chip::app::InteractionModelEngine::GetInstance()->GetExchangeManager(); VerifyOrReturnError(exchangeManager != nullptr, ToPyChipError(CHIP_ERROR_INCORRECT_STATE));