Skip to content

Commit

Permalink
[Python] Remove obsolete callback handling (#33665)
Browse files Browse the repository at this point in the history
The Call() function currently still has some callback handling code
the completeEvent and callbackRes variables. These are only used when
callbacks are in play, like pychip_DeviceController_Commission or
pychip_DeviceController_OpenCommissioningWindow. When calling these
functions CallAsyncWithCompleteCallback() needs to be used (and is
beeing used in all cases).

In practice, on single threaded applications this is not a problem.
However, when calling the SDK from multiple threads, then another Call()
Might accidentally release a call to CallAsyncWithCompleteCallback()
early.
  • Loading branch information
agners authored May 30, 2024
1 parent 92f8cd0 commit 17c5a00
Showing 1 changed file with 1 addition and 21 deletions.
22 changes: 1 addition & 21 deletions src/controller/python/chip/ChipStack.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
import os
import sys
import time
from ctypes import (CFUNCTYPE, POINTER, Structure, c_bool, c_char_p, c_int64, c_uint8, c_uint16, c_uint32, c_ulong, c_void_p,
py_object, pythonapi)
from ctypes import CFUNCTYPE, Structure, c_bool, c_char_p, c_int64, c_uint8, c_uint16, c_uint32, c_void_p, py_object, pythonapi
from threading import Condition, Event, Lock

import chip.native
Expand Down Expand Up @@ -194,9 +193,6 @@ def __call__(self):
pythonapi.Py_DecRef(py_object(self))


_CompleteFunct = CFUNCTYPE(None, c_void_p, c_void_p)
_ErrorFunct = CFUNCTYPE(None, c_void_p, c_void_p,
c_ulong, POINTER(DeviceStatusStruct))
_LogMessageFunct = CFUNCTYPE(
None, c_int64, c_int64, c_char_p, c_uint8, c_char_p)
_ChipThreadTaskRunnerFunct = CFUNCTYPE(None, py_object)
Expand Down Expand Up @@ -272,21 +268,11 @@ def __init__(self, persistentStoragePath: str, installDefaultLogHandler=True,
self.logger.addHandler(logHandler)
self.logger.setLevel(logging.DEBUG)

def HandleComplete(appState, reqState):
self.callbackRes = True
self.completeEvent.set()

def HandleError(appState, reqState, err, devStatusPtr):
self.callbackRes = self.ErrorToException(err, devStatusPtr)
self.completeEvent.set()

@_ChipThreadTaskRunnerFunct
def HandleChipThreadRun(callback):
callback()

self.cbHandleChipThreadRun = HandleChipThreadRun
self.cbHandleComplete = _CompleteFunct(HandleComplete)
self.cbHandleError = _ErrorFunct(HandleError)
# set by other modules(BLE) that require service by thread while thread blocks.
self.blockingCB = None

Expand Down Expand Up @@ -389,15 +375,9 @@ def Call(self, callFunct, timeoutMs: int = None):
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.
'''
# throw error if op in progress
self.callbackRes = None
self.completeEvent.clear()
# TODO: Lock probably no longer necessary, see https://github.com/project-chip/connectedhomeip/issues/33321.
with self.networkLock:
res = self.PostTaskOnChipThread(callFunct).Wait(timeoutMs)
self.completeEvent.set()
if res == 0 and self.callbackRes is not None:
return self.callbackRes
return res

async def CallAsync(self, callFunct, timeoutMs: int = None):
Expand Down

0 comments on commit 17c5a00

Please sign in to comment.