Persist Full Companion BLE subscriptions across reboot

docs/companion_radio_full.md

@@ -40,6 +40,35 @@ a busy radio rollback remains pending for recovery instead of reporting success.
 | LoRa self-update | No | No |
 | Optional self-update | WiFi on existing dual-app layouts; USB on single-app layouts | Bluetooth DFU with a compatible bootloader; USB always supported |
 
+### nRF52 Bluetooth reconnect qualification
+
+Full Companions persist the paired client's notification subscriptions (CCCDs)
+alongside its bond keys. A RAM-only subscription cache is insufficient: after
+reboot, a phone can retain its bond and subscription while the node has disabled
+notifications. Bluetooth then shows Connected, but Companion replies do not
+reach the app. The RAM cache remains useful for warm reconnects; the persistent
+bond record restores subscriptions after reboot.
+
+The nRF52 build force-includes `SoftDeviceSvcCompat.h` in both application and
+framework compilation. It prevents LTO from discarding memory reads and writes
+performed by Nordic supervisor calls, including queries used to save CCCDs.
+`test/test_nrf52_svc_compat.py` compiles a real ARM regression with the build's
+LTO and pointer-analysis options and compares it with the unprotected wrapper.
+
+On an already paired test node, run the hardware regression with `bleak`:
+
+```sh
+python3 scripts/check_nrf52_ble_reconnect.py \
+  --address D1:A0:86:CD:BB:C6 --board "Seeed Tracker T1000-E" \
+  --version <expected-firmware-hash> --reboot
+```
+
+This checks app-style NUL-terminated CLI commands, reboots the selected node,
+and reads the saved subscription before the host subscribes again. It also
+checks automatic release of a secured link with no Companion traffic, followed
+by a normal reconnect and working CLI. It does not clear pairing keys or change
+preferences. Fresh pairing with the current PIN is a separate initial gate.
+
 ### SenseCAP Indicator transport and rendering exception
 
 Both SenseCAP Indicator Full layouts keep USB available and start exactly one

scripts/check_nrf52_ble_reconnect.py

