Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Linux] Wrap BlueZ endpoint functions in a class #30063

Merged
merged 25 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
12b6388
Wrap BluezEndpoint in a class
arkq Oct 3, 2023
0092b02
Change pointer to reference in handle new device
arkq Oct 10, 2023
e69bf48
BluezEndpoint: Move some static functions to class
arkq Oct 12, 2023
d6a9e92
Remove unnecessary memory allocation
arkq Oct 25, 2023
b91344f
Make GATT service setup function names more descriptive
arkq Oct 25, 2023
1a93eff
Do not pass magic string to CreateGattService
arkq Oct 25, 2023
f6ebe92
More class encapsulation
arkq Oct 25, 2023
d2a32f3
Reduce number of signals on the D-Bus during initialization
arkq Oct 27, 2023
0160ca6
Keep D-Bus callbacks in BluezEndpoint class
arkq Oct 27, 2023
c2d12e4
Keep signal callbacks in BluezEndpoint class
arkq Oct 27, 2023
db00308
Get rid of explicit passing of this pointer
arkq Oct 27, 2023
b567a25
Keep data of BluezEndpoint private
arkq Oct 27, 2023
7064ad4
Fixup for naming convention
arkq Oct 27, 2023
fe7642d
Get rid of static class members
arkq Oct 27, 2023
a667125
Keep register app done callback in class
arkq Oct 27, 2023
5c7c0cc
Fix error handling when allocating BluezEndpoint
arkq Oct 28, 2023
6902b2b
Do not use assert() in the code
arkq Oct 30, 2023
43cfaeb
Fix crash when trying to read characteristic without prior write
arkq Oct 30, 2023
7edddec
Do not use pointer for BluezEndpoint
arkq Oct 30, 2023
1163c2f
Get rid of BluezEndpoint pointers
arkq Nov 13, 2023
d052a69
Fix double-free when destroying BluezEndpoint
arkq Nov 13, 2023
e8e785d
Use c++ map for storing BlueConnection objects
arkq Nov 13, 2023
73a7a91
Decouple BLE adv with BluezEndpoint private members
arkq Nov 13, 2023
b95b171
Prevent double free when calling shutdown twice
arkq Nov 13, 2023
833df1e
Keep SuccessOrExit in one line with function call
arkq Nov 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 17 additions & 22 deletions src/platform/Linux/BLEManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
* platform/<PLATFORM>/BLEManagerImpl.h after defining interface class. */
#include "platform/internal/BLEManager.h"

#include <cassert>
#include <type_traits>
#include <utility>

