From d5ef6194be4cb65ff7d0bc646fc036d07927d6c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Mon, 27 Nov 2023 11:23:26 +0100 Subject: [PATCH 1/8] Replace single bluez endpoint related objects with BluezEndpoint --- src/platform/Linux/BLEManagerImpl.cpp | 4 +- .../Linux/bluez/BluezAdvertisement.cpp | 50 +++++--------- src/platform/Linux/bluez/BluezAdvertisement.h | 7 +- src/platform/Linux/bluez/BluezEndpoint.h | 1 + .../Linux/bluez/ChipDeviceScanner.cpp | 69 +++++++------------ src/platform/Linux/bluez/ChipDeviceScanner.h | 7 +- 6 files changed, 52 insertions(+), 86 deletions(-) diff --git a/src/platform/Linux/BLEManagerImpl.cpp b/src/platform/Linux/BLEManagerImpl.cpp index 10a17be7607735..fdff640a4695c2 100644 --- a/src/platform/Linux/BLEManagerImpl.cpp +++ b/src/platform/Linux/BLEManagerImpl.cpp @@ -595,7 +595,7 @@ void BLEManagerImpl::DriveBLEState() // Configure advertising data if it hasn't been done yet. if (!mFlags.Has(Flags::kAdvertisingConfigured)) { - SuccessOrExit(err = mBLEAdvertisement.Init(mEndpoint, mBLEAdvType, mpBLEAdvUUID, mBLEAdvDurationMs)); + SuccessOrExit(err = mBLEAdvertisement.Init(&mEndpoint, mBLEAdvType, mpBLEAdvUUID, mBLEAdvDurationMs)); mFlags.Set(Flags::kAdvertisingConfigured); } @@ -668,7 +668,7 @@ void BLEManagerImpl::InitiateScan(BleScanState scanType) mBLEScanConfig.mBleScanState = scanType; - CHIP_ERROR err = mDeviceScanner.Init(mEndpoint.GetAdapter(), this); + CHIP_ERROR err = mDeviceScanner.Init(&mEndpoint, this); if (err != CHIP_NO_ERROR) { mBLEScanConfig.mBleScanState = BleScanState::kNotScanning; diff --git a/src/platform/Linux/bluez/BluezAdvertisement.cpp b/src/platform/Linux/bluez/BluezAdvertisement.cpp index 19212fbe717475..f59f0429c09e5c 100644 --- a/src/platform/Linux/bluez/BluezAdvertisement.cpp +++ b/src/platform/Linux/bluez/BluezAdvertisement.cpp @@ -62,7 +62,7 @@ BluezLEAdvertisement1 * BluezAdvertisement::CreateLEAdvertisement() g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, &mDeviceIdInfo, sizeof(mDeviceIdInfo), sizeof(uint8_t))); g_variant_builder_add(&serviceUUIDsBuilder, "s", mpAdvUUID); - localNamePtr = mpAdapterName; + localNamePtr = mEndpoint->GetAdapterName(); if (localNamePtr == nullptr) { g_snprintf(localName, sizeof(localName), "%s%04x", CHIP_DEVICE_CONFIG_BLE_DEVICE_NAME_PREFIX, getpid() & 0xffff); @@ -107,7 +107,7 @@ BluezLEAdvertisement1 * BluezAdvertisement::CreateLEAdvertisement() }), this); - g_dbus_object_manager_server_export(mpRoot, G_DBUS_OBJECT_SKELETON(object)); + g_dbus_object_manager_server_export(mEndpoint->GetGattApplicationObjectManager(), G_DBUS_OBJECT_SKELETON(object)); g_object_unref(object); return adv; @@ -116,7 +116,7 @@ BluezLEAdvertisement1 * BluezAdvertisement::CreateLEAdvertisement() gboolean BluezAdvertisement::BluezLEAdvertisement1Release(BluezLEAdvertisement1 * aAdv, GDBusMethodInvocation * aInvocation) { ChipLogDetail(DeviceLayer, "Release BLE adv object in %s", __func__); - g_dbus_object_manager_server_unexport(mpRoot, mpAdvPath); + g_dbus_object_manager_server_unexport(mEndpoint->GetGattApplicationObjectManager(), mpAdvPath); g_object_unref(mpAdv); mpAdv = nullptr; mIsAdvertising = false; @@ -133,7 +133,7 @@ CHIP_ERROR BluezAdvertisement::InitImpl() return CHIP_NO_ERROR; } -CHIP_ERROR BluezAdvertisement::Init(const BluezEndpoint & aEndpoint, ChipAdvType aAdvType, const char * aAdvUUID, +CHIP_ERROR BluezAdvertisement::Init(const BluezEndpoint * aEndpoint, ChipAdvType aAdvType, const char * aAdvUUID, uint32_t aAdvDurationMs) { GAutoPtr rootPath; @@ -142,11 +142,10 @@ CHIP_ERROR BluezAdvertisement::Init(const BluezEndpoint & aEndpoint, ChipAdvType VerifyOrExit(mpAdv == nullptr, err = CHIP_ERROR_INCORRECT_STATE; ChipLogError(DeviceLayer, "FAIL: BLE advertisement already initialized in %s", __func__)); - mpRoot = reinterpret_cast(g_object_ref(aEndpoint.GetGattApplicationObjectManager())); - mpAdapter = reinterpret_cast(g_object_ref(aEndpoint.GetAdapter())); - mpAdapterName = g_strdup(aEndpoint.GetAdapterName()); + mEndpoint = aEndpoint; - g_object_get(G_OBJECT(mpRoot), "object-path", &MakeUniquePointerReceiver(rootPath).Get(), nullptr); + g_object_get(G_OBJECT(mEndpoint->GetGattApplicationObjectManager()), "object-path", &MakeUniquePointerReceiver(rootPath).Get(), + nullptr); mpAdvPath = g_strdup_printf("%s/advertising", rootPath.get()); mAdvType = aAdvType; mpAdvUUID = g_strdup(aAdvUUID); @@ -159,8 +158,7 @@ CHIP_ERROR BluezAdvertisement::Init(const BluezEndpoint & aEndpoint, ChipAdvType mDeviceIdInfo.SetAdditionalDataFlag(true); #endif - err = PlatformMgrImpl().GLibMatterContextInvokeSync( - +[](BluezAdvertisement * self) { return self->InitImpl(); }, this); + err = PlatformMgrImpl().GLibMatterContextInvokeSync(+[](BluezAdvertisement * self) { return self->InitImpl(); }, this); VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Ble, "Failed to schedule BLE advertisement Init() on CHIPoBluez thread")); @@ -188,16 +186,6 @@ void BluezAdvertisement::Shutdown() // attached to the advertising object that may run on the glib thread. PlatformMgrImpl().GLibMatterContextInvokeSync( +[](BluezAdvertisement * self) { - if (self->mpRoot != nullptr) - { - g_object_unref(self->mpRoot); - self->mpRoot = nullptr; - } - if (self->mpAdapter != nullptr) - { - g_object_unref(self->mpAdapter); - self->mpAdapter = nullptr; - } if (self->mpAdv != nullptr) { g_object_unref(self->mpAdv); @@ -209,8 +197,6 @@ void BluezAdvertisement::Shutdown() g_free(mpAdvPath); mpAdvPath = nullptr; - g_free(mpAdapterName); - mpAdapterName = nullptr; g_free(mpAdvUUID); mpAdvUUID = nullptr; @@ -227,7 +213,7 @@ void BluezAdvertisement::StartDone(GObject * aObject, GAsyncResult * aResult) bluez_leadvertising_manager1_call_register_advertisement_finish(advMgr, aResult, &MakeUniquePointerReceiver(error).Get()); if (success == FALSE) { - g_dbus_object_manager_server_unexport(mpRoot, mpAdvPath); + g_dbus_object_manager_server_unexport(mEndpoint->GetGattApplicationObjectManager(), mpAdvPath); } VerifyOrExit(success == TRUE, ChipLogError(DeviceLayer, "FAIL: RegisterAdvertisement : %s", error->message)); @@ -247,9 +233,9 @@ CHIP_ERROR BluezAdvertisement::StartImpl() GVariant * options; VerifyOrExit(!mIsAdvertising, ChipLogError(DeviceLayer, "FAIL: Advertising has already been enabled in %s", __func__)); - VerifyOrExit(mpAdapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); + VerifyOrExit(mEndpoint->GetAdapter() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); - adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mpAdapter)); + adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mEndpoint->GetAdapter())); VerifyOrExit(adapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL adapter in %s", __func__)); advMgr = bluez_object_get_leadvertising_manager1(BLUEZ_OBJECT(adapter)); @@ -273,8 +259,8 @@ CHIP_ERROR BluezAdvertisement::Start() { VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); - CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync( - +[](BluezAdvertisement * self) { return self->StartImpl(); }, this); + CHIP_ERROR err = + PlatformMgrImpl().GLibMatterContextInvokeSync(+[](BluezAdvertisement * self) { return self->StartImpl(); }, this); VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Ble, "Failed to schedule BLE advertisement Start() on CHIPoBluez thread")); return err; @@ -291,7 +277,7 @@ void BluezAdvertisement::StopDone(GObject * aObject, GAsyncResult * aResult) if (success == FALSE) { - g_dbus_object_manager_server_unexport(mpRoot, mpAdvPath); + g_dbus_object_manager_server_unexport(mEndpoint->GetGattApplicationObjectManager(), mpAdvPath); } else { @@ -312,9 +298,9 @@ CHIP_ERROR BluezAdvertisement::StopImpl() BluezLEAdvertisingManager1 * advMgr = nullptr; VerifyOrExit(mIsAdvertising, ChipLogError(DeviceLayer, "FAIL: Advertising has already been disabled in %s", __func__)); - VerifyOrExit(mpAdapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); + VerifyOrExit(mEndpoint->GetAdapter() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); - adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mpAdapter)); + adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mEndpoint->GetAdapter())); VerifyOrExit(adapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL adapter in %s", __func__)); advMgr = bluez_object_get_leadvertising_manager1(BLUEZ_OBJECT(adapter)); @@ -335,8 +321,8 @@ CHIP_ERROR BluezAdvertisement::Stop() { VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); - CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync( - +[](BluezAdvertisement * self) { return self->StopImpl(); }, this); + CHIP_ERROR err = + PlatformMgrImpl().GLibMatterContextInvokeSync(+[](BluezAdvertisement * self) { return self->StopImpl(); }, this); VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Ble, "Failed to schedule BLE advertisement Stop() on CHIPoBluez thread")); return err; diff --git a/src/platform/Linux/bluez/BluezAdvertisement.h b/src/platform/Linux/bluez/BluezAdvertisement.h index 6ae2974d706b35..cbbdd9ce8bf0d4 100644 --- a/src/platform/Linux/bluez/BluezAdvertisement.h +++ b/src/platform/Linux/bluez/BluezAdvertisement.h @@ -41,7 +41,7 @@ class BluezAdvertisement BluezAdvertisement() = default; ~BluezAdvertisement() { Shutdown(); } - CHIP_ERROR Init(const BluezEndpoint & aEndpoint, ChipAdvType aAdvType, const char * aAdvUUID, uint32_t aAdvDurationMs); + CHIP_ERROR Init(const BluezEndpoint * aEndpoint, ChipAdvType aAdvType, const char * aAdvUUID, uint32_t aAdvDurationMs); void Shutdown(); /// Start BLE advertising. @@ -69,9 +69,8 @@ class BluezAdvertisement CHIP_ERROR StopImpl(); // Objects (interfaces) used by LE advertisement - GDBusObjectManagerServer * mpRoot = nullptr; - BluezAdapter1 * mpAdapter = nullptr; - BluezLEAdvertisement1 * mpAdv = nullptr; + const BluezEndpoint * mEndpoint = nullptr; + BluezLEAdvertisement1 * mpAdv = nullptr; bool mIsInitialized = false; bool mIsAdvertising = false; diff --git a/src/platform/Linux/bluez/BluezEndpoint.h b/src/platform/Linux/bluez/BluezEndpoint.h index 24b7ad140cde65..0b3f286b399247 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.h +++ b/src/platform/Linux/bluez/BluezEndpoint.h @@ -77,6 +77,7 @@ class BluezEndpoint CHIP_ERROR RegisterGattApplication(); GDBusObjectManagerServer * GetGattApplicationObjectManager() const { return mpRoot; } + GDBusObjectManager * GetObjectManager() const { return mpObjMgr; } CHIP_ERROR ConnectDevice(BluezDevice1 & aDevice); void CancelConnect(); diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index 1670dfc082bebd..86b6baeea3918f 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -52,33 +52,21 @@ bool BluezGetChipDeviceInfo(BluezDevice1 & aDevice, chip::Ble::ChipBLEDeviceIden } // namespace -CHIP_ERROR ChipDeviceScanner::Init(BluezAdapter1 * adapter, ChipDeviceScannerDelegate * delegate) +CHIP_ERROR ChipDeviceScanner::Init(const BluezEndpoint * aEndpoint, ChipDeviceScannerDelegate * delegate) { // Make this function idempotent by shutting down previously initialized state if any. Shutdown(); - mAdapter = BLUEZ_ADAPTER1(g_object_ref(adapter)); + mEndpoint = aEndpoint; mCancellable = g_cancellable_new(); mDelegate = delegate; - // Create the D-Bus object manager client object on the glib thread, so that all D-Bus signals - // will be delivered to the glib thread. - ReturnErrorOnFailure(PlatformMgrImpl().GLibMatterContextInvokeSync( - +[](ChipDeviceScanner * self) { - // When creating D-Bus proxy object, the thread default context must be initialized. - VerifyOrDie(g_main_context_get_thread_default() != nullptr); - - GAutoPtr err; - self->mManager = 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 */, self->mCancellable, &MakeUniquePointerReceiver(err).Get()); - VerifyOrReturnError(self->mManager != nullptr, CHIP_ERROR_INTERNAL, - ChipLogError(Ble, "Failed to get D-Bus object manager for device scanning: %s", err->message)); - return CHIP_NO_ERROR; - }, - this)); + g_cancellable_cancel(mCancellable); + + GAutoPtr err; + VerifyOrReturnError(mEndpoint->GetObjectManager() != nullptr, CHIP_ERROR_INTERNAL, + ChipLogError(Ble, "Failed to get D-Bus object manager for device scanning: %s", err->message)); mIsInitialized = true; return CHIP_NO_ERROR; @@ -97,20 +85,8 @@ void ChipDeviceScanner::Shutdown() chip::DeviceLayer::SystemLayer().CancelTimer(TimerExpiredCallback, this); } - // Release resources on the glib thread. This is necessary because the D-Bus manager client - // object handles D-Bus signals. Otherwise, we might face a race when the manager object is - // released during a D-Bus signal being processed. - PlatformMgrImpl().GLibMatterContextInvokeSync( - +[](ChipDeviceScanner * self) { - if (self->mManager != nullptr) - g_object_unref(self->mManager); - if (self->mAdapter != nullptr) - g_object_unref(self->mAdapter); - if (self->mCancellable != nullptr) - g_object_unref(self->mCancellable); - return CHIP_NO_ERROR; - }, - this); + if (mCancellable != nullptr) + g_object_unref(mCancellable); mIsInitialized = false; } @@ -166,13 +142,13 @@ CHIP_ERROR ChipDeviceScanner::StopScan() if (mObjectAddedSignal) { - g_signal_handler_disconnect(mManager, mObjectAddedSignal); + g_signal_handler_disconnect(mEndpoint->GetObjectManager(), mObjectAddedSignal); mObjectAddedSignal = 0; } if (mInterfaceChangedSignal) { - g_signal_handler_disconnect(mManager, mInterfaceChangedSignal); + g_signal_handler_disconnect(mEndpoint->GetObjectManager(), mInterfaceChangedSignal); mInterfaceChangedSignal = 0; } @@ -194,7 +170,7 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStopScan(ChipDeviceScanner * self) { GAutoPtr error; - if (!bluez_adapter1_call_stop_discovery_sync(self->mAdapter, nullptr /* not cancellable */, + if (!bluez_adapter1_call_stop_discovery_sync(self->mEndpoint->GetAdapter(), nullptr /* not cancellable */, &MakeUniquePointerReceiver(error).Get())) { ChipLogError(Ble, "Failed to stop discovery %s", error->message); @@ -224,7 +200,7 @@ void ChipDeviceScanner::SignalInterfaceChanged(GDBusObjectManagerClient * manage void ChipDeviceScanner::ReportDevice(BluezDevice1 & device) { - if (strcmp(bluez_device1_get_adapter(&device), g_dbus_proxy_get_object_path(G_DBUS_PROXY(mAdapter))) != 0) + if (strcmp(bluez_device1_get_adapter(&device), g_dbus_proxy_get_object_path(G_DBUS_PROXY(mEndpoint->GetAdapter()))) != 0) { return; } @@ -242,7 +218,7 @@ void ChipDeviceScanner::ReportDevice(BluezDevice1 & device) void ChipDeviceScanner::RemoveDevice(BluezDevice1 & device) { - if (strcmp(bluez_device1_get_adapter(&device), g_dbus_proxy_get_object_path(G_DBUS_PROXY(mAdapter))) != 0) + if (strcmp(bluez_device1_get_adapter(&device), g_dbus_proxy_get_object_path(G_DBUS_PROXY(mEndpoint->GetAdapter()))) != 0) { return; } @@ -257,7 +233,8 @@ void ChipDeviceScanner::RemoveDevice(BluezDevice1 & device) const auto devicePath = g_dbus_proxy_get_object_path(G_DBUS_PROXY(&device)); GAutoPtr error; - if (!bluez_adapter1_call_remove_device_sync(mAdapter, devicePath, nullptr, &MakeUniquePointerReceiver(error).Get())) + if (!bluez_adapter1_call_remove_device_sync(mEndpoint->GetAdapter(), devicePath, nullptr, + &MakeUniquePointerReceiver(error).Get())) { ChipLogDetail(Ble, "Failed to remove device %s: %s", StringOrNullMarker(devicePath), error->message); } @@ -267,12 +244,13 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStartScan(ChipDeviceScanner * self) { GAutoPtr error; - self->mObjectAddedSignal = g_signal_connect(self->mManager, "object-added", G_CALLBACK(SignalObjectAdded), self); - self->mInterfaceChangedSignal = - g_signal_connect(self->mManager, "interface-proxy-properties-changed", G_CALLBACK(SignalInterfaceChanged), self); + self->mObjectAddedSignal = + g_signal_connect(self->mEndpoint->GetObjectManager(), "object-added", G_CALLBACK(SignalObjectAdded), self); + self->mInterfaceChangedSignal = g_signal_connect(self->mEndpoint->GetObjectManager(), "interface-proxy-properties-changed", + G_CALLBACK(SignalInterfaceChanged), self); ChipLogProgress(Ble, "BLE removing known devices."); - for (BluezObject & object : BluezObjectList(self->mManager)) + for (BluezObject & object : BluezObjectList(self->mEndpoint->GetObjectManager())) { GAutoPtr device(bluez_object_get_device1(&object)); if (device.get() != nullptr) @@ -292,7 +270,7 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStartScan(ChipDeviceScanner * self) g_variant_builder_add(&filterBuilder, "{sv}", "Transport", g_variant_new_string("le")); GVariant * filter = g_variant_builder_end(&filterBuilder); - if (!bluez_adapter1_call_set_discovery_filter_sync(self->mAdapter, filter, self->mCancellable, + if (!bluez_adapter1_call_set_discovery_filter_sync(self->mEndpoint->GetAdapter(), filter, self->mCancellable, &MakeUniquePointerReceiver(error).Get())) { // Not critical: ignore if fails @@ -301,7 +279,8 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStartScan(ChipDeviceScanner * self) } ChipLogProgress(Ble, "BLE initiating scan."); - if (!bluez_adapter1_call_start_discovery_sync(self->mAdapter, self->mCancellable, &MakeUniquePointerReceiver(error).Get())) + if (!bluez_adapter1_call_start_discovery_sync(self->mEndpoint->GetAdapter(), self->mCancellable, + &MakeUniquePointerReceiver(error).Get())) { ChipLogError(Ble, "Failed to start discovery: %s", error->message); diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.h b/src/platform/Linux/bluez/ChipDeviceScanner.h index 16b8c91682650c..50d754068cdd88 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.h +++ b/src/platform/Linux/bluez/ChipDeviceScanner.h @@ -30,6 +30,8 @@ namespace chip { namespace DeviceLayer { namespace Internal { +class BluezEndpoint; + /// Receives callbacks when chip devices are being scanned class ChipDeviceScannerDelegate { @@ -60,7 +62,7 @@ class ChipDeviceScanner ~ChipDeviceScanner() { Shutdown(); } /// Initialize the scanner. - CHIP_ERROR Init(BluezAdapter1 * adapter, ChipDeviceScannerDelegate * delegate); + CHIP_ERROR Init(const BluezEndpoint * aEndpoint, ChipDeviceScannerDelegate * delegate); /// Release any resources associated with the scanner. void Shutdown(); @@ -90,8 +92,7 @@ class ChipDeviceScanner /// so that it can be re-discovered if it's still advertising. void RemoveDevice(BluezDevice1 & device); - GDBusObjectManager * mManager = nullptr; - BluezAdapter1 * mAdapter = nullptr; + const BluezEndpoint * mEndpoint = nullptr; GCancellable * mCancellable = nullptr; ChipDeviceScannerDelegate * mDelegate = nullptr; gulong mObjectAddedSignal = 0; From dcef95b6e94c01e6cb539a43fafefe8407c22b46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Wed, 6 Dec 2023 15:25:13 +0100 Subject: [PATCH 2/8] Ensure the existence of the object in the compilation process. --- src/platform/Linux/BLEManagerImpl.cpp | 4 +-- src/platform/Linux/BLEManagerImpl.h | 10 +++--- .../Linux/bluez/BluezAdvertisement.cpp | 25 +++++++-------- src/platform/Linux/bluez/BluezAdvertisement.h | 9 +++--- .../Linux/bluez/ChipDeviceScanner.cpp | 32 ++++++++----------- src/platform/Linux/bluez/ChipDeviceScanner.h | 7 ++-- 6 files changed, 40 insertions(+), 47 deletions(-) diff --git a/src/platform/Linux/BLEManagerImpl.cpp b/src/platform/Linux/BLEManagerImpl.cpp index fdff640a4695c2..b66150accaffc3 100644 --- a/src/platform/Linux/BLEManagerImpl.cpp +++ b/src/platform/Linux/BLEManagerImpl.cpp @@ -595,7 +595,7 @@ void BLEManagerImpl::DriveBLEState() // Configure advertising data if it hasn't been done yet. if (!mFlags.Has(Flags::kAdvertisingConfigured)) { - SuccessOrExit(err = mBLEAdvertisement.Init(&mEndpoint, mBLEAdvType, mpBLEAdvUUID, mBLEAdvDurationMs)); + SuccessOrExit(err = mBLEAdvertisement.Init(mBLEAdvType, mpBLEAdvUUID, mBLEAdvDurationMs)); mFlags.Set(Flags::kAdvertisingConfigured); } @@ -668,7 +668,7 @@ void BLEManagerImpl::InitiateScan(BleScanState scanType) mBLEScanConfig.mBleScanState = scanType; - CHIP_ERROR err = mDeviceScanner.Init(&mEndpoint, this); + CHIP_ERROR err = mDeviceScanner.Init(this); if (err != CHIP_NO_ERROR) { mBLEScanConfig.mBleScanState = BleScanState::kNotScanning; diff --git a/src/platform/Linux/BLEManagerImpl.h b/src/platform/Linux/BLEManagerImpl.h index 5cd3ed2460adf9..cd2b90f11e3389 100644 --- a/src/platform/Linux/BLEManagerImpl.h +++ b/src/platform/Linux/BLEManagerImpl.h @@ -188,12 +188,12 @@ class BLEManagerImpl final : public BLEManager, bool mIsCentral = false; BluezEndpoint mEndpoint; - BluezAdvertisement mBLEAdvertisement; - ChipAdvType mBLEAdvType = ChipAdvType::BLUEZ_ADV_TYPE_UNDIRECTED_CONNECTABLE_SCANNABLE; - uint16_t mBLEAdvDurationMs = 20; - const char * mpBLEAdvUUID = nullptr; + BluezAdvertisement mBLEAdvertisement = BluezAdvertisement(mEndpoint); + ChipAdvType mBLEAdvType = ChipAdvType::BLUEZ_ADV_TYPE_UNDIRECTED_CONNECTABLE_SCANNABLE; + uint16_t mBLEAdvDurationMs = 20; + const char * mpBLEAdvUUID = nullptr; - ChipDeviceScanner mDeviceScanner; + ChipDeviceScanner mDeviceScanner = ChipDeviceScanner(mEndpoint); BLEScanConfig mBLEScanConfig; }; diff --git a/src/platform/Linux/bluez/BluezAdvertisement.cpp b/src/platform/Linux/bluez/BluezAdvertisement.cpp index f59f0429c09e5c..be920a51c303d1 100644 --- a/src/platform/Linux/bluez/BluezAdvertisement.cpp +++ b/src/platform/Linux/bluez/BluezAdvertisement.cpp @@ -62,7 +62,7 @@ BluezLEAdvertisement1 * BluezAdvertisement::CreateLEAdvertisement() g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, &mDeviceIdInfo, sizeof(mDeviceIdInfo), sizeof(uint8_t))); g_variant_builder_add(&serviceUUIDsBuilder, "s", mpAdvUUID); - localNamePtr = mEndpoint->GetAdapterName(); + localNamePtr = mEndpoint.GetAdapterName(); if (localNamePtr == nullptr) { g_snprintf(localName, sizeof(localName), "%s%04x", CHIP_DEVICE_CONFIG_BLE_DEVICE_NAME_PREFIX, getpid() & 0xffff); @@ -107,7 +107,7 @@ BluezLEAdvertisement1 * BluezAdvertisement::CreateLEAdvertisement() }), this); - g_dbus_object_manager_server_export(mEndpoint->GetGattApplicationObjectManager(), G_DBUS_OBJECT_SKELETON(object)); + g_dbus_object_manager_server_export(mEndpoint.GetGattApplicationObjectManager(), G_DBUS_OBJECT_SKELETON(object)); g_object_unref(object); return adv; @@ -116,7 +116,7 @@ BluezLEAdvertisement1 * BluezAdvertisement::CreateLEAdvertisement() gboolean BluezAdvertisement::BluezLEAdvertisement1Release(BluezLEAdvertisement1 * aAdv, GDBusMethodInvocation * aInvocation) { ChipLogDetail(DeviceLayer, "Release BLE adv object in %s", __func__); - g_dbus_object_manager_server_unexport(mEndpoint->GetGattApplicationObjectManager(), mpAdvPath); + g_dbus_object_manager_server_unexport(mEndpoint.GetGattApplicationObjectManager(), mpAdvPath); g_object_unref(mpAdv); mpAdv = nullptr; mIsAdvertising = false; @@ -133,8 +133,7 @@ CHIP_ERROR BluezAdvertisement::InitImpl() return CHIP_NO_ERROR; } -CHIP_ERROR BluezAdvertisement::Init(const BluezEndpoint * aEndpoint, ChipAdvType aAdvType, const char * aAdvUUID, - uint32_t aAdvDurationMs) +CHIP_ERROR BluezAdvertisement::Init(ChipAdvType aAdvType, const char * aAdvUUID, uint32_t aAdvDurationMs) { GAutoPtr rootPath; CHIP_ERROR err; @@ -142,9 +141,7 @@ CHIP_ERROR BluezAdvertisement::Init(const BluezEndpoint * aEndpoint, ChipAdvType VerifyOrExit(mpAdv == nullptr, err = CHIP_ERROR_INCORRECT_STATE; ChipLogError(DeviceLayer, "FAIL: BLE advertisement already initialized in %s", __func__)); - mEndpoint = aEndpoint; - - g_object_get(G_OBJECT(mEndpoint->GetGattApplicationObjectManager()), "object-path", &MakeUniquePointerReceiver(rootPath).Get(), + g_object_get(G_OBJECT(mEndpoint.GetGattApplicationObjectManager()), "object-path", &MakeUniquePointerReceiver(rootPath).Get(), nullptr); mpAdvPath = g_strdup_printf("%s/advertising", rootPath.get()); mAdvType = aAdvType; @@ -213,7 +210,7 @@ void BluezAdvertisement::StartDone(GObject * aObject, GAsyncResult * aResult) bluez_leadvertising_manager1_call_register_advertisement_finish(advMgr, aResult, &MakeUniquePointerReceiver(error).Get()); if (success == FALSE) { - g_dbus_object_manager_server_unexport(mEndpoint->GetGattApplicationObjectManager(), mpAdvPath); + g_dbus_object_manager_server_unexport(mEndpoint.GetGattApplicationObjectManager(), mpAdvPath); } VerifyOrExit(success == TRUE, ChipLogError(DeviceLayer, "FAIL: RegisterAdvertisement : %s", error->message)); @@ -233,9 +230,9 @@ CHIP_ERROR BluezAdvertisement::StartImpl() GVariant * options; VerifyOrExit(!mIsAdvertising, ChipLogError(DeviceLayer, "FAIL: Advertising has already been enabled in %s", __func__)); - VerifyOrExit(mEndpoint->GetAdapter() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); + VerifyOrExit(mEndpoint.GetAdapter() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); - adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mEndpoint->GetAdapter())); + adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mEndpoint.GetAdapter())); VerifyOrExit(adapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL adapter in %s", __func__)); advMgr = bluez_object_get_leadvertising_manager1(BLUEZ_OBJECT(adapter)); @@ -277,7 +274,7 @@ void BluezAdvertisement::StopDone(GObject * aObject, GAsyncResult * aResult) if (success == FALSE) { - g_dbus_object_manager_server_unexport(mEndpoint->GetGattApplicationObjectManager(), mpAdvPath); + g_dbus_object_manager_server_unexport(mEndpoint.GetGattApplicationObjectManager(), mpAdvPath); } else { @@ -298,9 +295,9 @@ CHIP_ERROR BluezAdvertisement::StopImpl() BluezLEAdvertisingManager1 * advMgr = nullptr; VerifyOrExit(mIsAdvertising, ChipLogError(DeviceLayer, "FAIL: Advertising has already been disabled in %s", __func__)); - VerifyOrExit(mEndpoint->GetAdapter() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); + VerifyOrExit(mEndpoint.GetAdapter() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); - adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mEndpoint->GetAdapter())); + adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mEndpoint.GetAdapter())); VerifyOrExit(adapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL adapter in %s", __func__)); advMgr = bluez_object_get_leadvertising_manager1(BLUEZ_OBJECT(adapter)); diff --git a/src/platform/Linux/bluez/BluezAdvertisement.h b/src/platform/Linux/bluez/BluezAdvertisement.h index cbbdd9ce8bf0d4..a8cc9a5e39c4ab 100644 --- a/src/platform/Linux/bluez/BluezAdvertisement.h +++ b/src/platform/Linux/bluez/BluezAdvertisement.h @@ -38,10 +38,11 @@ class BluezEndpoint; class BluezAdvertisement { public: - BluezAdvertisement() = default; + BluezAdvertisement() = delete; + BluezAdvertisement(const BluezEndpoint & endpoint) : mEndpoint(endpoint){}; ~BluezAdvertisement() { Shutdown(); } - CHIP_ERROR Init(const BluezEndpoint * aEndpoint, ChipAdvType aAdvType, const char * aAdvUUID, uint32_t aAdvDurationMs); + CHIP_ERROR Init(ChipAdvType aAdvType, const char * aAdvUUID, uint32_t aAdvDurationMs); void Shutdown(); /// Start BLE advertising. @@ -69,8 +70,8 @@ class BluezAdvertisement CHIP_ERROR StopImpl(); // Objects (interfaces) used by LE advertisement - const BluezEndpoint * mEndpoint = nullptr; - BluezLEAdvertisement1 * mpAdv = nullptr; + const BluezEndpoint & mEndpoint; + BluezLEAdvertisement1 * mpAdv = nullptr; bool mIsInitialized = false; bool mIsAdvertising = false; diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index 86b6baeea3918f..7e51a6d0d0f6b8 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -52,22 +52,15 @@ bool BluezGetChipDeviceInfo(BluezDevice1 & aDevice, chip::Ble::ChipBLEDeviceIden } // namespace -CHIP_ERROR ChipDeviceScanner::Init(const BluezEndpoint * aEndpoint, ChipDeviceScannerDelegate * delegate) +CHIP_ERROR ChipDeviceScanner::Init(ChipDeviceScannerDelegate * delegate) { // Make this function idempotent by shutting down previously initialized state if any. Shutdown(); - mEndpoint = aEndpoint; mCancellable = g_cancellable_new(); mDelegate = delegate; - g_cancellable_cancel(mCancellable); - - GAutoPtr err; - VerifyOrReturnError(mEndpoint->GetObjectManager() != nullptr, CHIP_ERROR_INTERNAL, - ChipLogError(Ble, "Failed to get D-Bus object manager for device scanning: %s", err->message)); - mIsInitialized = true; return CHIP_NO_ERROR; } @@ -142,13 +135,13 @@ CHIP_ERROR ChipDeviceScanner::StopScan() if (mObjectAddedSignal) { - g_signal_handler_disconnect(mEndpoint->GetObjectManager(), mObjectAddedSignal); + g_signal_handler_disconnect(mEndpoint.GetObjectManager(), mObjectAddedSignal); mObjectAddedSignal = 0; } if (mInterfaceChangedSignal) { - g_signal_handler_disconnect(mEndpoint->GetObjectManager(), mInterfaceChangedSignal); + g_signal_handler_disconnect(mEndpoint.GetObjectManager(), mInterfaceChangedSignal); mInterfaceChangedSignal = 0; } @@ -170,7 +163,7 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStopScan(ChipDeviceScanner * self) { GAutoPtr error; - if (!bluez_adapter1_call_stop_discovery_sync(self->mEndpoint->GetAdapter(), nullptr /* not cancellable */, + if (!bluez_adapter1_call_stop_discovery_sync(self->mEndpoint.GetAdapter(), nullptr /* not cancellable */, &MakeUniquePointerReceiver(error).Get())) { ChipLogError(Ble, "Failed to stop discovery %s", error->message); @@ -200,7 +193,7 @@ void ChipDeviceScanner::SignalInterfaceChanged(GDBusObjectManagerClient * manage void ChipDeviceScanner::ReportDevice(BluezDevice1 & device) { - if (strcmp(bluez_device1_get_adapter(&device), g_dbus_proxy_get_object_path(G_DBUS_PROXY(mEndpoint->GetAdapter()))) != 0) + if (strcmp(bluez_device1_get_adapter(&device), g_dbus_proxy_get_object_path(G_DBUS_PROXY(mEndpoint.GetAdapter()))) != 0) { return; } @@ -218,7 +211,7 @@ void ChipDeviceScanner::ReportDevice(BluezDevice1 & device) void ChipDeviceScanner::RemoveDevice(BluezDevice1 & device) { - if (strcmp(bluez_device1_get_adapter(&device), g_dbus_proxy_get_object_path(G_DBUS_PROXY(mEndpoint->GetAdapter()))) != 0) + if (strcmp(bluez_device1_get_adapter(&device), g_dbus_proxy_get_object_path(G_DBUS_PROXY(mEndpoint.GetAdapter()))) != 0) { return; } @@ -233,7 +226,7 @@ void ChipDeviceScanner::RemoveDevice(BluezDevice1 & device) const auto devicePath = g_dbus_proxy_get_object_path(G_DBUS_PROXY(&device)); GAutoPtr error; - if (!bluez_adapter1_call_remove_device_sync(mEndpoint->GetAdapter(), devicePath, nullptr, + if (!bluez_adapter1_call_remove_device_sync(mEndpoint.GetAdapter(), devicePath, nullptr, &MakeUniquePointerReceiver(error).Get())) { ChipLogDetail(Ble, "Failed to remove device %s: %s", StringOrNullMarker(devicePath), error->message); @@ -245,12 +238,12 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStartScan(ChipDeviceScanner * self) GAutoPtr error; self->mObjectAddedSignal = - g_signal_connect(self->mEndpoint->GetObjectManager(), "object-added", G_CALLBACK(SignalObjectAdded), self); - self->mInterfaceChangedSignal = g_signal_connect(self->mEndpoint->GetObjectManager(), "interface-proxy-properties-changed", + g_signal_connect(self->mEndpoint.GetObjectManager(), "object-added", G_CALLBACK(SignalObjectAdded), self); + self->mInterfaceChangedSignal = g_signal_connect(self->mEndpoint.GetObjectManager(), "interface-proxy-properties-changed", G_CALLBACK(SignalInterfaceChanged), self); ChipLogProgress(Ble, "BLE removing known devices."); - for (BluezObject & object : BluezObjectList(self->mEndpoint->GetObjectManager())) + for (BluezObject & object : BluezObjectList(self->mEndpoint.GetObjectManager())) { GAutoPtr device(bluez_object_get_device1(&object)); if (device.get() != nullptr) @@ -270,7 +263,8 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStartScan(ChipDeviceScanner * self) g_variant_builder_add(&filterBuilder, "{sv}", "Transport", g_variant_new_string("le")); GVariant * filter = g_variant_builder_end(&filterBuilder); - if (!bluez_adapter1_call_set_discovery_filter_sync(self->mEndpoint->GetAdapter(), filter, self->mCancellable, + ChipLogProgress(Ble, "BLE setting discovery Name adapter. %s", self->mEndpoint.GetAdapterName()); + if (!bluez_adapter1_call_set_discovery_filter_sync(self->mEndpoint.GetAdapter(), filter, self->mCancellable, &MakeUniquePointerReceiver(error).Get())) { // Not critical: ignore if fails @@ -279,7 +273,7 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStartScan(ChipDeviceScanner * self) } ChipLogProgress(Ble, "BLE initiating scan."); - if (!bluez_adapter1_call_start_discovery_sync(self->mEndpoint->GetAdapter(), self->mCancellable, + if (!bluez_adapter1_call_start_discovery_sync(self->mEndpoint.GetAdapter(), self->mCancellable, &MakeUniquePointerReceiver(error).Get())) { ChipLogError(Ble, "Failed to start discovery: %s", error->message); diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.h b/src/platform/Linux/bluez/ChipDeviceScanner.h index 50d754068cdd88..eea4acdf75964d 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.h +++ b/src/platform/Linux/bluez/ChipDeviceScanner.h @@ -54,7 +54,8 @@ class ChipDeviceScannerDelegate class ChipDeviceScanner { public: - ChipDeviceScanner() = default; + ChipDeviceScanner() = delete; + ChipDeviceScanner(const BluezEndpoint & endpoint) : mEndpoint(endpoint){}; ChipDeviceScanner(ChipDeviceScanner &&) = default; ChipDeviceScanner(const ChipDeviceScanner &) = delete; ChipDeviceScanner & operator=(const ChipDeviceScanner &) = delete; @@ -62,7 +63,7 @@ class ChipDeviceScanner ~ChipDeviceScanner() { Shutdown(); } /// Initialize the scanner. - CHIP_ERROR Init(const BluezEndpoint * aEndpoint, ChipDeviceScannerDelegate * delegate); + CHIP_ERROR Init(ChipDeviceScannerDelegate * delegate); /// Release any resources associated with the scanner. void Shutdown(); @@ -92,7 +93,7 @@ class ChipDeviceScanner /// so that it can be re-discovered if it's still advertising. void RemoveDevice(BluezDevice1 & device); - const BluezEndpoint * mEndpoint = nullptr; + const BluezEndpoint & mEndpoint; GCancellable * mCancellable = nullptr; ChipDeviceScannerDelegate * mDelegate = nullptr; gulong mObjectAddedSignal = 0; From 31c2ef080590d7660122b5103116f7161cb441bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Tue, 12 Dec 2023 10:28:10 +0100 Subject: [PATCH 3/8] Convert glib object to GAutoPtr<> in Bluez --- src/platform/GLibTypeDeleter.h | 18 +++ src/platform/Linux/bluez/AdapterIterator.cpp | 28 +--- src/platform/Linux/bluez/AdapterIterator.h | 10 +- .../Linux/bluez/BluezAdvertisement.cpp | 8 +- src/platform/Linux/bluez/BluezAdvertisement.h | 7 +- src/platform/Linux/bluez/BluezConnection.cpp | 77 ++++------ src/platform/Linux/bluez/BluezConnection.h | 18 ++- src/platform/Linux/bluez/BluezEndpoint.cpp | 142 ++++++++---------- src/platform/Linux/bluez/BluezEndpoint.h | 26 ++-- .../Linux/bluez/ChipDeviceScanner.cpp | 11 +- src/platform/Linux/bluez/ChipDeviceScanner.h | 4 +- src/platform/Linux/bluez/Types.h | 23 +++ 12 files changed, 185 insertions(+), 187 deletions(-) diff --git a/src/platform/GLibTypeDeleter.h b/src/platform/GLibTypeDeleter.h index 93ff40f61c111d..366f3a2574d00d 100644 --- a/src/platform/GLibTypeDeleter.h +++ b/src/platform/GLibTypeDeleter.h @@ -114,6 +114,24 @@ struct GAutoPtrDeleter using deleter = GObjectDeleter; }; +template <> +struct GAutoPtrDeleter +{ + using deleter = GObjectDeleter; +}; + +template <> +struct GAutoPtrDeleter +{ + using deleter = GObjectDeleter; +}; + +template <> +struct GAutoPtrDeleter +{ + using deleter = GObjectDeleter; +}; + template <> struct GAutoPtrDeleter { diff --git a/src/platform/Linux/bluez/AdapterIterator.cpp b/src/platform/Linux/bluez/AdapterIterator.cpp index 3197be4e73e175..40acfc2aaca8e2 100644 --- a/src/platform/Linux/bluez/AdapterIterator.cpp +++ b/src/platform/Linux/bluez/AdapterIterator.cpp @@ -28,21 +28,10 @@ namespace Internal { AdapterIterator::~AdapterIterator() { - if (mManager != nullptr) - { - g_object_unref(mManager); - } - if (mObjectList != nullptr) { g_list_free_full(mObjectList, g_object_unref); } - - if (mCurrent.adapter != nullptr) - { - g_object_unref(mCurrent.adapter); - mCurrent.adapter = nullptr; - } } CHIP_ERROR AdapterIterator::Initialize(AdapterIterator * self) @@ -54,15 +43,16 @@ CHIP_ERROR AdapterIterator::Initialize(AdapterIterator * self) CHIP_ERROR err = CHIP_NO_ERROR; GAutoPtr error; - self->mManager = g_dbus_object_manager_client_new_for_bus_sync( + self->mManager = GAutoPtr(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 */, &MakeUniquePointerReceiver(error).Get()); + nullptr /*destroy notify */, nullptr /* cancellable */, &MakeUniquePointerReceiver(error).Get())); - VerifyOrExit(self->mManager != nullptr, ChipLogError(DeviceLayer, "Failed to get DBUS object manager for listing adapters."); + VerifyOrExit(self->mManager.get() != nullptr, + ChipLogError(DeviceLayer, "Failed to get DBUS object manager for listing adapters."); err = CHIP_ERROR_INTERNAL); - self->mObjectList = g_dbus_object_manager_get_objects(self->mManager); + self->mObjectList = g_dbus_object_manager_get_objects(self->mManager.get()); self->mCurrentListItem = self->mObjectList; exit: @@ -102,18 +92,14 @@ bool AdapterIterator::Advance() index = 0; } - if (mCurrent.adapter != nullptr) - { - g_object_unref(mCurrent.adapter); - mCurrent.adapter = nullptr; - } + mCurrent.adapter = nullptr; mCurrent.index = index; mCurrent.address = bluez_adapter1_get_address(adapter); mCurrent.alias = bluez_adapter1_get_alias(adapter); mCurrent.name = bluez_adapter1_get_name(adapter); mCurrent.powered = bluez_adapter1_get_powered(adapter); - mCurrent.adapter = adapter; + mCurrent.adapter = GAutoPtr(adapter); mCurrentListItem = mCurrentListItem->next; diff --git a/src/platform/Linux/bluez/AdapterIterator.h b/src/platform/Linux/bluez/AdapterIterator.h index 85697b5d544c9a..b8ad8bacc40e29 100644 --- a/src/platform/Linux/bluez/AdapterIterator.h +++ b/src/platform/Linux/bluez/AdapterIterator.h @@ -61,7 +61,7 @@ class AdapterIterator const char * GetAlias() const { return mCurrent.alias.c_str(); } const char * GetName() const { return mCurrent.name.c_str(); } bool IsPowered() const { return mCurrent.powered; } - BluezAdapter1 * GetAdapter() const { return mCurrent.adapter; } + BluezAdapter1 * GetAdapter() const { return mCurrent.adapter.get(); } private: /// Sets up the DBUS manager and loads the list @@ -76,9 +76,9 @@ class AdapterIterator static constexpr size_t kMaxAddressLength = 19; // xx:xx:xx:xx:xx:xx static constexpr size_t kMaxNameLength = 64; - GDBusObjectManager * mManager = nullptr; // DBus connection - GList * mObjectList = nullptr; // listing of objects on the bus - GList * mCurrentListItem = nullptr; // current item viewed in the list + GAutoPtr mManager; // DBus connection + GList * mObjectList = nullptr; // listing of objects on the bus + GList * mCurrentListItem = nullptr; // current item viewed in the list // data valid only if Next() returns true struct @@ -88,7 +88,7 @@ class AdapterIterator std::string alias; std::string name; bool powered; - BluezAdapter1 * adapter; + GAutoPtr adapter; } mCurrent = { 0 }; }; diff --git a/src/platform/Linux/bluez/BluezAdvertisement.cpp b/src/platform/Linux/bluez/BluezAdvertisement.cpp index be920a51c303d1..73ce801ed22c5c 100644 --- a/src/platform/Linux/bluez/BluezAdvertisement.cpp +++ b/src/platform/Linux/bluez/BluezAdvertisement.cpp @@ -117,7 +117,6 @@ gboolean BluezAdvertisement::BluezLEAdvertisement1Release(BluezLEAdvertisement1 { ChipLogDetail(DeviceLayer, "Release BLE adv object in %s", __func__); g_dbus_object_manager_server_unexport(mEndpoint.GetGattApplicationObjectManager(), mpAdvPath); - g_object_unref(mpAdv); mpAdv = nullptr; mIsAdvertising = false; return TRUE; @@ -129,7 +128,7 @@ CHIP_ERROR BluezAdvertisement::InitImpl() // all D-Bus signals will be delivered to the GLib global default main context. VerifyOrDie(g_main_context_get_thread_default() != nullptr); - mpAdv = CreateLEAdvertisement(); + mpAdv = GAutoPtr(CreateLEAdvertisement()); return CHIP_NO_ERROR; } @@ -138,7 +137,7 @@ CHIP_ERROR BluezAdvertisement::Init(ChipAdvType aAdvType, const char * aAdvUUID, GAutoPtr rootPath; CHIP_ERROR err; - VerifyOrExit(mpAdv == nullptr, err = CHIP_ERROR_INCORRECT_STATE; + VerifyOrExit(mpAdv.get() == nullptr, err = CHIP_ERROR_INCORRECT_STATE; ChipLogError(DeviceLayer, "FAIL: BLE advertisement already initialized in %s", __func__)); g_object_get(G_OBJECT(mEndpoint.GetGattApplicationObjectManager()), "object-path", &MakeUniquePointerReceiver(rootPath).Get(), @@ -183,9 +182,8 @@ void BluezAdvertisement::Shutdown() // attached to the advertising object that may run on the glib thread. PlatformMgrImpl().GLibMatterContextInvokeSync( +[](BluezAdvertisement * self) { - if (self->mpAdv != nullptr) + if (self->mpAdv.get() != nullptr) { - g_object_unref(self->mpAdv); self->mpAdv = nullptr; } return CHIP_NO_ERROR; diff --git a/src/platform/Linux/bluez/BluezAdvertisement.h b/src/platform/Linux/bluez/BluezAdvertisement.h index a8cc9a5e39c4ab..aef175b6b17a8a 100644 --- a/src/platform/Linux/bluez/BluezAdvertisement.h +++ b/src/platform/Linux/bluez/BluezAdvertisement.h @@ -71,15 +71,14 @@ class BluezAdvertisement // Objects (interfaces) used by LE advertisement const BluezEndpoint & mEndpoint; - BluezLEAdvertisement1 * mpAdv = nullptr; + GAutoPtr mpAdv; bool mIsInitialized = false; bool mIsAdvertising = false; Ble::ChipBLEDeviceIdentificationInfo mDeviceIdInfo; - char * mpAdvPath = nullptr; - char * mpAdapterName = nullptr; - char * mpAdvUUID = nullptr; + char * mpAdvPath = nullptr; + char * mpAdvUUID = nullptr; ChipAdvType mAdvType; uint16_t mAdvDurationMs = 0; }; diff --git a/src/platform/Linux/bluez/BluezConnection.cpp b/src/platform/Linux/bluez/BluezConnection.cpp index 39b5bb0bdb8ba6..82a0cb8b7a7b10 100644 --- a/src/platform/Linux/bluez/BluezConnection.cpp +++ b/src/platform/Linux/bluez/BluezConnection.cpp @@ -34,7 +34,6 @@ #include #include "BluezEndpoint.h" -#include "Types.h" namespace chip { namespace DeviceLayer { @@ -65,20 +64,7 @@ BluezConnection::BluezConnection(const BluezEndpoint & aEndpoint, BluezDevice1 * Init(aEndpoint); } -BluezConnection::~BluezConnection() -{ - g_object_unref(mpDevice); - if (mpService) - g_object_unref(mpService); - if (mpC1) - g_object_unref(mpC1); - if (mpC2) - g_object_unref(mpC2); -#if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING - if (mpC3) - g_object_unref(mpC2); -#endif -} +BluezConnection::~BluezConnection() {} BluezConnection::IOChannel::~IOChannel() { @@ -89,8 +75,7 @@ BluezConnection::IOChannel::~IOChannel() BluezConnection::ConnectionDataBundle::ConnectionDataBundle(const BluezConnection & aConn, const chip::System::PacketBufferHandle & aBuf) : - mConn(aConn), - mData(g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, aBuf->Start(), aBuf->DataLength(), sizeof(uint8_t))) + mConn(aConn), mData(g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, aBuf->Start(), aBuf->DataLength(), sizeof(uint8_t))) {} CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) @@ -101,13 +86,13 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) if (!aEndpoint.mIsCentral) { - mpService = BLUEZ_GATT_SERVICE1(g_object_ref(aEndpoint.mpService)); - mpC1 = BLUEZ_GATT_CHARACTERISTIC1(g_object_ref(aEndpoint.mpC1)); - mpC2 = BLUEZ_GATT_CHARACTERISTIC1(g_object_ref(aEndpoint.mpC2)); + mpService = GAutoPtr(BLUEZ_GATT_SERVICE1(g_object_ref(aEndpoint.mpService.get()))); + mpC1 = GAutoPtr(BLUEZ_GATT_CHARACTERISTIC1(g_object_ref(aEndpoint.mpC1.get()))); + mpC2 = GAutoPtr(BLUEZ_GATT_CHARACTERISTIC1(g_object_ref(aEndpoint.mpC2.get()))); } else { - objects = g_dbus_object_manager_get_objects(aEndpoint.mpObjMgr); + objects = g_dbus_object_manager_get_objects(aEndpoint.mpObjMgr.get()); for (l = objects; l != nullptr; l = l->next) { @@ -116,17 +101,17 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) if (service != nullptr) { - if ((BluezIsServiceOnDevice(service, mpDevice)) == TRUE && + if ((BluezIsServiceOnDevice(service, mpDevice.get())) == TRUE && (strcmp(bluez_gatt_service1_get_uuid(service), CHIP_BLE_UUID_SERVICE_STRING) == 0)) { - mpService = service; + mpService = GAutoPtr(service); break; } g_object_unref(service); } } - VerifyOrExit(mpService != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL service in %s", __func__)); + VerifyOrExit(mpService.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL service in %s", __func__)); for (l = objects; l != nullptr; l = l->next) { @@ -135,21 +120,21 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) if (char1 != nullptr) { - if ((BluezIsCharOnService(char1, mpService) == TRUE) && + if ((BluezIsCharOnService(char1, mpService.get()) == TRUE) && (strcmp(bluez_gatt_characteristic1_get_uuid(char1), CHIP_PLAT_BLE_UUID_C1_STRING) == 0)) { - mpC1 = char1; + mpC1 = GAutoPtr(char1); } - else if ((BluezIsCharOnService(char1, mpService) == TRUE) && + else if ((BluezIsCharOnService(char1, mpService.get()) == TRUE) && (strcmp(bluez_gatt_characteristic1_get_uuid(char1), CHIP_PLAT_BLE_UUID_C2_STRING) == 0)) { - mpC2 = char1; + mpC2 = GAutoPtr(char1); } #if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING - else if ((BluezIsCharOnService(char1, mpService) == TRUE) && + else if ((BluezIsCharOnService(char1, mpService.get()) == TRUE) && (strcmp(bluez_gatt_characteristic1_get_uuid(char1), CHIP_PLAT_BLE_UUID_C3_STRING) == 0)) { - mpC3 = char1; + mpC3 = GAutoPtr(char1); } #endif else @@ -163,8 +148,8 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) } } - VerifyOrExit(mpC1 != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL C1 in %s", __func__)); - VerifyOrExit(mpC2 != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL C2 in %s", __func__)); + VerifyOrExit(mpC1.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL C1 in %s", __func__)); + VerifyOrExit(mpC2.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL C2 in %s", __func__)); } exit: @@ -180,7 +165,7 @@ CHIP_ERROR BluezConnection::BluezDisconnect(BluezConnection * conn) ChipLogDetail(DeviceLayer, "%s peer=%s", __func__, conn->GetPeerAddress()); - success = bluez_device1_call_disconnect_sync(conn->mpDevice, nullptr, &MakeUniquePointerReceiver(error).Get()); + success = bluez_device1_call_disconnect_sync(conn->mpDevice.get(), nullptr, &MakeUniquePointerReceiver(error).Get()); VerifyOrExit(success == TRUE, ChipLogError(DeviceLayer, "FAIL: Disconnect: %s", error->message)); exit: @@ -194,7 +179,7 @@ CHIP_ERROR BluezConnection::CloseConnection() const char * BluezConnection::GetPeerAddress() const { - return bluez_device1_get_address(mpDevice); + return bluez_device1_get_address(mpDevice.get()); } gboolean BluezConnection::WriteHandlerCallback(GIOChannel * aChannel, GIOCondition aCond, BluezConnection * apConn) @@ -216,7 +201,7 @@ gboolean BluezConnection::WriteHandlerCallback(GIOChannel * aChannel, GIOConditi // Casting len to size_t is safe, since we ensured that it's not negative. newVal = g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, buf, static_cast(len), sizeof(uint8_t)); - bluez_gatt_characteristic1_set_value(apConn->mpC1, newVal); + bluez_gatt_characteristic1_set_value(apConn->mpC1.get(), newVal); BLEManagerImpl::HandleRXCharWrite(apConn, buf, static_cast(len)); isSuccess = true; @@ -278,7 +263,7 @@ CHIP_ERROR BluezConnection::SendIndicationImpl(ConnectionDataBundle * data) GAutoPtr error; size_t len, written; - if (bluez_gatt_characteristic1_get_notify_acquired(data->mConn.mpC2) == TRUE) + if (bluez_gatt_characteristic1_get_notify_acquired(data->mConn.mpC2.get()) == TRUE) { auto * buf = static_cast(g_variant_get_fixed_array(data->mData.get(), &len, sizeof(uint8_t))); VerifyOrExit(len <= static_cast(std::numeric_limits::max()), @@ -289,7 +274,7 @@ CHIP_ERROR BluezConnection::SendIndicationImpl(ConnectionDataBundle * data) } else { - bluez_gatt_characteristic1_set_value(data->mConn.mpC2, data->mData.release()); + bluez_gatt_characteristic1_set_value(data->mConn.mpC2.get(), data->mData.release()); } exit: @@ -299,7 +284,7 @@ CHIP_ERROR BluezConnection::SendIndicationImpl(ConnectionDataBundle * data) CHIP_ERROR BluezConnection::SendIndication(chip::System::PacketBufferHandle apBuf) { VerifyOrReturnError(!apBuf.IsNull(), CHIP_ERROR_INVALID_ARGUMENT, ChipLogError(DeviceLayer, "apBuf is NULL in %s", __func__)); - VerifyOrReturnError(mpC2 != nullptr, CHIP_ERROR_INTERNAL, ChipLogError(DeviceLayer, "C2 is NULL in %s", __func__)); + VerifyOrReturnError(mpC2.get() != nullptr, CHIP_ERROR_INTERNAL, ChipLogError(DeviceLayer, "C2 is NULL in %s", __func__)); ConnectionDataBundle bundle(*this, apBuf); return PlatformMgrImpl().GLibMatterContextInvokeSync(SendIndicationImpl, &bundle); @@ -325,8 +310,8 @@ CHIP_ERROR BluezConnection::SendWriteRequestImpl(ConnectionDataBundle * data) g_variant_builder_add(&optionsBuilder, "{sv}", "type", g_variant_new_string("request")); auto options = g_variant_builder_end(&optionsBuilder); - bluez_gatt_characteristic1_call_write_value(data->mConn.mpC1, data->mData.release(), options, nullptr, SendWriteRequestDone, - const_cast(&data->mConn)); + bluez_gatt_characteristic1_call_write_value(data->mConn.mpC1.get(), data->mData.release(), options, nullptr, + SendWriteRequestDone, const_cast(&data->mConn)); return CHIP_NO_ERROR; } @@ -334,7 +319,7 @@ CHIP_ERROR BluezConnection::SendWriteRequestImpl(ConnectionDataBundle * data) CHIP_ERROR BluezConnection::SendWriteRequest(chip::System::PacketBufferHandle apBuf) { VerifyOrReturnError(!apBuf.IsNull(), CHIP_ERROR_INVALID_ARGUMENT, ChipLogError(DeviceLayer, "apBuf is NULL in %s", __func__)); - VerifyOrReturnError(mpC1 != nullptr, CHIP_ERROR_INTERNAL, ChipLogError(DeviceLayer, "C1 is NULL in %s", __func__)); + VerifyOrReturnError(mpC1.get() != nullptr, CHIP_ERROR_INTERNAL, ChipLogError(DeviceLayer, "C1 is NULL in %s", __func__)); ConnectionDataBundle bundle(*this, apBuf); return PlatformMgrImpl().GLibMatterContextInvokeSync(SendWriteRequestImpl, &bundle); @@ -370,12 +355,12 @@ void BluezConnection::SubscribeCharacteristicDone(GObject * aObject, GAsyncResul CHIP_ERROR BluezConnection::SubscribeCharacteristicImpl(BluezConnection * connection) { BluezGattCharacteristic1 * c2 = nullptr; - VerifyOrExit(connection->mpC2 != nullptr, ChipLogError(DeviceLayer, "C2 is NULL in %s", __func__)); - c2 = BLUEZ_GATT_CHARACTERISTIC1(connection->mpC2); + VerifyOrExit(connection->mpC2.get() != nullptr, ChipLogError(DeviceLayer, "C2 is NULL in %s", __func__)); + c2 = BLUEZ_GATT_CHARACTERISTIC1(connection->mpC2.get()); // Get notifications on the TX characteristic change (e.g. indication is received) g_signal_connect(c2, "g-properties-changed", G_CALLBACK(OnCharacteristicChanged), connection); - bluez_gatt_characteristic1_call_start_notify(connection->mpC2, nullptr, SubscribeCharacteristicDone, connection); + bluez_gatt_characteristic1_call_start_notify(connection->mpC2.get(), nullptr, SubscribeCharacteristicDone, connection); exit: return CHIP_NO_ERROR; @@ -403,9 +388,9 @@ void BluezConnection::UnsubscribeCharacteristicDone(GObject * aObject, GAsyncRes CHIP_ERROR BluezConnection::UnsubscribeCharacteristicImpl(BluezConnection * connection) { - VerifyOrExit(connection->mpC2 != nullptr, ChipLogError(DeviceLayer, "C2 is NULL in %s", __func__)); + VerifyOrExit(connection->mpC2.get() != nullptr, ChipLogError(DeviceLayer, "C2 is NULL in %s", __func__)); - bluez_gatt_characteristic1_call_stop_notify(connection->mpC2, nullptr, UnsubscribeCharacteristicDone, connection); + bluez_gatt_characteristic1_call_stop_notify(connection->mpC2.get(), nullptr, UnsubscribeCharacteristicDone, connection); exit: return CHIP_NO_ERROR; diff --git a/src/platform/Linux/bluez/BluezConnection.h b/src/platform/Linux/bluez/BluezConnection.h index fa8a6392fa1ec5..babca3f138fc6a 100644 --- a/src/platform/Linux/bluez/BluezConnection.h +++ b/src/platform/Linux/bluez/BluezConnection.h @@ -28,6 +28,8 @@ #include #include +#include "Types.h" + namespace chip { namespace DeviceLayer { namespace Internal { @@ -115,20 +117,20 @@ class BluezConnection static void UnsubscribeCharacteristicDone(GObject * aObject, GAsyncResult * aResult, gpointer apConn); static CHIP_ERROR UnsubscribeCharacteristicImpl(BluezConnection * apConn); - BluezDevice1 * mpDevice; + GAutoPtr mpDevice; bool mNotifyAcquired = false; uint16_t mMtu = 0; - BluezGattService1 * mpService = nullptr; + GAutoPtr mpService; - BluezGattCharacteristic1 * mpC1 = nullptr; - IOChannel mC1Channel = { 0 }; - BluezGattCharacteristic1 * mpC2 = nullptr; - IOChannel mC2Channel = { 0 }; + GAutoPtr mpC1; + IOChannel mC1Channel = { 0 }; + GAutoPtr mpC2; + IOChannel mC2Channel = { 0 }; #if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING - BluezGattCharacteristic1 * mpC3 = nullptr; - IOChannel mC3Channel = { 0 }; + GAutoPtr mpC3; + IOChannel mC3Channel = { 0 }; #endif }; diff --git a/src/platform/Linux/bluez/BluezEndpoint.cpp b/src/platform/Linux/bluez/BluezEndpoint.cpp index 4c315f11a7d581..19a68ba38c736e 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.cpp +++ b/src/platform/Linux/bluez/BluezEndpoint.cpp @@ -252,7 +252,7 @@ BluezGattCharacteristic1 * BluezEndpoint::CreateGattCharacteristic(BluezGattServ bluez_gatt_characteristic1_set_value(characteristic, value); bluez_object_skeleton_set_gatt_characteristic1(object, characteristic); - g_dbus_object_manager_server_export(mpRoot, G_DBUS_OBJECT_SKELETON(object)); + g_dbus_object_manager_server_export(mpRoot.get(), G_DBUS_OBJECT_SKELETON(object)); g_object_unref(object); return characteristic; @@ -282,9 +282,9 @@ CHIP_ERROR BluezEndpoint::RegisterGattApplicationImpl() GVariantBuilder optionsBuilder; GVariant * options; - VerifyOrExit(mpAdapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); + VerifyOrExit(mpAdapter.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); - adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mpAdapter)); + adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mpAdapter.get())); VerifyOrExit(adapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL adapter in %s", __func__)); gattMgr = bluez_object_get_gatt_manager1(BLUEZ_OBJECT(adapter)); @@ -344,11 +344,11 @@ void BluezEndpoint::BluezSignalInterfacePropertiesChanged(GDBusObjectManagerClie GDBusProxy * aInterface, GVariant * aChangedProperties, const char * const * aInvalidatedProps) { - VerifyOrReturn(mpAdapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); + VerifyOrReturn(mpAdapter.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); VerifyOrReturn(strcmp(g_dbus_proxy_get_interface_name(aInterface), DEVICE_INTERFACE) == 0, ); BluezDevice1 * device = BLUEZ_DEVICE1(aInterface); - VerifyOrReturn(BluezIsDeviceOnAdapter(device, mpAdapter)); + VerifyOrReturn(BluezIsDeviceOnAdapter(device, mpAdapter.get())); UpdateConnectionTable(device); } @@ -386,7 +386,7 @@ void BluezEndpoint::BluezSignalOnObjectAdded(GDBusObjectManager * aManager, GDBu GAutoPtr device(bluez_object_get_device1(BLUEZ_OBJECT(aObject))); VerifyOrReturn(device.get() != nullptr); - if (BluezIsDeviceOnAdapter(device.get(), mpAdapter) == TRUE) + if (BluezIsDeviceOnAdapter(device.get(), mpAdapter.get()) == TRUE) { HandleNewDevice(device.get()); } @@ -395,7 +395,7 @@ void BluezEndpoint::BluezSignalOnObjectAdded(GDBusObjectManager * aManager, GDBu void BluezEndpoint::BluezSignalOnObjectRemoved(GDBusObjectManager * aManager, GDBusObject * aObject) { // TODO: for Device1, lookup connection, and call otPlatTobleHandleDisconnected - // for Adapter1: unclear, crash if this pertains to our adapter? at least null out the self->mpAdapter. + // for Adapter1: unclear, crash if this pertains to our adapter? at least null out the self->mpAdapter.get(). // for Characteristic1, or GattService -- handle here via calling otPlatTobleHandleDisconnected, or ignore. } @@ -415,7 +415,7 @@ BluezGattService1 * BluezEndpoint::CreateGattService(const char * aUUID) // includes -- unclear whether required. Might be filled in later bluez_object_skeleton_set_gatt_service1(object, service); - g_dbus_object_manager_server_export(mpRoot, G_DBUS_OBJECT_SKELETON(object)); + g_dbus_object_manager_server_export(mpRoot.get(), G_DBUS_OBJECT_SKELETON(object)); g_object_unref(object); return service; @@ -426,8 +426,8 @@ void BluezEndpoint::SetupAdapter() char expectedPath[32]; snprintf(expectedPath, sizeof(expectedPath), BLUEZ_PATH "/hci%u", mAdapterId); - GList * objects = g_dbus_object_manager_get_objects(mpObjMgr); - for (auto l = objects; l != nullptr && mpAdapter == nullptr; l = l->next) + GList * objects = g_dbus_object_manager_get_objects(mpObjMgr.get()); + for (auto l = objects; l != nullptr && mpAdapter.get() == nullptr; l = l->next) { BluezObject * object = BLUEZ_OBJECT(l->data); @@ -441,14 +441,14 @@ void BluezEndpoint::SetupAdapter() { if (strcmp(g_dbus_proxy_get_object_path(G_DBUS_PROXY(adapter)), expectedPath) == 0) { - mpAdapter = static_cast(g_object_ref(adapter)); + mpAdapter = GAutoPtr(g_object_ref(adapter)); } } else { if (strcmp(bluez_adapter1_get_address(adapter), mpAdapterAddr) == 0) { - mpAdapter = static_cast(g_object_ref(adapter)); + mpAdapter = GAutoPtr(g_object_ref(adapter)); } } } @@ -456,14 +456,14 @@ void BluezEndpoint::SetupAdapter() g_list_free_full(interfaces, g_object_unref); } - VerifyOrExit(mpAdapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); + VerifyOrExit(mpAdapter.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); - bluez_adapter1_set_powered(mpAdapter, TRUE); + bluez_adapter1_set_powered(mpAdapter.get(), TRUE); // Setting "Discoverable" to False on the adapter and to True on the advertisement convinces // Bluez to set "BR/EDR Not Supported" flag. Bluez doesn't provide API to do that explicitly // and the flag is necessary to force using LE transport. - bluez_adapter1_set_discoverable(mpAdapter, FALSE); + bluez_adapter1_set_discoverable(mpAdapter.get(), FALSE); exit: g_list_free_full(objects, g_object_unref); @@ -534,63 +534,66 @@ void BluezEndpoint::SetupGattService() static const char * const c3_flags[] = { "read", nullptr }; #endif - mpService = CreateGattService(CHIP_BLE_UUID_SERVICE_SHORT_STRING); + mpService = GAutoPtr(CreateGattService(CHIP_BLE_UUID_SERVICE_SHORT_STRING)); // C1 characteristic - mpC1 = CreateGattCharacteristic(mpService, "c1", CHIP_PLAT_BLE_UUID_C1_STRING, c1_flags); - g_signal_connect(mpC1, "handle-read-value", + mpC1 = + GAutoPtr(CreateGattCharacteristic(mpService.get(), "c1", CHIP_PLAT_BLE_UUID_C1_STRING, c1_flags)); + g_signal_connect(mpC1.get(), "handle-read-value", G_CALLBACK(+[](BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, GVariant * aOpt, BluezEndpoint * self) { return self->BluezCharacteristicReadValue(aChar, aInv, aOpt); }), this); - g_signal_connect(mpC1, "handle-acquire-write", + g_signal_connect(mpC1.get(), "handle-acquire-write", G_CALLBACK(+[](BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, GVariant * aOpt, BluezEndpoint * self) { return self->BluezCharacteristicAcquireWrite(aChar, aInv, aOpt); }), this); - g_signal_connect(mpC1, "handle-acquire-notify", G_CALLBACK(BluezCharacteristicAcquireNotifyError), nullptr); - g_signal_connect(mpC1, "handle-confirm", G_CALLBACK(BluezCharacteristicConfirmError), nullptr); + g_signal_connect(mpC1.get(), "handle-acquire-notify", G_CALLBACK(BluezCharacteristicAcquireNotifyError), nullptr); + g_signal_connect(mpC1.get(), "handle-confirm", G_CALLBACK(BluezCharacteristicConfirmError), nullptr); // C2 characteristic - mpC2 = CreateGattCharacteristic(mpService, "c2", CHIP_PLAT_BLE_UUID_C2_STRING, c2_flags); - g_signal_connect(mpC2, "handle-read-value", + mpC2 = + GAutoPtr(CreateGattCharacteristic(mpService.get(), "c2", CHIP_PLAT_BLE_UUID_C2_STRING, c2_flags)); + g_signal_connect(mpC2.get(), "handle-read-value", G_CALLBACK(+[](BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, GVariant * aOpt, BluezEndpoint * self) { return self->BluezCharacteristicReadValue(aChar, aInv, aOpt); }), this); - g_signal_connect(mpC2, "handle-acquire-write", G_CALLBACK(BluezCharacteristicAcquireWriteError), nullptr); - g_signal_connect(mpC2, "handle-acquire-notify", + g_signal_connect(mpC2.get(), "handle-acquire-write", G_CALLBACK(BluezCharacteristicAcquireWriteError), nullptr); + g_signal_connect(mpC2.get(), "handle-acquire-notify", G_CALLBACK(+[](BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, GVariant * aOpt, BluezEndpoint * self) { return self->BluezCharacteristicAcquireNotify(aChar, aInv, aOpt); }), this); - g_signal_connect(mpC2, "handle-confirm", + g_signal_connect(mpC2.get(), "handle-confirm", G_CALLBACK(+[](BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, BluezEndpoint * self) { return self->BluezCharacteristicConfirm(aChar, aInv); }), this); - ChipLogDetail(DeviceLayer, "CHIP BTP C1 %s", bluez_gatt_characteristic1_get_service(mpC1)); - ChipLogDetail(DeviceLayer, "CHIP BTP C2 %s", bluez_gatt_characteristic1_get_service(mpC2)); + ChipLogDetail(DeviceLayer, "CHIP BTP C1 %s", bluez_gatt_characteristic1_get_service(mpC1.get())); + ChipLogDetail(DeviceLayer, "CHIP BTP C2 %s", bluez_gatt_characteristic1_get_service(mpC2.get())); #if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING ChipLogDetail(DeviceLayer, "CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING is TRUE"); // Additional data characteristics - mpC3 = CreateGattCharacteristic(mpService, "c3", CHIP_PLAT_BLE_UUID_C3_STRING, c3_flags); - g_signal_connect(mpC3, "handle-read-value", + mpC3 = + GAutoPtr(CreateGattCharacteristic(mpService.get(), "c3", CHIP_PLAT_BLE_UUID_C3_STRING, c3_flags)); + g_signal_connect(mpC3.get(), "handle-read-value", G_CALLBACK(+[](BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, GVariant * aOpt, BluezEndpoint * self) { return self->BluezCharacteristicReadValue(aChar, aInv, aOpt); }), this); - g_signal_connect(mpC3, "handle-acquire-write", G_CALLBACK(BluezCharacteristicAcquireWriteError), nullptr); - g_signal_connect(mpC3, "handle-acquire-notify", + g_signal_connect(mpC3.get(), "handle-acquire-write", G_CALLBACK(BluezCharacteristicAcquireWriteError), nullptr); + g_signal_connect(mpC3.get(), "handle-acquire-notify", G_CALLBACK(+[](BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, GVariant * aOpt, BluezEndpoint * self) { return self->BluezCharacteristicAcquireNotify(aChar, aInv, aOpt); }), this); - g_signal_connect(mpC3, "handle-confirm", + g_signal_connect(mpC3.get(), "handle-confirm", G_CALLBACK(+[](BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, BluezEndpoint * self) { return self->BluezCharacteristicConfirm(aChar, aInv); }), this); // update the characteristic value - UpdateAdditionalDataCharacteristic(mpC3); - ChipLogDetail(DeviceLayer, "CHIP BTP C3 %s", bluez_gatt_characteristic1_get_service(mpC3)); + UpdateAdditionalDataCharacteristic(mpC3.get()); + ChipLogDetail(DeviceLayer, "CHIP BTP C3 %s", bluez_gatt_characteristic1_get_service(mpC3.get())); #else ChipLogDetail(DeviceLayer, "CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING is FALSE"); #endif @@ -601,13 +604,13 @@ void BluezEndpoint::SetupGattServer(GDBusConnection * aConn) VerifyOrReturn(!mIsCentral); mpRootPath = g_strdup_printf("/chipoble/%04x", getpid() & 0xffff); - mpRoot = g_dbus_object_manager_server_new(mpRootPath); + mpRoot = GAutoPtr(g_dbus_object_manager_server_new(mpRootPath)); SetupGattService(); // Set connection after the service is set up in order to reduce the number // of signals sent on the bus. - g_dbus_object_manager_server_set_connection(mpRoot, aConn); + g_dbus_object_manager_server_set_connection(mpRoot.get(), aConn); } CHIP_ERROR BluezEndpoint::StartupEndpointBindings() @@ -625,23 +628,24 @@ CHIP_ERROR BluezEndpoint::StartupEndpointBindings() SetupGattServer(conn.get()); - mpObjMgr = g_dbus_object_manager_client_new_sync( + mpObjMgr = GAutoPtr(g_dbus_object_manager_client_new_sync( conn.get(), 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 */, - &MakeUniquePointerReceiver(err).Get()); - VerifyOrReturnError(mpObjMgr != nullptr, CHIP_ERROR_INTERNAL, + &MakeUniquePointerReceiver(err).Get())); + VerifyOrReturnError(mpObjMgr.get() != nullptr, CHIP_ERROR_INTERNAL, ChipLogError(DeviceLayer, "FAIL: Error getting object manager client: %s", err->message)); - g_signal_connect(mpObjMgr, "object-added", G_CALLBACK(+[](GDBusObjectManager * aMgr, GDBusObject * aObj, BluezEndpoint * self) { + g_signal_connect(mpObjMgr.get(), "object-added", + G_CALLBACK(+[](GDBusObjectManager * aMgr, GDBusObject * aObj, BluezEndpoint * self) { return self->BluezSignalOnObjectAdded(aMgr, aObj); }), this); - g_signal_connect(mpObjMgr, "object-removed", + g_signal_connect(mpObjMgr.get(), "object-removed", G_CALLBACK(+[](GDBusObjectManager * aMgr, GDBusObject * aObj, BluezEndpoint * self) { return self->BluezSignalOnObjectRemoved(aMgr, aObj); }), this); - g_signal_connect(mpObjMgr, "interface-proxy-properties-changed", + g_signal_connect(mpObjMgr.get(), "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); @@ -678,11 +682,11 @@ CHIP_ERROR BluezEndpoint::Init(uint32_t aAdapterId, bool aIsCentral, const char } else { - mpConnectCancellable = g_cancellable_new(); + mpConnectCancellable = GAutoPtr(g_cancellable_new()); } - err = PlatformMgrImpl().GLibMatterContextInvokeSync( - +[](BluezEndpoint * self) { return self->StartupEndpointBindings(); }, this); + err = + PlatformMgrImpl().GLibMatterContextInvokeSync(+[](BluezEndpoint * self) { return self->StartupEndpointBindings(); }, this); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to schedule endpoint initialization")); ChipLogDetail(DeviceLayer, "BlueZ integration init success"); @@ -695,33 +699,15 @@ void BluezEndpoint::Shutdown() { VerifyOrReturn(mIsInitialized); - // Run endpoint cleanup on the CHIPoBluez thread. This is necessary because the - // cleanup function releases the D-Bus manager client object, which handles D-Bus - // signals. Otherwise, we will face race condition when the D-Bus signal is in - // the middle of being processed when the cleanup function is called. - PlatformMgrImpl().GLibMatterContextInvokeSync( - +[](BluezEndpoint * self) { - if (self->mpObjMgr != nullptr) - g_object_unref(self->mpObjMgr); - if (self->mpAdapter != nullptr) - g_object_unref(self->mpAdapter); - if (self->mpDevice != nullptr) - g_object_unref(self->mpDevice); - if (self->mpRoot != nullptr) - g_object_unref(self->mpRoot); - if (self->mpService != nullptr) - g_object_unref(self->mpService); - if (self->mpC1 != nullptr) - g_object_unref(self->mpC1); - if (self->mpC2 != nullptr) - g_object_unref(self->mpC2); - if (self->mpC3 != nullptr) - g_object_unref(self->mpC3); - if (self->mpConnectCancellable != nullptr) - g_object_unref(self->mpConnectCancellable); - return CHIP_NO_ERROR; - }, - this); + mpObjMgr = nullptr; + mpAdapter = nullptr; + mpDevice = nullptr; + mpRoot = nullptr; + mpService = nullptr; + mpC1 = nullptr; + mpC2 = nullptr; + mpC3 = nullptr; + mpConnectCancellable = nullptr; g_free(mpOwningName); g_free(mpAdapterName); @@ -756,7 +742,7 @@ void BluezEndpoint::ConnectDeviceDone(GObject * aObject, GAsyncResult * aResult, g_clear_error(&MakeUniquePointerReceiver(error).Get()); bluez_device1_call_disconnect_sync(device, nullptr, &MakeUniquePointerReceiver(error).Get()); - bluez_device1_call_connect(device, params->mEndpoint.mpConnectCancellable, ConnectDeviceDone, params); + bluez_device1_call_connect(device, params->mEndpoint.mpConnectCancellable.get(), ConnectDeviceDone, params); return; } @@ -772,8 +758,8 @@ void BluezEndpoint::ConnectDeviceDone(GObject * aObject, GAsyncResult * aResult, CHIP_ERROR BluezEndpoint::ConnectDeviceImpl(ConnectParams * apParams) { - g_cancellable_reset(apParams->mEndpoint.mpConnectCancellable); - bluez_device1_call_connect(apParams->mpDevice, apParams->mEndpoint.mpConnectCancellable, ConnectDeviceDone, apParams); + g_cancellable_reset(apParams->mEndpoint.mpConnectCancellable.get()); + bluez_device1_call_connect(apParams->mpDevice, apParams->mEndpoint.mpConnectCancellable.get(), ConnectDeviceDone, apParams); return CHIP_NO_ERROR; } @@ -794,8 +780,8 @@ CHIP_ERROR BluezEndpoint::ConnectDevice(BluezDevice1 & aDevice) void BluezEndpoint::CancelConnect() { - VerifyOrDie(mpConnectCancellable != nullptr); - g_cancellable_cancel(mpConnectCancellable); + VerifyOrDie(mpConnectCancellable.get() != nullptr); + g_cancellable_cancel(mpConnectCancellable.get()); } } // namespace Internal diff --git a/src/platform/Linux/bluez/BluezEndpoint.h b/src/platform/Linux/bluez/BluezEndpoint.h index 0b3f286b399247..85c8b1b314c435 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.h +++ b/src/platform/Linux/bluez/BluezEndpoint.h @@ -72,12 +72,12 @@ class BluezEndpoint CHIP_ERROR Init(uint32_t aAdapterId, bool aIsCentral, const char * apBleAddr, const char * apBleName); void Shutdown(); - BluezAdapter1 * GetAdapter() const { return mpAdapter; } + BluezAdapter1 * GetAdapter() const { return mpAdapter.get(); } const char * GetAdapterName() const { return mpAdapterName; } CHIP_ERROR RegisterGattApplication(); - GDBusObjectManagerServer * GetGattApplicationObjectManager() const { return mpRoot; } - GDBusObjectManager * GetObjectManager() const { return mpObjMgr; } + GDBusObjectManagerServer * GetGattApplicationObjectManager() const { return mpRoot.get(); } + GDBusObjectManager * GetObjectManager() const { return mpObjMgr.get(); } CHIP_ERROR ConnectDevice(BluezDevice1 & aDevice); void CancelConnect(); @@ -141,21 +141,21 @@ class BluezEndpoint char * mpServicePath = nullptr; // Objects (interfaces) subscribed to by this service - GDBusObjectManager * mpObjMgr = nullptr; - BluezAdapter1 * mpAdapter = nullptr; - BluezDevice1 * mpDevice = nullptr; + GAutoPtr mpObjMgr; + GAutoPtr mpAdapter; + GAutoPtr mpDevice; // Objects (interfaces) published by this service - GDBusObjectManagerServer * mpRoot = nullptr; - BluezGattService1 * mpService = nullptr; - BluezGattCharacteristic1 * mpC1 = nullptr; - BluezGattCharacteristic1 * mpC2 = nullptr; + GAutoPtr mpRoot; + GAutoPtr mpService; + GAutoPtr mpC1; + GAutoPtr mpC2; // additional data characteristics - BluezGattCharacteristic1 * mpC3 = nullptr; + GAutoPtr mpC3; std::unordered_map mConnMap; - GCancellable * mpConnectCancellable = nullptr; - char * mpPeerDevicePath = nullptr; + GAutoPtr mpConnectCancellable; + char * mpPeerDevicePath = nullptr; // Allow BluezConnection to access our private members friend class BluezConnection; diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index 7e51a6d0d0f6b8..b912a54cffbd1d 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -58,7 +58,7 @@ CHIP_ERROR ChipDeviceScanner::Init(ChipDeviceScannerDelegate * delegate) // Make this function idempotent by shutting down previously initialized state if any. Shutdown(); - mCancellable = g_cancellable_new(); + mCancellable = GAutoPtr(g_cancellable_new()); mDelegate = delegate; mIsInitialized = true; @@ -78,8 +78,7 @@ void ChipDeviceScanner::Shutdown() chip::DeviceLayer::SystemLayer().CancelTimer(TimerExpiredCallback, this); } - if (mCancellable != nullptr) - g_object_unref(mCancellable); + mCancellable = nullptr; mIsInitialized = false; } @@ -131,7 +130,7 @@ CHIP_ERROR ChipDeviceScanner::StopScan() VerifyOrReturnError(!mIsStopping, CHIP_NO_ERROR); mIsStopping = true; - g_cancellable_cancel(mCancellable); // in case we are currently running a scan + g_cancellable_cancel(mCancellable.get()); // in case we are currently running a scan if (mObjectAddedSignal) { @@ -264,7 +263,7 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStartScan(ChipDeviceScanner * self) GVariant * filter = g_variant_builder_end(&filterBuilder); ChipLogProgress(Ble, "BLE setting discovery Name adapter. %s", self->mEndpoint.GetAdapterName()); - if (!bluez_adapter1_call_set_discovery_filter_sync(self->mEndpoint.GetAdapter(), filter, self->mCancellable, + if (!bluez_adapter1_call_set_discovery_filter_sync(self->mEndpoint.GetAdapter(), filter, self->mCancellable.get(), &MakeUniquePointerReceiver(error).Get())) { // Not critical: ignore if fails @@ -273,7 +272,7 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStartScan(ChipDeviceScanner * self) } ChipLogProgress(Ble, "BLE initiating scan."); - if (!bluez_adapter1_call_start_discovery_sync(self->mEndpoint.GetAdapter(), self->mCancellable, + if (!bluez_adapter1_call_start_discovery_sync(self->mEndpoint.GetAdapter(), self->mCancellable.get(), &MakeUniquePointerReceiver(error).Get())) { ChipLogError(Ble, "Failed to start discovery: %s", error->message); diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.h b/src/platform/Linux/bluez/ChipDeviceScanner.h index eea4acdf75964d..ec63a1779a6bf2 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.h +++ b/src/platform/Linux/bluez/ChipDeviceScanner.h @@ -26,6 +26,8 @@ #include #include +#include "Types.h" + namespace chip { namespace DeviceLayer { namespace Internal { @@ -94,7 +96,7 @@ class ChipDeviceScanner void RemoveDevice(BluezDevice1 & device); const BluezEndpoint & mEndpoint; - GCancellable * mCancellable = nullptr; + GAutoPtr mCancellable; ChipDeviceScannerDelegate * mDelegate = nullptr; gulong mObjectAddedSignal = 0; gulong mInterfaceChangedSignal = 0; diff --git a/src/platform/Linux/bluez/Types.h b/src/platform/Linux/bluez/Types.h index 0153492b5b0e32..93811ca74b722b 100644 --- a/src/platform/Linux/bluez/Types.h +++ b/src/platform/Linux/bluez/Types.h @@ -59,6 +59,29 @@ struct GAutoPtrDeleter { using deleter = GObjectDeleter; }; +template <> +struct GAutoPtrDeleter +{ + using deleter = GObjectDeleter; +}; + +template <> +struct GAutoPtrDeleter +{ + using deleter = GObjectDeleter; +}; + +template <> +struct GAutoPtrDeleter +{ + using deleter = GObjectDeleter; +}; + +template <> +struct GAutoPtrDeleter +{ + using deleter = GObjectDeleter; +}; namespace DeviceLayer { namespace Internal { From 8c36b95bd6ad483fa0ade8d24315c1d936902550 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Wed, 13 Dec 2023 16:47:04 +0100 Subject: [PATCH 4/8] Use reset() for GAutoPtr<>: - Restore run cleanup on the CHIPoBluez thread - Use for GAutoPtr<> object clear - Use for re-init GAutoPtr<> --- src/platform/Linux/bluez/AdapterIterator.cpp | 6 +-- .../Linux/bluez/BluezAdvertisement.cpp | 12 +++--- src/platform/Linux/bluez/BluezConnection.cpp | 14 +++---- src/platform/Linux/bluez/BluezEndpoint.cpp | 39 ++++++++++++------- .../Linux/bluez/ChipDeviceScanner.cpp | 14 +++++-- 5 files changed, 49 insertions(+), 36 deletions(-) diff --git a/src/platform/Linux/bluez/AdapterIterator.cpp b/src/platform/Linux/bluez/AdapterIterator.cpp index 40acfc2aaca8e2..c5d25ab2f68e2f 100644 --- a/src/platform/Linux/bluez/AdapterIterator.cpp +++ b/src/platform/Linux/bluez/AdapterIterator.cpp @@ -43,7 +43,7 @@ CHIP_ERROR AdapterIterator::Initialize(AdapterIterator * self) CHIP_ERROR err = CHIP_NO_ERROR; GAutoPtr error; - self->mManager = GAutoPtr(g_dbus_object_manager_client_new_for_bus_sync( + self->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 */, &MakeUniquePointerReceiver(error).Get())); @@ -92,14 +92,12 @@ bool AdapterIterator::Advance() index = 0; } - mCurrent.adapter = nullptr; - mCurrent.index = index; mCurrent.address = bluez_adapter1_get_address(adapter); mCurrent.alias = bluez_adapter1_get_alias(adapter); mCurrent.name = bluez_adapter1_get_name(adapter); mCurrent.powered = bluez_adapter1_get_powered(adapter); - mCurrent.adapter = GAutoPtr(adapter); + mCurrent.adapter.reset(adapter); mCurrentListItem = mCurrentListItem->next; diff --git a/src/platform/Linux/bluez/BluezAdvertisement.cpp b/src/platform/Linux/bluez/BluezAdvertisement.cpp index 73ce801ed22c5c..d5c262c1653620 100644 --- a/src/platform/Linux/bluez/BluezAdvertisement.cpp +++ b/src/platform/Linux/bluez/BluezAdvertisement.cpp @@ -72,7 +72,7 @@ BluezLEAdvertisement1 * BluezAdvertisement::CreateLEAdvertisement() serviceData = g_variant_builder_end(&serviceDataBuilder); serviceUUID = g_variant_builder_end(&serviceUUIDsBuilder); - debugStr = GAutoPtr(g_variant_print(serviceData, TRUE)); + debugStr.reset(g_variant_print(serviceData, TRUE)); ChipLogDetail(DeviceLayer, "SET service data to %s", StringOrNullMarker(debugStr.get())); bluez_leadvertisement1_set_type_(adv, (mAdvType & BLUEZ_ADV_TYPE_CONNECTABLE) ? "peripheral" : "broadcast"); @@ -117,7 +117,7 @@ gboolean BluezAdvertisement::BluezLEAdvertisement1Release(BluezLEAdvertisement1 { ChipLogDetail(DeviceLayer, "Release BLE adv object in %s", __func__); g_dbus_object_manager_server_unexport(mEndpoint.GetGattApplicationObjectManager(), mpAdvPath); - mpAdv = nullptr; + mpAdv.reset(); mIsAdvertising = false; return TRUE; } @@ -128,7 +128,7 @@ CHIP_ERROR BluezAdvertisement::InitImpl() // all D-Bus signals will be delivered to the GLib global default main context. VerifyOrDie(g_main_context_get_thread_default() != nullptr); - mpAdv = GAutoPtr(CreateLEAdvertisement()); + mpAdv.reset(CreateLEAdvertisement()); return CHIP_NO_ERROR; } @@ -182,10 +182,8 @@ void BluezAdvertisement::Shutdown() // attached to the advertising object that may run on the glib thread. PlatformMgrImpl().GLibMatterContextInvokeSync( +[](BluezAdvertisement * self) { - if (self->mpAdv.get() != nullptr) - { - self->mpAdv = nullptr; - } + self->mpAdv.reset(); + return CHIP_NO_ERROR; }, this); diff --git a/src/platform/Linux/bluez/BluezConnection.cpp b/src/platform/Linux/bluez/BluezConnection.cpp index 82a0cb8b7a7b10..148303d26b0fc7 100644 --- a/src/platform/Linux/bluez/BluezConnection.cpp +++ b/src/platform/Linux/bluez/BluezConnection.cpp @@ -86,9 +86,9 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) if (!aEndpoint.mIsCentral) { - mpService = GAutoPtr(BLUEZ_GATT_SERVICE1(g_object_ref(aEndpoint.mpService.get()))); - mpC1 = GAutoPtr(BLUEZ_GATT_CHARACTERISTIC1(g_object_ref(aEndpoint.mpC1.get()))); - mpC2 = GAutoPtr(BLUEZ_GATT_CHARACTERISTIC1(g_object_ref(aEndpoint.mpC2.get()))); + mpService.reset(BLUEZ_GATT_SERVICE1(g_object_ref(aEndpoint.mpService.get()))); + mpC1.reset(BLUEZ_GATT_CHARACTERISTIC1(g_object_ref(aEndpoint.mpC1.get()))); + mpC2.reset(BLUEZ_GATT_CHARACTERISTIC1(g_object_ref(aEndpoint.mpC2.get()))); } else { @@ -104,7 +104,7 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) if ((BluezIsServiceOnDevice(service, mpDevice.get())) == TRUE && (strcmp(bluez_gatt_service1_get_uuid(service), CHIP_BLE_UUID_SERVICE_STRING) == 0)) { - mpService = GAutoPtr(service); + mpService.reset(service); break; } g_object_unref(service); @@ -123,18 +123,18 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) if ((BluezIsCharOnService(char1, mpService.get()) == TRUE) && (strcmp(bluez_gatt_characteristic1_get_uuid(char1), CHIP_PLAT_BLE_UUID_C1_STRING) == 0)) { - mpC1 = GAutoPtr(char1); + mpC1.reset(char1); } else if ((BluezIsCharOnService(char1, mpService.get()) == TRUE) && (strcmp(bluez_gatt_characteristic1_get_uuid(char1), CHIP_PLAT_BLE_UUID_C2_STRING) == 0)) { - mpC2 = GAutoPtr(char1); + mpC2.reset(char1); } #if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING else if ((BluezIsCharOnService(char1, mpService.get()) == TRUE) && (strcmp(bluez_gatt_characteristic1_get_uuid(char1), CHIP_PLAT_BLE_UUID_C3_STRING) == 0)) { - mpC3 = GAutoPtr(char1); + mpC3.reset(char1); } #endif else diff --git a/src/platform/Linux/bluez/BluezEndpoint.cpp b/src/platform/Linux/bluez/BluezEndpoint.cpp index 19a68ba38c736e..8036a77d873afb 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.cpp +++ b/src/platform/Linux/bluez/BluezEndpoint.cpp @@ -441,14 +441,14 @@ void BluezEndpoint::SetupAdapter() { if (strcmp(g_dbus_proxy_get_object_path(G_DBUS_PROXY(adapter)), expectedPath) == 0) { - mpAdapter = GAutoPtr(g_object_ref(adapter)); + mpAdapter.reset(g_object_ref(adapter)); } } else { if (strcmp(bluez_adapter1_get_address(adapter), mpAdapterAddr) == 0) { - mpAdapter = GAutoPtr(g_object_ref(adapter)); + mpAdapter.reset(g_object_ref(adapter)); } } } @@ -534,7 +534,7 @@ void BluezEndpoint::SetupGattService() static const char * const c3_flags[] = { "read", nullptr }; #endif - mpService = GAutoPtr(CreateGattService(CHIP_BLE_UUID_SERVICE_SHORT_STRING)); + mpService.reset(CreateGattService(CHIP_BLE_UUID_SERVICE_SHORT_STRING)); // C1 characteristic mpC1 = @@ -604,7 +604,7 @@ void BluezEndpoint::SetupGattServer(GDBusConnection * aConn) VerifyOrReturn(!mIsCentral); mpRootPath = g_strdup_printf("/chipoble/%04x", getpid() & 0xffff); - mpRoot = GAutoPtr(g_dbus_object_manager_server_new(mpRootPath)); + mpRoot.reset(g_dbus_object_manager_server_new(mpRootPath)); SetupGattService(); @@ -628,7 +628,7 @@ CHIP_ERROR BluezEndpoint::StartupEndpointBindings() SetupGattServer(conn.get()); - mpObjMgr = GAutoPtr(g_dbus_object_manager_client_new_sync( + mpObjMgr.reset(g_dbus_object_manager_client_new_sync( conn.get(), 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 */, &MakeUniquePointerReceiver(err).Get())); @@ -682,7 +682,7 @@ CHIP_ERROR BluezEndpoint::Init(uint32_t aAdapterId, bool aIsCentral, const char } else { - mpConnectCancellable = GAutoPtr(g_cancellable_new()); + mpConnectCancellable.reset(g_cancellable_new()); } err = @@ -699,15 +699,24 @@ void BluezEndpoint::Shutdown() { VerifyOrReturn(mIsInitialized); - mpObjMgr = nullptr; - mpAdapter = nullptr; - mpDevice = nullptr; - mpRoot = nullptr; - mpService = nullptr; - mpC1 = nullptr; - mpC2 = nullptr; - mpC3 = nullptr; - mpConnectCancellable = nullptr; + // Run endpoint cleanup on the CHIPoBluez thread. This is necessary because the + // cleanup function releases the D-Bus manager client object, which handles D-Bus + // signals. Otherwise, we will face race condition when the D-Bus signal is in + // the middle of being processed when the cleanup function is called. + PlatformMgrImpl().GLibMatterContextInvokeSync( + +[](BluezEndpoint * self) { + mpObjMgr.reset(); + mpAdapter.reset(); + mpDevice.reset(); + mpRoot.reset(); + mpService.reset(); + mpC1.reset(); + mpC2.reset(); + mpC3.reset(); + mpConnectCancellable.reset(); + return CHIP_NO_ERROR; + }, + this); g_free(mpOwningName); g_free(mpAdapterName); diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index b912a54cffbd1d..23c26796bf87de 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -58,8 +58,8 @@ CHIP_ERROR ChipDeviceScanner::Init(ChipDeviceScannerDelegate * delegate) // Make this function idempotent by shutting down previously initialized state if any. Shutdown(); - mCancellable = GAutoPtr(g_cancellable_new()); - mDelegate = delegate; + mCancellable.reset(g_cancellable_new()); + mDelegate = delegate; mIsInitialized = true; return CHIP_NO_ERROR; @@ -78,7 +78,15 @@ void ChipDeviceScanner::Shutdown() chip::DeviceLayer::SystemLayer().CancelTimer(TimerExpiredCallback, this); } - mCancellable = nullptr; + // Release resources on the glib thread. This is necessary because the D-Bus manager client + // object handles D-Bus signals. Otherwise, we might face a race when the manager object is + // released during a D-Bus signal being processed. + PlatformMgrImpl().GLibMatterContextInvokeSync( + +[](ChipDeviceScanner * self) { + mCancellable.reset(); + return CHIP_NO_ERROR; + }, + this); mIsInitialized = false; } From 06844772dabaeb055f116b815041d110b9fd7fe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Thu, 14 Dec 2023 09:39:38 +0100 Subject: [PATCH 5/8] Cleanup: - Remove unnecessary delete operator from classes - Fix lost self in lambda function - Remove not needed comment - Restyled by clang-format --- .../Linux/bluez/BluezAdvertisement.cpp | 11 +++++---- src/platform/Linux/bluez/BluezAdvertisement.h | 2 -- src/platform/Linux/bluez/BluezConnection.cpp | 3 ++- src/platform/Linux/bluez/BluezEndpoint.cpp | 24 +++++++++---------- .../Linux/bluez/ChipDeviceScanner.cpp | 2 +- src/platform/Linux/bluez/ChipDeviceScanner.h | 1 - 6 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/platform/Linux/bluez/BluezAdvertisement.cpp b/src/platform/Linux/bluez/BluezAdvertisement.cpp index d5c262c1653620..f3eca1cdd106fd 100644 --- a/src/platform/Linux/bluez/BluezAdvertisement.cpp +++ b/src/platform/Linux/bluez/BluezAdvertisement.cpp @@ -154,7 +154,8 @@ CHIP_ERROR BluezAdvertisement::Init(ChipAdvType aAdvType, const char * aAdvUUID, mDeviceIdInfo.SetAdditionalDataFlag(true); #endif - err = PlatformMgrImpl().GLibMatterContextInvokeSync(+[](BluezAdvertisement * self) { return self->InitImpl(); }, this); + err = PlatformMgrImpl().GLibMatterContextInvokeSync( + +[](BluezAdvertisement * self) { return self->InitImpl(); }, this); VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Ble, "Failed to schedule BLE advertisement Init() on CHIPoBluez thread")); @@ -252,8 +253,8 @@ CHIP_ERROR BluezAdvertisement::Start() { VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); - CHIP_ERROR err = - PlatformMgrImpl().GLibMatterContextInvokeSync(+[](BluezAdvertisement * self) { return self->StartImpl(); }, this); + CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync( + +[](BluezAdvertisement * self) { return self->StartImpl(); }, this); VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Ble, "Failed to schedule BLE advertisement Start() on CHIPoBluez thread")); return err; @@ -314,8 +315,8 @@ CHIP_ERROR BluezAdvertisement::Stop() { VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); - CHIP_ERROR err = - PlatformMgrImpl().GLibMatterContextInvokeSync(+[](BluezAdvertisement * self) { return self->StopImpl(); }, this); + CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync( + +[](BluezAdvertisement * self) { return self->StopImpl(); }, this); VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Ble, "Failed to schedule BLE advertisement Stop() on CHIPoBluez thread")); return err; diff --git a/src/platform/Linux/bluez/BluezAdvertisement.h b/src/platform/Linux/bluez/BluezAdvertisement.h index aef175b6b17a8a..09a03b3ecfedbf 100644 --- a/src/platform/Linux/bluez/BluezAdvertisement.h +++ b/src/platform/Linux/bluez/BluezAdvertisement.h @@ -38,7 +38,6 @@ class BluezEndpoint; class BluezAdvertisement { public: - BluezAdvertisement() = delete; BluezAdvertisement(const BluezEndpoint & endpoint) : mEndpoint(endpoint){}; ~BluezAdvertisement() { Shutdown(); } @@ -69,7 +68,6 @@ class BluezAdvertisement void StopDone(GObject * aObject, GAsyncResult * aResult); CHIP_ERROR StopImpl(); - // Objects (interfaces) used by LE advertisement const BluezEndpoint & mEndpoint; GAutoPtr mpAdv; diff --git a/src/platform/Linux/bluez/BluezConnection.cpp b/src/platform/Linux/bluez/BluezConnection.cpp index 148303d26b0fc7..9704a498b3466d 100644 --- a/src/platform/Linux/bluez/BluezConnection.cpp +++ b/src/platform/Linux/bluez/BluezConnection.cpp @@ -75,7 +75,8 @@ BluezConnection::IOChannel::~IOChannel() BluezConnection::ConnectionDataBundle::ConnectionDataBundle(const BluezConnection & aConn, const chip::System::PacketBufferHandle & aBuf) : - mConn(aConn), mData(g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, aBuf->Start(), aBuf->DataLength(), sizeof(uint8_t))) + mConn(aConn), + mData(g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, aBuf->Start(), aBuf->DataLength(), sizeof(uint8_t))) {} CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) diff --git a/src/platform/Linux/bluez/BluezEndpoint.cpp b/src/platform/Linux/bluez/BluezEndpoint.cpp index 8036a77d873afb..7ee3afe4fe1045 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.cpp +++ b/src/platform/Linux/bluez/BluezEndpoint.cpp @@ -395,7 +395,7 @@ void BluezEndpoint::BluezSignalOnObjectAdded(GDBusObjectManager * aManager, GDBu void BluezEndpoint::BluezSignalOnObjectRemoved(GDBusObjectManager * aManager, GDBusObject * aObject) { // TODO: for Device1, lookup connection, and call otPlatTobleHandleDisconnected - // for Adapter1: unclear, crash if this pertains to our adapter? at least null out the self->mpAdapter.get(). + // for Adapter1: unclear, crash if this pertains to our adapter? at least null out the self->mpAdapter. // for Characteristic1, or GattService -- handle here via calling otPlatTobleHandleDisconnected, or ignore. } @@ -685,8 +685,8 @@ CHIP_ERROR BluezEndpoint::Init(uint32_t aAdapterId, bool aIsCentral, const char mpConnectCancellable.reset(g_cancellable_new()); } - err = - PlatformMgrImpl().GLibMatterContextInvokeSync(+[](BluezEndpoint * self) { return self->StartupEndpointBindings(); }, this); + err = PlatformMgrImpl().GLibMatterContextInvokeSync( + +[](BluezEndpoint * self) { return self->StartupEndpointBindings(); }, this); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to schedule endpoint initialization")); ChipLogDetail(DeviceLayer, "BlueZ integration init success"); @@ -705,15 +705,15 @@ void BluezEndpoint::Shutdown() // the middle of being processed when the cleanup function is called. PlatformMgrImpl().GLibMatterContextInvokeSync( +[](BluezEndpoint * self) { - mpObjMgr.reset(); - mpAdapter.reset(); - mpDevice.reset(); - mpRoot.reset(); - mpService.reset(); - mpC1.reset(); - mpC2.reset(); - mpC3.reset(); - mpConnectCancellable.reset(); + self->mpObjMgr.reset(); + self->mpAdapter.reset(); + self->mpDevice.reset(); + self->mpRoot.reset(); + self->mpService.reset(); + self->mpC1.reset(); + self->mpC2.reset(); + self->mpC3.reset(); + self->mpConnectCancellable.reset(); return CHIP_NO_ERROR; }, this); diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index 23c26796bf87de..6994ee05bd7305 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -83,7 +83,7 @@ void ChipDeviceScanner::Shutdown() // released during a D-Bus signal being processed. PlatformMgrImpl().GLibMatterContextInvokeSync( +[](ChipDeviceScanner * self) { - mCancellable.reset(); + self->mCancellable.reset(); return CHIP_NO_ERROR; }, this); diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.h b/src/platform/Linux/bluez/ChipDeviceScanner.h index ec63a1779a6bf2..4c74df651ef6a0 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.h +++ b/src/platform/Linux/bluez/ChipDeviceScanner.h @@ -56,7 +56,6 @@ class ChipDeviceScannerDelegate class ChipDeviceScanner { public: - ChipDeviceScanner() = delete; ChipDeviceScanner(const BluezEndpoint & endpoint) : mEndpoint(endpoint){}; ChipDeviceScanner(ChipDeviceScanner &&) = default; ChipDeviceScanner(const ChipDeviceScanner &) = delete; From 48559cff1dd66ea60a5b436efe71e8ab7afbf39b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Thu, 14 Dec 2023 14:33:23 +0100 Subject: [PATCH 6/8] Fixes CI and other - Fix missing mpC3 getter --- src/platform/Linux/bluez/BluezConnection.cpp | 5 +---- src/platform/Linux/bluez/BluezConnection.h | 2 +- src/platform/Linux/bluez/BluezEndpoint.cpp | 19 ++++++++----------- src/platform/Linux/bluez/BluezEndpoint.h | 2 +- .../Linux/bluez/ChipDeviceScanner.cpp | 10 +++++----- 5 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/platform/Linux/bluez/BluezConnection.cpp b/src/platform/Linux/bluez/BluezConnection.cpp index 9704a498b3466d..a5695c14bef4ed 100644 --- a/src/platform/Linux/bluez/BluezConnection.cpp +++ b/src/platform/Linux/bluez/BluezConnection.cpp @@ -64,8 +64,6 @@ BluezConnection::BluezConnection(const BluezEndpoint & aEndpoint, BluezDevice1 * Init(aEndpoint); } -BluezConnection::~BluezConnection() {} - BluezConnection::IOChannel::~IOChannel() { if (mWatchSource != nullptr) @@ -75,8 +73,7 @@ BluezConnection::IOChannel::~IOChannel() BluezConnection::ConnectionDataBundle::ConnectionDataBundle(const BluezConnection & aConn, const chip::System::PacketBufferHandle & aBuf) : - mConn(aConn), - mData(g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, aBuf->Start(), aBuf->DataLength(), sizeof(uint8_t))) + mConn(aConn), mData(g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, aBuf->Start(), aBuf->DataLength(), sizeof(uint8_t))) {} CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) diff --git a/src/platform/Linux/bluez/BluezConnection.h b/src/platform/Linux/bluez/BluezConnection.h index babca3f138fc6a..88eeab40ce695a 100644 --- a/src/platform/Linux/bluez/BluezConnection.h +++ b/src/platform/Linux/bluez/BluezConnection.h @@ -40,7 +40,7 @@ class BluezConnection { public: BluezConnection(const BluezEndpoint & aEndpoint, BluezDevice1 * apDevice); - ~BluezConnection(); + ~BluezConnection() = default; const char * GetPeerAddress() const; diff --git a/src/platform/Linux/bluez/BluezEndpoint.cpp b/src/platform/Linux/bluez/BluezEndpoint.cpp index 7ee3afe4fe1045..fb742ea4fbf991 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.cpp +++ b/src/platform/Linux/bluez/BluezEndpoint.cpp @@ -161,7 +161,7 @@ gboolean BluezEndpoint::BluezCharacteristicAcquireNotify(BluezGattCharacteristic uint16_t mtu; #if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING - isAdditionalAdvertising = (aChar == mpC3); + isAdditionalAdvertising = (aChar == mpC3.get()); #endif if (bluez_gatt_characteristic1_get_notifying(aChar)) @@ -441,14 +441,14 @@ void BluezEndpoint::SetupAdapter() { if (strcmp(g_dbus_proxy_get_object_path(G_DBUS_PROXY(adapter)), expectedPath) == 0) { - mpAdapter.reset(g_object_ref(adapter)); + mpAdapter.reset(static_cast(g_object_ref(adapter))); } } else { if (strcmp(bluez_adapter1_get_address(adapter), mpAdapterAddr) == 0) { - mpAdapter.reset(g_object_ref(adapter)); + mpAdapter.reset(static_cast(g_object_ref(adapter))); } } } @@ -537,8 +537,7 @@ void BluezEndpoint::SetupGattService() mpService.reset(CreateGattService(CHIP_BLE_UUID_SERVICE_SHORT_STRING)); // C1 characteristic - mpC1 = - GAutoPtr(CreateGattCharacteristic(mpService.get(), "c1", CHIP_PLAT_BLE_UUID_C1_STRING, c1_flags)); + mpC1.reset(CreateGattCharacteristic(mpService.get(), "c1", CHIP_PLAT_BLE_UUID_C1_STRING, c1_flags)); g_signal_connect(mpC1.get(), "handle-read-value", G_CALLBACK(+[](BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, GVariant * aOpt, BluezEndpoint * self) { return self->BluezCharacteristicReadValue(aChar, aInv, aOpt); }), @@ -551,8 +550,7 @@ void BluezEndpoint::SetupGattService() g_signal_connect(mpC1.get(), "handle-confirm", G_CALLBACK(BluezCharacteristicConfirmError), nullptr); // C2 characteristic - mpC2 = - GAutoPtr(CreateGattCharacteristic(mpService.get(), "c2", CHIP_PLAT_BLE_UUID_C2_STRING, c2_flags)); + mpC2.reset(CreateGattCharacteristic(mpService.get(), "c2", CHIP_PLAT_BLE_UUID_C2_STRING, c2_flags)); g_signal_connect(mpC2.get(), "handle-read-value", G_CALLBACK(+[](BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, GVariant * aOpt, BluezEndpoint * self) { return self->BluezCharacteristicReadValue(aChar, aInv, aOpt); }), @@ -574,8 +572,7 @@ void BluezEndpoint::SetupGattService() #if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING ChipLogDetail(DeviceLayer, "CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING is TRUE"); // Additional data characteristics - mpC3 = - GAutoPtr(CreateGattCharacteristic(mpService.get(), "c3", CHIP_PLAT_BLE_UUID_C3_STRING, c3_flags)); + mpC3.reset(CreateGattCharacteristic(mpService.get(), "c3", CHIP_PLAT_BLE_UUID_C3_STRING, c3_flags)); g_signal_connect(mpC3.get(), "handle-read-value", G_CALLBACK(+[](BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, GVariant * aOpt, BluezEndpoint * self) { return self->BluezCharacteristicReadValue(aChar, aInv, aOpt); }), @@ -685,8 +682,8 @@ CHIP_ERROR BluezEndpoint::Init(uint32_t aAdapterId, bool aIsCentral, const char mpConnectCancellable.reset(g_cancellable_new()); } - err = PlatformMgrImpl().GLibMatterContextInvokeSync( - +[](BluezEndpoint * self) { return self->StartupEndpointBindings(); }, this); + err = + PlatformMgrImpl().GLibMatterContextInvokeSync(+[](BluezEndpoint * self) { return self->StartupEndpointBindings(); }, this); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to schedule endpoint initialization")); ChipLogDetail(DeviceLayer, "BlueZ integration init success"); diff --git a/src/platform/Linux/bluez/BluezEndpoint.h b/src/platform/Linux/bluez/BluezEndpoint.h index 85c8b1b314c435..2a5bd5ad0beb31 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.h +++ b/src/platform/Linux/bluez/BluezEndpoint.h @@ -77,7 +77,7 @@ class BluezEndpoint CHIP_ERROR RegisterGattApplication(); GDBusObjectManagerServer * GetGattApplicationObjectManager() const { return mpRoot.get(); } - GDBusObjectManager * GetObjectManager() const { return mpObjMgr.get(); } + GDBusObjectManager * GetBluezObjectManager() const { return mpObjMgr.get(); } CHIP_ERROR ConnectDevice(BluezDevice1 & aDevice); void CancelConnect(); diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index 6994ee05bd7305..816443ed12341a 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -142,13 +142,13 @@ CHIP_ERROR ChipDeviceScanner::StopScan() if (mObjectAddedSignal) { - g_signal_handler_disconnect(mEndpoint.GetObjectManager(), mObjectAddedSignal); + g_signal_handler_disconnect(mEndpoint.GetBluezObjectManager(), mObjectAddedSignal); mObjectAddedSignal = 0; } if (mInterfaceChangedSignal) { - g_signal_handler_disconnect(mEndpoint.GetObjectManager(), mInterfaceChangedSignal); + g_signal_handler_disconnect(mEndpoint.GetBluezObjectManager(), mInterfaceChangedSignal); mInterfaceChangedSignal = 0; } @@ -245,12 +245,12 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStartScan(ChipDeviceScanner * self) GAutoPtr error; self->mObjectAddedSignal = - g_signal_connect(self->mEndpoint.GetObjectManager(), "object-added", G_CALLBACK(SignalObjectAdded), self); - self->mInterfaceChangedSignal = g_signal_connect(self->mEndpoint.GetObjectManager(), "interface-proxy-properties-changed", + g_signal_connect(self->mEndpoint.GetBluezObjectManager(), "object-added", G_CALLBACK(SignalObjectAdded), self); + self->mInterfaceChangedSignal = g_signal_connect(self->mEndpoint.GetBluezObjectManager(), "interface-proxy-properties-changed", G_CALLBACK(SignalInterfaceChanged), self); ChipLogProgress(Ble, "BLE removing known devices."); - for (BluezObject & object : BluezObjectList(self->mEndpoint.GetObjectManager())) + for (BluezObject & object : BluezObjectList(self->mEndpoint.GetBluezObjectManager())) { GAutoPtr device(bluez_object_get_device1(&object)); if (device.get() != nullptr) From 576fac83bcc11022a27a7e8d6ab7d7f0a3aced5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Thu, 14 Dec 2023 15:01:04 +0100 Subject: [PATCH 7/8] Reducing duplicate types in BLE manager - Restyled by clang-format --- src/platform/Linux/BLEManagerImpl.h | 10 +++++----- src/platform/Linux/bluez/BluezConnection.cpp | 3 ++- src/platform/Linux/bluez/BluezEndpoint.cpp | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/platform/Linux/BLEManagerImpl.h b/src/platform/Linux/BLEManagerImpl.h index cd2b90f11e3389..23342dc1b2be6c 100644 --- a/src/platform/Linux/BLEManagerImpl.h +++ b/src/platform/Linux/BLEManagerImpl.h @@ -188,12 +188,12 @@ class BLEManagerImpl final : public BLEManager, bool mIsCentral = false; BluezEndpoint mEndpoint; - BluezAdvertisement mBLEAdvertisement = BluezAdvertisement(mEndpoint); - ChipAdvType mBLEAdvType = ChipAdvType::BLUEZ_ADV_TYPE_UNDIRECTED_CONNECTABLE_SCANNABLE; - uint16_t mBLEAdvDurationMs = 20; - const char * mpBLEAdvUUID = nullptr; + BluezAdvertisement mBLEAdvertisement{ mEndpoint }; + ChipAdvType mBLEAdvType = ChipAdvType::BLUEZ_ADV_TYPE_UNDIRECTED_CONNECTABLE_SCANNABLE; + uint16_t mBLEAdvDurationMs = 20; + const char * mpBLEAdvUUID = nullptr; - ChipDeviceScanner mDeviceScanner = ChipDeviceScanner(mEndpoint); + ChipDeviceScanner mDeviceScanner{ mEndpoint }; BLEScanConfig mBLEScanConfig; }; diff --git a/src/platform/Linux/bluez/BluezConnection.cpp b/src/platform/Linux/bluez/BluezConnection.cpp index a5695c14bef4ed..6775bf649ad646 100644 --- a/src/platform/Linux/bluez/BluezConnection.cpp +++ b/src/platform/Linux/bluez/BluezConnection.cpp @@ -73,7 +73,8 @@ BluezConnection::IOChannel::~IOChannel() BluezConnection::ConnectionDataBundle::ConnectionDataBundle(const BluezConnection & aConn, const chip::System::PacketBufferHandle & aBuf) : - mConn(aConn), mData(g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, aBuf->Start(), aBuf->DataLength(), sizeof(uint8_t))) + mConn(aConn), + mData(g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, aBuf->Start(), aBuf->DataLength(), sizeof(uint8_t))) {} CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) diff --git a/src/platform/Linux/bluez/BluezEndpoint.cpp b/src/platform/Linux/bluez/BluezEndpoint.cpp index fb742ea4fbf991..71086981ea94b2 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.cpp +++ b/src/platform/Linux/bluez/BluezEndpoint.cpp @@ -682,8 +682,8 @@ CHIP_ERROR BluezEndpoint::Init(uint32_t aAdapterId, bool aIsCentral, const char mpConnectCancellable.reset(g_cancellable_new()); } - err = - PlatformMgrImpl().GLibMatterContextInvokeSync(+[](BluezEndpoint * self) { return self->StartupEndpointBindings(); }, this); + err = PlatformMgrImpl().GLibMatterContextInvokeSync( + +[](BluezEndpoint * self) { return self->StartupEndpointBindings(); }, this); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to schedule endpoint initialization")); ChipLogDetail(DeviceLayer, "BlueZ integration init success"); From 7fba468b2ab7e285c4929a59dcb92729be9e30c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Michalak-Szmaci=C5=84ski?= Date: Thu, 14 Dec 2023 16:47:21 +0100 Subject: [PATCH 8/8] WIP - Python BLE API - Restyled by clang-format --- src/controller/python/chip/ble/LinuxImpl.cpp | 12 ++++++++++-- src/platform/Linux/bluez/BluezEndpoint.h | 1 + 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/controller/python/chip/ble/LinuxImpl.cpp b/src/controller/python/chip/ble/LinuxImpl.cpp index 6218d9c66d4c83..6e0338dc9e2b56 100644 --- a/src/controller/python/chip/ble/LinuxImpl.cpp +++ b/src/controller/python/chip/ble/LinuxImpl.cpp @@ -93,7 +93,13 @@ 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) + { + // It's a hack of sorts on how the python API works. + // One more thing to consider is what should the api implementation look like with BLEManager running? + mEndpoint.SetAdapter(adapter); + return mScanner.Init(this); + } CHIP_ERROR ScannerStartScan(chip::System::Clock::Timeout timeout) { return mScanner.StartScan(timeout); } void OnDeviceScanned(BluezDevice1 & device, const chip::Ble::ChipBLEDeviceIdentificationInfo & info) override @@ -124,7 +130,8 @@ class ScannerDelegateImpl : public ChipDeviceScannerDelegate } private: - ChipDeviceScanner mScanner; + BluezEndpoint mEndpoint; + ChipDeviceScanner mScanner{ mEndpoint }; PyObject * const mContext; const DeviceScannedCallback mScanCallback; const ScanCompleteCallback mCompleteCallback; @@ -142,6 +149,7 @@ extern "C" void * pychip_ble_start_scanning(PyObject * context, void * adapter, std::make_unique(context, scanCallback, completeCallback, errorCallback); CHIP_ERROR err = delegate->ScannerInit(static_cast(adapter)); + VerifyOrReturnError(err == CHIP_NO_ERROR, nullptr); chip::DeviceLayer::PlatformMgr().LockChipStack(); diff --git a/src/platform/Linux/bluez/BluezEndpoint.h b/src/platform/Linux/bluez/BluezEndpoint.h index 2a5bd5ad0beb31..2a18ef72cae7bb 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.h +++ b/src/platform/Linux/bluez/BluezEndpoint.h @@ -72,6 +72,7 @@ class BluezEndpoint CHIP_ERROR Init(uint32_t aAdapterId, bool aIsCentral, const char * apBleAddr, const char * apBleName); void Shutdown(); + void SetAdapter(BluezAdapter1 * apAdapter) { mpAdapter.reset(apAdapter); }; BluezAdapter1 * GetAdapter() const { return mpAdapter.get(); } const char * GetAdapterName() const { return mpAdapterName; }