Skip to content

Commit

Permalink
Merge pull request #52 from home-assistant-libs/add-dependent-patch
Browse files Browse the repository at this point in the history
Add dependent patch which introduces _DeviceAvailableCallback
  • Loading branch information
agners authored Mar 28, 2024
2 parents 97f55c5 + b7aa56c commit aed5835
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 0 deletions.
170 changes: 170 additions & 0 deletions 0012-Python-Keep-reference-to-callback-function-on-timeou.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
From f5f84d9e4a7296680a606ab4c6ddd0e265374efc Mon Sep 17 00:00:00 2001
From: Stefan Agner <stefan@agner.ch>
Date: Wed, 13 Dec 2023 19:51:20 +0100
Subject: [PATCH] [Python] Keep reference to callback function on timeout
(#30877)

* [Python] Keep reference to callback function on timeout

When using a timeout when calling GetConnectedDeviceSync() the callback
function DeviceAvailableCallback() gets potentially GC'ed. Make sure
we hold a reference to the instance.

* Use correct namespace for PyObject

* Fix types in pychip_GetConnectedDeviceByNodeId

* Call callback with context (self)

* Correctly pass context

* Use separate closure function
---
.../ChipDeviceController-ScriptBinding.cpp | 17 ++++-----
src/controller/python/chip/ChipDeviceCtrl.py | 35 ++++++++++++-------
2 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp
index da742b2463..504b952d8a 100644
--- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp
+++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp
@@ -87,7 +87,7 @@ using namespace chip::DeviceLayer;
extern "C" {
typedef void (*ConstructBytesArrayFunct)(const uint8_t * dataBuf, uint32_t dataLen);
typedef void (*LogMessageFunct)(uint64_t time, uint64_t timeUS, const char * moduleName, uint8_t category, const char * msg);
-typedef void (*DeviceAvailableFunc)(DeviceProxy * device, PyChipError err);
+typedef void (*DeviceAvailableFunc)(chip::Controller::Python::PyObject * context, DeviceProxy * device, PyChipError err);
typedef void (*ChipThreadTaskRunnerFunct)(intptr_t context);
typedef void (*DeviceUnpairingCompleteFunct)(uint64_t nodeId, PyChipError error);
}
@@ -202,7 +202,7 @@ const char * pychip_Stack_StatusReportToString(uint32_t profileId, uint16_t stat
void pychip_Stack_SetLogFunct(LogMessageFunct logFunct);

PyChipError pychip_GetConnectedDeviceByNodeId(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId,
- DeviceAvailableFunc callback);
+ chip::Controller::Python::PyObject * context, DeviceAvailableFunc callback);
PyChipError pychip_FreeOperationalDeviceProxy(chip::OperationalDeviceProxy * deviceProxy);
PyChipError pychip_GetLocalSessionId(chip::OperationalDeviceProxy * deviceProxy, uint16_t * localSessionId);
PyChipError pychip_GetNumSessionsToPeer(chip::OperationalDeviceProxy * deviceProxy, uint32_t * numSessions);
@@ -706,36 +706,37 @@ namespace {

struct GetDeviceCallbacks
{
- GetDeviceCallbacks(DeviceAvailableFunc callback) :
- mOnSuccess(OnDeviceConnectedFn, this), mOnFailure(OnConnectionFailureFn, this), mCallback(callback)
+ GetDeviceCallbacks(chip::Controller::Python::PyObject * context, DeviceAvailableFunc callback) :
+ mOnSuccess(OnDeviceConnectedFn, this), mOnFailure(OnConnectionFailureFn, this), mContext(context), mCallback(callback)
{}

static void OnDeviceConnectedFn(void * context, Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle)
{
auto * self = static_cast<GetDeviceCallbacks *>(context);
auto * operationalDeviceProxy = new OperationalDeviceProxy(&exchangeMgr, sessionHandle);
- self->mCallback(operationalDeviceProxy, ToPyChipError(CHIP_NO_ERROR));
+ self->mCallback(self->mContext, operationalDeviceProxy, ToPyChipError(CHIP_NO_ERROR));
delete self;
}

static void OnConnectionFailureFn(void * context, const ScopedNodeId & peerId, CHIP_ERROR error)
{
auto * self = static_cast<GetDeviceCallbacks *>(context);
- self->mCallback(nullptr, ToPyChipError(error));
+ self->mCallback(self->mContext, nullptr, ToPyChipError(error));
delete self;
}

Callback::Callback<OnDeviceConnected> mOnSuccess;
Callback::Callback<OnDeviceConnectionFailure> mOnFailure;
+ chip::Controller::Python::PyObject * const mContext;
DeviceAvailableFunc mCallback;
};
} // anonymous namespace

PyChipError pychip_GetConnectedDeviceByNodeId(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId,
- DeviceAvailableFunc callback)
+ chip::Controller::Python::PyObject * context, DeviceAvailableFunc callback)
{
VerifyOrReturnError(devCtrl != nullptr, ToPyChipError(CHIP_ERROR_INVALID_ARGUMENT));
- auto * callbacks = new GetDeviceCallbacks(callback);
+ auto * callbacks = new GetDeviceCallbacks(context, callback);
return ToPyChipError(devCtrl->GetConnectedDevice(nodeId, &callbacks->mOnSuccess, &callbacks->mOnFailure));
}

diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py
index 08dbdff224..1572829591 100644
--- a/src/controller/python/chip/ChipDeviceCtrl.py
+++ b/src/controller/python/chip/ChipDeviceCtrl.py
@@ -76,7 +76,7 @@ _DevicePairingDelegate_OnFabricCheckFunct = CFUNCTYPE(
#
# CHIP_ERROR is actually signed, so using c_uint32 is weird, but everything
# else seems to do it.
-_DeviceAvailableFunct = CFUNCTYPE(None, c_void_p, PyChipError)
+_DeviceAvailableCallbackFunct = CFUNCTYPE(None, py_object, c_void_p, PyChipError)

_IssueNOCChainCallbackPythonCallbackFunct = CFUNCTYPE(
None, py_object, PyChipError, c_void_p, c_size_t, c_void_p, c_size_t, c_void_p, c_size_t, c_void_p, c_size_t, c_uint64)
@@ -100,6 +100,11 @@ class NOCChain:
adminSubject: int


+@_DeviceAvailableCallbackFunct
+def _DeviceAvailableCallback(closure, device, err):
+ closure.deviceAvailable(device, err)
+
+
@_IssueNOCChainCallbackPythonCallbackFunct
def _IssueNOCChainCallbackPythonCallback(devCtrl, status: PyChipError, noc: c_void_p, nocLen: int, icac: c_void_p,
icacLen: int, rcac: c_void_p, rcacLen: int, ipk: c_void_p, ipkLen: int, adminSubject: int):
@@ -743,16 +748,6 @@ class ChipDeviceControllerBase():
returnErr = None
deviceAvailableCV = threading.Condition()

- @_DeviceAvailableFunct
- def DeviceAvailableCallback(device, err):
- nonlocal returnDevice
- nonlocal returnErr
- nonlocal deviceAvailableCV
- with deviceAvailableCV:
- returnDevice = c_void_p(device)
- returnErr = err
- deviceAvailableCV.notify_all()
-
if allowPASE:
res = self._ChipStack.Call(lambda: self._dmLib.pychip_GetDeviceBeingCommissioned(
self.devCtrl, nodeid, byref(returnDevice)), timeoutMs)
@@ -760,8 +755,22 @@ class ChipDeviceControllerBase():
print('Using PASE connection')
return DeviceProxyWrapper(returnDevice)

+ class DeviceAvailableClosure():
+ def deviceAvailable(self, device, err):
+ nonlocal returnDevice
+ nonlocal returnErr
+ nonlocal deviceAvailableCV
+ with deviceAvailableCV:
+ returnDevice = c_void_p(device)
+ returnErr = err
+ deviceAvailableCV.notify_all()
+ ctypes.pythonapi.Py_DecRef(ctypes.py_object(self))
+
+ closure = DeviceAvailableClosure()
+ ctypes.pythonapi.Py_IncRef(ctypes.py_object(closure))
self._ChipStack.Call(lambda: self._dmLib.pychip_GetConnectedDeviceByNodeId(
- self.devCtrl, nodeid, DeviceAvailableCallback), timeoutMs).raise_on_error()
+ self.devCtrl, nodeid, ctypes.py_object(closure), _DeviceAvailableCallback),
+ timeoutMs).raise_on_error()

# The callback might have been received synchronously (during self._ChipStack.Call()).
# Check if the device is already set before waiting for the callback.
@@ -1491,7 +1500,7 @@ class ChipDeviceControllerBase():
self._dmLib.pychip_ScriptDevicePairingDelegate_SetFabricCheckCallback.restype = PyChipError

self._dmLib.pychip_GetConnectedDeviceByNodeId.argtypes = [
- c_void_p, c_uint64, _DeviceAvailableFunct]
+ c_void_p, c_uint64, py_object, _DeviceAvailableCallbackFunct]
self._dmLib.pychip_GetConnectedDeviceByNodeId.restype = PyChipError

self._dmLib.pychip_FreeOperationalDeviceProxy.argtypes = [
--
2.44.0

0 comments on commit aed5835

Please sign in to comment.