Expand Down Expand Up @@ -63,9 +62,8 @@ const ChipBleUUID ChipUUID_CHIPoBLEChar_TX = { { 0x18, 0xEE, 0x2E, 0xF5, 0x26, 0

void HandleConnectTimeout(chip::System::Layer *, void * apEndpoint)
{
assert(apEndpoint != nullptr);

CancelConnect(static_cast<BluezEndpoint *>(apEndpoint));
VerifyOrDie(apEndpoint != nullptr);
static_cast<BluezEndpoint *>(apEndpoint)->CancelConnect();
BLEManagerImpl::HandleConnectFailed(CHIP_ERROR_TIMEOUT);
}

Expand Down Expand Up @@ -105,8 +103,10 @@ void BLEManagerImpl::_Shutdown()
mDeviceScanner.Shutdown();
// Stop advertising and free resources.
mBLEAdvertisement.Shutdown();
// Make sure that the endpoint is not used by the timer.
DeviceLayer::SystemLayer().CancelTimer(HandleConnectTimeout, &mEndpoint);
// Release BLE connection resources (unregister from BlueZ).
ShutdownBluezBleLayer(mpEndpoint);
mEndpoint.Shutdown();
mFlags.Clear(Flags::kBluezBLELayerInitialized);
}

Expand Down Expand Up @@ -571,16 +571,14 @@ void BLEManagerImpl::DriveBLEState()
// Initializes the Bluez BLE layer if needed.
if (mServiceMode == ConnectivityManager::kCHIPoBLEServiceMode_Enabled && !mFlags.Has(Flags::kBluezBLELayerInitialized))
{
err = InitBluezBleLayer(mAdapterId, mIsCentral, nullptr, mDeviceName, mpEndpoint);
SuccessOrExit(err);
SuccessOrExit(err = mEndpoint.Init(mAdapterId, mIsCentral, nullptr, mDeviceName));
mFlags.Set(Flags::kBluezBLELayerInitialized);
}

// Register the CHIPoBLE application with the Bluez BLE layer if needed.
if (!mIsCentral && mServiceMode == ConnectivityManager::kCHIPoBLEServiceMode_Enabled && !mFlags.Has(Flags::kAppRegistered))
{
err = BluezGattsAppRegister(mpEndpoint);
SuccessOrExit(err);
SuccessOrExit(err = mEndpoint.RegisterGattApplication());
mFlags.Set(Flags::kControlOpInProgress);
ExitNow();
}
Expand All @@ -597,15 +595,13 @@ void BLEManagerImpl::DriveBLEState()
// Configure advertising data if it hasn't been done yet.
if (!mFlags.Has(Flags::kAdvertisingConfigured))
{
err = mBLEAdvertisement.Init(mpEndpoint, mBLEAdvType, mpBLEAdvUUID, mBLEAdvDurationMs);
SuccessOrExit(err);
SuccessOrExit(err = mBLEAdvertisement.Init(mEndpoint, mBLEAdvType, mpBLEAdvUUID, mBLEAdvDurationMs));
mFlags.Set(Flags::kAdvertisingConfigured);
}

// Start advertising. This is an asynchronous step. BLE manager will be notified of
// advertising start completion via a call to NotifyBLEPeripheralAdvStartComplete.
err = mBLEAdvertisement.Start();
SuccessOrExit(err);
SuccessOrExit(err = mBLEAdvertisement.Start());
mFlags.Set(Flags::kControlOpInProgress);
ExitNow();
}
Expand All @@ -616,8 +612,7 @@ void BLEManagerImpl::DriveBLEState()
{
if (mFlags.Has(Flags::kAdvertising))
{
err = mBLEAdvertisement.Stop();
SuccessOrExit(err);
SuccessOrExit(err = mBLEAdvertisement.Stop());
mFlags.Set(Flags::kControlOpInProgress);

ExitNow();
Expand Down Expand Up @@ -653,14 +648,14 @@ void BLEManagerImpl::InitiateScan(BleScanState scanType)
return;
}

if (mpEndpoint == nullptr)
if (!mFlags.Has(Flags::kBluezBLELayerInitialized))
{
BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, CHIP_ERROR_INCORRECT_STATE);
ChipLogError(Ble, "BLE Layer is not yet initialized");
return;
}

