From 6f7995b78cc5712dadf4600a1cdd6808add3e26c Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Wed, 27 Mar 2024 18:17:31 +0100 Subject: [PATCH] [Linux] Dispatch BlueZ signals in BluezObjectManager (#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 | 35 +++++ src/platform/Linux/BLEManagerImpl.h | 2 + src/platform/Linux/CHIPDevicePlatformEvent.h | 7 + src/platform/Linux/bluez/BluezConnection.cpp | 8 +- src/platform/Linux/bluez/BluezConnection.h | 2 +- src/platform/Linux/bluez/BluezEndpoint.cpp | 89 +++-------- src/platform/Linux/bluez/BluezEndpoint.h | 17 +-- .../Linux/bluez/BluezObjectManager.cpp | 142 ++++++++++++++++++ src/platform/Linux/bluez/BluezObjectManager.h | 39 +++++ .../Linux/bluez/ChipDeviceScanner.cpp | 45 +----- src/platform/Linux/bluez/ChipDeviceScanner.h | 17 +-- 11 files changed, 278 insertions(+), 125 deletions(-) diff --git a/src/platform/Linux/BLEManagerImpl.cpp b/src/platform/Linux/BLEManagerImpl.cpp index 36b74308638300..c1d9eb725b1a30 100644 --- a/src/platform/Linux/BLEManagerImpl.cpp +++ b/src/platform/Linux/BLEManagerImpl.cpp @@ -37,6 +37,7 @@ #include #include +#include #include #include #include @@ -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; diff --git a/src/platform/Linux/BLEManagerImpl.h b/src/platform/Linux/BLEManagerImpl.h index 24533762040db6..a0339b856a9855 100644 --- a/src/platform/Linux/BLEManagerImpl.h +++ b/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); diff --git a/src/platform/Linux/CHIPDevicePlatformEvent.h b/src/platform/Linux/CHIPDevicePlatformEvent.h index 019dcb31e3403a..18ed55a1ef28d4 100644 --- a/src/platform/Linux/CHIPDevicePlatformEvent.h +++ b/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; diff --git a/src/platform/Linux/bluez/BluezConnection.cpp b/src/platform/Linux/bluez/BluezConnection.cpp index 87c74bb917f54c..722cb2478d1665 100644 --- a/src/platform/Linux/bluez/BluezConnection.cpp +++ b/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(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(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(g_object_ref(apDevice))) +BluezConnection::BluezConnection(const BluezEndpoint & aEndpoint, BluezDevice1 & aDevice) : + mDevice(reinterpret_cast(g_object_ref(&aDevice))) { Init(aEndpoint); } diff --git a/src/platform/Linux/bluez/BluezConnection.h b/src/platform/Linux/bluez/BluezConnection.h index dab977ee9ad864..a967977a21cf39 100644 --- a/src/platform/Linux/bluez/BluezConnection.h +++ b/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; diff --git a/src/platform/Linux/bluez/BluezEndpoint.cpp b/src/platform/Linux/bluez/BluezEndpoint.cpp index d8969d9b58a62f..7cc5814a0e96de 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.cpp +++ b/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(apDevice)); + const char * objectPath = g_dbus_proxy_get_object_path(reinterpret_cast(&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(device)); + const char * objectPath = g_dbus_proxy_get_object_path(reinterpret_cast(&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(*this, device); + conn = chip::Platform::New(*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 device(bluez_object_get_device1(reinterpret_cast(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 err; - GAutoPtr 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(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())); diff --git a/src/platform/Linux/bluez/BluezEndpoint.h b/src/platform/Linux/bluez/BluezEndpoint.h index 2f9d7e76b78193..39189b962f0ed1 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.h +++ b/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(); diff --git a/src/platform/Linux/bluez/BluezObjectManager.cpp b/src/platform/Linux/bluez/BluezObjectManager.cpp index ba858c5fd53941..1025dda92cf95f 100644 --- a/src/platform/Linux/bluez/BluezObjectManager.cpp +++ b/src/platform/Linux/bluez/BluezObjectManager.cpp @@ -17,6 +17,10 @@ #include "BluezObjectManager.h" +#include +#include +#include + #include #include #include @@ -35,6 +39,15 @@ namespace chip { namespace DeviceLayer { namespace Internal { +namespace { + +const char * GetAdapterObjectPath(BluezAdapter1 * aAdapter) +{ + return g_dbus_proxy_get_object_path(reinterpret_cast(aAdapter)); +} + +} // namespace + CHIP_ERROR BluezObjectManager::Init() { return PlatformMgrImpl().GLibMatterContextInvokeSync( @@ -94,6 +107,23 @@ BluezAdapter1 * BluezObjectManager::GetAdapter(const char * aAdapterAddress) return nullptr; } +CHIP_ERROR BluezObjectManager::SubscribeDeviceNotifications(BluezAdapter1 * aAdapter, + BluezObjectManagerAdapterNotificationsDelegate * aDelegate) +{ + std::lock_guard lock(mSubscriptionsMutex); + mSubscriptions.emplace_back(GetAdapterObjectPath(aAdapter), aDelegate); + return CHIP_NO_ERROR; +} + +CHIP_ERROR BluezObjectManager::UnsubscribeDeviceNotifications(BluezAdapter1 * aAdapter, + BluezObjectManagerAdapterNotificationsDelegate * aDelegate) +{ + std::lock_guard lock(mSubscriptionsMutex); + const auto item = std::make_pair(std::string(GetAdapterObjectPath(aAdapter)), aDelegate); + mSubscriptions.erase(std::remove(mSubscriptions.begin(), mSubscriptions.end(), item), mSubscriptions.end()); + return CHIP_NO_ERROR; +} + CHIP_ERROR BluezObjectManager::SetupAdapter(BluezAdapter1 * aAdapter) { // Make sure the adapter is powered on. @@ -105,6 +135,32 @@ CHIP_ERROR BluezObjectManager::SetupAdapter(BluezAdapter1 * aAdapter) return CHIP_NO_ERROR; } +void BluezObjectManager::NotifyAdapterAdded(BluezAdapter1 * aAdapter) +{ + unsigned int adapterId = 0; + sscanf(GetAdapterObjectPath(aAdapter), BLUEZ_PATH "/hci%u", &adapterId); + // Notify the application that new adapter has been just added + BLEManagerImpl::NotifyBLEAdapterAdded(adapterId, bluez_adapter1_get_address(aAdapter)); +} + +void BluezObjectManager::NotifyAdapterRemoved(BluezAdapter1 * aAdapter) +{ + unsigned int adapterId = 0; + sscanf(GetAdapterObjectPath(aAdapter), BLUEZ_PATH "/hci%u", &adapterId); + // Notify the application that the adapter is no longer available + BLEManagerImpl::NotifyBLEAdapterRemoved(adapterId, bluez_adapter1_get_address(aAdapter)); +} + +void BluezObjectManager::RemoveAdapterSubscriptions(BluezAdapter1 * aAdapter) +{ + std::lock_guard lock(mSubscriptionsMutex); + const auto adapterPath = GetAdapterObjectPath(aAdapter); + // Remove all device notification subscriptions for the given adapter + mSubscriptions.erase(std::remove_if(mSubscriptions.begin(), mSubscriptions.end(), + [adapterPath](const auto & subscription) { return subscription.first == adapterPath; }), + mSubscriptions.end()); +} + CHIP_ERROR BluezObjectManager::SetupDBusConnection() { GAutoPtr err; @@ -114,6 +170,75 @@ CHIP_ERROR BluezObjectManager::SetupDBusConnection() return CHIP_NO_ERROR; } +BluezObjectManager::NotificationsDelegates BluezObjectManager::GetDeviceNotificationsDelegates(BluezDevice1 * device) +{ + const char * deviceAdapterPath = bluez_device1_get_adapter(device); + NotificationsDelegates delegates; + + std::lock_guard lock(mSubscriptionsMutex); + for (auto & [adapterPath, delegate] : mSubscriptions) + { + if (adapterPath == deviceAdapterPath) + { + delegates.push_back(delegate); + } + } + + return delegates; +} + +void BluezObjectManager::OnObjectAdded(GDBusObjectManager * aMgr, GDBusObject * aObj) +{ + GAutoPtr adapter(bluez_object_get_adapter1(reinterpret_cast(aObj))); + if (adapter) + { + NotifyAdapterAdded(adapter.get()); + return; + } + + GAutoPtr device(bluez_object_get_device1(reinterpret_cast(aObj))); + if (device) + { + for (auto delegate : GetDeviceNotificationsDelegates(device.get())) + { + delegate->OnDeviceAdded(*device.get()); + } + } +} + +void BluezObjectManager::OnObjectRemoved(GDBusObjectManager * aMgr, GDBusObject * aObj) +{ + GAutoPtr adapter(bluez_object_get_adapter1(reinterpret_cast(aObj))); + if (adapter) + { + RemoveAdapterSubscriptions(adapter.get()); + NotifyAdapterRemoved(adapter.get()); + return; + } + + GAutoPtr device(bluez_object_get_device1(reinterpret_cast(aObj))); + if (device) + { + for (auto delegate : GetDeviceNotificationsDelegates(device.get())) + { + delegate->OnDeviceRemoved(*device.get()); + } + } +} + +void BluezObjectManager::OnInterfacePropertiesChanged(GDBusObjectManagerClient * aMgr, GDBusObjectProxy * aObj, GDBusProxy * aIface, + GVariant * aChangedProps, const char * const * aInvalidatedProps) +{ + GAutoPtr device(bluez_object_get_device1(reinterpret_cast(aObj))); + if (device) + { + for (auto delegate : GetDeviceNotificationsDelegates(device.get())) + { + delegate->OnDevicePropertyChanged(*device.get(), aChangedProps, aInvalidatedProps); + } + } +} + CHIP_ERROR BluezObjectManager::SetupObjectManager() { // When connecting to signals, the thread default context must be initialized. Otherwise, @@ -128,6 +253,23 @@ CHIP_ERROR BluezObjectManager::SetupObjectManager() VerifyOrReturnError(mObjectManager, CHIP_ERROR_INTERNAL, ChipLogError(DeviceLayer, "FAIL: Get D-Bus object manager client: %s", err->message)); + g_signal_connect(mObjectManager.get(), "object-added", + G_CALLBACK(+[](GDBusObjectManager * mgr, GDBusObject * obj, BluezObjectManager * self) { + return self->OnObjectAdded(mgr, obj); + }), + this); + g_signal_connect(mObjectManager.get(), "object-removed", + G_CALLBACK(+[](GDBusObjectManager * mgr, GDBusObject * obj, BluezObjectManager * self) { + return self->OnObjectRemoved(mgr, obj); + }), + this); + g_signal_connect(mObjectManager.get(), "interface-proxy-properties-changed", + G_CALLBACK(+[](GDBusObjectManagerClient * mgr, GDBusObjectProxy * obj, GDBusProxy * iface, GVariant * changed, + const char * const * invalidated, BluezObjectManager * self) { + return self->OnInterfacePropertiesChanged(mgr, obj, iface, changed, invalidated); + }), + this); + return CHIP_NO_ERROR; } diff --git a/src/platform/Linux/bluez/BluezObjectManager.h b/src/platform/Linux/bluez/BluezObjectManager.h index 04abd47dfb5c6d..4943451ac60ad7 100644 --- a/src/platform/Linux/bluez/BluezObjectManager.h +++ b/src/platform/Linux/bluez/BluezObjectManager.h @@ -17,11 +17,16 @@ #pragma once +#include +#include +#include + #include #include #include #include +#include #include "BluezObjectList.h" @@ -29,6 +34,17 @@ namespace chip { namespace DeviceLayer { namespace Internal { +/// Delegate for receiving notifications about various events on the Adapter1 +/// interface managed by the BlueZ object manager. +class BluezObjectManagerAdapterNotificationsDelegate +{ +public: + virtual ~BluezObjectManagerAdapterNotificationsDelegate() {} + virtual void OnDeviceAdded(BluezDevice1 & device) = 0; + virtual void OnDevicePropertyChanged(BluezDevice1 & device, GVariant * changedProps, const char * const * invalidatedProps) = 0; + virtual void OnDeviceRemoved(BluezDevice1 & device) = 0; +}; + class BluezObjectManager { public: @@ -54,13 +70,36 @@ class BluezObjectManager // Get the adapter with the given Bluetooth address. BluezAdapter1 * GetAdapter(const char * aAdapterAddress); + // Subscribe to notifications associated with the given adapter. + // In case when the adapter is removed, the subscription will be automatically canceled. + CHIP_ERROR SubscribeDeviceNotifications(BluezAdapter1 * adapter, BluezObjectManagerAdapterNotificationsDelegate * delegate); + + // Unsubscribe from notifications associated with the given adapter. + CHIP_ERROR UnsubscribeDeviceNotifications(BluezAdapter1 * adapter, BluezObjectManagerAdapterNotificationsDelegate * delegate); + private: CHIP_ERROR SetupDBusConnection(); CHIP_ERROR SetupObjectManager(); CHIP_ERROR SetupAdapter(BluezAdapter1 * aAdapter); + void NotifyAdapterAdded(BluezAdapter1 * aAdapter); + void NotifyAdapterRemoved(BluezAdapter1 * aAdapter); + void RemoveAdapterSubscriptions(BluezAdapter1 * aAdapter); + + using NotificationsDelegates = std::vector; + NotificationsDelegates GetDeviceNotificationsDelegates(BluezDevice1 * device); + + void OnObjectAdded(GDBusObjectManager * aMgr, GDBusObject * aObj); + void OnObjectRemoved(GDBusObjectManager * aMgr, GDBusObject * aObj); + void OnInterfacePropertiesChanged(GDBusObjectManagerClient * aMgr, GDBusObjectProxy * aObj, GDBusProxy * aIface, + GVariant * aChangedProps, const char * const * aInvalidatedProps); + GAutoPtr mConnection; GAutoPtr mObjectManager; + + std::mutex mSubscriptionsMutex; + std::vector> + mSubscriptions CHIP_GUARDED_BY(mSubscriptionsMutex); }; // Helper function to convert glib error returned by bluez_*_call_*() functions to CHIP_ERROR. diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index cd1c45956c5636..4fcb604b863dc6 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -171,17 +171,7 @@ CHIP_ERROR ChipDeviceScanner::StopScanImpl() g_cancellable_cancel(mCancellable.get()); mCancellable.reset(); - if (mObjectAddedSignal) - { - g_signal_handler_disconnect(mObjectManager.GetObjectManager(), mObjectAddedSignal); - mObjectAddedSignal = 0; - } - - if (mPropertiesChangedSignal) - { - g_signal_handler_disconnect(mObjectManager.GetObjectManager(), mPropertiesChangedSignal); - mPropertiesChangedSignal = 0; - } + mObjectManager.UnsubscribeDeviceNotifications(mAdapter.get(), this); GAutoPtr error; if (!bluez_adapter1_call_stop_discovery_sync(mAdapter.get(), nullptr /* not cancellable */, &error.GetReceiver())) @@ -198,22 +188,15 @@ CHIP_ERROR ChipDeviceScanner::StopScanImpl() return CHIP_NO_ERROR; } -void ChipDeviceScanner::SignalObjectAdded(GDBusObjectManager * aManager, GDBusObject * aObject) +void ChipDeviceScanner::OnDeviceAdded(BluezDevice1 & device) { - GAutoPtr device(bluez_object_get_device1(reinterpret_cast(aObject))); - VerifyOrReturn(device); - - ReportDevice(*device.get()); + ReportDevice(device); } -void ChipDeviceScanner::SignalInterfacePropertiesChanged(GDBusObjectManagerClient * aManager, GDBusObjectProxy * aObject, - GDBusProxy * aInterface, GVariant * aChangedProperties, - const char * const * aInvalidatedProps) +void ChipDeviceScanner::OnDevicePropertyChanged(BluezDevice1 & device, GVariant * changedProps, + const char * const * invalidatedProps) { - GAutoPtr device(bluez_object_get_device1(reinterpret_cast(aObject))); - VerifyOrReturn(device); - - ReportDevice(*device.get()); + ReportDevice(device); } void ChipDeviceScanner::ReportDevice(BluezDevice1 & device) @@ -254,20 +237,8 @@ void ChipDeviceScanner::RemoveDevice(BluezDevice1 & device) CHIP_ERROR ChipDeviceScanner::StartScanImpl() { - - mObjectAddedSignal = g_signal_connect(mObjectManager.GetObjectManager(), "object-added", - G_CALLBACK(+[](GDBusObjectManager * aMgr, GDBusObject * aObj, ChipDeviceScanner * self) { - return self->SignalObjectAdded(aMgr, aObj); - }), - this); - - mPropertiesChangedSignal = g_signal_connect( - mObjectManager.GetObjectManager(), "interface-proxy-properties-changed", - G_CALLBACK(+[](GDBusObjectManagerClient * aMgr, GDBusObjectProxy * aObj, GDBusProxy * aIface, GVariant * aChangedProps, - const char * const * aInvalidatedProps, ChipDeviceScanner * self) { - return self->SignalInterfacePropertiesChanged(aMgr, aObj, aIface, aChangedProps, aInvalidatedProps); - }), - this); + CHIP_ERROR err = mObjectManager.SubscribeDeviceNotifications(mAdapter.get(), this); + ReturnErrorOnFailure(err); ChipLogProgress(Ble, "BLE removing known devices"); for (BluezObject & object : mObjectManager.GetObjects()) diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.h b/src/platform/Linux/bluez/ChipDeviceScanner.h index 43c24491d6e330..d0bde89a9b28ab 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.h +++ b/src/platform/Linux/bluez/ChipDeviceScanner.h @@ -53,7 +53,7 @@ class ChipDeviceScannerDelegate /// Allows scanning for CHIP devices /// /// Will perform scan operations and call back whenever a device is discovered. -class ChipDeviceScanner +class ChipDeviceScanner : public BluezObjectManagerAdapterNotificationsDelegate { public: ChipDeviceScanner(BluezObjectManager & aObjectManager) : mObjectManager(aObjectManager) {} @@ -78,6 +78,11 @@ class ChipDeviceScanner /// Stop any currently running scan CHIP_ERROR StopScan(); + /// 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: enum ChipDeviceScannerState { @@ -97,10 +102,6 @@ class ChipDeviceScanner CHIP_ERROR StopScanImpl(); static void TimerExpiredCallback(chip::System::Layer * layer, void * appState); - void SignalObjectAdded(GDBusObjectManager * aManager, GDBusObject * aObject); - void SignalInterfacePropertiesChanged(GDBusObjectManagerClient * aManager, GDBusObjectProxy * aObject, GDBusProxy * aInterface, - GVariant * aChangedProperties, const char * const * aInvalidatedProps); - /// Check if a given device is a CHIP device and if yes, report it as discovered void ReportDevice(BluezDevice1 & device); @@ -111,10 +112,8 @@ class ChipDeviceScanner BluezObjectManager & mObjectManager; GAutoPtr mAdapter; - ChipDeviceScannerDelegate * mDelegate = nullptr; - unsigned long mObjectAddedSignal = 0; - unsigned long mPropertiesChangedSignal = 0; - ChipDeviceScannerState mScannerState = ChipDeviceScannerState::SCANNER_UNINITIALIZED; + 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 mCancellable;