[Linux] Handle BLE scan timeout in BLEManager instead of scanner class (project-chip#32770)

* [Linux] Handle BLE scan timeout in BLEManager instead of scanner class

* Simplify scanning check

* Simplify error handling in InitiateScan()

* Update Python binding code

* Fix compilation without logging

Author: arkq

src/controller/python/chip/ble/LinuxImpl.cpp

@@ -19,13 +19,18 @@
 #include <cstdint>
 #include <memory>
 
+#include <ble/CHIPBleServiceData.h>
 #include <lib/core/CHIPError.h>
-#include <lib/support/CHIPMem.h>
+#include <lib/support/CodeUtils.h>
 #include <platform/CHIPDeviceLayer.h>
+#include <platform/Linux/BlePlatformConfig.h>
 #include <platform/Linux/bluez/AdapterIterator.h>
 #include <platform/Linux/bluez/BluezObjectManager.h>
+#include <platform/Linux/bluez/ChipDeviceScanner.h>
 #include <platform/Linux/dbus/bluez/DbusBluez.h>
-#include <platform/internal/BLEManager.h>
+#include <platform/PlatformManager.h>
+#include <system/SystemClock.h>
+#include <system/SystemLayer.h>
 
 using namespace chip::DeviceLayer::Internal;
 
@@ -107,9 +112,29 @@ class ScannerDelegateImpl : public ChipDeviceScannerDelegate
 
     void ScannerShutdown() { mBluezObjectManager.Shutdown(); }
 
-    CHIP_ERROR ScannerStartScan(chip::System::Clock::Timeout timeout) { return mScanner.StartScan(timeout); }
+    CHIP_ERROR ScannerStartScan(chip::System::Clock::Timeout timeout)
+    {
+        CHIP_ERROR err = mScanner.StartScan();
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err);
+
+        err = chip::DeviceLayer::SystemLayer().StartTimer(timeout, HandleScannerTimer, this);
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err, mScanner.StopScan());
 
-    CHIP_ERROR ScannerStopScan() { return mScanner.StopScan(); }
+        return CHIP_NO_ERROR;
+    }
+
+    CHIP_ERROR ScannerStopScan()
+    {
+        chip::DeviceLayer::SystemLayer().CancelTimer(HandleScannerTimer, this);
+        return mScanner.StopScan();
+    }
+
+    static void HandleScannerTimer(chip::System::Layer *, void * appState)
+    {
+        auto * delegate = static_cast<ScannerDelegateImpl *>(appState);
+        delegate->OnScanError(CHIP_ERROR_TIMEOUT);
+        delegate->mScanner.StopScan();
+    }
 
     void OnDeviceScanned(BluezDevice1 & device, const chip::Ble::ChipBLEDeviceIdentificationInfo & info) override
     {

src/platform/Linux/BLEManagerImpl.cpp

@@ -746,42 +746,50 @@ void BLEManagerImpl::HandleAdvertisingTimer(chip::System::Layer *, void * appSta
 
 void BLEManagerImpl::InitiateScan(BleScanState scanType)
 {
+    CHIP_ERROR err = CHIP_ERROR_INCORRECT_STATE;
+
     DriveBLEState();
 
-    if (scanType == BleScanState::kNotScanning)
-    {
-        ChipLogError(Ble, "Invalid scan type requested");
-        BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, CHIP_ERROR_INCORRECT_STATE);
-        return;
-    }
-
-    if (!mFlags.Has(Flags::kBluezAdapterAvailable))
-    {
-        BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, BLE_ERROR_ADAPTER_UNAVAILABLE);
-        return;
-    }
+    VerifyOrExit(scanType != BleScanState::kNotScanning,
+                 ChipLogError(Ble, "Invalid scan type requested: %d", to_underlying(scanType)));
+    VerifyOrExit(!mDeviceScanner.IsScanning(), ChipLogError(Ble, "BLE scan already in progress"));
+    VerifyOrExit(mFlags.Has(Flags::kBluezAdapterAvailable), err = BLE_ERROR_ADAPTER_UNAVAILABLE);
 
     mBLEScanConfig.mBleScanState = scanType;
 
-    CHIP_ERROR err = mDeviceScanner.Init(mAdapter.get(), this);
-    if (err != CHIP_NO_ERROR)
-    {
+    err = mDeviceScanner.Init(mAdapter.get(), this);
+    VerifyOrExit(err == CHIP_NO_ERROR, {
         mBLEScanConfig.mBleScanState = BleScanState::kNotScanning;
-        ChipLogError(Ble, "Failed to create a BLE device scanner: %" CHIP_ERROR_FORMAT, err.Format());
-        BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, CHIP_ERROR_INTERNAL);
-        return;
-    }
+        ChipLogError(Ble, "Failed to create BLE device scanner: %" CHIP_ERROR_FORMAT, err.Format());
+    });
 