@@ -0,0 +1,160 @@
+#!/usr/bin/env python3
+"""Hardware regression: preserve a bonded UART subscription across reboot.
+
+Requires bleak and an already paired test device. This reboots that device;
+it does not clear bonds or change its preferences. Pair with the device's
+current PIN before running. Only the selected BLE address is accessed.
+"""
+
+import argparse
+import asyncio
+import json
+import sys
+import time
+
+from bleak import BleakClient
+
+
+RX = "6e400002-b5a3-f393-e0a9-e50e24dcca9e"
+TX = "6e400003-b5a3-f393-e0a9-e50e24dcca9e"
+REV = "00001534-1212-efde-1523-785feabcd123"
+
+
+def report(test, **values):
+    print(json.dumps(dict(test=test, **values)), flush=True)
+
+
+async def paired_client(address):
+    if sys.platform != "linux":
+        return BleakClient(address, timeout=30)
+    # Use BlueZ's existing paired device, rather than requiring a new scanner
+    # sighting immediately after every disconnect. This is also how a phone
+    # reconnects to a known paired peripheral.
+    from bleak.backends.device import BLEDevice
+    from dbus_fast import BusType, Message, MessageType
+    from dbus_fast.aio import MessageBus
+
+    path = "/org/bluez/hci0/dev_" + address.upper().replace(":", "_")
+    bus = await MessageBus(bus_type=BusType.SYSTEM).connect()
+    try:
+        reply = await bus.call(Message(
+            destination="org.bluez", path=path,
+            interface="org.freedesktop.DBus.Properties", member="GetAll",
+            signature="s", body=["org.bluez.Device1"]))
+        if reply.message_type == MessageType.ERROR:
+            raise RuntimeError("Pair the selected device before running this test")
+        props = {key: value.value for key, value in reply.body[0].items()}
+        if props.get("Address", "").upper() != address.upper() or not props.get("Paired"):
+            raise RuntimeError("Selected BlueZ device is not paired")
+        device = BLEDevice(address, props.get("Name"), {"path": path, "props": props})
+        return BleakClient(device, timeout=30)
+    finally:
+        bus.disconnect()
+
+
+async def prepare(client):
+    # BlueZ needs to exchange MTU before using larger UART notifications.
+    acquire = getattr(client._backend, "_acquire_mtu", None)
+    if acquire:
+        await acquire()
+    # An encrypted characteristic forces bond-based security without an
+    # application write or a notification subscription.
+    await client.read_gatt_char(REV)
+
+
+async def read_cccd(client):
+    char = client.services.get_characteristic(TX)
+    desc = next(d for d in char.descriptors if d.uuid.startswith("00002902-"))
+    return int.from_bytes(await client.read_gatt_descriptor(desc.handle), "little")
+
+
+async def wait_disconnected(client, timeout):
+    deadline = time.monotonic() + timeout
+    while client.is_connected:
+        if time.monotonic() >= deadline:
+            raise TimeoutError("Device did not release the Bluetooth link")
+        await asyncio.sleep(0.1)
+
+
+async def check_commands(client, args):
+    queue = asyncio.Queue()
+    await client.start_notify(TX, lambda _, data: queue.put_nowait(bytes(data)))
+
+    async def request(payload, prefix):
+        await client.write_gatt_char(RX, payload, response=True)
+        deadline = time.monotonic() + 10
+        while True:
+            data = await asyncio.wait_for(queue.get(), deadline - time.monotonic())
+            if data.startswith(prefix):
+                return data
+            if data.startswith(b"\x01"):
+                raise RuntimeError("Companion command returned error " + data.hex())
+
+    info = await request(b"\x16\x0e", b"\x0d")
+    board = info[20:60].split(b"\0")[0].decode("ascii")
+    if board != args.board:
+        raise RuntimeError("Wrong board: " + board)
+    report("device_identity", board=board, address=args.address, protocol=info[1])
+    for cmd in ("ver", "get tx", "get radio", "get name"):
+        data = await request(b"\x42B1|" + cmd.encode("ascii") + b"\0", b"\x1dB1|")
+        reply = data[4:].decode("ascii")
+        if cmd == "ver" and args.version not in reply:
+            raise RuntimeError("Unexpected firmware: " + reply)
+        if "Unknown command" in reply or "Not Supported" in reply:
+            raise RuntimeError(reply)
+        report("app_cli", command=cmd, trailing_nul=True, reply=reply)
+
+
+async def run(args):
+    async with await paired_client(args.address) as client:
+        await prepare(client)
+        await check_commands(client, args)
+        value = await read_cccd(client)
+        if value != 1:
+            raise RuntimeError("UART notifications not enabled: " + str(value))
+        # Give the callback worker time to persist the new subscription.
+        await asyncio.sleep(3)
+        report("subscription_before_reboot", cccd=value)
+        try:
+            await client.write_gatt_char(RX, b"\x13reboot", response=True)
+        except Exception:
+            if client.is_connected:
+                raise
+        await wait_disconnected(client, 10)
+        report("application_reboot", passed=True)
+
+    await asyncio.sleep(10)
+    async with await paired_client(args.address) as client:
+        await prepare(client)
+        # MUST precede start_notify: otherwise the host's subscription write
+        # would conceal lost persistent CCCD state.
+        value = await read_cccd(client)
+        report("bonded_cold_reconnect_without_resubscribing", cccd=value)
+        if value != 1:
+            raise RuntimeError("Bond survived reboot but UART subscription was lost")
+        # Also reproduce iOS Connected / no Companion session. No UART writes
+        # occur on this connection; firmware must release it automatically.
+        started = time.monotonic()
+        await wait_disconnected(client, 22)
+        report("secured_link_without_app_recovery",
+               seconds=round(time.monotonic() - started, 2), passed=True)
+
+    await asyncio.sleep(2)
+    async with await paired_client(args.address) as client:
+        await prepare(client)
+        await check_commands(client, args)
+        report("normal_reconnect_after_recovery", passed=True)
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--address", required=True)
+    parser.add_argument("--board", required=True, help="Exact DeviceInfo model")
+    parser.add_argument("--version", required=True, help="Expected CLI version substring")
+    parser.add_argument("--reboot", action="store_true", required=True,
+                        help="Acknowledge that the selected test node will reboot")
+    asyncio.run(run(parser.parse_args()))
+
+
+if __name__ == "__main__":
+    main()

scripts/nrf52_ble_cccd_fix.py

