[Linux] Extract BlueZ client manager to separate file (project-chip#32647)

* Cleanup BLE event processing function

* Disallow copying BluezObjectList class

* Log before BleConnectionDelegate::OnConnectionError call

* Extract BlueZ client manager to separate file

* Update Python binding implementation

* Handle scan start error codes properly

* Helper function to convert D-Bus error to CHIP error

Author: arkq

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

@@ -16,9 +16,15 @@
  *    limitations under the License.
  */
 
+#include <cstdint>
+#include <memory>
+
+#include <lib/core/CHIPError.h>
 #include <lib/support/CHIPMem.h>
 #include <platform/CHIPDeviceLayer.h>
 #include <platform/Linux/bluez/AdapterIterator.h>
+#include <platform/Linux/bluez/BluezObjectManager.h>
+#include <platform/Linux/dbus/bluez/DbusBluez.h>
 #include <platform/internal/BLEManager.h>
 
 using namespace chip::DeviceLayer::Internal;
@@ -93,9 +99,18 @@ class ScannerDelegateImpl : public ChipDeviceScannerDelegate
         mScanCallback(scanCallback), mCompleteCallback(completeCallback), mErrorCallback(errorCallback)
     {}
 
-    CHIP_ERROR ScannerInit(BluezAdapter1 * adapter) { return mScanner.Init(adapter, this); }
+    CHIP_ERROR ScannerInit(BluezAdapter1 * adapter)
+    {
+        ReturnErrorOnFailure(mBluezObjectManager.Init());
+        return mScanner.Init(adapter, this);
+    }
+
+    void ScannerShutdown() { mBluezObjectManager.Shutdown(); }
+
     CHIP_ERROR ScannerStartScan(chip::System::Clock::Timeout timeout) { return mScanner.StartScan(timeout); }
 
+    CHIP_ERROR ScannerStopScan() { return mScanner.StopScan(); }
+
     void OnDeviceScanned(BluezDevice1 & device, const chip::Ble::ChipBLEDeviceIdentificationInfo & info) override
     {
         if (mScanCallback)
@@ -122,7 +137,8 @@ class ScannerDelegateImpl : public ChipDeviceScannerDelegate
     }
 
 private:
-    ChipDeviceScanner mScanner;
+    BluezObjectManager mBluezObjectManager;
+    ChipDeviceScanner mScanner{ mBluezObjectManager };
     PyObject * const mContext;
     const DeviceScannedCallback mScanCallback;
     const ScanCompleteCallback mCompleteCallback;
@@ -152,6 +168,10 @@ extern "C" void * pychip_ble_scanner_start(PyObject * context, void * adapter, u
 
 extern "C" void pychip_ble_scanner_delete(void * scanner)
 {
+    auto * delegate = static_cast<ScannerDelegateImpl *>(scanner);
     chip::DeviceLayer::StackLock lock;
-    delete static_cast<ScannerDelegateImpl *>(scanner);
+    // Make sure that the scanner is stopped before deleting the delegate.
+    delegate->ScannerStopScan();
+    delegate->ScannerShutdown();
+    delete delegate;
 }

src/platform/Linux/BLEManagerImpl.cpp

@@ -35,6 +35,7 @@
 #include <type_traits>
 #include <utility>
 
+#include <ble/BleError.h>
 #include <ble/CHIPBleServiceData.h>
 #include <lib/support/CodeUtils.h>
 #include <lib/support/SafeInt.h>
@@ -123,6 +124,7 @@ void BLEManagerImpl::_Shutdown()
     DeviceLayer::SystemLayer().CancelTimer(HandleConnectTimeout, &mEndpoint);
     // Release BLE connection resources (unregister from BlueZ).
     mEndpoint.Shutdown();
+    mBluezObjectManager.Shutdown();
     mFlags.Clear(Flags::kBluezBLELayerInitialized);
 }
 
@@ -251,8 +253,7 @@ void BLEManagerImpl::_OnPlatformEvent(const ChipDeviceEvent * event)
 
 void BLEManagerImpl::HandlePlatformSpecificBLEEvent(const ChipDeviceEvent * apEvent)
 {
-    CHIP_ERROR err         = CHIP_NO_ERROR;
-    bool controlOpComplete = false;
+    CHIP_ERROR err = CHIP_NO_ERROR;
     ChipLogDetail(DeviceLayer, "HandlePlatformSpecificBLEEvent %d", apEvent->Type);
     switch (apEvent->Type)
     {
@@ -288,37 +289,37 @@ void BLEManagerImpl::HandlePlatformSpecificBLEEvent(const ChipDeviceEvent * apEv
         break;
     case DeviceEventType::kPlatformLinuxBLEPeripheralAdvStartComplete:
         SuccessOrExit(err = apEvent->Platform.BLEPeripheralAdvStartComplete.mError);
-        sInstance.mFlags.Clear(Flags::kControlOpInProgress).Clear(Flags::kAdvertisingRefreshNeeded);
+        mFlags.Clear(Flags::kControlOpInProgress).Clear(Flags::kAdvertisingRefreshNeeded);
         // Do not restart the timer if it is still active. This is to avoid the timer from being restarted
         // if the advertising is stopped due to a premature release.
         if (!DeviceLayer::SystemLayer().IsTimerActive(HandleAdvertisingTimer, this))
         {
             // Start a timer to make sure that the fast advertising is stopped after specified timeout.
             SuccessOrExit(err = DeviceLayer::SystemLayer().StartTimer(kFastAdvertiseTimeout, HandleAdvertisingTimer, this));
         }
-        sInstance.mFlags.Set(Flags::kAdvertising);
+        mFlags.Set(Flags::kAdvertising);
         break;
     case DeviceEventType::kPlatformLinuxBLEPeripheralAdvStopComplete:
         SuccessOrExit(err = apEvent->Platform.BLEPeripheralAdvStopComplete.mError);
-        sInstance.mFlags.Clear(Flags::kControlOpInProgress).Clear(Flags::kAdvertisingRefreshNeeded);
+        mFlags.Clear(Flags::kControlOpInProgress).Clear(Flags::kAdvertisingRefreshNeeded);
         DeviceLayer::SystemLayer().CancelTimer(HandleAdvertisingTimer, this);
-
         // Transition to the not Advertising state...
-        if (sInstance.mFlags.Has(Flags::kAdvertising))
+        if (mFlags.Has(Flags::kAdvertising))
         {
-            sInstance.mFlags.Clear(Flags::kAdvertising);
+            mFlags.Clear(Flags::kAdvertising);
             ChipLogProgress(DeviceLayer, "CHIPoBLE advertising stopped");
         }
         break;
     case DeviceEventType::kPlatformLinuxBLEPeripheralAdvReleased:
         // If the advertising was stopped due to a premature release, check if it needs to be restarted.
-        sInstance.mFlags.Clear(Flags::kAdvertising);
+        mFlags.Clear(Flags::kAdvertising);
         DriveBLEState();
         break;
     case DeviceEventType::kPlatformLinuxBLEPeripheralRegisterAppComplete:
         SuccessOrExit(err = apEvent->Platform.BLEPeripheralRegisterAppComplete.mError);
+        mFlags.Clear(Flags::kControlOpInProgress);
         mFlags.Set(Flags::kAppRegistered);
-        controlOpComplete = true;
+        DriveBLEState();
         break;
     default:
         break;
@@ -330,13 +331,7 @@ void BLEManagerImpl::HandlePlatformSpecificBLEEvent(const ChipDeviceEvent * apEv
         ChipLogError(DeviceLayer, "Disabling CHIPoBLE service due to error: %s", ErrorStr(err));
         mServiceMode = ConnectivityManager::kCHIPoBLEServiceMode_Disabled;
         DeviceLayer::SystemLayer().CancelTimer(HandleAdvertisingTimer, this);
-        sInstance.mFlags.Clear(Flags::kControlOpInProgress);
-    }
-
-    if (controlOpComplete)
-    {
         mFlags.Clear(Flags::kControlOpInProgress);
-        DriveBLEState();
     }
 }
 
@@ -587,11 +582,25 @@ void BLEManagerImpl::DriveBLEState()
     // If there's already a control operation in progress, wait until it completes.
     VerifyOrExit(!mFlags.Has(Flags::kControlOpInProgress), /* */);
 
-    // Initializes the Bluez BLE layer if needed.
-    if (mServiceMode == ConnectivityManager::kCHIPoBLEServiceMode_Enabled && !mFlags.Has(Flags::kBluezBLELayerInitialized))
+    // Initializes the BlueZ BLE layer if needed.
+    if (mServiceMode == ConnectivityManager::kCHIPoBLEServiceMode_Enabled)
     {
-        SuccessOrExit(err = mEndpoint.Init(mIsCentral, mAdapterId));
-        mFlags.Set(Flags::kBluezBLELayerInitialized);
+        if (!mFlags.Has(Flags::kBluezManagerInitialized))
+        {
+            SuccessOrExit(err = mBluezObjectManager.Init());
+            mFlags.Set(Flags::kBluezManagerInitialized);
+        }
+        if (!mFlags.Has(Flags::kBluezAdapterAvailable))
+        {
+            mAdapter.reset(mBluezObjectManager.GetAdapter(mAdapterId));
+            VerifyOrExit(mAdapter, err = BLE_ERROR_ADAPTER_UNAVAILABLE);
+            mFlags.Set(Flags::kBluezAdapterAvailable);
+        }
+        if (!mFlags.Has(Flags::kBluezBLELayerInitialized))
+        {
+            SuccessOrExit(err = mEndpoint.Init(mAdapter.get(), mIsCentral));
+            mFlags.Set(Flags::kBluezBLELayerInitialized);
+        }
     }
 
     // Register the CHIPoBLE application with the Bluez BLE layer if needed.
@@ -614,7 +623,7 @@ void BLEManagerImpl::DriveBLEState()
             // Configure advertising data if it hasn't been done yet.
             if (!mFlags.Has(Flags::kAdvertisingConfigured))
             {
-                SuccessOrExit(err = mBLEAdvertisement.Init(mEndpoint, mpBLEAdvUUID, mDeviceName));
+                SuccessOrExit(err = mBLEAdvertisement.Init(mAdapter.get(), mpBLEAdvUUID, mDeviceName));
                 mFlags.Set(Flags::kAdvertisingConfigured);
             }
 
@@ -724,41 +733,33 @@ void BLEManagerImpl::InitiateScan(BleScanState scanType)
 
     if (scanType == BleScanState::kNotScanning)
     {
-        BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, CHIP_ERROR_INCORRECT_STATE);
         ChipLogError(Ble, "Invalid scan type requested");
-        return;
-    }
-
-    if (!mFlags.Has(Flags::kBluezBLELayerInitialized))
-    {
         BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, CHIP_ERROR_INCORRECT_STATE);
-        ChipLogError(Ble, "BLE Layer is not yet initialized");
         return;
     }
 
-    if (mEndpoint.GetAdapter() == nullptr)
+    if (!mFlags.Has(Flags::kBluezAdapterAvailable))
     {
-        BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, CHIP_ERROR_INCORRECT_STATE);
-        ChipLogError(Ble, "No adapter available for new connection establishment");
+        BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, BLE_ERROR_ADAPTER_UNAVAILABLE);
         return;
     }
 
     mBLEScanConfig.mBleScanState = scanType;
 