-    err = mDeviceScanner.StartScan(kNewConnectionScanTimeout);
+    err = mDeviceScanner.StartScan();
+    VerifyOrExit(err == CHIP_NO_ERROR, {
+        mBLEScanConfig.mBleScanState = BleScanState::kNotScanning;
+        ChipLogError(Ble, "Failed to start BLE scan: %" CHIP_ERROR_FORMAT, err.Format());
+    });
+
+    err = DeviceLayer::SystemLayer().StartTimer(kNewConnectionScanTimeout, HandleScannerTimer, this);
+    VerifyOrExit(err == CHIP_NO_ERROR, {
+        mBLEScanConfig.mBleScanState = BleScanState::kNotScanning;
+        mDeviceScanner.StopScan();
+        ChipLogError(Ble, "Failed to start BLE scan timeout: %" CHIP_ERROR_FORMAT, err.Format());
+    });
+
+exit:
     if (err != CHIP_NO_ERROR)
     {
-        mBLEScanConfig.mBleScanState = BleScanState::kNotScanning;
-        ChipLogError(Ble, "Failed to start a BLE can: %" CHIP_ERROR_FORMAT, err.Format());
         BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, err);
-        return;
     }
 }
 
+void BLEManagerImpl::HandleScannerTimer(chip::System::Layer *, void * appState)
+{
+    auto * manager = static_cast<BLEManagerImpl *>(appState);
+    manager->OnScanError(CHIP_ERROR_TIMEOUT);
+    manager->mDeviceScanner.StopScan();
+}
+
 void BLEManagerImpl::CleanScanConfig()
 {
     if (mBLEScanConfig.mBleScanState == BleScanState::kConnecting)
@@ -805,7 +813,10 @@ CHIP_ERROR BLEManagerImpl::CancelConnection()
         mEndpoint.CancelConnect();
     // If in discovery mode, stop scan.
     else if (mBLEScanConfig.mBleScanState != BleScanState::kNotScanning)
+    {
+        DeviceLayer::SystemLayer().CancelTimer(HandleScannerTimer, this);
         mDeviceScanner.StopScan();
+    }
     return CHIP_NO_ERROR;
 }
 
@@ -894,6 +905,7 @@ void BLEManagerImpl::OnDeviceScanned(BluezDevice1 & device, const chip::Ble::Chi
     // We StartScan in the ChipStack thread.
     // StopScan should also be performed in the ChipStack thread.
     // At the same time, the scan timer also needs to be canceled in the ChipStack thread.
+    DeviceLayer::SystemLayer().CancelTimer(HandleScannerTimer, this);
     mDeviceScanner.StopScan();
     // Stop scanning and then start connecting timer
     DeviceLayer::SystemLayer().StartTimer(kConnectTimeout, HandleConnectTimeout, &mEndpoint);

src/platform/Linux/BLEManagerImpl.h

@@ -187,6 +187,7 @@ class BLEManagerImpl final : public BLEManager,
     BluezAdvertisement::AdvertisingIntervals GetAdvertisingIntervals() const;
     static void HandleAdvertisingTimer(chip::System::Layer *, void * appState);
     void InitiateScan(BleScanState scanType);
+    static void HandleScannerTimer(chip::System::Layer *, void * appState);
     void CleanScanConfig();
 
     CHIPoBLEServiceMode mServiceMode;

src/platform/Linux/bluez/ChipDeviceScanner.cpp

@@ -89,11 +89,10 @@ void ChipDeviceScanner::Shutdown()
     mScannerState = ChipDeviceScannerState::SCANNER_UNINITIALIZED;
 }
 
