Skip to content

Commit

Permalink
[Linux] Reuse AdapterIterator in all places where listing managed obj…
Browse files Browse the repository at this point in the history
…ects (#32372)

* Use dedicated iterator for listing BlueZ objects

* Use GAutoPtr to manage GDBusObjectManager instance

* Wrap static function with lambda

* Fix iterator initialization for AdapterIterator
  • Loading branch information
arkq authored and pull[bot] committed Jun 6, 2024
1 parent 16a543d commit 1438254
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 89 deletions.
6 changes: 6 additions & 0 deletions src/platform/GLibTypeDeleter.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ struct GAutoPtrDeleter<GDBusConnection>
using deleter = GObjectDeleter;
};

template <>
struct GAutoPtrDeleter<GDBusObjectManager>
{
using deleter = GObjectDeleter;
};

template <>
struct GAutoPtrDeleter<GDBusObjectManagerServer>
{
Expand Down
64 changes: 17 additions & 47 deletions src/platform/Linux/bluez/AdapterIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,69 +26,38 @@ namespace chip {
namespace DeviceLayer {
namespace Internal {

AdapterIterator::~AdapterIterator()
{
if (mManager != nullptr)
{
g_object_unref(mManager);
}

if (mObjectList != nullptr)
{
g_list_free_full(mObjectList, g_object_unref);
}
}

CHIP_ERROR AdapterIterator::Initialize(AdapterIterator * self)
CHIP_ERROR AdapterIterator::Initialize()
{
// When creating D-Bus proxy object, the thread default context must be initialized. Otherwise,
// all D-Bus signals will be delivered to the GLib global default main context.
VerifyOrDie(g_main_context_get_thread_default() != nullptr);

CHIP_ERROR err = CHIP_NO_ERROR;
GAutoPtr<GError> error;

self->mManager = g_dbus_object_manager_client_new_for_bus_sync(
mManager.reset(g_dbus_object_manager_client_new_for_bus_sync(
G_BUS_TYPE_SYSTEM, G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_NONE, BLUEZ_INTERFACE, "/",
bluez_object_manager_client_get_proxy_type, nullptr /* unused user data in the Proxy Type Func */,
nullptr /* destroy notify */, nullptr /* cancellable */, &error.GetReceiver());
nullptr /* destroy notify */, nullptr /* cancellable */, &error.GetReceiver()));

VerifyOrExit(self->mManager != nullptr, ChipLogError(DeviceLayer, "Failed to get DBUS object manager for listing adapters.");
err = CHIP_ERROR_INTERNAL);
VerifyOrReturnError(mManager, CHIP_ERROR_INTERNAL,
ChipLogError(DeviceLayer, "Failed to get D-Bus object manager for listing adapters: %s", error->message));

self->mObjectList = g_dbus_object_manager_get_objects(self->mManager);
self->mCurrentListItem = self->mObjectList;
mObjectList.Init(mManager.get());
mIterator = mObjectList.begin();

exit:
if (error != nullptr)
{
ChipLogError(DeviceLayer, "DBus error: %s", error->message);
}

return err;
return CHIP_NO_ERROR;
}

bool AdapterIterator::Advance()
{
if (mCurrentListItem == nullptr)
for (; mIterator != BluezObjectList::end(); ++mIterator)
{
return false;
}

while (mCurrentListItem != nullptr)
{
BluezAdapter1 * adapter = bluez_object_get_adapter1(BLUEZ_OBJECT(mCurrentListItem->data));
if (adapter == nullptr)
BluezAdapter1 * adapter = bluez_object_get_adapter1(&(*mIterator));
if (adapter != nullptr)
{
mCurrentListItem = mCurrentListItem->next;
continue;
mCurrentAdapter.reset(adapter);
++mIterator;
return true;
}

mCurrentAdapter.reset(adapter);

mCurrentListItem = mCurrentListItem->next;

return true;
}

return false;
Expand All @@ -112,9 +81,10 @@ uint32_t AdapterIterator::GetIndex() const

bool AdapterIterator::Next()
{
if (mManager == nullptr)
if (!mManager)
{
CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(Initialize, this);
CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(
+[](AdapterIterator * self) { return self->Initialize(); }, this);
VerifyOrReturnError(err == CHIP_NO_ERROR, false, ChipLogError(DeviceLayer, "Failed to initialize adapter iterator"));
}

Expand Down
14 changes: 7 additions & 7 deletions src/platform/Linux/bluez/AdapterIterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@

#pragma once

#include <string>
#include <cstdint>

#include <gio/gio.h>

#include <lib/core/CHIPError.h>
#include <platform/GLibTypeDeleter.h>
#include <platform/Linux/dbus/bluez/DbusBluez.h>

#include "BluezObjectList.h"
#include "Types.h"

namespace chip {
Expand All @@ -46,8 +48,6 @@ namespace Internal {
class AdapterIterator
{
public:
~AdapterIterator();

/// Moves to the next DBUS interface.
///
/// MUST be called before any of the 'current value' methods are
Expand All @@ -65,17 +65,17 @@ class AdapterIterator

private:
/// Sets up the DBUS manager and loads the list
static CHIP_ERROR Initialize(AdapterIterator * self);
CHIP_ERROR Initialize();

/// Loads the next value in the list.
///
/// Returns true if a value could be loaded, false if no more items to
/// iterate through.
bool Advance();

GDBusObjectManager * mManager = nullptr; // DBus connection
GList * mObjectList = nullptr; // listing of objects on the bus
GList * mCurrentListItem = nullptr; // current item viewed in the list
GAutoPtr<GDBusObjectManager> mManager;
BluezObjectList mObjectList;
BluezObjectIterator mIterator;
// Data valid only if Next() returns true
GAutoPtr<BluezAdapter1> mCurrentAdapter;
};
Expand Down
17 changes: 5 additions & 12 deletions src/platform/Linux/bluez/BluezConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <system/SystemPacketBuffer.h>

#include "BluezEndpoint.h"
#include "BluezObjectList.h"
#include "Types.h"

namespace chip {
Expand Down Expand Up @@ -95,10 +96,6 @@ BluezConnection::ConnectionDataBundle::ConnectionDataBundle(const BluezConnectio

CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint)
{
// populate the service and the characteristics
GList * objects = nullptr;
GList * l;

if (!aEndpoint.mIsCentral)
{
mpService = reinterpret_cast<BluezGattService1 *>(g_object_ref(aEndpoint.mpService));
Expand All @@ -107,11 +104,9 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint)
}
else
{
objects = g_dbus_object_manager_get_objects(aEndpoint.mpObjMgr);

for (l = objects; l != nullptr; l = l->next)
for (BluezObject & object : BluezObjectList(aEndpoint.mpObjMgr))
{
BluezGattService1 * service = bluez_object_get_gatt_service1(BLUEZ_OBJECT(l->data));
BluezGattService1 * service = bluez_object_get_gatt_service1(&object);
if (service != nullptr)
{
if ((BluezIsServiceOnDevice(service, mpDevice)) == TRUE &&
Expand All @@ -126,9 +121,9 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint)

VerifyOrExit(mpService != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL service in %s", __func__));

for (l = objects; l != nullptr; l = l->next)
for (BluezObject & object : BluezObjectList(aEndpoint.mpObjMgr))
{
BluezGattCharacteristic1 * char1 = bluez_object_get_gatt_characteristic1(BLUEZ_OBJECT(l->data));
BluezGattCharacteristic1 * char1 = bluez_object_get_gatt_characteristic1(&object);
if (char1 != nullptr)
{
if ((BluezIsCharOnService(char1, mpService) == TRUE) &&
Expand Down Expand Up @@ -164,8 +159,6 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint)
}

exit:
if (objects != nullptr)
g_list_free_full(objects, g_object_unref);
return CHIP_NO_ERROR;
}

Expand Down
13 changes: 6 additions & 7 deletions src/platform/Linux/bluez/BluezEndpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
#include <system/SystemPacketBuffer.h>

#include "BluezConnection.h"
#include "BluezObjectList.h"
#include "Types.h"

namespace chip {
Expand Down Expand Up @@ -420,15 +421,14 @@ BluezGattService1 * BluezEndpoint::CreateGattService(const char * aUUID)
return service;
}

void BluezEndpoint::SetupAdapter()
CHIP_ERROR 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 && mAdapter.get() == nullptr; l = l->next)
for (BluezObject & object : BluezObjectList(mpObjMgr))
{
GAutoPtr<BluezAdapter1> adapter(bluez_object_get_adapter1(BLUEZ_OBJECT(l->data)));
GAutoPtr<BluezAdapter1> adapter(bluez_object_get_adapter1(&object));
if (adapter.get() != nullptr)
{
if (mpAdapterAddr == nullptr) // no adapter address provided, bind to the hci indicated by nodeid
Expand All @@ -450,7 +450,7 @@ void BluezEndpoint::SetupAdapter()
}
}

VerifyOrExit(mAdapter.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mAdapter in %s", __func__));
VerifyOrReturnError(mAdapter, CHIP_ERROR_INTERNAL, ChipLogError(DeviceLayer, "FAIL: NULL mAdapter in %s", __func__));

bluez_adapter1_set_powered(mAdapter.get(), TRUE);

Expand All @@ -459,8 +459,7 @@ void BluezEndpoint::SetupAdapter()
// and the flag is necessary to force using LE transport.
bluez_adapter1_set_discoverable(mAdapter.get(), FALSE);

exit:
g_list_free_full(objects, g_object_unref);
return CHIP_NO_ERROR;
}

BluezConnection * BluezEndpoint::GetBluezConnection(const char * aPath)
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Linux/bluez/BluezEndpoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class BluezEndpoint
private:
CHIP_ERROR StartupEndpointBindings();

void SetupAdapter();
CHIP_ERROR SetupAdapter();
void SetupGattServer(GDBusConnection * aConn);
void SetupGattService();

Expand Down
2 changes: 2 additions & 0 deletions src/platform/Linux/bluez/BluezObjectIterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

#pragma once

#include <iterator>

#include <glib.h>

#include <platform/CHIPDeviceConfig.h>
Expand Down
29 changes: 14 additions & 15 deletions src/platform/Linux/bluez/BluezObjectList.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,27 +35,26 @@ namespace Internal {
class BluezObjectList
{
public:
explicit BluezObjectList(GDBusObjectManager * manager) { Initialize(manager); }
BluezObjectList() = default;
explicit BluezObjectList(GDBusObjectManager * manager) { Init(manager); }

~BluezObjectList() { g_list_free_full(mObjectList, g_object_unref); }

BluezObjectIterator begin() const { return BluezObjectIterator(mObjectList); }
BluezObjectIterator end() const { return BluezObjectIterator(); }

protected:
BluezObjectList() {}

void Initialize(GDBusObjectManager * manager)
~BluezObjectList()
{
if (manager == nullptr)
{
ChipLogError(DeviceLayer, "Manager is NULL in %s", __func__);
return;
}
if (mObjectList != nullptr)
g_list_free_full(mObjectList, g_object_unref);
}

CHIP_ERROR Init(GDBusObjectManager * manager)
{
VerifyOrReturnError(manager != nullptr, CHIP_ERROR_INVALID_ARGUMENT,
ChipLogError(DeviceLayer, "Manager is NULL in %s", __func__));
mObjectList = g_dbus_object_manager_get_objects(manager);
return CHIP_NO_ERROR;
}

BluezObjectIterator begin() const { return BluezObjectIterator(mObjectList); }
static BluezObjectIterator end() { return BluezObjectIterator(); }

private:
GList * mObjectList = nullptr;
};
Expand Down

0 comments on commit 1438254

Please sign in to comment.