[Python] Delete BLE scanner handle in an explicit way (#32574)
* Normalize BT MAC letter case passed to DiscoverAsync
* Remove obsolete docstrings
* [Python] Delete BLE scanner handle in an explicit way
* Update callback names
* Get rid of obsolete comments
* Unref manager client on glib thread
diff --git a/src/controller/python/chip/ble/LinuxImpl.cpp b/src/controller/python/chip/ble/LinuxImpl.cpp
index 6218d9c..852e1f7 100644
--- a/src/controller/python/chip/ble/LinuxImpl.cpp
+++ b/src/controller/python/chip/ble/LinuxImpl.cpp
@@ -111,11 +111,9 @@
{
mCompleteCallback(mContext);
}
-
- delete this;
}
- virtual void OnScanError(CHIP_ERROR error) override
+ void OnScanError(CHIP_ERROR error) override
{
if (mErrorCallback)
{
@@ -133,10 +131,10 @@
} // namespace
-extern "C" void * pychip_ble_start_scanning(PyObject * context, void * adapter, uint32_t timeoutMs,
- ScannerDelegateImpl::DeviceScannedCallback scanCallback,
- ScannerDelegateImpl::ScanCompleteCallback completeCallback,
- ScannerDelegateImpl::ScanErrorCallback errorCallback)
+extern "C" void * pychip_ble_scanner_start(PyObject * context, void * adapter, uint32_t timeoutMs,
+ ScannerDelegateImpl::DeviceScannedCallback scanCallback,
+ ScannerDelegateImpl::ScanCompleteCallback completeCallback,
+ ScannerDelegateImpl::ScanErrorCallback errorCallback)
{
std::unique_ptr<ScannerDelegateImpl> delegate =
std::make_unique<ScannerDelegateImpl>(context, scanCallback, completeCallback, errorCallback);
@@ -151,3 +149,9 @@
return delegate.release();
}
+
+extern "C" void pychip_ble_scanner_delete(void * scanner)
+{
+ chip::DeviceLayer::StackLock lock;
+ delete static_cast<ScannerDelegateImpl *>(scanner);
+}
diff --git a/src/controller/python/chip/ble/darwin/Scanning.mm b/src/controller/python/chip/ble/darwin/Scanning.mm
index caf18c6..d3fb928 100644
--- a/src/controller/python/chip/ble/darwin/Scanning.mm
+++ b/src/controller/python/chip/ble/darwin/Scanning.mm
@@ -131,7 +131,7 @@
@end
-extern "C" void * pychip_ble_start_scanning(PyObject * context, void * adapter, uint32_t timeout,
+extern "C" void * pychip_ble_scanner_start(PyObject * context, void * adapter, uint32_t timeout,
DeviceScannedCallback scanCallback, ScanCompleteCallback completeCallback, ScanErrorCallback errorCallback)
{
// NOTE: adapter is ignored as it does not apply to mac
@@ -144,3 +144,8 @@
return (__bridge_retained void *) (scanner);
}
+
+extern "C" void pychip_ble_scanner_delete(void * scanner)
+{
+ CFRelease((CFTypeRef) scanner);
+}
diff --git a/src/controller/python/chip/ble/library_handle.py b/src/controller/python/chip/ble/library_handle.py
index 2dbb52f..00f2a77 100644
--- a/src/controller/python/chip/ble/library_handle.py
+++ b/src/controller/python/chip/ble/library_handle.py
@@ -57,8 +57,10 @@
setter.Set('pychip_ble_adapter_list_get_raw_adapter',
VoidPointer, [VoidPointer])
- setter.Set('pychip_ble_start_scanning', VoidPointer, [
- py_object, VoidPointer, c_uint32, DeviceScannedCallback, ScanDoneCallback, ScanErrorCallback
+ setter.Set('pychip_ble_scanner_start', VoidPointer, [
+ py_object, VoidPointer, c_uint32, DeviceScannedCallback,
+ ScanDoneCallback, ScanErrorCallback,
])
+ setter.Set('pychip_ble_scanner_delete', None, [VoidPointer])
return handle
diff --git a/src/controller/python/chip/ble/scan_devices.py b/src/controller/python/chip/ble/scan_devices.py
index 5e9ee24..86cc0a3 100644
--- a/src/controller/python/chip/ble/scan_devices.py
+++ b/src/controller/python/chip/ble/scan_devices.py
@@ -17,6 +17,7 @@
import ctypes
from dataclasses import dataclass
from queue import Queue
+from threading import Thread
from typing import Generator
from chip.ble.library_handle import _GetBleLibraryHandle
@@ -26,36 +27,68 @@
@DeviceScannedCallback
def ScanFoundCallback(closure, address: str, discriminator: int, vendor: int,
product: int):
- closure.DeviceFound(address, discriminator, vendor, product)
+ closure.OnDeviceScanned(address, discriminator, vendor, product)
@ScanDoneCallback
def ScanDoneCallback(closure):
- closure.ScanCompleted()
+ closure.OnScanComplete()
@ScanErrorCallback
def ScanErrorCallback(closure, errorCode: int):
- closure.ScanErrorCallback(errorCode)
+ closure.OnScanError(errorCode)
-def DiscoverAsync(timeoutMs: int, scanCallback, doneCallback, errorCallback, adapter=None):
- """Initiate a BLE discovery of devices with the given timeout.
+@dataclass
+class DeviceInfo:
+ address: str
+ discriminator: int
+ vendor: int
+ product: int
- NOTE: devices are not guaranteed to be unique. New entries are returned
+
+class _DeviceInfoReceiver:
+ """Uses a queue to notify of objects received asynchronously
+ from a BLE scan.
+
+ Internal queue gets filled on DeviceFound and ends with None when
+ ScanCompleted.
+ """
+
+ def __init__(self):
+ self.queue = Queue()
+
+ def OnDeviceScanned(self, address, discriminator, vendor, product):
+ self.queue.put(DeviceInfo(address, discriminator, vendor, product))
+
+ def OnScanComplete(self):
+ self.queue.put(None)
+
+ def OnScanError(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.
+
+ NOTE: Devices are not guaranteed to be unique. New entries are returned
as soon as the underlying BLE manager detects changes.
Args:
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.
"""
- if adapter and not isinstance(adapter, str):
- adapter = adapter.address
+ if adapter:
+ if isinstance(adapter, str):
+ adapter = adapter.upper()
+ else:
+ adapter = adapter.address
handle = _GetBleLibraryHandle()
@@ -69,87 +102,49 @@
nativeList).decode('utf8')):
continue
- class ScannerClosure:
-
- def DeviceFound(self, *args):
- scanCallback(*args)
-
- 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))
-
- scanner = handle.pychip_ble_start_scanning(
- ctypes.py_object(closure),
- handle.pychip_ble_adapter_list_get_raw_adapter(
- nativeList), timeoutMs,
- ScanFoundCallback, ScanDoneCallback, ScanErrorCallback)
+ receiver = _DeviceInfoReceiver()
+ scanner = handle.pychip_ble_scanner_start(
+ ctypes.py_object(receiver),
+ handle.pychip_ble_adapter_list_get_raw_adapter(nativeList),
+ timeoutMs, ScanFoundCallback, ScanDoneCallback, ScanErrorCallback)
if scanner == 0:
- raise Exception('Failed to initiate scan')
+ raise Exception('Failed to start BLE scan')
+
+ while True:
+ data = receiver.queue.get()
+ if not data:
+ break
+ yield data
+
+ handle.pychip_ble_scanner_delete(scanner)
break
finally:
handle.pychip_ble_adapter_list_delete(nativeList)
-@dataclass
-class DeviceInfo:
- address: str
- discriminator: int
- vendor: int
- product: int
+def DiscoverAsync(timeoutMs: int, scanCallback, doneCallback, errorCallback, adapter=None):
+ """Discover BLE devices over the specified period of time without blocking.
-
-class _DeviceInfoReceiver:
- """Uses a queue to notify of objects received asynchronously
- from a ble scan.
-
- Internal queue gets filled on DeviceFound and ends with None when
- ScanCompleted.
- """
-
- def __init__(self):
- self.queue = Queue()
-
- def DeviceFound(self, address, discriminator, vendor, product):
- self.queue.put(DeviceInfo(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.
-
- NOTE: devices are not guaranteed to be unique. New entries are returned
+ NOTE: Devices are not guaranteed to be unique. The scanCallback is called
as soon as the underlying BLE manager detects changes.
Args:
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.
"""
- receiver = _DeviceInfoReceiver()
- DiscoverAsync(timeoutMs, receiver.DeviceFound,
- receiver.ScanCompleted, receiver.ScanError, adapter)
+ def _DiscoverAsync(timeoutMs, scanCallback, doneCallback, errorCallback, adapter):
+ for device in DiscoverSync(timeoutMs, adapter):
+ scanCallback(device.address, device.discriminator, device.vendor, device.product)
+ doneCallback()
- while True:
- data = receiver.queue.get()
- if not data:
- break
- yield data
+ t = Thread(target=_DiscoverAsync,
+ args=(timeoutMs, scanCallback, doneCallback, errorCallback, adapter),
+ daemon=True)
+ t.start()
diff --git a/src/platform/Linux/bluez/AdapterIterator.cpp b/src/platform/Linux/bluez/AdapterIterator.cpp
index ced1724..0455359 100644
--- a/src/platform/Linux/bluez/AdapterIterator.cpp
+++ b/src/platform/Linux/bluez/AdapterIterator.cpp
@@ -47,6 +47,18 @@
return CHIP_NO_ERROR;
}
+CHIP_ERROR AdapterIterator::Shutdown()
+{
+ // Release resources on the glib thread to synchronize with potential signal handlers
+ // attached to the manager client object that may run on the glib thread.
+ return PlatformMgrImpl().GLibMatterContextInvokeSync(
+ +[](AdapterIterator * self) {
+ self->mManager.reset();
+ return CHIP_NO_ERROR;
+ },
+ this);
+}
+
bool AdapterIterator::Advance()
{
for (; mIterator != BluezObjectList::end(); ++mIterator)
diff --git a/src/platform/Linux/bluez/AdapterIterator.h b/src/platform/Linux/bluez/AdapterIterator.h
index 38af64f..c998df6 100644
--- a/src/platform/Linux/bluez/AdapterIterator.h
+++ b/src/platform/Linux/bluez/AdapterIterator.h
@@ -48,6 +48,9 @@
class AdapterIterator
{
public:
+ AdapterIterator() = default;
+ ~AdapterIterator() { Shutdown(); }
+
/// Moves to the next DBUS interface.
///
/// MUST be called before any of the 'current value' methods are
@@ -66,6 +69,8 @@
private:
/// Sets up the DBUS manager and loads the list
CHIP_ERROR Initialize();
+ /// Destroys the DBUS manager
+ CHIP_ERROR Shutdown();
/// Loads the next value in the list.
///
diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp
index 5523bf8..ce52b84 100644
--- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp
+++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp
@@ -112,16 +112,12 @@
VerifyOrReturnError(mTimerState == ScannerTimerState::TIMER_CANCELED, CHIP_ERROR_INCORRECT_STATE);
mCancellable.reset(g_cancellable_new());
- if (PlatformMgrImpl().GLibMatterContextInvokeSync(MainLoopStartScan, this) != CHIP_NO_ERROR)
+ CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(MainLoopStartScan, this);
+ if (err != CHIP_NO_ERROR)
{
- ChipLogError(Ble, "Failed to schedule BLE scan start.");
-
- ChipDeviceScannerDelegate * delegate = this->mDelegate;
- // callback is explicitly allowed to delete the scanner (hence no more
- // references to 'self' here)
- delegate->OnScanComplete();
-
- return CHIP_ERROR_INTERNAL;
+ ChipLogError(Ble, "Failed to initiate BLE scan start: %" CHIP_ERROR_FORMAT, err.Format());
+ mDelegate->OnScanComplete();
+ return err;
}
// Here need to set the Bluetooth scanning status immediately.
@@ -129,14 +125,14 @@
// calling StopScan will be effective.
mScannerState = ChipDeviceScannerState::SCANNER_SCANNING;
- CHIP_ERROR err = chip::DeviceLayer::SystemLayer().StartTimer(timeout, TimerExpiredCallback, static_cast<void *>(this));
-
+ err = chip::DeviceLayer::SystemLayer().StartTimer(timeout, TimerExpiredCallback, static_cast<void *>(this));
if (err != CHIP_NO_ERROR)
{
- ChipLogError(Ble, "Failed to schedule scan timeout.");
+ ChipLogError(Ble, "Failed to schedule scan timeout: %" CHIP_ERROR_FORMAT, err.Format());
StopScan();
return err;
}
+
mTimerState = ScannerTimerState::TIMER_STARTED;
ChipLogDetail(Ble, "ChipDeviceScanner has started scanning!");
@@ -157,9 +153,10 @@
assertChipStackLockedByCurrentThread();
VerifyOrReturnError(mScannerState == ChipDeviceScannerState::SCANNER_SCANNING, CHIP_NO_ERROR);
- if (PlatformMgrImpl().GLibMatterContextInvokeSync(MainLoopStopScan, this) != CHIP_NO_ERROR)
+ CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(MainLoopStopScan, this);
+ if (err != CHIP_NO_ERROR)
{
- ChipLogError(Ble, "Failed to schedule BLE scan stop.");
+ ChipLogError(Ble, "Failed to initiate BLE scan stop: %" CHIP_ERROR_FORMAT, err.Format());
return CHIP_ERROR_INTERNAL;
}
@@ -176,10 +173,7 @@
// Reset timer status
mTimerState = ScannerTimerState::TIMER_CANCELED;
- ChipDeviceScannerDelegate * delegate = this->mDelegate;
- // callback is explicitly allowed to delete the scanner (hence no more
- // references to 'self' here)
- delegate->OnScanComplete();
+ mDelegate->OnScanComplete();
return CHIP_NO_ERROR;
}
