Fix implementation of OnChipScanComplete and OnScanComplete - second …

…PR (project-chip#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
lecndav · Mar 22, 2023 · 03b8129 · 03b8129
1 parent 1ca1eb0
commit 03b8129
Show file tree

Hide file tree

Showing 14 changed files with 93 additions and 49 deletions.
diff --git 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<ChipDeviceScanner> 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<ChipDeviceScanner> 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<ScannerDelegateImpl> delegate = std::make_unique<ScannerDelegateImpl>(context, scanCallback, completeCallback);
+    std::unique_ptr<ScannerDelegateImpl> delegate =
+        std::make_unique<ScannerDelegateImpl>(context, scanCallback, completeCallback, errorCallback);
 
     std::unique_ptr<ChipDeviceScanner> scanner = ChipDeviceScanner::Create(static_cast<BluezAdapter1 *>(adapter), delegate.get());
 

diff --git 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 <CBCentralManagerDelegate>
@@ -23,10 +24,12 @@ @interface ChipDeviceBleScanner : NSObject <CBCentralManagerDelegate>
 @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
@@ -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
@@ -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
@@ -150,7 +150,9 @@ CHIP_ERROR ChipDeviceScanner::StartScan(System::Clock::Timeout timeout)
 
 void ChipDeviceScanner::TimerExpiredCallback(chip::System::Layer * layer, void * appState)
 {
-    static_cast<ChipDeviceScanner *>(appState)->StopScan();
+    ChipDeviceScanner * chipDeviceScanner = static_cast<ChipDeviceScanner *>(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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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