From d6a783e6b71ed05bbcc56e31ec6bb81efd6e7c31 Mon Sep 17 00:00:00 2001 From: Jakub Date: Tue, 28 Feb 2023 18:07:45 +0100 Subject: [PATCH] Fix implementation of OnChipScanComplete and OnScanComplete - second PR (#24873) * Remove on connection error from on scan complete Create a separate error handler for scan errors * Fix typo * Remove redundant variable set * Rewrite OnScanComplete to make it more readable. Create a separate function to handle scan errors. * Add missing override * Style fix * Change log level * Call an error in darwin on StopTimeoutReached * Error as int --- src/controller/python/chip/ble/LinuxImpl.cpp | 22 ++++++++++--- .../python/chip/ble/darwin/Scanning.mm | 11 +++++-- src/platform/Linux/BLEManagerImpl.cpp | 21 ++++++++---- src/platform/Linux/BLEManagerImpl.h | 1 + .../Linux/bluez/ChipDeviceScanner.cpp | 4 ++- src/platform/Linux/bluez/ChipDeviceScanner.h | 3 ++ src/platform/Tizen/BLEManagerImpl.cpp | 24 +++++++++----- src/platform/Tizen/BLEManagerImpl.h | 3 +- src/platform/Tizen/ChipDeviceScanner.cpp | 2 +- src/platform/Tizen/ChipDeviceScanner.h | 5 ++- src/platform/webos/BLEManagerImpl.cpp | 33 ++++++++----------- src/platform/webos/BLEManagerImpl.h | 5 ++- src/platform/webos/ChipDeviceScanner.cpp | 2 +- src/platform/webos/ChipDeviceScanner.h | 6 ++-- 14 files changed, 93 insertions(+), 49 deletions(-) diff --git a/src/controller/python/chip/ble/LinuxImpl.cpp b/src/controller/python/chip/ble/LinuxImpl.cpp index c084f957d44d1c..2aa869ed1a530f 100644 --- a/src/controller/python/chip/ble/LinuxImpl.cpp +++ b/src/controller/python/chip/ble/LinuxImpl.cpp @@ -68,9 +68,12 @@ class ScannerDelegateImpl : public ChipDeviceScannerDelegate using DeviceScannedCallback = void (*)(PyObject * context, const char * address, uint16_t discriminator, uint16_t vendorId, uint16_t productId); using ScanCompleteCallback = void (*)(PyObject * context); + using ScanErrorCallback = void (*)(PyObject * context, CHIP_ERROR::StorageType error); - ScannerDelegateImpl(PyObject * context, DeviceScannedCallback scanCallback, ScanCompleteCallback completeCallback) : - mContext(context), mScanCallback(scanCallback), mCompleteCallback(completeCallback) + ScannerDelegateImpl(PyObject * context, DeviceScannedCallback scanCallback, ScanCompleteCallback completeCallback, + ScanErrorCallback errorCallback) : + mContext(context), + mScanCallback(scanCallback), mCompleteCallback(completeCallback), mErrorCallback(errorCallback) {} void SetScanner(std::unique_ptr scanner) { mScanner = std::move(scanner); } @@ -94,20 +97,31 @@ class ScannerDelegateImpl : public ChipDeviceScannerDelegate delete this; } + virtual void OnScanError(CHIP_ERROR error) override + { + if (mErrorCallback) + { + mErrorCallback(mContext, error.AsInteger()); + } + } + private: std::unique_ptr mScanner; PyObject * const mContext; const DeviceScannedCallback mScanCallback; const ScanCompleteCallback mCompleteCallback; + const ScanErrorCallback mErrorCallback; }; } // namespace extern "C" void * pychip_ble_start_scanning(PyObject * context, void * adapter, uint32_t timeoutMs, ScannerDelegateImpl::DeviceScannedCallback scanCallback, - ScannerDelegateImpl::ScanCompleteCallback completeCallback) + ScannerDelegateImpl::ScanCompleteCallback completeCallback, + ScannerDelegateImpl::ScanErrorCallback errorCallback) { - std::unique_ptr delegate = std::make_unique(context, scanCallback, completeCallback); + std::unique_ptr delegate = + std::make_unique(context, scanCallback, completeCallback, errorCallback); std::unique_ptr scanner = ChipDeviceScanner::Create(static_cast(adapter), delegate.get()); diff --git a/src/controller/python/chip/ble/darwin/Scanning.mm b/src/controller/python/chip/ble/darwin/Scanning.mm index cfe653ca9481ab..c26395281e5529 100644 --- a/src/controller/python/chip/ble/darwin/Scanning.mm +++ b/src/controller/python/chip/ble/darwin/Scanning.mm @@ -11,6 +11,7 @@ using DeviceScannedCallback = void (*)(PyObject * context, const char * address, uint16_t discriminator, uint16_t vendorId, uint16_t productId); using ScanCompleteCallback = void (*)(PyObject * context); +using ScanErrorCallback = void (*)(PyObject * context, uint32_t error); } @interface ChipDeviceBleScanner : NSObject @@ -23,10 +24,12 @@ @interface ChipDeviceBleScanner : NSObject @property (assign, nonatomic) PyObject * context; @property (assign, nonatomic) DeviceScannedCallback scanCallback; @property (assign, nonatomic) ScanCompleteCallback completeCallback; +@property (assign, nonatomic) ScanErrorCallback errorCallback; - (id)initWithContext:(PyObject *)context scanCallback:(DeviceScannedCallback)scanCallback completeCallback:(ScanCompleteCallback)completeCallback + errorCallback:(ScanErrorCallback)errorCallback timeoutMs:(uint32_t)timeout; - (void)stopTimeoutReached; @@ -38,6 +41,7 @@ @implementation ChipDeviceBleScanner - (id)initWithContext:(PyObject *)context scanCallback:(DeviceScannedCallback)scanCallback completeCallback:(ScanCompleteCallback)completeCallback + errorCallback:(ScanErrorCallback)errorCallback timeoutMs:(uint32_t)timeout { self = [super init]; @@ -50,6 +54,7 @@ - (id)initWithContext:(PyObject *)context _context = context; _scanCallback = scanCallback; _completeCallback = completeCallback; + _errorCallback = errorCallback; dispatch_source_set_event_handler(_timer, ^{ [self stopTimeoutReached]; @@ -100,6 +105,7 @@ - (void)stopTimeoutReached ChipLogProgress(Ble, "Scan timeout reached."); _completeCallback(_context); + _errorCallback(_context, CHIP_ERROR_TIMEOUT.AsInteger()); dispatch_source_cancel(_timer); [self.centralManager stopScan]; @@ -125,14 +131,15 @@ - (void)centralManager:(CBCentralManager *)central didConnectPeripheral:(CBPerip @end -extern "C" void * pychip_ble_start_scanning( - PyObject * context, void * adapter, uint32_t timeout, DeviceScannedCallback scanCallback, ScanCompleteCallback completeCallback) +extern "C" void * pychip_ble_start_scanning(PyObject * context, void * adapter, uint32_t timeout, + DeviceScannedCallback scanCallback, ScanCompleteCallback completeCallback, ScanErrorCallback errorCallback) { // NOTE: adapter is ignored as it does not apply to mac ChipDeviceBleScanner * scanner = [[ChipDeviceBleScanner alloc] initWithContext:context scanCallback:scanCallback completeCallback:completeCallback + errorCallback:errorCallback timeoutMs:timeout]; return (__bridge_retained void *) (scanner); diff --git a/src/platform/Linux/BLEManagerImpl.cpp b/src/platform/Linux/BLEManagerImpl.cpp index 2c2a8794c66105..c9ae6ae36bf771 100644 --- a/src/platform/Linux/BLEManagerImpl.cpp +++ b/src/platform/Linux/BLEManagerImpl.cpp @@ -790,7 +790,7 @@ void BLEManagerImpl::OnDeviceScanned(BluezDevice1 * device, const chip::Ble::Chi } else { - // Internal consistency eerror + // Internal consistency error ChipLogError(Ble, "Unknown discovery type. Ignoring scanned device."); return; } @@ -804,15 +804,24 @@ void BLEManagerImpl::OnDeviceScanned(BluezDevice1 * device, const chip::Ble::Chi void BLEManagerImpl::OnScanComplete() { - if (mBLEScanConfig.mBleScanState != BleScanState::kScanForDiscriminator && - mBLEScanConfig.mBleScanState != BleScanState::kScanForAddress) + switch (mBLEScanConfig.mBleScanState) { + case BleScanState::kNotScanning: ChipLogProgress(Ble, "Scan complete notification without an active scan."); - return; + break; + case BleScanState::kScanForAddress: + case BleScanState::kScanForDiscriminator: + mBLEScanConfig.mBleScanState = BleScanState::kNotScanning; + ChipLogProgress(Ble, "Scan complete. No matching device found."); + break; + case BleScanState::kConnecting: + break; } +} - BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, CHIP_ERROR_TIMEOUT); - mBLEScanConfig.mBleScanState = BleScanState::kNotScanning; +void BLEManagerImpl::OnScanError(CHIP_ERROR err) +{ + ChipLogError(Ble, "BLE scan error: %" CHIP_ERROR_FORMAT, err.Format()); } } // namespace Internal diff --git a/src/platform/Linux/BLEManagerImpl.h b/src/platform/Linux/BLEManagerImpl.h index 8c0d06b066cbd8..ef7cb53ede7a50 100644 --- a/src/platform/Linux/BLEManagerImpl.h +++ b/src/platform/Linux/BLEManagerImpl.h @@ -91,6 +91,7 @@ class BLEManagerImpl final : public BLEManager, public: CHIP_ERROR ConfigureBle(uint32_t aAdapterId, bool aIsCentral); + void OnScanError(CHIP_ERROR error) override; // Driven by BlueZ IO static void HandleNewConnection(BLE_CONNECTION_OBJECT conId); diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index 46e17ce5b17089..04c522ba34aa66 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -150,7 +150,9 @@ CHIP_ERROR ChipDeviceScanner::StartScan(System::Clock::Timeout timeout) void ChipDeviceScanner::TimerExpiredCallback(chip::System::Layer * layer, void * appState) { - static_cast(appState)->StopScan(); + ChipDeviceScanner * chipDeviceScanner = static_cast(appState); + chipDeviceScanner->mDelegate->OnScanError(CHIP_ERROR_TIMEOUT); + chipDeviceScanner->StopScan(); } CHIP_ERROR ChipDeviceScanner::StopScan() diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.h b/src/platform/Linux/bluez/ChipDeviceScanner.h index bbe84ac07268ba..f81f923dfb3b47 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.h +++ b/src/platform/Linux/bluez/ChipDeviceScanner.h @@ -44,6 +44,9 @@ class ChipDeviceScannerDelegate // Called when a scan was completed (stopped or timed out) virtual void OnScanComplete() = 0; + + // Call on scan error + virtual void OnScanError(CHIP_ERROR) = 0; }; /// Allows scanning for CHIP devices diff --git a/src/platform/Tizen/BLEManagerImpl.cpp b/src/platform/Tizen/BLEManagerImpl.cpp index c33326f659a9ee..54527546da655a 100644 --- a/src/platform/Tizen/BLEManagerImpl.cpp +++ b/src/platform/Tizen/BLEManagerImpl.cpp @@ -515,18 +515,26 @@ void BLEManagerImpl::OnChipDeviceScanned(void * device, const Ble::ChipBLEDevice ConnectHandler(deviceInfo->remote_address); } -void BLEManagerImpl::OnChipScanComplete() +void BLEManagerImpl::OnScanComplete() { - if (mBLEScanConfig.mBleScanState != BleScanState::kScanForDiscriminator && - mBLEScanConfig.mBleScanState != BleScanState::kScanForAddress) + switch (mBLEScanConfig.mBleScanState) { - ChipLogProgress(DeviceLayer, "Scan complete notification without an active scan."); - return; + case BleScanState::kNotScanning: + ChipLogProgress(Ble, "Scan complete notification without an active scan."); + break; + case BleScanState::kScanForAddress: + case BleScanState::kScanForDiscriminator: + mBLEScanConfig.mBleScanState = BleScanState::kNotScanning; + ChipLogProgress(Ble, "Scan complete. No matching device found."); + break; + case BleScanState::kConnecting: + break; } +} - ChipLogError(DeviceLayer, "Scan Completed with Timeout: Notify Upstream."); - BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, CHIP_ERROR_TIMEOUT); - mBLEScanConfig.mBleScanState = BleScanState::kNotScanning; +void BLEManagerImpl::OnScanError(CHIP_ERROR err) +{ + ChipLogDetail(Ble, "BLE scan error: %" CHIP_ERROR_FORMAT, err.Format()); } int BLEManagerImpl::RegisterGATTServer() diff --git a/src/platform/Tizen/BLEManagerImpl.h b/src/platform/Tizen/BLEManagerImpl.h index 7723d2dcf62221..a685a85205de65 100644 --- a/src/platform/Tizen/BLEManagerImpl.h +++ b/src/platform/Tizen/BLEManagerImpl.h @@ -138,7 +138,8 @@ class BLEManagerImpl final : public BLEManager, // ===== Members that implement virtual methods on ChipDeviceScannerDelegate void OnChipDeviceScanned(void * device, const Ble::ChipBLEDeviceIdentificationInfo & info) override; - void OnChipScanComplete() override; + void OnScanComplete() override; + void OnScanError(CHIP_ERROR err) override; // ===== Members for internal use by the following friends. diff --git a/src/platform/Tizen/ChipDeviceScanner.cpp b/src/platform/Tizen/ChipDeviceScanner.cpp index 204895347cb45c..2fb48203a1ca1e 100644 --- a/src/platform/Tizen/ChipDeviceScanner.cpp +++ b/src/platform/Tizen/ChipDeviceScanner.cpp @@ -242,7 +242,7 @@ CHIP_ERROR ChipDeviceScanner::StopChipScan() UnRegisterScanFilter(); // Report to Impl class - mDelegate->OnChipScanComplete(); + mDelegate->OnScanComplete(); mIsScanning = false; diff --git a/src/platform/Tizen/ChipDeviceScanner.h b/src/platform/Tizen/ChipDeviceScanner.h index f95f4782ece551..ac8049fdd1ecdd 100644 --- a/src/platform/Tizen/ChipDeviceScanner.h +++ b/src/platform/Tizen/ChipDeviceScanner.h @@ -65,7 +65,10 @@ class ChipDeviceScannerDelegate virtual void OnChipDeviceScanned(void * device, const chip::Ble::ChipBLEDeviceIdentificationInfo & info) = 0; // Called when a scan was completed (stopped or timed out) - virtual void OnChipScanComplete(void) = 0; + virtual void OnScanComplete(void) = 0; + + // Called on scan error + virtual void OnScanError(CHIP_ERROR err) = 0; }; /// Allows scanning for CHIP devices diff --git a/src/platform/webos/BLEManagerImpl.cpp b/src/platform/webos/BLEManagerImpl.cpp index 4aef02746ebc5a..3ea67b6ad1c486 100644 --- a/src/platform/webos/BLEManagerImpl.cpp +++ b/src/platform/webos/BLEManagerImpl.cpp @@ -917,31 +917,26 @@ void BLEManagerImpl::NotifyBLEPeripheralAdvStopComplete(bool aIsSuccess, void * PlatformMgr().PostEventOrDie(&event); } -void BLEManagerImpl::OnChipScanComplete() -{ - if (mBLEScanConfig.mBleScanState != BleScanState::kScanForDiscriminator && - mBLEScanConfig.mBleScanState != BleScanState::kScanForAddress) - { - ChipLogProgress(DeviceLayer, "Scan complete notification without an active scan."); - return; - } - - ChipLogError(DeviceLayer, "Scan Completed with Timeout: Notify Upstream."); - BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, CHIP_ERROR_TIMEOUT); - mBLEScanConfig.mBleScanState = BleScanState::kNotScanning; -} - void BLEManagerImpl::OnScanComplete() { - if (mBLEScanConfig.mBleScanState != BleScanState::kScanForDiscriminator && - mBLEScanConfig.mBleScanState != BleScanState::kScanForAddress) + switch (mBLEScanConfig.mBleScanState) { + case BleScanState::kNotScanning: ChipLogProgress(Ble, "Scan complete notification without an active scan."); - return; + break; + case BleScanState::kScanForAddress: + case BleScanState::kScanForDiscriminator: + mBLEScanConfig.mBleScanState = BleScanState::kNotScanning; + ChipLogProgress(Ble, "Scan complete. No matching device found."); + break; + case BleScanState::kConnecting: + break; } +} - BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, CHIP_ERROR_TIMEOUT); - mBLEScanConfig.mBleScanState = BleScanState::kNotScanning; +void BLEManagerImpl::OnScanError(CHIP_ERROR err) +{ + ChipLogDetail(Ble, "BLE scan error: %" CHIP_ERROR_FORMAT, err.Format()); } bool BLEManagerImpl::gattGetServiceCb(LSHandle * sh, LSMessage * message, void * userData) diff --git a/src/platform/webos/BLEManagerImpl.h b/src/platform/webos/BLEManagerImpl.h index a9a8a0ac88312d..2ab9a5844e4fdc 100644 --- a/src/platform/webos/BLEManagerImpl.h +++ b/src/platform/webos/BLEManagerImpl.h @@ -135,10 +135,9 @@ class BLEManagerImpl final : public BLEManager, CHIP_ERROR CancelConnection() override; // ===== Members that implement virtual methods on ChipDeviceScannerDelegate - void OnScanComplete() override; void OnChipDeviceScanned(char * address) override; - void OnChipScanComplete() override; - + void OnScanComplete() override; + void OnScanError(CHIP_ERROR err) override; // ===== Members for internal use by the following friends. friend BLEManager & BLEMgr(); diff --git a/src/platform/webos/ChipDeviceScanner.cpp b/src/platform/webos/ChipDeviceScanner.cpp index 7e3a4b71b9f0a9..c79112b820bec3 100644 --- a/src/platform/webos/ChipDeviceScanner.cpp +++ b/src/platform/webos/ChipDeviceScanner.cpp @@ -336,7 +336,7 @@ CHIP_ERROR ChipDeviceScanner::StopChipScan() ChipLogProgress(DeviceLayer, "CHIP Scanner Async Thread Quit Done..Wait for Thread Windup...!"); // Report to Impl class - mDelegate->OnChipScanComplete(); + mDelegate->OnScanComplete(); mIsScanning = false; diff --git a/src/platform/webos/ChipDeviceScanner.h b/src/platform/webos/ChipDeviceScanner.h index d3481204c642a5..084d45d9ac0201 100644 --- a/src/platform/webos/ChipDeviceScanner.h +++ b/src/platform/webos/ChipDeviceScanner.h @@ -43,8 +43,10 @@ class ChipDeviceScannerDelegate virtual void OnChipDeviceScanned(char * address) = 0; // Called when a scan was completed (stopped or timed out) - virtual void OnScanComplete() = 0; - virtual void OnChipScanComplete() = 0; + virtual void OnScanComplete() = 0; + + // Called on scan error + virtual void OnScanError(CHIP_ERROR err) = 0; }; /// Allows scanning for CHIP devices