Skip to content

Commit b7aa56c

Browse files
committed
Add dependent patch which introduces _DeviceAvailableCallback
The patch which implements async GetConnectedDevice requires the _DeviceAvailableCallback which got introduced with the fix which keeps a reference to callback function when using a timeout. Reorder the patch in a sensible order. The main/v1.3 branch already has this change upstream.
1 parent 97f55c5 commit b7aa56c

2 files changed

+170
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
From f5f84d9e4a7296680a606ab4c6ddd0e265374efc Mon Sep 17 00:00:00 2001
2+
From: Stefan Agner <stefan@agner.ch>
3+
Date: Wed, 13 Dec 2023 19:51:20 +0100
4+
Subject: [PATCH] [Python] Keep reference to callback function on timeout
5+
(#30877)
6+
7+
* [Python] Keep reference to callback function on timeout
8+
9+
When using a timeout when calling GetConnectedDeviceSync() the callback
10+
function DeviceAvailableCallback() gets potentially GC'ed. Make sure
11+
we hold a reference to the instance.
12+
13+
* Use correct namespace for PyObject
14+
15+
* Fix types in pychip_GetConnectedDeviceByNodeId
16+
17+
* Call callback with context (self)
18+
19+
* Correctly pass context
20+
21+
* Use separate closure function
22+
---
23+
.../ChipDeviceController-ScriptBinding.cpp | 17 ++++-----
24+
src/controller/python/chip/ChipDeviceCtrl.py | 35 ++++++++++++-------
25+
2 files changed, 31 insertions(+), 21 deletions(-)
26+
27+
diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp
28+
index da742b2463..504b952d8a 100644
29+
--- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp
30+
+++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp
31+
@@ -87,7 +87,7 @@ using namespace chip::DeviceLayer;
32+
extern "C" {
33+
typedef void (*ConstructBytesArrayFunct)(const uint8_t * dataBuf, uint32_t dataLen);
34+
typedef void (*LogMessageFunct)(uint64_t time, uint64_t timeUS, const char * moduleName, uint8_t category, const char * msg);
35+
-typedef void (*DeviceAvailableFunc)(DeviceProxy * device, PyChipError err);
36+
+typedef void (*DeviceAvailableFunc)(chip::Controller::Python::PyObject * context, DeviceProxy * device, PyChipError err);
37+
typedef void (*ChipThreadTaskRunnerFunct)(intptr_t context);
38+
typedef void (*DeviceUnpairingCompleteFunct)(uint64_t nodeId, PyChipError error);
39+
}
40+
@@ -202,7 +202,7 @@ const char * pychip_Stack_StatusReportToString(uint32_t profileId, uint16_t stat
41+
void pychip_Stack_SetLogFunct(LogMessageFunct logFunct);
42+
43+
PyChipError pychip_GetConnectedDeviceByNodeId(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId,
44+
- DeviceAvailableFunc callback);
45+
+ chip::Controller::Python::PyObject * context, DeviceAvailableFunc callback);
46+
PyChipError pychip_FreeOperationalDeviceProxy(chip::OperationalDeviceProxy * deviceProxy);
47+
PyChipError pychip_GetLocalSessionId(chip::OperationalDeviceProxy * deviceProxy, uint16_t * localSessionId);
48+
PyChipError pychip_GetNumSessionsToPeer(chip::OperationalDeviceProxy * deviceProxy, uint32_t * numSessions);
49+
@@ -706,36 +706,37 @@ namespace {
50+
51+
struct GetDeviceCallbacks
52+
{
53+
- GetDeviceCallbacks(DeviceAvailableFunc callback) :
54+
- mOnSuccess(OnDeviceConnectedFn, this), mOnFailure(OnConnectionFailureFn, this), mCallback(callback)
55+
+ GetDeviceCallbacks(chip::Controller::Python::PyObject * context, DeviceAvailableFunc callback) :
56+
+ mOnSuccess(OnDeviceConnectedFn, this), mOnFailure(OnConnectionFailureFn, this), mContext(context), mCallback(callback)
57+
{}
58+
59+
static void OnDeviceConnectedFn(void * context, Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle)
60+
{
61+
auto * self = static_cast<GetDeviceCallbacks *>(context);
62+
auto * operationalDeviceProxy = new OperationalDeviceProxy(&exchangeMgr, sessionHandle);
63+
- self->mCallback(operationalDeviceProxy, ToPyChipError(CHIP_NO_ERROR));
64+
+ self->mCallback(self->mContext, operationalDeviceProxy, ToPyChipError(CHIP_NO_ERROR));
65+
delete self;
66+
}
67+
68+
static void OnConnectionFailureFn(void * context, const ScopedNodeId & peerId, CHIP_ERROR error)
69+
{
70+
auto * self = static_cast<GetDeviceCallbacks *>(context);
71+
- self->mCallback(nullptr, ToPyChipError(error));
72+
+ self->mCallback(self->mContext, nullptr, ToPyChipError(error));
73+
delete self;
74+
}
75+
76+
Callback::Callback<OnDeviceConnected> mOnSuccess;
77+
Callback::Callback<OnDeviceConnectionFailure> mOnFailure;
78+
+ chip::Controller::Python::PyObject * const mContext;
79+
DeviceAvailableFunc mCallback;
80+
};
81+
} // anonymous namespace
82+
83+
PyChipError pychip_GetConnectedDeviceByNodeId(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId,
84+
- DeviceAvailableFunc callback)
85+
+ chip::Controller::Python::PyObject * context, DeviceAvailableFunc callback)
86+
{
87+
VerifyOrReturnError(devCtrl != nullptr, ToPyChipError(CHIP_ERROR_INVALID_ARGUMENT));
88+
- auto * callbacks = new GetDeviceCallbacks(callback);
89+
+ auto * callbacks = new GetDeviceCallbacks(context, callback);
90+
return ToPyChipError(devCtrl->GetConnectedDevice(nodeId, &callbacks->mOnSuccess, &callbacks->mOnFailure));
91+
}
92+
93+
diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py
94+
index 08dbdff224..1572829591 100644
95+
--- a/src/controller/python/chip/ChipDeviceCtrl.py
96+
+++ b/src/controller/python/chip/ChipDeviceCtrl.py
97+
@@ -76,7 +76,7 @@ _DevicePairingDelegate_OnFabricCheckFunct = CFUNCTYPE(
98+
#
99+
# CHIP_ERROR is actually signed, so using c_uint32 is weird, but everything
100+
# else seems to do it.
101+
-_DeviceAvailableFunct = CFUNCTYPE(None, c_void_p, PyChipError)
102+
+_DeviceAvailableCallbackFunct = CFUNCTYPE(None, py_object, c_void_p, PyChipError)
103+
104+
_IssueNOCChainCallbackPythonCallbackFunct = CFUNCTYPE(
105+
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)
106+
@@ -100,6 +100,11 @@ class NOCChain:
107+
adminSubject: int
108+
109+
110+
+@_DeviceAvailableCallbackFunct
111+
+def _DeviceAvailableCallback(closure, device, err):
112+
+ closure.deviceAvailable(device, err)
113+
+
114+
+
115+
@_IssueNOCChainCallbackPythonCallbackFunct
116+
def _IssueNOCChainCallbackPythonCallback(devCtrl, status: PyChipError, noc: c_void_p, nocLen: int, icac: c_void_p,
117+
icacLen: int, rcac: c_void_p, rcacLen: int, ipk: c_void_p, ipkLen: int, adminSubject: int):
118+
@@ -743,16 +748,6 @@ class ChipDeviceControllerBase():
119+
returnErr = None
120+
deviceAvailableCV = threading.Condition()
121+
122+
- @_DeviceAvailableFunct
123+
- def DeviceAvailableCallback(device, err):
124+
- nonlocal returnDevice
125+
- nonlocal returnErr
126+
- nonlocal deviceAvailableCV
127+
- with deviceAvailableCV:
128+
- returnDevice = c_void_p(device)
129+
- returnErr = err
130+
- deviceAvailableCV.notify_all()
131+
-
132+
if allowPASE:
133+
res = self._ChipStack.Call(lambda: self._dmLib.pychip_GetDeviceBeingCommissioned(
134+
self.devCtrl, nodeid, byref(returnDevice)), timeoutMs)
135+
@@ -760,8 +755,22 @@ class ChipDeviceControllerBase():
136+
print('Using PASE connection')
137+
return DeviceProxyWrapper(returnDevice)
138+
139+
+ class DeviceAvailableClosure():
140+
+ def deviceAvailable(self, device, err):
141+
+ nonlocal returnDevice
142+
+ nonlocal returnErr
143+
+ nonlocal deviceAvailableCV
144+
+ with deviceAvailableCV:
145+
+ returnDevice = c_void_p(device)
146+
+ returnErr = err
147+
+ deviceAvailableCV.notify_all()
148+
+ ctypes.pythonapi.Py_DecRef(ctypes.py_object(self))
149+
+
150+
+ closure = DeviceAvailableClosure()
151+
+ ctypes.pythonapi.Py_IncRef(ctypes.py_object(closure))
152+
self._ChipStack.Call(lambda: self._dmLib.pychip_GetConnectedDeviceByNodeId(
153+
- self.devCtrl, nodeid, DeviceAvailableCallback), timeoutMs).raise_on_error()
154+
+ self.devCtrl, nodeid, ctypes.py_object(closure), _DeviceAvailableCallback),
155+
+ timeoutMs).raise_on_error()
156+
157+
# The callback might have been received synchronously (during self._ChipStack.Call()).
158+
# Check if the device is already set before waiting for the callback.
159+
@@ -1491,7 +1500,7 @@ class ChipDeviceControllerBase():
160+
self._dmLib.pychip_ScriptDevicePairingDelegate_SetFabricCheckCallback.restype = PyChipError
161+
162+
self._dmLib.pychip_GetConnectedDeviceByNodeId.argtypes = [
163+
- c_void_p, c_uint64, _DeviceAvailableFunct]
164+
+ c_void_p, c_uint64, py_object, _DeviceAvailableCallbackFunct]
165+
self._dmLib.pychip_GetConnectedDeviceByNodeId.restype = PyChipError
166+
167+
self._dmLib.pychip_FreeOperationalDeviceProxy.argtypes = [
168+
--
169+
2.44.0
170+

0 commit comments

Comments
 (0)