Keep nRF52 Full Companion BLE subscriptions across reconnects

scripts/nrf52_ble_cccd_fix.py

@@ -3,8 +3,8 @@
 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 client re-subscribes on connection;
-this workaround sacrifices CCCD persistence, not key-based reconnection.
+persistence still works. The Companion keeps recent CCCD state in RAM across
+BLE reconnects, without a live flash write.
 Patch a private build copy; never edit PlatformIO's shared framework cache.
 """
 
@@ -21,6 +21,25 @@ FIXED_SAVE = """      {
 #endif
       }
 """
+OLD_LOAD = "        if ( !loadCccd() )  sd_ble_gatts_sys_attr_set(_conn_hdl, NULL, 0, 0);\n"
+FIXED_LOAD = """#if defined(COMPANION_RADIO_FULL) && COMPANION_RADIO_FULL
+        if ( !mesh_nrf52_restore_ram_cccd(_conn_hdl, &_bond_id_addr) &&
+             !loadCccd() )
+        {
+          sd_ble_gatts_sys_attr_set(_conn_hdl, NULL, 0, 0);
+        }
+#else
+        if ( !loadCccd() )  sd_ble_gatts_sys_attr_set(_conn_hdl, NULL, 0, 0);
+#endif
+"""
+OLD_CONNECTION_INCLUDE = '#include "bluefruit.h"\n'
+FIXED_CONNECTION_INCLUDE = """#include "bluefruit.h"
+
+#if defined(COMPANION_RADIO_FULL) && COMPANION_RADIO_FULL
+bool mesh_nrf52_restore_ram_cccd(uint16_t conn_handle,
+                                  const ble_gap_addr_t* peer);
+#endif
+"""
 
 
 def patched_source(source):
@@ -34,20 +53,36 @@ def patched_source(source):
     return source.replace(OLD_SAVE, FIXED_SAVE)
 
 
+def patched_connection_source(source):
+    source = source.replace("\r\n", "\n")
+    if FIXED_LOAD in source and FIXED_CONNECTION_INCLUDE in source:
+        return source
+    if source.count(OLD_LOAD) != 1 or source.count(OLD_CONNECTION_INCLUDE) != 1:
+        raise RuntimeError(
+            "nRF52 BLE CCCD fix: unrecognized BLEConnection source; review before building"
+        )
+    return source.replace(OLD_CONNECTION_INCLUDE, FIXED_CONNECTION_INCLUDE).replace(
+        OLD_LOAD, FIXED_LOAD
+    )
+
+
 def replace_framework_source(build_env, node):
     source = Path(node.srcnode().get_abspath())
-    if source.name != "BLEGatt.cpp" or source.parent.name != "src":
+    if source.name not in ("BLEGatt.cpp", "BLEConnection.cpp") or source.parent.name != "src":
         return node
-    patched = patched_source(source.read_text(encoding="utf-8"))
+    original = source.read_text(encoding="utf-8")
+    patched = (patched_source(original) if source.name == "BLEGatt.cpp"
+               else patched_connection_source(original))
     build_env.AppendUnique(CPPPATH=[str(source.parent)])
-    destination = Path(build_env.subst("$BUILD_DIR")) / "patched-nrf52-ble" / "BLEGatt.cpp"
+    destination = Path(build_env.subst("$BUILD_DIR")) / "patched-nrf52-ble" / source.name
     destination.parent.mkdir(parents=True, exist_ok=True)
     if not destination.exists() or destination.read_text(encoding="utf-8") != patched:
         destination.write_text(patched, encoding="utf-8")
-    print("nRF52 BLE: Full Companion CCCD persistence workaround enabled")
+    print(f"nRF52 BLE: Full Companion CCCD workaround in {source.name}")
     return build_env.File(str(destination))
 
 
 if "Import" in globals():
     Import("env")
     env.AddBuildMiddleware(replace_framework_source, "*BLEGatt.cpp")
+    env.AddBuildMiddleware(replace_framework_source, "*BLEConnection.cpp")

src/helpers/nrf52/SerialBLEInterface.cpp

@@ -29,6 +29,94 @@
 
 static SerialBLEInterface* instance = nullptr;
 
+#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.
+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;
+struct CccdRamCache {
+  uint8_t attrs[CCCD_RAM_CACHE_MAX] = {};
+  uint16_t length = 0;
+  ble_gap_addr_t peer = {};
+  std::atomic<bool> valid{false};
+};
+static CccdRamCache cccd_ram_cache;
+static bool cccd_written_this_connection = false;
+
+static bool cccdPeersMatch(const ble_gap_addr_t& a,
+                           const ble_gap_addr_t& b) {
+  if (a.addr_type == b.addr_type
+      && memcmp(a.addr, b.addr, sizeof(a.addr)) == 0) return true;
+
+  // iPhones may reconnect from a different resolvable private address. The
+  // stored bond maps both addresses to the same stable peer identity.
+  ble_gap_addr_t a_lookup = a;
+  ble_gap_addr_t b_lookup = b;
+  bond_keys_t a_keys;
+  bond_keys_t b_keys;
+  return bond_load_keys(BLE_GAP_ROLE_PERIPH, &a_lookup, &a_keys)
+      && bond_load_keys(BLE_GAP_ROLE_PERIPH, &b_lookup, &b_keys)
+      && a_keys.peer_id.id_addr_info.addr_type
+             == b_keys.peer_id.id_addr_info.addr_type
+      && memcmp(a_keys.peer_id.id_addr_info.addr,
+                b_keys.peer_id.id_addr_info.addr,
+                sizeof(a_keys.peer_id.id_addr_info.addr)) == 0;
+}
+
+bool mesh_nrf52_restore_ram_cccd(uint16_t handle,
+                                  const ble_gap_addr_t* peer) {
+  if (!cccd_ram_cache.valid.load(std::memory_order_acquire)
+      || cccd_written_this_connection || !peer
+      || !cccdPeersMatch(cccd_ram_cache.peer, *peer)) {
+    BLE_DEBUG_PRINTLN("CCCD RAM not restored at initialization: valid=%u written=%u",
+                      (unsigned)cccd_ram_cache.valid.load(std::memory_order_relaxed),
+                      (unsigned)cccd_written_this_connection);
+    return false;
+  }
+  // The first sys_attr_set on a connection must contain the saved CCCD.
+  // Bluefruit's no-file fallback initializes everything to off otherwise.
+  const uint32_t status = sd_ble_gatts_sys_attr_set(
+      handle, cccd_ram_cache.attrs, cccd_ram_cache.length,
+      CCCD_SYS_ATTR_FLAGS);
+  if (status != NRF_SUCCESS) {
+    BLE_DEBUG_PRINTLN("CCCD RAM initialization failed: %lu",
+                      (unsigned long)status);
+    cccd_ram_cache.valid.store(false, std::memory_order_release);
+    return false;
+  }
+  BLE_DEBUG_PRINTLN("CCCD RAM initialized: len=%u",
+                    (unsigned)cccd_ram_cache.length);
+  return true;
+}
+
+static void captureCccdInRam(uint16_t handle, const ble_gap_addr_t& peer) {
+  cccd_ram_cache.valid.store(false, std::memory_order_release);
+  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.
+  __asm__ __volatile__("" ::: "memory");
+  if (status != NRF_SUCCESS || length == 0
+      || length > CCCD_RAM_CACHE_MAX) {
+    BLE_DEBUG_PRINTLN("CCCD RAM capture: status=%lu len=%u",
+                      (unsigned long)status, (unsigned)length);
+    return;
+  }
+  cccd_ram_cache.length = length;
+  cccd_ram_cache.peer = peer;
+  cccd_ram_cache.valid.store(true, std::memory_order_release);
+  BLE_DEBUG_PRINTLN("CCCD RAM captured: len=%u", (unsigned)length);
+}
+
+static void captureCccdDeferred(uint16_t handle, ble_gap_addr_t* peer) {
+  if (peer) captureCccdInRam(handle, *peer);
+}
+#endif
+
 static bool isBondAuthenticationFailure(uint8_t reason) {
   return reason == BLE_HCI_AUTHENTICATION_FAILURE ||
          reason == BLE_HCI_STATUS_CODE_PIN_OR_KEY_MISSING ||
@@ -202,6 +290,11 @@ void SerialBLEInterface::onBLEEvent(ble_evt_t* evt) {
       instance->_peer_address = connected->peer_addr;
       instance->_peer_address_valid = true;
       instance->_bond_removed_for_connection = false;
+#if defined(COMPANION_RADIO_FULL) && COMPANION_RADIO_FULL
+      cccd_written_this_connection = false;
+      BLE_DEBUG_PRINTLN("CCCD RAM connect: cached=%u",
+                        (unsigned)cccd_ram_cache.valid.load(std::memory_order_relaxed));
+#endif
     }
   } else if (evt->header.evt_id == BLE_GAP_EVT_CONN_SEC_UPDATE) {
     uint16_t conn_handle = evt->evt.gap_evt.conn_handle;
@@ -218,6 +311,30 @@ void SerialBLEInterface::onBLEEvent(ble_evt_t* evt) {
       instance->_security_timer.cancel();
       sd_ble_gap_disconnect(conn_handle, BLE_HCI_REMOTE_USER_TERMINATED_CONNECTION);
     }
+#if defined(COMPANION_RADIO_FULL) && COMPANION_RADIO_FULL
+  } else if (evt->header.evt_id == BLE_GATTS_EVT_WRITE) {
+    const ble_gatts_evt_write_t& write = evt->evt.gatts_evt.params.write;
+    if (write.uuid.type == BLE_UUID_TYPE_BLE
+        && write.uuid.uuid == BLE_UUID_DESCRIPTOR_CLIENT_CHAR_CONFIG
+        && instance->_peer_address_valid) {
+      cccd_written_this_connection = true;
+      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.
+      // 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);
+      if (!ada_callback(&instance->_peer_address,
+                        sizeof(instance->_peer_address), captureCccdDeferred,
+                        evt->evt.gatts_evt.conn_handle,
+                        &instance->_peer_address)) {
+        BLE_DEBUG_PRINTLN("CCCD RAM capture could not be queued");
+      }
+    }
+#endif
+  } else if (evt->header.evt_id == BLE_GATTS_EVT_SYS_ATTR_MISSING) {
+    BLE_DEBUG_PRINTLN("CCCD RAM system attributes missing");
   } else if (evt->header.evt_id == BLE_GAP_EVT_DISCONNECTED) {
     ble_gap_evt_disconnected_t const* disconnected = &evt->evt.gap_evt.params.disconnected;
     if (isBondAuthenticationFailure(disconnected->reason)) {
@@ -227,6 +344,9 @@ void SerialBLEInterface::onBLEEvent(ble_evt_t* evt) {
             "bond authentication disconnect");
       }
     }
+#if defined(COMPANION_RADIO_FULL) && COMPANION_RADIO_FULL
+    cccd_written_this_connection = false;
+#endif
     instance->_peer_address_valid = false;
   } else if (evt->header.evt_id == BLE_GAP_EVT_CONN_PARAM_UPDATE_REQUEST) {
     uint16_t conn_handle = evt->evt.gap_evt.conn_handle;

test/test_nrf52_ble_cccd_fix.py

@@ -27,6 +27,32 @@ class Nrf52BleCccdFixTest(unittest.TestCase):
         ini = (ROOT / "platformio.ini").read_text(encoding="utf-8")
         self.assertIn("pre:scripts/nrf52_ble_cccd_fix.py", ini)
 
+    def test_full_companion_initializes_cccd_from_ram_before_empty_fallback(self):
+        patched = module.patched_connection_source(
+            module.OLD_CONNECTION_INCLUDE + module.OLD_LOAD
+        )
+        self.assertIn("mesh_nrf52_restore_ram_cccd", patched)
+        self.assertLess(patched.index("!mesh_nrf52_restore_ram_cccd"),
+                        patched.index("!loadCccd()"))
+        self.assertLess(patched.index("!loadCccd()"),
+                        patched.index("sd_ble_gatts_sys_attr_set(_conn_hdl, NULL"))
+        self.assertEqual(module.patched_connection_source(patched), patched)
+
+    def test_full_companion_restores_same_peers_cccd_without_flash(self):
+        app = (ROOT / "src/helpers/nrf52/SerialBLEInterface.cpp").read_text(encoding="utf-8")
+        self.assertIn("captureCccdInRam", app)
+        self.assertIn("std::atomic<bool> valid", app)
+        self.assertIn("cccdPeersMatch", app)
+        self.assertIn("BLE_UUID_DESCRIPTOR_CLIENT_CHAR_CONFIG", app)
+        self.assertIn("sd_ble_gatts_sys_attr_get", app)
+        self.assertIn("sd_ble_gatts_sys_attr_set", app)
+        self.assertIn('__asm__ __volatile__("" ::: "memory")', app)
+        self.assertIn("cccd_written_this_connection || !peer", app)
+        self.assertIn("bool mesh_nrf52_restore_ram_cccd", app)
+        self.assertIn("captureCccdDeferred", app)
+        self.assertIn("ada_callback(&instance->_peer_address", app)
+        self.assertNotIn("mesh_nrf52_service_cccd_save", app)
+
 
 if __name__ == "__main__":
     unittest.main()

tools/hil/t1000e_ble_cccd_reconnect.py

@@ -0,0 +1,126 @@
+#!/usr/bin/env python3
+"""Read the bonded BLE UART CCCD across two T1000-E connections.
+
+This is local-only: it subscribes to notifications once, disconnects, then
+reconnects without re-subscribing and reads the descriptor. It never changes
+radio settings, sends LoRa traffic, or erases storage.
+"""
+
+import asyncio
+import argparse
+import json
+import os
+import sys
+
+from bleak import BleakClient, BleakScanner
+
+
+ADDRESS = None
+PIN = "123456"
+UART_TX = "6e400003-b5a3-f393-e0a9-e50e24dcca9e"
+
+
+async def pair_with_pin():
+    from winrt.windows.devices.bluetooth import BluetoothLEDevice
+    from winrt.windows.devices.enumeration import (
+        DevicePairingKinds,
+        DevicePairingProtectionLevel,
+    )
+
+    address = int(ADDRESS.replace(":", ""), 16)
+    device = await BluetoothLEDevice.from_bluetooth_address_async(address)
+    if device is None:
+        raise RuntimeError("Windows cannot open the advertising BLE device")
+    pairing = device.device_information.pairing
+    if pairing.is_paired:
+        return "already paired"
+    custom = pairing.custom
+
+    def on_request(_sender, args):
+        if args.pairing_kind == DevicePairingKinds.PROVIDE_PIN:
+            args.accept(PIN)
+        elif args.pairing_kind == DevicePairingKinds.CONFIRM_ONLY:
+            args.accept()
+
+    token = custom.add_pairing_requested(on_request)
+    try:
+        result = await custom.pair_async(
+            DevicePairingKinds.PROVIDE_PIN | DevicePairingKinds.CONFIRM_ONLY,
+            DevicePairingProtectionLevel.ENCRYPTION_AND_AUTHENTICATION,
+        )
+        return str(result.status)
+    finally:
+        custom.remove_pairing_requested(token)
+        device.close()
+
+
+async def descriptor_state(subscribe):
+    async with BleakClient(ADDRESS, timeout=20) as client:
+        tx = client.services.get_characteristic(UART_TX)
+        if tx is None:
+            raise RuntimeError("Nordic UART TX characteristic unavailable")
+        cccd = next(
+            (item for item in tx.descriptors if item.uuid.lower().startswith("00002902")),
+            None,
+        )
+        if cccd is None:
+            raise RuntimeError("Nordic UART CCCD unavailable")
+        if subscribe:
+            await client.start_notify(tx, lambda *_: None)
+            await asyncio.sleep(0.4)
+        value = await client.read_gatt_descriptor(cccd.handle)
+        return {"cccd": bytes(value).hex(), "handle": cccd.handle}
+
+
+async def subscribe_then_abort():
+    client = BleakClient(ADDRESS, timeout=20)
+    await client.connect()
+    tx = client.services.get_characteristic(UART_TX)
+    await client.start_notify(tx, lambda *_: None)
+    await asyncio.sleep(0.4)
+    cccd = next(item for item in tx.descriptors
+                if item.uuid.lower().startswith("00002902"))
+    value = await client.read_gatt_descriptor(cccd.handle)
+    print(json.dumps({"subscribed_before_abrupt_exit": bytes(value).hex()}), flush=True)
+
+
+async def main(pair, mode):
+    discovered = await BleakScanner.discover(timeout=10)
+    if not any(item.address.upper() == ADDRESS for item in discovered):
+        raise RuntimeError("T1000-E is not advertising; stop the phone connection first")
+    if pair:
+        print("pair:", await pair_with_pin())
+        await asyncio.sleep(1)
+    if mode == "abort":
+        await subscribe_then_abort()
+        os._exit(0)
+    elif mode == "subscribe":
+        print(json.dumps({"subscribed": await descriptor_state(True)}))
+    elif mode == "inspect":
+        print(json.dumps({"without_subscribe": await descriptor_state(False)}))
+    else:
+        first = await descriptor_state(True)
+        await asyncio.sleep(1)
+        second = await descriptor_state(False)
+        print(json.dumps({"first": first, "reconnect_without_subscribe": second}))
+        if first["cccd"] != "0100" or second["cccd"] != "0100":
+            raise RuntimeError("UART notifications did not survive reconnect")
+
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--address", required=True,
+                        help="Bluetooth address of the test T1000-E")
+    parser.add_argument("--pin", default="123456",
+                        help="pairing PIN, used only with --pair")
+    parser.add_argument("--pair", action="store_true")
+    parser.add_argument("--mode", choices=("roundtrip", "subscribe", "inspect", "abort"),
+                        default="roundtrip")
+    args = parser.parse_args()
+    ADDRESS = args.address.upper()
+    PIN = args.pin
+    try:
+        asyncio.run(main(args.pair, args.mode))
+    except Exception as exc:
+        print(f"BLE CCCD test failed: {exc}", file=sys.stderr)
+        raise

tools/hil/t1000e_ble_usb_log.py

@@ -0,0 +1,28 @@
+#!/usr/bin/env python3
+"""Print the local T1000-E USB debug stream for a bounded interval."""
+
+import argparse
+import sys
+import time
+
+import serial
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--port", required=True)
+    parser.add_argument("--seconds", type=int, default=55)
+    args = parser.parse_args()
+    deadline = time.monotonic() + args.seconds
+    with serial.Serial(args.port, 115200, timeout=0.1) as port:
+        port.dtr = True
+        port.rts = False
+        while time.monotonic() < deadline:
+            data = port.read(port.in_waiting or 1)
+            if data:
+                sys.stdout.write(data.decode(errors="replace"))
+                sys.stdout.flush()
+
+
+if __name__ == "__main__":
+    main()