diff --git a/scripts/nrf52_ble_cccd_fix.py b/scripts/nrf52_ble_cccd_fix.py index 754446b9..fa099aa1 100644 --- a/scripts/nrf52_ble_cccd_fix.py +++ b/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") diff --git a/src/helpers/nrf52/SerialBLEInterface.cpp b/src/helpers/nrf52/SerialBLEInterface.cpp index c7c05c67..638f19fc 100644 --- a/src/helpers/nrf52/SerialBLEInterface.cpp +++ b/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 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; diff --git a/test/test_nrf52_ble_cccd_fix.py b/test/test_nrf52_ble_cccd_fix.py index 26bda754..eeece660 100644 --- a/test/test_nrf52_ble_cccd_fix.py +++ b/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 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() diff --git a/tools/hil/t1000e_ble_cccd_reconnect.py b/tools/hil/t1000e_ble_cccd_reconnect.py new file mode 100644 index 00000000..a9d2a7f4 --- /dev/null +++ b/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 diff --git a/tools/hil/t1000e_ble_usb_log.py b/tools/hil/t1000e_ble_usb_log.py new file mode 100644 index 00000000..bbcfdf49 --- /dev/null +++ b/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()