-    CHIP_ERROR err = mDeviceScanner.Init(mEndpoint.GetAdapter(), this);
+    CHIP_ERROR err = mDeviceScanner.Init(mAdapter.get(), this);
     if (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);
-        ChipLogError(Ble, "Failed to create a BLE device scanner");
         return;
     }
 
     err = mDeviceScanner.StartScan(kNewConnectionScanTimeout);
     if (err != CHIP_NO_ERROR)
     {
         mBLEScanConfig.mBleScanState = BleScanState::kNotScanning;
-        ChipLogError(Ble, "Failed to start a BLE can: %s", chip::ErrorStr(err));
+        ChipLogError(Ble, "Failed to start a BLE can: %" CHIP_ERROR_FORMAT, err.Format());
         BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, err);
         return;
     }

src/platform/Linux/BLEManagerImpl.h

@@ -31,6 +31,7 @@
 
 #include "bluez/BluezAdvertisement.h"
 #include "bluez/BluezEndpoint.h"
+#include "bluez/BluezObjectManager.h"
 #include "bluez/ChipDeviceScanner.h"
 #include "bluez/Types.h"
 
@@ -159,16 +160,18 @@ class BLEManagerImpl final : public BLEManager,
     enum class Flags : uint16_t
     {
         kAsyncInitCompleted       = 0x0001, /**< One-time asynchronous initialization actions have been performed. */
-        kBluezBLELayerInitialized = 0x0002, /**< The Bluez layer has been initialized. */
-        kAppRegistered            = 0x0004, /**< The CHIPoBLE application has been registered with the Bluez layer. */
-        kAdvertisingConfigured    = 0x0008, /**< CHIPoBLE advertising has been configured in the Bluez layer. */
-        kAdvertising              = 0x0010, /**< The system is currently CHIPoBLE advertising. */
-        kControlOpInProgress      = 0x0020, /**< An async control operation has been issued to the ESP BLE layer. */
-        kAdvertisingEnabled       = 0x0040, /**< The application has enabled CHIPoBLE advertising. */
-        kFastAdvertisingEnabled   = 0x0080, /**< The application has enabled fast advertising. */
-        kUseCustomDeviceName      = 0x0100, /**< The application has configured a custom BLE device name. */
-        kAdvertisingRefreshNeeded = 0x0200, /**< The advertising configuration/state in BLE layer needs to be updated. */
-        kExtAdvertisingEnabled    = 0x0400, /**< The application has enabled CHIPoBLE extended advertising. */
+        kBluezManagerInitialized  = 0x0002, /**< The BlueZ object manager has been initialized. */
+        kBluezAdapterAvailable    = 0x0004, /**< Selected BlueZ adapter is available for use. */
+        kBluezBLELayerInitialized = 0x0008, /**< The BlueZ BLE layer has been initialized. */
+        kAppRegistered            = 0x0010, /**< The CHIPoBLE application has been registered with the Bluez layer. */
+        kAdvertisingConfigured    = 0x0020, /**< CHIPoBLE advertising has been configured in the Bluez layer. */
+        kAdvertising              = 0x0040, /**< The system is currently CHIPoBLE advertising. */
+        kControlOpInProgress      = 0x0080, /**< An async control operation has been issued to the ESP BLE layer. */
+        kAdvertisingEnabled       = 0x0100, /**< The application has enabled CHIPoBLE advertising. */
+        kFastAdvertisingEnabled   = 0x0200, /**< The application has enabled fast advertising. */
+        kUseCustomDeviceName      = 0x0400, /**< The application has configured a custom BLE device name. */
+        kAdvertisingRefreshNeeded = 0x0800, /**< The advertising configuration/state in BLE layer needs to be updated. */
+        kExtAdvertisingEnabled    = 0x1000, /**< The application has enabled CHIPoBLE extended advertising. */
     };
 
     enum
