diff --git a/src/controller/python/chip/ble/library_handle.py b/src/controller/python/chip/ble/library_handle.py index 52f83d65b180b8..2dbb52f28bf429 100644 --- a/src/controller/python/chip/ble/library_handle.py +++ b/src/controller/python/chip/ble/library_handle.py @@ -18,7 +18,7 @@ from ctypes import c_bool, c_char_p, c_uint32, c_void_p, py_object import chip.native -from chip.ble.types import DeviceScannedCallback, ScanDoneCallback +from chip.ble.types import DeviceScannedCallback, ScanDoneCallback, ScanErrorCallback # This prevents python auto-casting c_void_p to integers and @@ -58,7 +58,7 @@ def _GetBleLibraryHandle() -> ctypes.CDLL: VoidPointer, [VoidPointer]) setter.Set('pychip_ble_start_scanning', VoidPointer, [ - py_object, VoidPointer, c_uint32, DeviceScannedCallback, ScanDoneCallback + py_object, VoidPointer, c_uint32, DeviceScannedCallback, ScanDoneCallback, ScanErrorCallback ]) return handle diff --git a/src/controller/python/chip/ble/scan_devices.py b/src/controller/python/chip/ble/scan_devices.py index 700f39b7e1de62..5422afb7cbb88b 100644 --- a/src/controller/python/chip/ble/scan_devices.py +++ b/src/controller/python/chip/ble/scan_devices.py @@ -20,7 +20,7 @@ from typing import Generator from chip.ble.library_handle import _GetBleLibraryHandle -from chip.ble.types import DeviceScannedCallback, ScanDoneCallback +from chip.ble.types import DeviceScannedCallback, ScanDoneCallback, ScanErrorCallback @DeviceScannedCallback @@ -34,7 +34,12 @@ def ScanDoneCallback(closure): closure.ScanCompleted() -def DiscoverAsync(timeoutMs: int, scanCallback, doneCallback, adapter=None): +@ScanErrorCallback +def ScanErrorCallback(closure, errorCode: int): + closure.ScanErrorCallback(errorCode) + + +def DiscoverAsync(timeoutMs: int, scanCallback, doneCallback, errorCallback, adapter=None): """Initiate a BLE discovery of devices with the given timeout. NOTE: devices are not guaranteed to be unique. New entries are returned @@ -44,6 +49,7 @@ def DiscoverAsync(timeoutMs: int, scanCallback, doneCallback, adapter=None): timeoutMs: scan will complete after this time scanCallback: callback when a device is found doneCallback: callback when the scan is complete + errorCallback: callback when error occurred during scan adapter: what adapter to choose. Either an AdapterInfo object or a string with the adapter address. If None, the first adapter on the system is used. @@ -72,6 +78,9 @@ def ScanCompleted(self, *args): doneCallback(*args) ctypes.pythonapi.Py_DecRef(ctypes.py_object(self)) + def ScanErrorCallback(self, *args): + errorCallback(*args) + closure = ScannerClosure() ctypes.pythonapi.Py_IncRef(ctypes.py_object(closure)) @@ -79,7 +88,7 @@ def ScanCompleted(self, *args): ctypes.py_object(closure), handle.pychip_ble_adapter_list_get_raw_adapter( nativeList), timeoutMs, - ScanFoundCallback, ScanDoneCallback) + ScanFoundCallback, ScanDoneCallback, ScanErrorCallback) if scanner == 0: raise Exception('Failed to initiate scan') @@ -113,6 +122,12 @@ def DeviceFound(self, address, discriminator, vendor, product): def ScanCompleted(self): self.queue.put(None) + def ScanError(self, errorCode): + # TODO need to determine what we do with this error. Most of the time this + # error is just a timeout introduced in PR #24873, right before we get a + # ScanCompleted. + pass + def DiscoverSync(timeoutMs: int, adapter=None) -> Generator[DeviceInfo, None, None]: """Discover BLE devices over the specified period of time. @@ -131,7 +146,7 @@ def DiscoverSync(timeoutMs: int, adapter=None) -> Generator[DeviceInfo, None, No receiver = _DeviceInfoReceiver() DiscoverAsync(timeoutMs, receiver.DeviceFound, - receiver.ScanCompleted, adapter) + receiver.ScanCompleted, receiver.ScanError, adapter) while True: data = receiver.queue.get() diff --git a/src/controller/python/chip/ble/types.py b/src/controller/python/chip/ble/types.py index b56a2e5670b012..94b1789e9c0673 100644 --- a/src/controller/python/chip/ble/types.py +++ b/src/controller/python/chip/ble/types.py @@ -20,3 +20,5 @@ c_uint16, c_uint16) ScanDoneCallback = CFUNCTYPE(None, py_object) + +ScanErrorCallback = CFUNCTYPE(None, py_object, c_uint16) diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index 0172c820c5a0cc..7c47e735d5dfc1 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -78,8 +78,16 @@ ChipDeviceScanner::~ChipDeviceScanner() { StopScan(); - // In case the timeout timer is still active - chip::DeviceLayer::SystemLayer().CancelTimer(TimerExpiredCallback, this); + // mTimerExpired should only be set to true in the TimerExpiredCallback, which means we are in that callback + // right now so there is no need to cancel the timer. Doing so would result in deadlock trying to aquire the + // chip stack lock which we already currently have. + if (!mTimerExpired) + { + // In case the timeout timer is still active + DeviceLayer::PlatformMgr().LockChipStack(); + chip::DeviceLayer::SystemLayer().CancelTimer(TimerExpiredCallback, this); + DeviceLayer::PlatformMgr().UnlockChipStack(); + } g_object_unref(mManager); g_object_unref(mCancellable); @@ -136,7 +144,9 @@ CHIP_ERROR ChipDeviceScanner::StartScan(System::Clock::Timeout timeout) return CHIP_ERROR_INTERNAL; } + DeviceLayer::PlatformMgr().LockChipStack(); CHIP_ERROR err = chip::DeviceLayer::SystemLayer().StartTimer(timeout, TimerExpiredCallback, static_cast(this)); + DeviceLayer::PlatformMgr().UnlockChipStack(); if (err != CHIP_NO_ERROR) { @@ -144,6 +154,7 @@ CHIP_ERROR ChipDeviceScanner::StartScan(System::Clock::Timeout timeout) StopScan(); return err; } + mTimerExpired = false; return CHIP_NO_ERROR; } @@ -151,6 +162,7 @@ CHIP_ERROR ChipDeviceScanner::StartScan(System::Clock::Timeout timeout) void ChipDeviceScanner::TimerExpiredCallback(chip::System::Layer * layer, void * appState) { ChipDeviceScanner * chipDeviceScanner = static_cast(appState); + chipDeviceScanner->MarkTimerExpired(); chipDeviceScanner->mDelegate->OnScanError(CHIP_ERROR_TIMEOUT); chipDeviceScanner->StopScan(); } @@ -180,6 +192,11 @@ CHIP_ERROR ChipDeviceScanner::StopScan() return CHIP_ERROR_INTERNAL; } + ChipDeviceScannerDelegate * delegate = this->mDelegate; + // callback is explicitly allowed to delete the scanner (hence no more + // references to 'self' here) + delegate->OnScanComplete(); + return CHIP_NO_ERROR; } @@ -192,12 +209,7 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStopScan(ChipDeviceScanner * self) ChipLogError(Ble, "Failed to stop discovery %s", error->message); g_error_free(error); } - ChipDeviceScannerDelegate * delegate = self->mDelegate; - self->mIsScanning = false; - - // callback is explicitly allowed to delete the scanner (hence no more - // references to 'self' here) - delegate->OnScanComplete(); + self->mIsScanning = false; return CHIP_NO_ERROR; } diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.h b/src/platform/Linux/bluez/ChipDeviceScanner.h index ad12ba9b31c8d8..4dbb244c8772da 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.h +++ b/src/platform/Linux/bluez/ChipDeviceScanner.h @@ -71,6 +71,9 @@ class ChipDeviceScanner /// Stop any currently running scan CHIP_ERROR StopScan(); + /// Should only be called by TimerExpiredCallback. + void MarkTimerExpired() { mTimerExpired = true; } + /// Create a new device scanner /// /// Convenience method to allocate any required variables. @@ -101,6 +104,8 @@ class ChipDeviceScanner gulong mInterfaceChangedSignal = 0; bool mIsScanning = false; bool mIsStopping = false; + /// Used to track if timer has alread expired and doesn't need to be canceled. + bool mTimerExpired = false; }; } // namespace Internal