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

arkq · web-flow · commit e47f748fd32f · 2024-04-23T11:43:11.000Z
* Keep all timer handlers in BLEManagerImpl class

* [Linux] Handle BLE adapter re-appearance

* Simplify casting for callbacks

* Fix adapter added notification

* Add TODO for potential issue with LE-only adapters
diff --git a/src/platform/Linux/BLEManagerImpl.cpp b/src/platform/Linux/BLEManagerImpl.cpp
@@ -77,13 +77,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;
@@ -116,16 +109,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)
@@ -262,15 +256,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:
@@ -344,9 +353,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);
     }
 }
@@ -680,9 +687,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);
     }
 }
 
@@ -768,7 +789,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();
@@ -782,7 +803,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);
@@ -792,7 +813,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;
 }
@@ -813,7 +834,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;
@@ -904,10 +925,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);
@@ -916,6 +937,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)
diff --git a/src/platform/Linux/BLEManagerImpl.h b/src/platform/Linux/BLEManagerImpl.h
@@ -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;
 
diff --git a/src/platform/Linux/bluez/BluezObjectManager.cpp b/src/platform/Linux/bluez/BluezObjectManager.cpp
@@ -188,16 +188,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()))
@@ -207,17 +216,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()))
@@ -227,10 +236,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()))
@@ -256,18 +277,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);
 
diff --git a/src/platform/Linux/bluez/BluezObjectManager.h b/src/platform/Linux/bluez/BluezObjectManager.h
@@ -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;