-CHIP_ERROR ChipDeviceScanner::StartScan(System::Clock::Timeout timeout)
+CHIP_ERROR ChipDeviceScanner::StartScan()
 {
     assertChipStackLockedByCurrentThread();
     VerifyOrReturnError(mScannerState != ChipDeviceScannerState::SCANNER_SCANNING, CHIP_ERROR_INCORRECT_STATE);
-    VerifyOrReturnError(mTimerState == ScannerTimerState::TIMER_CANCELED, CHIP_ERROR_INCORRECT_STATE);
 
     mCancellable.reset(g_cancellable_new());
     CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(
@@ -105,34 +104,12 @@ CHIP_ERROR ChipDeviceScanner::StartScan(System::Clock::Timeout timeout)
         return err;
     }
 
-    // Here need to set the Bluetooth scanning status immediately.
-    // So that if the timer fails to start in the next step,
-    // calling StopScan will be effective.
     mScannerState = ChipDeviceScannerState::SCANNER_SCANNING;
-
-    err = chip::DeviceLayer::SystemLayer().StartTimer(timeout, TimerExpiredCallback, static_cast<void *>(this));
-    if (err != CHIP_NO_ERROR)
-    {
-        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!");
 
     return CHIP_NO_ERROR;
 }
 
-void ChipDeviceScanner::TimerExpiredCallback(chip::System::Layer * layer, void * appState)
-{
-    ChipDeviceScanner * chipDeviceScanner = static_cast<ChipDeviceScanner *>(appState);
-    chipDeviceScanner->mTimerState        = ScannerTimerState::TIMER_EXPIRED;
-    chipDeviceScanner->mDelegate->OnScanError(CHIP_ERROR_TIMEOUT);
-    chipDeviceScanner->StopScan();
-}
-
 CHIP_ERROR ChipDeviceScanner::StopScan()
 {
     assertChipStackLockedByCurrentThread();
@@ -151,14 +128,6 @@ CHIP_ERROR ChipDeviceScanner::StopScan()
 
     ChipLogDetail(Ble, "ChipDeviceScanner has stopped scanning!");
 
-    if (mTimerState == ScannerTimerState::TIMER_STARTED)
-    {
-        chip::DeviceLayer::SystemLayer().CancelTimer(TimerExpiredCallback, this);
-    }
-
-    // Reset timer status
-    mTimerState = ScannerTimerState::TIMER_CANCELED;
-
     mDelegate->OnScanComplete();
 
     return CHIP_NO_ERROR;

src/platform/Linux/bluez/ChipDeviceScanner.h

@@ -73,11 +73,14 @@ class ChipDeviceScanner : public BluezObjectManagerAdapterNotificationsDelegate
     ///
     /// This method must be called while in the Matter context (from the Matter event
     /// loop, or while holding the Matter stack lock).
-    CHIP_ERROR StartScan(System::Clock::Timeout timeout);
+    CHIP_ERROR StartScan();
 
     /// Stop any currently running scan
     CHIP_ERROR StopScan();
 
+    /// Check if the scanner is active
+    bool IsScanning() const { return mScannerState == ChipDeviceScannerState::SCANNER_SCANNING; }
+
     /// Members that implement virtual methods on BluezObjectManagerAdapterNotificationsDelegate
     void OnDeviceAdded(BluezDevice1 & device) override;
     void OnDevicePropertyChanged(BluezDevice1 & device, GVariant * changedProps, const char * const * invalidatedProps) override;
@@ -91,16 +94,8 @@ class ChipDeviceScanner : public BluezObjectManagerAdapterNotificationsDelegate
         SCANNER_SCANNING
     };
 
-    enum ScannerTimerState
-    {
-        TIMER_CANCELED,
-        TIMER_STARTED,
-        TIMER_EXPIRED
-    };
-
     CHIP_ERROR StartScanImpl();
     CHIP_ERROR StopScanImpl();
-    static void TimerExpiredCallback(chip::System::Layer * layer, void * appState);
 
     /// Check if a given device is a CHIP device and if yes, report it as discovered
     void ReportDevice(BluezDevice1 & device);
@@ -114,8 +109,6 @@ class ChipDeviceScanner : public BluezObjectManagerAdapterNotificationsDelegate
 
     ChipDeviceScannerDelegate * mDelegate = nullptr;
     ChipDeviceScannerState mScannerState  = ChipDeviceScannerState::SCANNER_UNINITIALIZED;
-    /// Used to track if timer has already expired and doesn't need to be canceled.
-    ScannerTimerState mTimerState = ScannerTimerState::TIMER_CANCELED;
     GAutoPtr<GCancellable> mCancellable;
 };