@@ -1,10 +1,11 @@
 """Apply nRF52 Bluefruit fixes in private build copies of framework sources.
 
-The pinned core saves every bonded CCCD write from its callback worker into
-InternalFS. On a T1000-E Full Companion this reproducibly resets the radio as
-the client enables notifications, even after formatting InternalFS. Bond-key
-persistence still works. The Companion keeps recent CCCD state in RAM across
-BLE reconnects, without a live flash write.
+Keep bonded CCCD persistence enabled, including Full Companions. The global
+SoftDeviceSvcCompat.h guard protects the framework's system-attribute queries
+from LTO miscompilation. The previous Full-only RAM workaround lost client
+notification subscriptions after reboot while retaining the pairing keys.
+Keep the RAM cache as a warm reconnect optimization, with the framework's
+persistent bond record as the cold boot fallback.
 Patch a private build copy; never edit PlatformIO's shared framework cache.
 With the pinned GCC 14/LTO toolchain, the optimized Bluefruit.begin() fails
 on the RAK4631 repeater: S140 rejects a valid single-peripheral role request
@@ -19,9 +20,8 @@ OLD_SAVE = """      {
       }
 """
 FIXED_SAVE = """      {
-#if !defined(COMPANION_RADIO_FULL) || !COMPANION_RADIO_FULL
+        // SoftDeviceSvcCompat.h protects system-attribute query side effects.
         conn->saveCccd();
-#endif
       }
 """
 OLD_LOAD = "        if ( !loadCccd() )  sd_ble_gatts_sys_attr_set(_conn_hdl, NULL, 0, 0);\n"

src/helpers/nrf52/SerialBLEInterface.cpp

@@ -52,10 +52,10 @@ static bool formatDeviceName(char* dest, size_t capacity,
 }
 
 #if defined(COMPANION_RADIO_FULL) && COMPANION_RADIO_FULL
-// Bluefruit's bonded CCCD flash write resets some nRF52 Full Companions.
-// Retain the small SoftDevice system-attribute image in RAM for reconnects
-// during this boot. A reboot intentionally discards it; the client can then
-// subscribe normally without a risky flash write.
+// Cache the small SoftDevice system-attribute image for warm reconnects.
+// Bluefruit also persists it in the peer's bond record for cold boot; losing
+// that record leaves clients that retain their subscription unable to receive
+// replies after reboot. SoftDeviceSvcCompat.h protects its SDK queries under LTO.
 static constexpr uint16_t CCCD_RAM_CACHE_MAX = 128;
 static constexpr uint32_t CCCD_SYS_ATTR_FLAGS =
     BLE_GATTS_SYS_ATTR_FLAG_SYS_SRVCS | BLE_GATTS_SYS_ATTR_FLAG_USR_SRVCS;
@@ -119,8 +119,8 @@ static void captureCccdInRam(uint16_t handle, const ble_gap_addr_t& peer) {
   uint16_t length = sizeof(cccd_ram_cache.attrs);
   const uint32_t status = sd_ble_gatts_sys_attr_get(
       handle, cccd_ram_cache.attrs, &length, CCCD_SYS_ATTR_FLAGS);
-  // The SoftDevice SVC writes through this pointer, but its GCC wrapper has
-  // no memory clobber. Force LTO to reload the returned length from memory.
+  // Retain a local reload barrier in addition to the global SVC ABI guard.
+  // The original SDK wrapper does not declare writes through this pointer.
   __asm__ __volatile__("" ::: "memory");
   if (status != NRF_SUCCESS || length == 0
       || length > CCCD_RAM_CACHE_MAX) {
@@ -352,7 +352,7 @@ void SerialBLEInterface::onBLEEvent(ble_evt_t* evt) {
       BLE_DEBUG_PRINTLN("CCCD RAM write: len=%u value=%u",
                         (unsigned)write.len,
                         (unsigned)(write.len ? write.data[0] : 0));
-      // Mirror Bluefruit's deferred CCCD save timing, but copy only to RAM.
+      // Mirror Bluefruit's deferred CCCD save timing for the warm RAM cache.
       // Reading during the BLE event is too early: sys_attr_get succeeds but
       // returns the pre-write CCCD even though notifyEnabled() is already on.
       cccd_ram_cache.valid.store(false, std::memory_order_release);

test/test_nrf52_ble_cccd_fix.py

@@ -13,9 +13,10 @@ spec.loader.exec_module(module)
 
 
 class Nrf52BleCccdFixTest(unittest.TestCase):
-    def test_only_full_companions_skip_persistence(self):
+    def test_full_companions_also_persist_subscriptions(self):
         patched = module.patched_source("before\n" + module.OLD_SAVE + "after\n")
-        self.assertIn("!defined(COMPANION_RADIO_FULL) || !COMPANION_RADIO_FULL", patched)
+        self.assertNotIn("#if", patched)
+        self.assertIn("SoftDeviceSvcCompat.h", patched)
         self.assertIn("conn->saveCccd();", patched)
         self.assertEqual(module.patched_source(patched), patched)