[Linux] Dispatch BlueZ signals in BluezObjectManager (project-chip#32709

)

* Notification facility for BLE adapter added/removed

* Dispatch BlueZ signals in BluezObjectManager

* Get rid of pointers in delegate public API

* Fix deadlock when calling callback under mutex

src/platform/Linux/BLEManagerImpl.cpp

@@ -37,6 +37,7 @@
 
 #include <ble/BleError.h>
 #include <ble/CHIPBleServiceData.h>
+#include <lib/support/CHIPMemString.h>
 #include <lib/support/CodeUtils.h>
 #include <lib/support/SafeInt.h>
 #include <platform/CHIPDeviceLayer.h>
@@ -257,6 +258,22 @@ void BLEManagerImpl::HandlePlatformSpecificBLEEvent(const ChipDeviceEvent * apEv
     ChipLogDetail(DeviceLayer, "HandlePlatformSpecificBLEEvent %d", apEvent->Type);
     switch (apEvent->Type)
     {
+    case DeviceEventType::kPlatformLinuxBLEAdapterAdded:
+        ChipLogDetail(DeviceLayer, "BLE adapter added: id=%u address=%s", apEvent->Platform.BLEAdapter.mAdapterId,
+                      apEvent->Platform.BLEAdapter.mAdapterAddress);
+        if (apEvent->Platform.BLEAdapter.mAdapterId == mAdapterId)
+        {
+            // TODO: Handle adapter added
+        }
+        break;
+    case DeviceEventType::kPlatformLinuxBLEAdapterRemoved:
+        ChipLogDetail(DeviceLayer, "BLE adapter removed: id=%u address=%s", apEvent->Platform.BLEAdapter.mAdapterId,
+                      apEvent->Platform.BLEAdapter.mAdapterAddress);
+        if (apEvent->Platform.BLEAdapter.mAdapterId == mAdapterId)
+        {
+            // TODO: Handle adapter removed
+        }
+        break;
     case DeviceEventType::kPlatformLinuxBLECentralConnected:
         if (mBLEScanConfig.mBleScanState == BleScanState::kConnecting)
         {
@@ -792,6 +809,24 @@ CHIP_ERROR BLEManagerImpl::CancelConnection()
     return CHIP_NO_ERROR;
 }
 
+void BLEManagerImpl::NotifyBLEAdapterAdded(unsigned int aAdapterId, const char * aAdapterAddress)
+{
+    ChipDeviceEvent event;
+    event.Type                           = DeviceEventType::kPlatformLinuxBLEAdapterAdded;
+    event.Platform.BLEAdapter.mAdapterId = aAdapterId;
+    Platform::CopyString(event.Platform.BLEAdapter.mAdapterAddress, aAdapterAddress);
+    PlatformMgr().PostEventOrDie(&event);
+}
+
+void BLEManagerImpl::NotifyBLEAdapterRemoved(unsigned int aAdapterId, const char * aAdapterAddress)
+{
+    ChipDeviceEvent event;
+    event.Type                           = DeviceEventType::kPlatformLinuxBLEAdapterRemoved;
+    event.Platform.BLEAdapter.mAdapterId = aAdapterId;
+    Platform::CopyString(event.Platform.BLEAdapter.mAdapterAddress, aAdapterAddress);
+    PlatformMgr().PostEventOrDie(&event);
+}
+
 void BLEManagerImpl::NotifyBLEPeripheralRegisterAppComplete(CHIP_ERROR error)
 {
     ChipDeviceEvent event;

src/platform/Linux/BLEManagerImpl.h

@@ -93,6 +93,8 @@ class BLEManagerImpl final : public BLEManager,
     static void HandleTXCharCCCDWrite(BLE_CONNECTION_OBJECT user_data);
     static void HandleTXComplete(BLE_CONNECTION_OBJECT user_data);
 
+    static void NotifyBLEAdapterAdded(unsigned int aAdapterId, const char * aAdapterAddress);
+    static void NotifyBLEAdapterRemoved(unsigned int aAdapterId, const char * aAdapterAddress);
     static void NotifyBLEPeripheralRegisterAppComplete(CHIP_ERROR error);
     static void NotifyBLEPeripheralAdvStartComplete(CHIP_ERROR error);
     static void NotifyBLEPeripheralAdvStopComplete(CHIP_ERROR error);

src/platform/Linux/CHIPDevicePlatformEvent.h

@@ -45,6 +45,8 @@ enum PublicPlatformSpecificEventTypes
 enum InternalPlatformSpecificEventTypes
 {
     kPlatformLinuxEvent = kRange_InternalPlatformSpecific,
+    kPlatformLinuxBLEAdapterAdded,
+    kPlatformLinuxBLEAdapterRemoved,
     kPlatformLinuxBLECentralConnected,
     kPlatformLinuxBLECentralConnectFailed,
     kPlatformLinuxBLEWriteComplete,
@@ -67,6 +69,11 @@ struct ChipDevicePlatformEvent
 {
     union
     {
+        struct
+        {
+            unsigned int mAdapterId;
+            char mAdapterAddress[18];
+        } BLEAdapter;
         struct
         {
             BLE_CONNECTION_OBJECT mConnection;

src/platform/Linux/bluez/BluezConnection.cpp

@@ -45,22 +45,22 @@ namespace {
 gboolean BluezIsServiceOnDevice(BluezGattService1 * aService, BluezDevice1 * aDevice)
 {
     const auto * servicePath = bluez_gatt_service1_get_device(aService);
-    const auto * devicePath  = g_dbus_proxy_get_object_path(G_DBUS_PROXY(aDevice));
+    const auto * devicePath  = g_dbus_proxy_get_object_path(reinterpret_cast<GDBusProxy *>(aDevice));
     return strcmp(servicePath, devicePath) == 0 ? TRUE : FALSE;
 }
 
 gboolean BluezIsCharOnService(BluezGattCharacteristic1 * aChar, BluezGattService1 * aService)
 {
     const auto * charPath    = bluez_gatt_characteristic1_get_service(aChar);
-    const auto * servicePath = g_dbus_proxy_get_object_path(G_DBUS_PROXY(aService));
+    const auto * servicePath = g_dbus_proxy_get_object_path(reinterpret_cast<GDBusProxy *>(aService));
     ChipLogDetail(DeviceLayer, "Char %s on service %s", charPath, servicePath);
     return strcmp(charPath, servicePath) == 0 ? TRUE : FALSE;
 }
 
 } // namespace
 
-BluezConnection::BluezConnection(const BluezEndpoint & aEndpoint, BluezDevice1 * apDevice) :
-    mDevice(reinterpret_cast<BluezDevice1 *>(g_object_ref(apDevice)))
+BluezConnection::BluezConnection(const BluezEndpoint & aEndpoint, BluezDevice1 & aDevice) :
+    mDevice(reinterpret_cast<BluezDevice1 *>(g_object_ref(&aDevice)))
 {
     Init(aEndpoint);
 }

src/platform/Linux/bluez/BluezConnection.h

@@ -39,7 +39,7 @@ class BluezEndpoint;
 class BluezConnection
 {
 public:
-    BluezConnection(const BluezEndpoint & aEndpoint, BluezDevice1 * apDevice);
+    BluezConnection(const BluezEndpoint & aEndpoint, BluezDevice1 & aDevice);
     ~BluezConnection() = default;
 
     const char * GetPeerAddress() const;

src/platform/Linux/bluez/BluezEndpoint.cpp

@@ -228,11 +228,6 @@ static gboolean BluezCharacteristicConfirmError(BluezGattCharacteristic1 * aChar
     return TRUE;
 }
 
-static gboolean BluezIsDeviceOnAdapter(BluezDevice1 * aDevice, BluezAdapter1 * aAdapter)
-{
-    return strcmp(bluez_device1_get_adapter(aDevice), g_dbus_proxy_get_object_path(G_DBUS_PROXY(aAdapter))) == 0 ? TRUE : FALSE;
-}
-
 BluezGattCharacteristic1 * BluezEndpoint::CreateGattCharacteristic(BluezGattService1 * aService, const char * aCharName,
                                                                    const char * aUUID, const char * const * aFlags)
 {
@@ -305,12 +300,12 @@ CHIP_ERROR BluezEndpoint::RegisterGattApplicationImpl()
 }
 
 /// Update the table of open BLE connections whenever a new device is spotted or its attributes have changed.
-void BluezEndpoint::UpdateConnectionTable(BluezDevice1 * apDevice)
+void BluezEndpoint::UpdateConnectionTable(BluezDevice1 & aDevice)
 {
-    const char * objectPath      = g_dbus_proxy_get_object_path(reinterpret_cast<GDBusProxy *>(apDevice));
+    const char * objectPath      = g_dbus_proxy_get_object_path(reinterpret_cast<GDBusProxy *>(&aDevice));
     BluezConnection * connection = GetBluezConnection(objectPath);
 
-    if (connection != nullptr && !bluez_device1_get_connected(apDevice))
+    if (connection != nullptr && !bluez_device1_get_connected(&aDevice))
     {
         ChipLogDetail(DeviceLayer, "Bluez disconnected");
         BLEManagerImpl::CHIPoBluez_ConnectionClosed(connection);
@@ -323,35 +318,22 @@ void BluezEndpoint::UpdateConnectionTable(BluezDevice1 * apDevice)
 
     if (connection == nullptr)
     {
-        HandleNewDevice(apDevice);
+        HandleNewDevice(aDevice);
     }
 }
 
-void BluezEndpoint::BluezSignalInterfacePropertiesChanged(GDBusObjectManagerClient * aManager, GDBusObjectProxy * aObject,
-                                                          GDBusProxy * aInterface, GVariant * aChangedProperties,
-                                                          const char * const * aInvalidatedProps)
+void BluezEndpoint::HandleNewDevice(BluezDevice1 & aDevice)
 {
-    VerifyOrReturn(mAdapter, ChipLogError(DeviceLayer, "FAIL: NULL mAdapter in %s", __func__));
-    VerifyOrReturn(strcmp(g_dbus_proxy_get_interface_name(aInterface), DEVICE_INTERFACE) == 0, );
-
-    BluezDevice1 * device = BLUEZ_DEVICE1(aInterface);
-    VerifyOrReturn(BluezIsDeviceOnAdapter(device, mAdapter.get()));
-
-    UpdateConnectionTable(device);
-}
+    VerifyOrReturn(bluez_device1_get_connected(&aDevice));
+    VerifyOrReturn(!mIsCentral || bluez_device1_get_services_resolved(&aDevice));
 
-void BluezEndpoint::HandleNewDevice(BluezDevice1 * device)
-{
-    VerifyOrReturn(bluez_device1_get_connected(device));
-    VerifyOrReturn(!mIsCentral || bluez_device1_get_services_resolved(device));
-
-    const char * objectPath = g_dbus_proxy_get_object_path(reinterpret_cast<GDBusProxy *>(device));
+    const char * objectPath = g_dbus_proxy_get_object_path(reinterpret_cast<GDBusProxy *>(&aDevice));
     BluezConnection * conn  = GetBluezConnection(objectPath);
     VerifyOrReturn(conn == nullptr,
                    ChipLogError(DeviceLayer, "FAIL: Connection already tracked: conn=%p device=%s path=%s", conn,
                                 conn->GetPeerAddress(), objectPath));
 
-    conn                       = chip::Platform::New<BluezConnection>(*this, device);
+    conn                       = chip::Platform::New<BluezConnection>(*this, aDevice);
     mpPeerDevicePath           = g_strdup(objectPath);
     mConnMap[mpPeerDevicePath] = conn;
 
@@ -360,24 +342,20 @@ void BluezEndpoint::HandleNewDevice(BluezDevice1 * device)
     BLEManagerImpl::HandleNewConnection(conn);
 }
 
-void BluezEndpoint::BluezSignalOnObjectAdded(GDBusObjectManager * aManager, GDBusObject * aObject)
+void BluezEndpoint::OnDeviceAdded(BluezDevice1 & device)
 {
-    // TODO: right now we do not handle addition/removal of adapters
-    // Primary focus here is to handle addition of a device
-    GAutoPtr<BluezDevice1> device(bluez_object_get_device1(reinterpret_cast<BluezObject *>(aObject)));
-    VerifyOrReturn(device);
+    HandleNewDevice(device);
+}
 
-    if (BluezIsDeviceOnAdapter(device.get(), mAdapter.get()) == TRUE)
-    {
-        HandleNewDevice(device.get());
-    }
+void BluezEndpoint::OnDevicePropertyChanged(BluezDevice1 & device, GVariant * changedProps, const char * const * invalidatedProps)
+{
+    UpdateConnectionTable(device);
 }
 
-void BluezEndpoint::BluezSignalOnObjectRemoved(GDBusObjectManager * aManager, GDBusObject * aObject)
+void BluezEndpoint::OnDeviceRemoved(BluezDevice1 & device)
 {
-    // TODO: for Device1, lookup connection, and call otPlatTobleHandleDisconnected
-    // for Adapter1: unclear, crash if this pertains to our adapter? at least null out the self->mAdapter.
-    // for Characteristic1, or GattService -- handle here via calling otPlatTobleHandleDisconnected, or ignore.
+    // Handling device removal is not necessary because disconnection is already handled
+    // in the OnDevicePropertyChanged() - we are checking for the "Connected" property.
 }
 
 BluezGattService1 * BluezEndpoint::CreateGattService(const char * aUUID)
@@ -545,30 +523,7 @@ void BluezEndpoint::SetupGattServer(GDBusConnection * aConn)
 
 CHIP_ERROR BluezEndpoint::SetupEndpointBindings()
 {
-    GAutoPtr<GError> err;
-    GAutoPtr<GDBusConnection> conn(g_bus_get_sync(G_BUS_TYPE_SYSTEM, nullptr, &err.GetReceiver()));
-    VerifyOrReturnError(conn != nullptr, CHIP_ERROR_INTERNAL,
-                        ChipLogError(DeviceLayer, "FAIL: get bus sync in %s, error: %s", __func__, err->message));
-
-    SetupGattServer(conn.get());
-
-    g_signal_connect(mObjectManager.GetObjectManager(), "object-added",
-                     G_CALLBACK(+[](GDBusObjectManager * aMgr, GDBusObject * aObj, BluezEndpoint * self) {
-                         return self->BluezSignalOnObjectAdded(aMgr, aObj);
-                     }),
-                     this);
-    g_signal_connect(mObjectManager.GetObjectManager(), "object-removed",
-                     G_CALLBACK(+[](GDBusObjectManager * aMgr, GDBusObject * aObj, BluezEndpoint * self) {
-                         return self->BluezSignalOnObjectRemoved(aMgr, aObj);
-                     }),
-                     this);
-    g_signal_connect(mObjectManager.GetObjectManager(), "interface-proxy-properties-changed",
-                     G_CALLBACK(+[](GDBusObjectManagerClient * aMgr, GDBusObjectProxy * aObj, GDBusProxy * aIface,
-                                    GVariant * aChangedProps, const char * const * aInvalidatedProps, BluezEndpoint * self) {
-                         return self->BluezSignalInterfacePropertiesChanged(aMgr, aObj, aIface, aChangedProps, aInvalidatedProps);
-                     }),
-                     this);
-
+    SetupGattServer(mObjectManager.GetConnection());
     return CHIP_NO_ERROR;
 }
 
@@ -586,7 +541,11 @@ CHIP_ERROR BluezEndpoint::Init(BluezAdapter1 * apAdapter, bool aIsCentral)
     mAdapter.reset(reinterpret_cast<BluezAdapter1 *>(g_object_ref(apAdapter)));
     mIsCentral = aIsCentral;
 
-    CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(
+    CHIP_ERROR err = mObjectManager.SubscribeDeviceNotifications(mAdapter.get(), this);
+    VerifyOrReturnError(err == CHIP_NO_ERROR, err,
+                        ChipLogError(DeviceLayer, "Failed to subscribe for notifications: %" CHIP_ERROR_FORMAT, err.Format()));
+
+    err = PlatformMgrImpl().GLibMatterContextInvokeSync(
         +[](BluezEndpoint * self) { return self->SetupEndpointBindings(); }, this);
     VerifyOrReturnError(err == CHIP_NO_ERROR, err,
                         ChipLogError(DeviceLayer, "Failed to schedule endpoint initialization: %" CHIP_ERROR_FORMAT, err.Format()));

src/platform/Linux/bluez/BluezEndpoint.h

@@ -65,7 +65,7 @@ namespace chip {
 namespace DeviceLayer {
 namespace Internal {
 
-class BluezEndpoint
+class BluezEndpoint : public BluezObjectManagerAdapterNotificationsDelegate
 {
 public:
     BluezEndpoint(BluezObjectManager & aObjectManager) : mObjectManager(aObjectManager) {}
@@ -80,6 +80,11 @@ class BluezEndpoint
     CHIP_ERROR ConnectDevice(BluezDevice1 & aDevice);
     void CancelConnect();
 
+    // Members that implement virtual methods on BluezObjectManagerAdapterNotificationsDelegate
+    void OnDeviceAdded(BluezDevice1 & device) override;
+    void OnDevicePropertyChanged(BluezDevice1 & device, GVariant * changedProps, const char * const * invalidatedProps) override;
+    void OnDeviceRemoved(BluezDevice1 & device) override;
+
 private:
     CHIP_ERROR SetupEndpointBindings();
     void SetupGattServer(GDBusConnection * aConn);
@@ -89,8 +94,8 @@ class BluezEndpoint
     BluezGattCharacteristic1 * CreateGattCharacteristic(BluezGattService1 * aService, const char * aCharName, const char * aUUID,
                                                         const char * const * aFlags);
 
-    void HandleNewDevice(BluezDevice1 * aDevice);
-    void UpdateConnectionTable(BluezDevice1 * aDevice);
+    void HandleNewDevice(BluezDevice1 & aDevice);
+    void UpdateConnectionTable(BluezDevice1 & aDevice);
     BluezConnection * GetBluezConnection(const char * aPath);
     BluezConnection * GetBluezConnectionViaDevice();
 
@@ -99,12 +104,6 @@ class BluezEndpoint
     gboolean BluezCharacteristicAcquireNotify(BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, GVariant * aOptions);
     gboolean BluezCharacteristicConfirm(BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv);
 
-    void BluezSignalOnObjectAdded(GDBusObjectManager * aManager, GDBusObject * aObject);
-    void BluezSignalOnObjectRemoved(GDBusObjectManager * aManager, GDBusObject * aObject);
-    void BluezSignalInterfacePropertiesChanged(GDBusObjectManagerClient * aManager, GDBusObjectProxy * aObject,
-                                               GDBusProxy * aInterface, GVariant * aChangedProperties,
-                                               const char * const * aInvalidatedProps);
-
     void RegisterGattApplicationDone(GObject * aObject, GAsyncResult * aResult);
     CHIP_ERROR RegisterGattApplicationImpl();