From 301ef3758d278f48e577c5086684584a93a8bf1f Mon Sep 17 00:00:00 2001 From: mikecarper Date: Wed, 30 Sep 2026 23:20:14 -0700 Subject: [PATCH] Persist Full Companion BLE subscriptions across reboot --- docs/companion_radio_full.md | 29 ++++ scripts/check_nrf52_ble_reconnect.py | 160 +++++++++++++++++++++++ scripts/nrf52_ble_cccd_fix.py | 14 +- src/helpers/nrf52/SerialBLEInterface.cpp | 14 +- test/test_nrf52_ble_cccd_fix.py | 5 +- 5 files changed, 206 insertions(+), 16 deletions(-) create mode 100644 scripts/check_nrf52_ble_reconnect.py diff --git a/docs/companion_radio_full.md b/docs/companion_radio_full.md index 1f1995104..d03f1a003 100644 --- a/docs/companion_radio_full.md +++ b/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 --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 diff --git a/scripts/check_nrf52_ble_reconnect.py b/scripts/check_nrf52_ble_reconnect.py new file mode 100644 index 000000000..a6e689698 --- /dev/null +++ b/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() diff --git a/scripts/nrf52_ble_cccd_fix.py b/scripts/nrf52_ble_cccd_fix.py index 3ed399c8b..15c75aaea 100644 --- a/scripts/nrf52_ble_cccd_fix.py +++ b/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" diff --git a/src/helpers/nrf52/SerialBLEInterface.cpp b/src/helpers/nrf52/SerialBLEInterface.cpp index a12128653..af99def87 100644 --- a/src/helpers/nrf52/SerialBLEInterface.cpp +++ b/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); diff --git a/test/test_nrf52_ble_cccd_fix.py b/test/test_nrf52_ble_cccd_fix.py index eeeed3fcf..32fd4e0f6 100644 --- a/test/test_nrf52_ble_cccd_fix.py +++ b/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)