if (mpEndpoint->mpAdapter == nullptr)
if (mEndpoint.GetAdapter() == nullptr)
{
BleConnectionDelegate::OnConnectionError(mBLEScanConfig.mAppState, CHIP_ERROR_INCORRECT_STATE);
ChipLogError(Ble, "No adapter available for new connection establishment");
Expand All @@ -669,7 +664,7 @@ void BLEManagerImpl::InitiateScan(BleScanState scanType)

mBLEScanConfig.mBleScanState = scanType;

CHIP_ERROR err = mDeviceScanner.Init(mpEndpoint->mpAdapter, this);
CHIP_ERROR err = mDeviceScanner.Init(mEndpoint.GetAdapter(), this);
if (err != CHIP_NO_ERROR)
{
mBLEScanConfig.mBleScanState = BleScanState::kNotScanning;
Expand All @@ -691,7 +686,7 @@ void BLEManagerImpl::InitiateScan(BleScanState scanType)
void BLEManagerImpl::CleanScanConfig()
{
if (mBLEScanConfig.mBleScanState == BleScanState::kConnecting)
DeviceLayer::SystemLayer().CancelTimer(HandleConnectTimeout, mpEndpoint);
DeviceLayer::SystemLayer().CancelTimer(HandleConnectTimeout, &mEndpoint);

mBLEScanConfig.mBleScanState = BleScanState::kNotScanning;
}
Expand All @@ -713,7 +708,7 @@ void BLEManagerImpl::NewConnection(BleLayer * bleLayer, void * appState, const S
CHIP_ERROR BLEManagerImpl::CancelConnection()
{
if (mBLEScanConfig.mBleScanState == BleScanState::kConnecting)
arkq marked this conversation as resolved.
Show resolved Hide resolved
CancelConnect(mpEndpoint);
mEndpoint.CancelConnect();
// If in discovery mode, stop scan.
else if (mBLEScanConfig.mBleScanState != BleScanState::kNotScanning)
mDeviceScanner.StopScan();
Expand Down Expand Up @@ -777,12 +772,12 @@ void BLEManagerImpl::OnDeviceScanned(BluezDevice1 & device, const chip::Ble::Chi
mBLEScanConfig.mBleScanState = BleScanState::kConnecting;

chip::DeviceLayer::PlatformMgr().LockChipStack();
DeviceLayer::SystemLayer().StartTimer(kConnectTimeout, HandleConnectTimeout, mpEndpoint);
DeviceLayer::SystemLayer().StartTimer(kConnectTimeout, HandleConnectTimeout, &mEndpoint);
chip::DeviceLayer::PlatformMgr().UnlockChipStack();

mDeviceScanner.StopScan();

ConnectDevice(device, mpEndpoint);
mEndpoint.ConnectDevice(device);
}

void BLEManagerImpl::OnScanComplete()
Expand Down
4 changes: 2 additions & 2 deletions src/platform/Linux/BLEManagerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ class BLEManagerImpl final : public BLEManager,

uint32_t mAdapterId = 0;
char mDeviceName[kMaxDeviceNameLength + 1];
bool mIsCentral = false;
BluezEndpoint * mpEndpoint = nullptr;
bool mIsCentral = false;
BluezEndpoint mEndpoint;

BluezAdvertisement mBLEAdvertisement;
ChipAdvType mBLEAdvType = ChipAdvType::BLUEZ_ADV_TYPE_UNDIRECTED_CONNECTABLE_SCANNABLE;
Expand Down
10 changes: 4 additions & 6 deletions src/platform/Linux/bluez/BluezAdvertisement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,20 +133,18 @@ CHIP_ERROR BluezAdvertisement::InitImpl()
return CHIP_NO_ERROR;
}

CHIP_ERROR BluezAdvertisement::Init(BluezEndpoint * apEndpoint, ChipAdvType aAdvType, const char * aAdvUUID,
CHIP_ERROR BluezAdvertisement::Init(const BluezEndpoint & aEndpoint, ChipAdvType aAdvType, const char * aAdvUUID,
uint32_t aAdvDurationMs)
{
GAutoPtr<char> rootPath;
CHIP_ERROR err;

VerifyOrExit(apEndpoint != nullptr, err = CHIP_ERROR_INCORRECT_STATE;
ChipLogError(DeviceLayer, "FAIL: NULL endpoint in %s", __func__));
VerifyOrExit(mpAdv == nullptr, err = CHIP_ERROR_INCORRECT_STATE;
ChipLogError(DeviceLayer, "FAIL: BLE advertisement already initialized in %s", __func__));

mpRoot = reinterpret_cast<GDBusObjectManagerServer *>(g_object_ref(apEndpoint->mpRoot));
mpAdapter = reinterpret_cast<BluezAdapter1 *>(g_object_ref(apEndpoint->mpAdapter));
mpAdapterName = g_strdup(apEndpoint->mpAdapterName);
mpRoot = reinterpret_cast<GDBusObjectManagerServer *>(g_object_ref(aEndpoint.GetGattApplicationObjectManager()));
mpAdapter = reinterpret_cast<BluezAdapter1 *>(g_object_ref(aEndpoint.GetAdapter()));
mpAdapterName = g_strdup(aEndpoint.GetAdapterName());

g_object_get(G_OBJECT(mpRoot), "object-path", &MakeUniquePointerReceiver(rootPath).Get(), nullptr);
mpAdvPath = g_strdup_printf("%s/advertising", rootPath.get());
Expand Down
4 changes: 2 additions & 2 deletions src/platform/Linux/bluez/BluezAdvertisement.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ namespace chip {
namespace DeviceLayer {
namespace Internal {

struct BluezEndpoint;
class BluezEndpoint;

class BluezAdvertisement
{
public:
BluezAdvertisement() = default;
~BluezAdvertisement() { Shutdown(); }

CHIP_ERROR Init(BluezEndpoint * apEndpoint, 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.
Expand Down
18 changes: 9 additions & 9 deletions src/platform/Linux/bluez/BluezConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ gboolean BluezIsCharOnService(BluezGattCharacteristic1 * aChar, BluezGattService

} // namespace

BluezConnection::BluezConnection(BluezEndpoint * apEndpoint, BluezDevice1 * apDevice) :
mpEndpoint(apEndpoint), mpDevice(BLUEZ_DEVICE1(g_object_ref(apDevice)))
BluezConnection::BluezConnection(const BluezEndpoint & aEndpoint, BluezDevice1 * apDevice) :
mpDevice(BLUEZ_DEVICE1(g_object_ref(apDevice)))
{
Init();
Init(aEndpoint);
}

BluezConnection::~BluezConnection()
Expand Down Expand Up @@ -93,21 +93,21 @@ BluezConnection::ConnectionDataBundle::ConnectionDataBundle(const BluezConnectio
mData(g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, aBuf->Start(), aBuf->DataLength(), sizeof(uint8_t)))
{}

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

if (!mpEndpoint->mIsCentral)
if (!aEndpoint.mIsCentral)
{
mpService = BLUEZ_GATT_SERVICE1(g_object_ref(mpEndpoint->mpService));
mpC1 = BLUEZ_GATT_CHARACTERISTIC1(g_object_ref(mpEndpoint->mpC1));
mpC2 = BLUEZ_GATT_CHARACTERISTIC1(g_object_ref(mpEndpoint->mpC2));
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));
}
else
{
objects = g_dbus_object_manager_get_objects(mpEndpoint->mpObjMgr);
objects = g_dbus_object_manager_get_objects(aEndpoint.mpObjMgr);

for (l = objects; l != nullptr; l = l->next)
{
Expand Down
7 changes: 3 additions & 4 deletions src/platform/Linux/bluez/BluezConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ namespace chip {
namespace DeviceLayer {
namespace Internal {

struct BluezEndpoint;
class BluezEndpoint;

class BluezConnection
{
public:
BluezConnection(BluezEndpoint * apEndpoint, BluezDevice1 * apDevice);
BluezConnection(const BluezEndpoint & aEndpoint, BluezDevice1 * apDevice);
~BluezConnection();

const char * GetPeerAddress() const;
Expand Down Expand Up @@ -95,7 +95,7 @@ class BluezConnection
GAutoPtr<GVariant> mData;
};

CHIP_ERROR Init();
CHIP_ERROR Init(const BluezEndpoint & aEndpoint);

static CHIP_ERROR BluezDisconnect(BluezConnection * apConn);

Expand All @@ -115,7 +115,6 @@ class BluezConnection
static void UnsubscribeCharacteristicDone(GObject * aObject, GAsyncResult * aResult, gpointer apConn);
static CHIP_ERROR UnsubscribeCharacteristicImpl(BluezConnection * apConn);

BluezEndpoint * mpEndpoint;
BluezDevice1 * mpDevice;

bool mNotifyAcquired = false;
Expand Down
Loading
Loading