Bluetooth and Wi-Fi improvements (#622)

- Enable BT by default in DTS, and create separate radio on/of functionality (like recent Wi-Fi changes) - Bluetooth event handling now uses queued subscriptions, improving event delivery and application responsiveness. - Improved synchronization of Bluetooth advertising, connections, HID, MIDI, and SPP operations. - Enhanced cleanup during Bluetooth shutdown and subscription removal. - Clarified Bluetooth start and stop behavior. - Improved wifi driver subscription stability to prevent endless loops
2026-08-26 00:30:03 +02:00
parent ab75d2022d
commit 0ee2415f3b
50 changed files with 458 additions and 270 deletions
@@ -26,10 +26,10 @@ struct PeerRecord {
    int profileId = 0;
 };

-// Wrapper around device start & radio on
+// Turns the BT radio on (dev is already started - see ble0's devicetree status)
 bool start(Device* dev);

-// Wrapper around device stop & radio off
+// Turns the BT radio off (dev stays started)
 bool stop(Device* dev);

 bool isRadioOnOrPending(Device* dev);
@@ -7,9 +7,6 @@
 #include <Tactility/bluetooth/Bluetooth.h>
 #include <tactility/drivers/bluetooth.h>

-#include <atomic>
-#include <memory>
-
 namespace tt::app::btmanage {

 struct Context {
@@ -19,13 +16,7 @@ struct Context {
    State state;
    View view = View(&bindings, &state);
    Device* btDevice = nullptr;
-    bool callbackRegistered = false;
-
-    // Bumped right before the BT event callback is unregistered at the end of appMain(), to
-    // invalidate any BT event already dispatched to the main task for this instance. Kept in
-    // its own heap allocation, independent of Context's (stack-local) lifetime, so a dispatched
-    // callback can check it without touching a possibly already-destroyed Context.
-    std::shared_ptr<std::atomic<int>> generation = std::make_shared<std::atomic<int>>(0);
+    BtEventSubscription btEventSub {};

    void lock() { mutex.lock(); }
    void unlock() { mutex.unlock(); }
@@ -34,15 +25,4 @@ struct Context {
 void onBtEvent(Context* ctx, const struct BtEvent& event);
 void requestViewUpdate(Context* ctx);

-// Re-attempts registering the device event callback. Needed because the BLE driver only
-// allocates its callback list while the device is started/on: a registration attempted while
-// the radio is off silently no-ops, so this must be called again right after a successful
-// bluetooth::start(). Idempotent: no-ops if already registered for this device.
-void registerDeviceCallback(Context* ctx, Device* dev);
-
-// Call after bluetooth::stop(): the driver frees its callback list on stop, so the
-// registration state must be cleared here too, without touching the (now-dangling) driver-side
-// list.
-void forgetCallbackRegistration(Context* ctx);
-
 } // namespace tt::app::btmanage
@@ -3,8 +3,6 @@
 #include <Tactility/app/btmanage/BtManagePrivate.h>
 #include <Tactility/app/btmanage/View.h>

-#include <Tactility/Tactility.h>
-
 #include <app/event.h>
 #include <app/manager.h>
 #include <app/manifest.h>
@@ -13,6 +11,7 @@
 #include <lvgl_window_manager/window_manager.h>

 #include <tactility/check.h>
+#include <tactility/device.h>
 #include <tactility/log.h>

 namespace tt::app::btmanage {
@@ -22,26 +21,17 @@ constexpr auto* TAG = "BtManage";
 extern const ::AppManifest manifest;


-static void onBtToggled(void* context, bool requestOn) {
+static void onBtToggled(void* /*context*/, bool requestOn) {
 #if defined(CONFIG_BT_NIMBLE_ENABLED)
-    auto* ctx = static_cast<Context*>(context);
    Device* dev;
    if (device_get_first_by_type(&BLUETOOTH_TYPE, &dev) == ERROR_NONE) {
        bool radio_on = bluetooth::isRadioOnOrPending(dev);
        if (requestOn && !radio_on) {
            LOG_I(TAG, "Turning on");
-            if (bluetooth::start(dev)) {
-                // The driver only allocates its callback list once the device is started,
-                // so the registration attempted at startup (while radio was off) was a
-                // no-op. Register again now that the device is actually up.
-                registerDeviceCallback(ctx, dev);
-            }
+            bluetooth::start(dev);
        } else if (!requestOn && radio_on) {
            LOG_I(TAG, "Turning off");
-            if (bluetooth::stop(dev)) {
-                // A completed stop frees the driver's callback list.
-                forgetCallbackRegistration(ctx);
-            }
+            bluetooth::stop(dev);
        }
        device_put(dev);
    } else {
@@ -85,8 +75,6 @@ static void onForgetPeer(const std::array<uint8_t, 6>& addr) {
    bluetooth::unpair(addr);
 }

-static void onKernelBtEvent(Device* /*device*/, void* context, BtEvent event);
-
 void requestViewUpdate(Context* ctx) {
    // Lock order must match appMain()'s setup/teardown: both run under the LVGL lock
    // and then take `ctx->mutex` internally. Taking `mutex` before lvgl_lock() here would
@@ -143,50 +131,6 @@ void onBtEvent(Context* ctx, const BtEvent& event) {
    requestViewUpdate(ctx);
 }

-static void onKernelBtEvent(Device* /*device*/, void* context, BtEvent event) {
-    // BT event callbacks can fire from the NimBLE host task (e.g. DISCONNECT during
-    // nimble_port_stop shutdown). Calling onBtEvent() synchronously from the NimBLE
-    // task would block it on the LVGL mutex (held by the LVGL task waiting in
-    // nimble_port_stop), creating a permanent deadlock. Dispatch to the main task so
-    // the NimBLE host task is never blocked by BtManage's state updates or LVGL lock.
-    auto* ctx = static_cast<Context*>(context);
-    // Captured while `ctx` is still guaranteed valid (the callback is only invoked while
-    // registered, i.e. before appMain()'s cleanup removes it). Comparing this later -
-    // without dereferencing `ctx` - lets the dispatched lambda detect a stale event from an
-    // instance that has since closed (and had its Context destroyed) without a UAF: the
-    // generation bump in appMain()'s cleanup always happens before window_manager_remove()
-    // destroys ctx's widgets, and this dispatched lambda always re-reads the live generation
-    // at run time (not at dispatch time), so a bump landing anywhere before this lambda
-    // actually runs is enough to make it skip touching ctx.
-    auto generation = ctx->generation;
-    int expectedGeneration = generation->load();
-    getMainDispatcher().dispatch([ctx, generation, expectedGeneration, event] {
-        if (generation->load() != expectedGeneration) {
-            return;
-        }
-        onBtEvent(ctx, event);
-    });
-}
-
-void registerDeviceCallback(Context* ctx, Device* dev) {
-    ctx->lock();
-    if (ctx->btDevice == dev && !ctx->callbackRegistered) {
-        // Only latch the flag on success: while the radio is off the driver has no
-        // callback list yet, so this add is a silent no-op and must be retried once
-        // bluetooth::start() actually brings the device up.
-        if (bluetooth_add_event_callback(dev, ctx, onKernelBtEvent) == ERROR_NONE) {
-            ctx->callbackRegistered = true;
-        }
-    }
-    ctx->unlock();
-}
-
-void forgetCallbackRegistration(Context* ctx) {
-    ctx->lock();
-    ctx->callbackRegistered = false;
-    ctx->unlock();
-}
-
 void onBackPressed(lv_event_t* event) {
    auto* ctx = static_cast<Context*>(lv_event_get_user_data(event));
    AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} };
@@ -229,13 +173,18 @@ int32_t appMain(int argc, char* argv[]) {
    AppEventSubscription sub {};
    check(app_event_subscribe(&sub, &event_group) == ERROR_NONE);

-    WindowId window = window_manager_create(appInstanceId, createWidgets, &ctx);
-
-    ctx.btDevice = dev;
-    if (ctx.btDevice) {
-        registerDeviceCallback(&ctx, ctx.btDevice);
+    // dev is started for the process lifetime once ble0 is enabled in the devicetree -
+    // subscribe once here rather than resubscribing on every bluetooth::start()/stop() toggle.
+    if (dev != nullptr) {
+        if (bluetooth_event_subscribe(dev, &ctx.btEventSub, &event_group) == ERROR_NONE) {
+            ctx.btDevice = dev;
+        } else {
+            LOG_W(TAG, "Failed to subscribe to BT events");
+        }
    }

+    WindowId window = window_manager_create(appInstanceId, createWidgets, &ctx);
+
    auto radio_state = bluetooth::getRadioState();
    bool can_scan = radio_state == bluetooth::RadioState::On;
    LOG_I(TAG, "Radio: %s, Scanning: %d, Can scan: %d",
@@ -261,21 +210,24 @@ int32_t appMain(int argc, char* argv[]) {
            }
            if (shouldClose) break;
        }
+
+        if (ctx.btDevice != nullptr) {
+            BtEvent bt_event {};
+            while (bluetooth_event_poll(&ctx.btEventSub, &bt_event) == ERROR_NONE) {
+                onBtEvent(&ctx, bt_event);
+            }
+        }
    }

-    // Invalidate any BT event dispatched-but-not-yet-run for this instance before doing
-    // anything else, so it can't race the teardown below (see onKernelBtEvent()).
-    ctx.generation->fetch_add(1);
-
    if (ctx.btDevice) {
-        if (ctx.callbackRegistered) {
-            bluetooth_remove_event_callback(ctx.btDevice, onKernelBtEvent);
-            ctx.callbackRegistered = false;
-        }
-        device_put(ctx.btDevice);
+        bluetooth_event_unsubscribe(ctx.btDevice, &ctx.btEventSub);
        ctx.btDevice = nullptr;
    }

+    if (dev != nullptr) {
+        device_put(dev);
+    }
+
    window_manager_remove(window);
    check(app_event_unsubscribe(&sub) == ERROR_NONE);
    task_event_group_destruct(&event_group);
@@ -58,8 +58,7 @@ void updateViews(const Context* ctx) {
    }
 }

-void onKernelBtEvent(struct Device* /*device*/, void* context, struct BtEvent /*event*/) {
-    auto* ctx = static_cast<Context*>(context);
+void onBtEvent(Context* ctx) {
    lvgl_lock();
    updateViews(ctx);
    lvgl_unlock();
@@ -196,18 +195,21 @@ int32_t appMain(int argc, char* argv[]) {
    }


-    Device* btDevice = nullptr;
-    if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &btDevice) == ERROR_NONE) {
-        bluetooth_add_event_callback(btDevice, &ctx, onKernelBtEvent);
-        device_put(btDevice);
-    }
-
    TaskEventGroup event_group {};
    task_event_group_construct(&event_group);

    AppEventSubscription sub {};
    check(app_event_subscribe(&sub, &event_group) == ERROR_NONE);

+    Device* btDevice = nullptr;
+    BtEventSubscription btSub {};
+    if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &btDevice) == ERROR_NONE) {
+        if (bluetooth_event_subscribe(btDevice, &btSub, &event_group) != ERROR_NONE) {
+            device_put(btDevice);
+            btDevice = nullptr;
+        }
+    }
+
    WindowId window = window_manager_create(appInstanceId, createWidgets, &ctx);

    bool shouldClose = false;
@@ -239,10 +241,17 @@ int32_t appMain(int argc, char* argv[]) {
            }
            if (shouldClose) break;
        }
+
+        if (btDevice != nullptr) {
+            BtEvent bt_event {};
+            while (bluetooth_event_poll(&btSub, &bt_event) == ERROR_NONE) {
+                onBtEvent(&ctx);
+            }
+        }
    }

-    if (device_get_first_active_by_type(&BLUETOOTH_TYPE, &btDevice) == ERROR_NONE) {
-        bluetooth_remove_event_callback(btDevice, onKernelBtEvent);
+    if (btDevice != nullptr) {
+        bluetooth_event_unsubscribe(btDevice, &btSub);
        device_put(btDevice);
    }

@@ -11,6 +11,7 @@

 #include <Tactility/Mutex.h>
 #include <Tactility/Tactility.h>
+#include <Tactility/Thread.h>
 #include <tactility/check.h>
 #include <tactility/device.h>
 #include <tactility/drivers/bluetooth.h>
@@ -20,6 +21,7 @@
 #include <tactility/log.h>

 #include <array>
+#include <atomic>
 #include <cstring>
 #include <vector>

@@ -103,12 +105,17 @@ static void cachePeerRecord(const BtPeerRecord& krecord) {
    scan_results_cache.push_back(std::move(rec));
 }

-// ---- Bridge callback (registered with kernel driver) ----
-// This callback listens to platform driver events to perform auto-start logic
-// and settings management. Consumers should register their own callbacks via
-// bluetooth_add_event_callback() to receive events directly.
+// ---- Bridge thread (subscribed to the kernel driver) ----
+// This thread listens to platform driver events to perform auto-start logic and settings
+// management. Consumers should subscribe directly via bluetooth_event_subscribe() to receive
+// events themselves.

-static void bt_event_bridge(Device*, void* /*context*/, BtEvent event) {
+TaskEventGroup btEventGroup {};
+BtEventSubscription btEventSub {};
+Thread* btEventThread = nullptr;
+std::atomic<bool> btEventThreadRunning {false};
+
+static void bt_event_bridge(BtEvent event) {
    switch (event.type) {
        case BT_EVENT_RADIO_STATE_CHANGED:
            switch (event.radio_state) {
@@ -250,6 +257,57 @@ static void bt_event_bridge(Device*, void* /*context*/, BtEvent event) {
    }
 }

+// ---- Bridge thread lifecycle ----
+// Runs bt_event_bridge() on its own stack instead of whichever thread published the event, by
+// blocking in task_event_group_wait_any() rather than being called back directly.
+
+constexpr configSTACK_DEPTH_TYPE BT_EVENT_THREAD_STACK_SIZE = 4096;
+
+Device* btEventDevice = nullptr;
+
+int32_t btEventThreadMain() {
+    while (btEventThreadRunning.load()) {
+        task_event_group_wait_any(&btEventGroup, nullptr, pdMS_TO_TICKS(250));
+
+        BtEvent event {};
+        while (bluetooth_event_poll(&btEventSub, &event) == ERROR_NONE) {
+            bt_event_bridge(event);
+        }
+    }
+    return 0;
+}
+
+bool startBtEventThread(Device* dev) {
+    if (btEventThread != nullptr) {
+        return true; // already running
+    }
+
+    task_event_group_construct(&btEventGroup);
+    if (bluetooth_event_subscribe(dev, &btEventSub, &btEventGroup) != ERROR_NONE) {
+        task_event_group_destruct(&btEventGroup);
+        return false;
+    }
+
+    btEventDevice = dev;
+    btEventThreadRunning = true;
+    btEventThread = new Thread("bt-events", BT_EVENT_THREAD_STACK_SIZE, [] { return btEventThreadMain(); });
+    btEventThread->start();
+    return true;
+}
+
+void stopBtEventThread() {
+    if (btEventThread == nullptr) return;
+
+    btEventThreadRunning = false;
+    btEventThread->join();
+    delete btEventThread;
+    btEventThread = nullptr;
+
+    bluetooth_event_unsubscribe(btEventDevice, &btEventSub);
+    task_event_group_destruct(&btEventGroup);
+    btEventDevice = nullptr;
+}
+
 // ---- systemStart ----

 void systemStart() {
@@ -271,26 +329,20 @@ bool isRadioOnOrPending(Device* dev) {
    return state == BT_RADIO_STATE_ON || state == BT_RADIO_STATE_ON_PENDING;
 }

+// dev is started (device_start()) once, at kernel_init (see ble0's devicetree status) and never
+// stopped for the process lifetime - this only toggles the radio itself, so callers subscribed
+// directly to the driver (e.g. BtManage) stay subscribed across on/off toggles instead of
+// having to resubscribe.
 bool start(Device* dev) {
-    LOG_I(TAG, "Auto-enabling BLE on boot");
-    if (!device_is_ready(dev)) {
-        LOG_I(TAG, "Starting BLE device");
-        if (device_start(dev) != ERROR_NONE) {
-            LOG_E(TAG, "Failed to start BLE device");
-            return false;
-        }
-    }
-
-    // TODO: Fix bug where repeatedly calling start would add this callback multiple times
-    if (bluetooth_add_event_callback(dev, nullptr, bt_event_bridge) != ERROR_NONE) {
-        LOG_E(TAG, "Failed to set BLE callback");
+    // TODO: Fix bug where repeatedly calling start would try to subscribe the bridge thread twice
+    if (!startBtEventThread(dev)) {
+        LOG_E(TAG, "Failed to subscribe to BLE events");
    }

    LOG_I(TAG, "Enabling BT radio");
    if (bluetooth_set_radio_enabled(dev, true) != ERROR_NONE) {
        LOG_E(TAG, "Failed to enable BLE radio");
-        // Add bridge again
-        bluetooth_remove_event_callback(dev, bt_event_bridge);
+        stopBtEventThread();
        return false;
    }

@@ -308,19 +360,12 @@ bool stop(Device* dev) {
        return true;
    }

-    if (bluetooth_remove_event_callback(dev, bt_event_bridge) != ERROR_NONE) {
-        LOG_E(TAG, "Failed to remove BLE callback");
-    }
+    stopBtEventThread();

    if (bluetooth_set_radio_enabled(dev, false) != ERROR_NONE) {
        LOG_E(TAG, "Failed to disable BT radio");
-        // Re-register bridge
-        bluetooth_add_event_callback(dev, nullptr, bt_event_bridge);
-        return false;
-    }
-
-    if (device_stop(dev) != ERROR_NONE) {
-        LOG_E(TAG, "Failed to stop BT device");
+        // Re-subscribe bridge
+        startBtEventThread(dev);
        return false;
    }