From e8e785d152898d12fa3ff9c148aa367d021fbbf8 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Mon, 13 Nov 2023 13:22:14 +0100 Subject: [PATCH] Use c++ map for storing BlueConnection objects --- src/platform/Linux/bluez/BluezEndpoint.cpp | 36 ++++++++++------------ src/platform/Linux/bluez/BluezEndpoint.h | 17 +++++----- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/platform/Linux/bluez/BluezEndpoint.cpp b/src/platform/Linux/bluez/BluezEndpoint.cpp index af110771fcaf89..8b334a770b7642 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.cpp +++ b/src/platform/Linux/bluez/BluezEndpoint.cpp @@ -308,16 +308,16 @@ CHIP_ERROR BluezEndpoint::RegisterGattApplicationImpl() void BluezEndpoint::UpdateConnectionTable(BluezDevice1 * apDevice) { const char * objectPath = g_dbus_proxy_get_object_path(G_DBUS_PROXY(apDevice)); - BluezConnection * connection = static_cast(g_hash_table_lookup(mpConnMap, objectPath)); + BluezConnection * connection = GetBluezConnection(objectPath); if (connection != nullptr && !bluez_device1_get_connected(apDevice)) { ChipLogDetail(DeviceLayer, "Bluez disconnected"); BLEManagerImpl::CHIPoBluez_ConnectionClosed(connection); + mConnMap.erase(objectPath); // TODO: the connection object should be released after BLEManagerImpl finishes cleaning up its resources // after the disconnection. Releasing it here doesn't cause any issues, but it's error-prone. chip::Platform::Delete(connection); - g_hash_table_remove(mpConnMap, objectPath); return; } @@ -329,9 +329,9 @@ void BluezEndpoint::UpdateConnectionTable(BluezDevice1 * apDevice) if (connection == nullptr && bluez_device1_get_connected(apDevice) && (!mIsCentral || bluez_device1_get_services_resolved(apDevice))) { - connection = chip::Platform::New(*this, apDevice); - mpPeerDevicePath = g_strdup(objectPath); - g_hash_table_insert(mpConnMap, mpPeerDevicePath, connection); + connection = chip::Platform::New(*this, apDevice); + mpPeerDevicePath = g_strdup(objectPath); + mConnMap[mpPeerDevicePath] = connection; ChipLogDetail(DeviceLayer, "New BLE connection: conn %p, device %s, path %s", connection, connection->GetPeerAddress(), mpPeerDevicePath); @@ -364,14 +364,14 @@ void BluezEndpoint::HandleNewDevice(BluezDevice1 * device) BluezConnection * conn; VerifyOrExit(bluez_device1_get_connected(device), ChipLogError(DeviceLayer, "FAIL: device is not connected")); - conn = static_cast(g_hash_table_lookup(mpConnMap, g_dbus_proxy_get_object_path(G_DBUS_PROXY(device)))); + conn = GetBluezConnection(g_dbus_proxy_get_object_path(G_DBUS_PROXY(device))); VerifyOrExit(conn == nullptr, ChipLogError(DeviceLayer, "FAIL: connection already tracked: conn: %p new device: %s", conn, g_dbus_proxy_get_object_path(G_DBUS_PROXY(device)))); - conn = chip::Platform::New(*this, device); - mpPeerDevicePath = g_strdup(g_dbus_proxy_get_object_path(G_DBUS_PROXY(device))); - g_hash_table_insert(mpConnMap, mpPeerDevicePath, conn); + conn = chip::Platform::New(*this, device); + mpPeerDevicePath = g_strdup(g_dbus_proxy_get_object_path(G_DBUS_PROXY(device))); + mConnMap[mpPeerDevicePath] = conn; ChipLogDetail(DeviceLayer, "BLE device connected: conn %p, device %s, path %s", conn, conn->GetPeerAddress(), mpPeerDevicePath); @@ -469,9 +469,15 @@ void BluezEndpoint::SetupAdapter() g_list_free_full(objects, g_object_unref); } +BluezConnection * BluezEndpoint::GetBluezConnection(const char * aPath) +{ + auto it = mConnMap.find(aPath); + return (it != mConnMap.end()) ? it->second : nullptr; +} + BluezConnection * BluezEndpoint::GetBluezConnectionViaDevice() { - return static_cast(g_hash_table_lookup(mpConnMap, mpPeerDevicePath)); + return GetBluezConnection(mpPeerDevicePath); } #if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING @@ -647,16 +653,6 @@ CHIP_ERROR BluezEndpoint::StartupEndpointBindings() return CHIP_NO_ERROR; } -BluezEndpoint::BluezEndpoint() -{ - mpConnMap = g_hash_table_new(g_str_hash, g_str_equal); -} - -BluezEndpoint::~BluezEndpoint() -{ - g_hash_table_destroy(mpConnMap); -} - CHIP_ERROR BluezEndpoint::RegisterGattApplication() { CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync( diff --git a/src/platform/Linux/bluez/BluezEndpoint.h b/src/platform/Linux/bluez/BluezEndpoint.h index aa83998acd528b..c01976f6d3d873 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.h +++ b/src/platform/Linux/bluez/BluezEndpoint.h @@ -46,6 +46,8 @@ #pragma once #include +#include +#include #include #include @@ -64,8 +66,8 @@ namespace Internal { class BluezEndpoint { public: - BluezEndpoint(); - ~BluezEndpoint(); + BluezEndpoint() = default; + ~BluezEndpoint() = default; CHIP_ERROR Init(uint32_t aAdapterId, bool aIsCentral, const char * apBleAddr, const char * apBleName); void Shutdown(); @@ -101,6 +103,7 @@ class BluezEndpoint void HandleNewDevice(BluezDevice1 * aDevice); void UpdateConnectionTable(BluezDevice1 * aDevice); + BluezConnection * GetBluezConnection(const char * aPath); BluezConnection * GetBluezConnectionViaDevice(); gboolean BluezCharacteristicReadValue(BluezGattCharacteristic1 * aChar, GDBusMethodInvocation * aInv, GVariant * aOptions); @@ -120,10 +123,13 @@ class BluezEndpoint static void ConnectDeviceDone(GObject * aObject, GAsyncResult * aResult, gpointer apParams); static CHIP_ERROR ConnectDeviceImpl(ConnectParams * apParams); + bool mIsCentral = false; + // Bus owning name char * mpOwningName = nullptr; // Adapter properties + uint32_t mAdapterId = 0; char * mpAdapterName = nullptr; char * mpAdapterAddr = nullptr; @@ -144,12 +150,9 @@ class BluezEndpoint // additional data characteristics BluezGattCharacteristic1 * mpC3 = nullptr; - // map device path to the connection - GHashTable * mpConnMap = nullptr; - uint32_t mAdapterId = 0; - bool mIsCentral = false; - char * mpPeerDevicePath = nullptr; + std::unordered_map mConnMap; GCancellable * mpConnectCancellable = nullptr; + char * mpPeerDevicePath = nullptr; // Allow BluezAdvertisement and BluezConnection to access our private members // TODO: Fix this tight coupling