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.

Sign up for GitHub

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] Handle BLE adapter re-appearance due to BlueZ restart #32847

Merged
merged 5 commits into from
Apr 23, 2024
Merged

[Linux] Handle BLE adapter re-appearance due to BlueZ restart #32847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
927a489
Keep all timer handlers in BLEManagerImpl class
arkq Mar 22, 2024
eb67cac
[Linux] Handle BLE adapter re-appearance
arkq Mar 22, 2024
841b256
Simplify casting for callbacks
arkq Mar 22, 2024
5563747
Fix adapter added notification
arkq Mar 25, 2024
7ba16fb
Add TODO for potential issue with LE-only adapters
arkq Apr 4, 2024
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
80 changes: 54 additions & 26 deletions src/platform/Linux/BLEManagerImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,6 @@ const ChipBleUUID ChipUUID_CHIPoBLEChar_RX = { { 0x18, 0xEE, 0x2E, 0xF5, 0x26, 0
const ChipBleUUID ChipUUID_CHIPoBLEChar_TX = { { 0x18, 0xEE, 0x2E, 0xF5, 0x26, 0x3D, 0x45, 0x59, 0x95, 0x9F, 0x4F, 0x9C, 0x42, 0x9F,
0x9D, 0x12 } };

void HandleConnectTimeout(chip::System::Layer *, void * apEndpoint)
{
VerifyOrDie(apEndpoint != nullptr);
static_cast<BluezEndpoint *>(apEndpoint)->CancelConnect();
BLEManagerImpl::HandleConnectFailed(CHIP_ERROR_TIMEOUT);
}

} // namespace

BLEManagerImpl BLEManagerImpl::sInstance;
Expand Down Expand Up @@ -117,16 +110,17 @@ CHIP_ERROR BLEManagerImpl::_Init()

void BLEManagerImpl::_Shutdown()
{
// Ensure scan resources are cleared (e.g. timeout timers).
// Make sure that timers are stopped before shutting down the BLE layer.
DeviceLayer::SystemLayer().CancelTimer(HandleScanTimer, this);
DeviceLayer::SystemLayer().CancelTimer(HandleAdvertisingTimer, this);
DeviceLayer::SystemLayer().CancelTimer(HandleConnectTimer, this);

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).
mEndpoint.Shutdown();

mBluezObjectManager.Shutdown();
mFlags.Clear(Flags::kBluezBLELayerInitialized);
mFlags.Clear(Flags::kBluezManagerInitialized);
}

CHIP_ERROR BLEManagerImpl::_SetAdvertisingEnabled(bool val)
Expand Down Expand Up @@ -263,15 +257,30 @@ void BLEManagerImpl::HandlePlatformSpecificBLEEvent(const ChipDeviceEvent * apEv
apEvent->Platform.BLEAdapter.mAdapterAddress);
if (apEvent->Platform.BLEAdapter.mAdapterId == mAdapterId)
{
// TODO: Handle adapter added
mServiceMode = ConnectivityManager::kCHIPoBLEServiceMode_Enabled;
DriveBLEState();
}
break;
case DeviceEventType::kPlatformLinuxBLEAdapterRemoved:
ChipLogDetail(DeviceLayer, "BLE adapter removed: id=%u address=%s", apEvent->Platform.BLEAdapter.mAdapterId,
apEvent->Platform.BLEAdapter.mAdapterAddress);
if (apEvent->Platform.BLEAdapter.mAdapterId == mAdapterId)
{
// TODO: Handle adapter removed
// Shutdown all BLE operations and release resources
mDeviceScanner.Shutdown();
mBLEAdvertisement.Shutdown();
mEndpoint.Shutdown();
// Drop reference to the adapter
mAdapter.reset();
// Clear all flags related to BlueZ BLE operations
mFlags.Clear(Flags::kBluezAdapterAvailable);
mFlags.Clear(Flags::kBluezBLELayerInitialized);
mFlags.Clear(Flags::kAdvertisingConfigured);
mFlags.Clear(Flags::kAppRegistered);
mFlags.Clear(Flags::kAdvertising);
CleanScanConfig();
// Indicate that the adapter is no longer available
err = BLE_ERROR_ADAPTER_UNAVAILABLE;
}
break;
case DeviceEventType::kPlatformLinuxBLECentralConnected:
Expand Down Expand Up @@ -345,9 +354,7 @@ void BLEManagerImpl::HandlePlatformSpecificBLEEvent(const ChipDeviceEvent * apEv
exit:
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Disabling CHIPoBLE service due to error: %s", ErrorStr(err));
mServiceMode = ConnectivityManager::kCHIPoBLEServiceMode_Disabled;
DeviceLayer::SystemLayer().CancelTimer(HandleAdvertisingTimer, this);
DisableBLEService(err);
mFlags.Clear(Flags::kControlOpInProgress);
}
}
Expand Down Expand Up @@ -681,9 +688,23 @@ void BLEManagerImpl::DriveBLEState()
exit:
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Disabling CHIPoBLE service due to error: %s", ErrorStr(err));
DisableBLEService(err);
}
}

