Skip to content

Commit

Permalink
Fix chip-repl write attribute for ARM64 Apple Platform devices (#25524)
Browse files Browse the repository at this point in the history
  • Loading branch information
tehampson authored and pull[bot] committed Jul 29, 2023
1 parent 44cda71 commit 3298077
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 13 deletions.
22 changes: 17 additions & 5 deletions src/controller/python/chip/clusters/Attribute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
)

Expand Down Expand Up @@ -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
Expand Down
28 changes: 20 additions & 8 deletions src/controller/python/chip/clusters/attribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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, ...);
}
Expand Down Expand Up @@ -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<uint16_t>(timedWriteTimeoutMsSizeT);
uint16_t interactionTimeoutMs = static_cast<uint16_t>(interactionTimeoutMsSizeT);
uint16_t busyWaitMs = static_cast<uint16_t>(busyWaitMsSizeT);

std::unique_ptr<WriteClientCallback> callback = std::make_unique<WriteClientCallback>(appContext);
std::unique_ptr<WriteClient> client = std::make_unique<WriteClient>(
app::InteractionModelEngine::GetInstance()->GetExchangeManager(), callback->GetChunkedCallback(),
Expand Down Expand Up @@ -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<chip::GroupId>(groupIdSizeT);
uint16_t busyWaitMs = static_cast<uint16_t>(busyWaitMsSizeT);

chip::Messaging::ExchangeManager * exchangeManager = chip::app::InteractionModelEngine::GetInstance()->GetExchangeManager();
VerifyOrReturnError(exchangeManager != nullptr, ToPyChipError(CHIP_ERROR_INCORRECT_STATE));

Expand Down

0 comments on commit 3298077

Please sign in to comment.