@@ -187,15 +190,18 @@ class BLEManagerImpl final : public BLEManager,
     CHIPoBLEServiceMode mServiceMode;
     BitFlags<Flags> mFlags;
 
+    BluezObjectManager mBluezObjectManager;
+    GAutoPtr<BluezAdapter1> mAdapter;
     uint32_t mAdapterId = 0;
+
     char mDeviceName[kMaxDeviceNameLength + 1];
     bool mIsCentral = false;
-    BluezEndpoint mEndpoint;
+    BluezEndpoint mEndpoint{ mBluezObjectManager };
 
-    BluezAdvertisement mBLEAdvertisement;
+    BluezAdvertisement mBLEAdvertisement{ mEndpoint };
     const char * mpBLEAdvUUID = nullptr;
 
-    ChipDeviceScanner mDeviceScanner;
+    ChipDeviceScanner mDeviceScanner{ mBluezObjectManager };
     BLEScanConfig mBLEScanConfig;
 };
 

src/platform/Linux/BUILD.gn

@@ -103,6 +103,8 @@ static_library("Linux") {
       "bluez/BluezEndpoint.h",
       "bluez/BluezObjectIterator.h",
       "bluez/BluezObjectList.h",
+      "bluez/BluezObjectManager.cpp",
+      "bluez/BluezObjectManager.h",
       "bluez/ChipDeviceScanner.cpp",
       "bluez/ChipDeviceScanner.h",
       "bluez/Types.h",

src/platform/Linux/bluez/AdapterIterator.cpp

@@ -17,48 +17,25 @@
 
 #include "AdapterIterator.h"
 
+#include <cstdio>
+
+#include <gio/gio.h>
+
+#include <lib/core/CHIPError.h>
 #include <lib/support/CodeUtils.h>
 #include <lib/support/logging/CHIPLogging.h>
 #include <platform/GLibTypeDeleter.h>
-#include <platform/PlatformManager.h>
+#include <platform/Linux/dbus/bluez/DbusBluez.h>
+
+#include "BluezObjectIterator.h"
+#include "BluezObjectList.h"
+#include "BluezObjectManager.h"
+#include "Types.h"
 
 namespace chip {
 namespace DeviceLayer {
 namespace Internal {
 
-CHIP_ERROR AdapterIterator::Initialize()
-{
-    // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise,
-    // all D-Bus signals will be delivered to the GLib global default main context.
-    VerifyOrDie(g_main_context_get_thread_default() != nullptr);
-
-    GAutoPtr<GError> error;
-    mManager.reset(g_dbus_object_manager_client_new_for_bus_sync(
-        G_BUS_TYPE_SYSTEM, G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_NONE, BLUEZ_INTERFACE, "/",
-        bluez_object_manager_client_get_proxy_type, nullptr /* unused user data in the Proxy Type Func */,
-        nullptr /* destroy notify */, nullptr /* cancellable */, &error.GetReceiver()));
-
-    VerifyOrReturnError(mManager, CHIP_ERROR_INTERNAL,
-                        ChipLogError(DeviceLayer, "Failed to get D-Bus object manager for listing adapters: %s", error->message));
-
-    mObjectList.Init(mManager.get());
-    mIterator = mObjectList.begin();
-
-    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)
@@ -93,11 +70,15 @@ uint32_t AdapterIterator::GetIndex() const
 
 bool AdapterIterator::Next()
 {
-    if (!mManager)
+    if (!mIsInitialized)
     {
-        CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(
-            +[](AdapterIterator * self) { return self->Initialize(); }, this);
-        VerifyOrReturnError(err == CHIP_NO_ERROR, false, ChipLogError(DeviceLayer, "Failed to initialize adapter iterator"));
+        CHIP_ERROR err = mObjectManager.Init();
+        VerifyOrReturnError(
+            err == CHIP_NO_ERROR, false,
+            ChipLogError(DeviceLayer, "Failed to initialize BlueZ object manager: %" CHIP_ERROR_FORMAT, err.Format()));
+        mObjectList.Init(mObjectManager.GetObjectManager());
+        mIterator      = mObjectList.begin();
+        mIsInitialized = true;
     }
 
     return Advance();