void BLEManagerImpl::DisableBLEService(CHIP_ERROR err)
{
ChipLogError(DeviceLayer, "Disabling CHIPoBLE service due to error: %" CHIP_ERROR_FORMAT, err.Format());
mServiceMode = ConnectivityManager::kCHIPoBLEServiceMode_Disabled;
// Stop all timers if the error is other than BLE adapter unavailable. In case of BLE adapter
// beeing unavailable, we will keep timers running, as the adapter might become available in
// the nearest future (e.g. BlueZ restart due to crash). By doing that we will ensure that BLE
// adapter reappearance will not extend timeouts for the ongoing operations.
if (err != BLE_ERROR_ADAPTER_UNAVAILABLE)
{
DeviceLayer::SystemLayer().CancelTimer(HandleScanTimer, this);
DeviceLayer::SystemLayer().CancelTimer(HandleAdvertisingTimer, this);
mServiceMode = ConnectivityManager::kCHIPoBLEServiceMode_Disabled;
DeviceLayer::SystemLayer().CancelTimer(HandleConnectTimer, this);
}
}

Expand Down Expand Up @@ -769,7 +790,7 @@ void BLEManagerImpl::InitiateScan(BleScanState scanType)
ChipLogError(Ble, "Failed to start BLE scan: %" CHIP_ERROR_FORMAT, err.Format());
});

err = DeviceLayer::SystemLayer().StartTimer(kNewConnectionScanTimeout, HandleScannerTimer, this);
err = DeviceLayer::SystemLayer().StartTimer(kNewConnectionScanTimeout, HandleScanTimer, this);
VerifyOrExit(err == CHIP_NO_ERROR, {
mBLEScanConfig.mBleScanState = BleScanState::kNotScanning;
mDeviceScanner.StopScan();
Expand All @@ -783,7 +804,7 @@ void BLEManagerImpl::InitiateScan(BleScanState scanType)
}
}

void BLEManagerImpl::HandleScannerTimer(chip::System::Layer *, void * appState)
void BLEManagerImpl::HandleScanTimer(chip::System::Layer *, void * appState)
{
auto * manager = static_cast<BLEManagerImpl *>(appState);
manager->OnScanError(CHIP_ERROR_TIMEOUT);
Expand All @@ -793,7 +814,7 @@ void BLEManagerImpl::HandleScannerTimer(chip::System::Layer *, void * appState)
void BLEManagerImpl::CleanScanConfig()
{
if (mBLEScanConfig.mBleScanState == BleScanState::kConnecting)
DeviceLayer::SystemLayer().CancelTimer(HandleConnectTimeout, &mEndpoint);
DeviceLayer::SystemLayer().CancelTimer(HandleConnectTimer, this);

mBLEScanConfig.mBleScanState = BleScanState::kNotScanning;
}
Expand All @@ -814,7 +835,7 @@ CHIP_ERROR BLEManagerImpl::CancelConnection()
// If in discovery mode, stop scan.
else if (mBLEScanConfig.mBleScanState != BleScanState::kNotScanning)
{
DeviceLayer::SystemLayer().CancelTimer(HandleScannerTimer, this);
DeviceLayer::SystemLayer().CancelTimer(HandleScanTimer, this);
mDeviceScanner.StopScan();
}
return CHIP_NO_ERROR;
Expand Down Expand Up @@ -905,10 +926,10 @@ void BLEManagerImpl::OnDeviceScanned(BluezDevice1 & device, const chip::Ble::Chi
// We StartScan in the ChipStack thread.
// StopScan should also be performed in the ChipStack thread.
// At the same time, the scan timer also needs to be canceled in the ChipStack thread.
DeviceLayer::SystemLayer().CancelTimer(HandleScannerTimer, this);
DeviceLayer::SystemLayer().CancelTimer(HandleScanTimer, this);
mDeviceScanner.StopScan();
// Stop scanning and then start connecting timer
DeviceLayer::SystemLayer().StartTimer(kConnectTimeout, HandleConnectTimeout, &mEndpoint);
DeviceLayer::SystemLayer().StartTimer(kConnectTimeout, HandleConnectTimer, this);
chip::DeviceLayer::PlatformMgr().UnlockChipStack();

CHIP_ERROR err = mEndpoint.ConnectDevice(device);
Expand All @@ -917,6 +938,13 @@ void BLEManagerImpl::OnDeviceScanned(BluezDevice1 & device, const chip::Ble::Chi
ChipLogProgress(Ble, "New device connected: %s", address);
}

void BLEManagerImpl::HandleConnectTimer(chip::System::Layer *, void * appState)
{
auto * manager = static_cast<BLEManagerImpl *>(appState);
manager->mEndpoint.CancelConnect();
BLEManagerImpl::HandleConnectFailed(CHIP_ERROR_TIMEOUT);
}

void BLEManagerImpl::OnScanComplete()
{
switch (mBLEScanConfig.mBleScanState)
Expand Down
7 changes: 5 additions & 2 deletions src/platform/Linux/BLEManagerImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,15 @@ class BLEManagerImpl final : public BLEManager,
};

void DriveBLEState();
void DisableBLEService(CHIP_ERROR err);
BluezAdvertisement::AdvertisingIntervals GetAdvertisingIntervals() const;
static void HandleAdvertisingTimer(chip::System::Layer *, void * appState);
void InitiateScan(BleScanState scanType);
static void HandleScannerTimer(chip::System::Layer *, void * appState);
void CleanScanConfig();

static void HandleAdvertisingTimer(chip::System::Layer *, void * appState);
static void HandleScanTimer(chip::System::Layer *, void * appState);
static void HandleConnectTimer(chip::System::Layer *, void * appState);

CHIPoBLEServiceMode mServiceMode;
BitFlags<Flags> mFlags;

Expand Down
46 changes: 34 additions & 12 deletions src/platform/Linux/bluez/BluezObjectManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,16 +187,25 @@ BluezObjectManager::NotificationsDelegates BluezObjectManager::GetDeviceNotifica
return delegates;
}

void BluezObjectManager::OnObjectAdded(GDBusObjectManager * aMgr, GDBusObject * aObj)
void BluezObjectManager::OnObjectAdded(GDBusObjectManager * aMgr, BluezObject * aObj)
{
GAutoPtr<BluezAdapter1> adapter(bluez_object_get_adapter1(reinterpret_cast<BluezObject *>(aObj)));
if (adapter)
GAutoPtr<BluezAdapter1> adapter(bluez_object_get_adapter1(aObj));
// Verify that the adapter is properly initialized - the class property must be set.
// BlueZ can export adapter objects on the bus before it is fully initialized. Such
// adapter objects are not usable and must be ignored.
//
// TODO: Find a better way to determine whether the adapter interface exposed by
// BlueZ D-Bus service is fully functional. The current approach is based on
// the assumption that the class property is non-zero, which is true only
// for BR/EDR + LE adapters. LE-only adapters do not have HCI command to read
// the class property and BlueZ sets it to 0 as a default value.
if (adapter && bluez_adapter1_get_class(adapter.get()) != 0)
{
NotifyAdapterAdded(adapter.get());
return;
}

GAutoPtr<BluezDevice1> device(bluez_object_get_device1(reinterpret_cast<BluezObject *>(aObj)));
GAutoPtr<BluezDevice1> device(bluez_object_get_device1(aObj));
if (device)
{
for (auto delegate : GetDeviceNotificationsDelegates(device.get()))
Expand All @@ -206,17 +215,17 @@ void BluezObjectManager::OnObjectAdded(GDBusObjectManager * aMgr, GDBusObject *
}
}

void BluezObjectManager::OnObjectRemoved(GDBusObjectManager * aMgr, GDBusObject * aObj)
void BluezObjectManager::OnObjectRemoved(GDBusObjectManager * aMgr, BluezObject * aObj)
{
GAutoPtr<BluezAdapter1> adapter(bluez_object_get_adapter1(reinterpret_cast<BluezObject *>(aObj)));
GAutoPtr<BluezAdapter1> adapter(bluez_object_get_adapter1(aObj));
if (adapter)
{
RemoveAdapterSubscriptions(adapter.get());
NotifyAdapterRemoved(adapter.get());
return;
}

GAutoPtr<BluezDevice1> device(bluez_object_get_device1(reinterpret_cast<BluezObject *>(aObj)));
GAutoPtr<BluezDevice1> device(bluez_object_get_device1(aObj));
if (device)
{
for (auto delegate : GetDeviceNotificationsDelegates(device.get()))
Expand All @@ -226,10 +235,22 @@ void BluezObjectManager::OnObjectRemoved(GDBusObjectManager * aMgr, GDBusObject
}
}

void BluezObjectManager::OnInterfacePropertiesChanged(GDBusObjectManagerClient * aMgr, GDBusObjectProxy * aObj, GDBusProxy * aIface,
void BluezObjectManager::OnInterfacePropertiesChanged(GDBusObjectManagerClient * aMgr, BluezObject * aObj, GDBusProxy * aIface,
GVariant * aChangedProps, const char * const * aInvalidatedProps)
{
GAutoPtr<BluezDevice1> device(bluez_object_get_device1(reinterpret_cast<BluezObject *>(aObj)));
uint32_t classValue = 0;
GAutoPtr<BluezAdapter1> adapter(bluez_object_get_adapter1(aObj));
// When the adapter's readonly class property is set, it means that the adapter has been
// fully initialized and is ready to be used. It's most likely that the adapter has been
// previously ignored in the OnObjectAdded callback, so now we can notify the application
// about the new adapter.
if (adapter && g_variant_lookup(aChangedProps, "Class", "u", &classValue) && classValue != 0)
{
NotifyAdapterAdded(adapter.get());
return;
}

GAutoPtr<BluezDevice1> device(bluez_object_get_device1(aObj));
if (device)
{
for (auto delegate : GetDeviceNotificationsDelegates(device.get()))
Expand All @@ -255,18 +276,19 @@ CHIP_ERROR BluezObjectManager::SetupObjectManager()

g_signal_connect(mObjectManager.get(), "object-added",
G_CALLBACK(+[](GDBusObjectManager * mgr, GDBusObject * obj, BluezObjectManager * self) {
return self->OnObjectAdded(mgr, obj);
return self->OnObjectAdded(mgr, reinterpret_cast<BluezObject *>(obj));
}),
this);
g_signal_connect(mObjectManager.get(), "object-removed",
G_CALLBACK(+[](GDBusObjectManager * mgr, GDBusObject * obj, BluezObjectManager * self) {
return self->OnObjectRemoved(mgr, obj);
return self->OnObjectRemoved(mgr, reinterpret_cast<BluezObject *>(obj));
}),
this);
g_signal_connect(mObjectManager.get(), "interface-proxy-properties-changed",
G_CALLBACK(+[](GDBusObjectManagerClient * mgr, GDBusObjectProxy * obj, GDBusProxy * iface, GVariant * changed,
const char * const * invalidated, BluezObjectManager * self) {
return self->OnInterfacePropertiesChanged(mgr, obj, iface, changed, invalidated);
return self->OnInterfacePropertiesChanged(mgr, reinterpret_cast<BluezObject *>(obj), iface, changed,
invalidated);
}),
this);

Expand Down
6 changes: 3 additions & 3 deletions src/platform/Linux/bluez/BluezObjectManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ class BluezObjectManager
using NotificationsDelegates = std::vector<BluezObjectManagerAdapterNotificationsDelegate *>;
NotificationsDelegates GetDeviceNotificationsDelegates(BluezDevice1 * device);

void OnObjectAdded(GDBusObjectManager * aMgr, GDBusObject * aObj);
void OnObjectRemoved(GDBusObjectManager * aMgr, GDBusObject * aObj);
void OnInterfacePropertiesChanged(GDBusObjectManagerClient * aMgr, GDBusObjectProxy * aObj, GDBusProxy * aIface,
void OnObjectAdded(GDBusObjectManager * aMgr, BluezObject * aObj);
void OnObjectRemoved(GDBusObjectManager * aMgr, BluezObject * aObj);
void OnInterfacePropertiesChanged(GDBusObjectManagerClient * aMgr, BluezObject * aObj, GDBusProxy * aIface,
GVariant * aChangedProps, const char * const * aInvalidatedProps);

GAutoPtr<GDBusConnection> mConnection;
Expand Down
Loading