From f2f23f89782ac1d6eee73e0bb76979440c8b8777 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Fri, 19 Jun 2026 12:21:05 -0500 Subject: [PATCH] NimBLE params overhaul and try-fix for incompatible bond cleanup (#10741) * NimBLE params overhaul and try-fix for incompatible bond cleanup * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Address PR review: remove dead clearNVS(), defer bond-purge log below init - Delete unused clearNVS() (no callers; should have been removed in #10264). - Move purgeIncompatibleBleBonds() after the "Init the NimBLE" log so bond-cleanup output doesn't appear to precede module init; it still runs before BLEDevice::init() reads the store. * Update MAX_SATELLITE_NODES and WARM_NODE_COUNT definitions for ESP32-S3 PSRAM support --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/mesh/mesh-pb-constants.h | 21 ++++--- src/nimble/NimbleBluetooth.cpp | 107 +++++++++++++++++++++++++++++--- variants/esp32/esp32-common.ini | 18 +++++- 3 files changed, 126 insertions(+), 20 deletions(-) diff --git a/src/mesh/mesh-pb-constants.h b/src/mesh/mesh-pb-constants.h index c7edb98a3..6f9f4bc91 100644 --- a/src/mesh/mesh-pb-constants.h +++ b/src/mesh/mesh-pb-constants.h @@ -105,14 +105,17 @@ static inline int get_max_num_nodes() /// Per-map cap (position/telemetry/environment/status): only the freshest /// MAX_SATELLITE_NODES nodes keep satellite payloads, the rest just the -/// NodeInfoLite header. RAM-bound (the maps are internal-SRAM, not PSRAM), so -/// flash-rich hosts get a cap >= their hot store (satellites for every node, as -/// before the cap existed) while constrained parts stay at 40. +/// NodeInfoLite header. RAM-bound: the four maps live in internal SRAM (not +/// PSRAM). PSRAM-equipped ESP32-S3 (and native) keep the full 250; other ESP32 +/// (no-PSRAM, incl. S3) get 80 -- ~32 KB worst case, affordable now the warm +/// tier is trimmed; nRF52840 and other tight parts stay at 40. #ifndef MAX_SATELLITE_NODES -#if defined(CONFIG_IDF_TARGET_ESP32S3) || defined(ARCH_PORTDUINO) -#define MAX_SATELLITE_NODES 250 +#if defined(ARCH_PORTDUINO) || (defined(CONFIG_IDF_TARGET_ESP32S3) && defined(BOARD_HAS_PSRAM)) +#define MAX_SATELLITE_NODES 250 // native / PSRAM-equipped ESP32-S3 +#elif defined(ARCH_ESP32) +#define MAX_SATELLITE_NODES 80 // no-PSRAM ESP32 (incl. ESP32-S3) #else -#define MAX_SATELLITE_NODES 40 // nRF52840 (28 KB LittleFS) and generic ESP32 +#define MAX_SATELLITE_NODES 40 // nRF52840 (28 KB LittleFS) and other constrained parts #endif // platform #endif // MAX_SATELLITE_NODES @@ -127,9 +130,11 @@ static inline int get_max_num_nodes() // architecture.h via configuration.h) isn't defined this early in every include // chain. Backed by the raw-flash ring below LittleFS — see WarmNodeStore.h. #define WARM_NODE_COUNT 200 -#elif defined(CONFIG_IDF_TARGET_ESP32S3) -#define WARM_NODE_COUNT 2000 // PSRAM-backed when available; warm.dat ~80 KB +#elif defined(CONFIG_IDF_TARGET_ESP32S3) && defined(BOARD_HAS_PSRAM) +#define WARM_NODE_COUNT 2000 // ESP32-S3 with PSRAM (external); warm.dat ~80 KB #else +// generic ESP32 and no-PSRAM ESP32-S3: ~12.5 KB in internal heap (calloc fallback in +// WarmNodeStore), leaving room for the BLE controller. PSRAM-equipped S3 takes the 2000 case above. #define WARM_NODE_COUNT 320 #endif // platform #endif // WARM_NODE_COUNT diff --git a/src/nimble/NimbleBluetooth.cpp b/src/nimble/NimbleBluetooth.cpp index 5722a6691..3ea0aab31 100644 --- a/src/nimble/NimbleBluetooth.cpp +++ b/src/nimble/NimbleBluetooth.cpp @@ -21,7 +21,12 @@ #include "PowerStatus.h" #include "host/ble_gap.h" +#include "host/ble_hs.h" #include "host/ble_store.h" +#ifdef ARCH_ESP32 +#include +#include +#endif namespace { @@ -30,6 +35,56 @@ constexpr uint16_t kPreferredBleTxOctets = 251; constexpr uint16_t kPreferredBleTxTimeUs = (kPreferredBleTxOctets + 14) * 8; } // namespace +#ifdef ARCH_ESP32 +// Discard NimBLE bonds left in an incompatible on-disk format. The ESP-IDF/NimBLE upgrade changed +// the length of the fixed-size bond records (ble_store_value_sec), so the new host rejects every +// old record on each boot ("NVS data size mismatch for obj_type 1 ...") with no auto-recovery -- +// pairing stays broken until a factory reset. Wipe the bond namespace once when a stored record's +// size differs from this build's struct; a same-size store is left untouched, so this never loops. +// Adapted from https://github.com/h2zero/NimBLE-Arduino/issues/740 +static void purgeIncompatibleBleBonds() +{ + esp_err_t initErr = nvs_flash_init(); + if (initErr != ESP_OK) { + LOG_WARN("purgeIncompatibleBleBonds: nvs_flash_init failed, err=%d", (int)initErr); + return; // NVS should already be up; if not, nothing safe to do here + } + + nvs_handle_t handle = 0; + esp_err_t err = nvs_open("nimble_bond", NVS_READWRITE, &handle); + if (err == ESP_ERR_NVS_NOT_FOUND) { + return; // no bonds stored yet + } + if (err != ESP_OK) { + LOG_ERROR("nimble_bond open failed, err=%d", err); + return; + } + + // Probe the first record of each fixed-size object type (bonds are written from index 1); a + // stored size differing from this build's struct means the store predates a format change. + size_t sz = 0; + bool mismatch = (nvs_get_blob(handle, "our_sec_1", nullptr, &sz) == ESP_OK && sz != sizeof(struct ble_store_value_sec)) || + (nvs_get_blob(handle, "peer_sec_1", nullptr, &sz) == ESP_OK && sz != sizeof(struct ble_store_value_sec)) || + (nvs_get_blob(handle, "cccd_sec_1", nullptr, &sz) == ESP_OK && sz != sizeof(struct ble_store_value_cccd)); + + bool wiped = false; + if (mismatch) { + LOG_WARN("Wiping incompatible NimBLE bonds (on-disk format changed)"); + wiped = nvs_erase_all(handle) == ESP_OK && nvs_commit(handle) == ESP_OK; + if (!wiped) { + LOG_ERROR("Failed to erase nimble_bond namespace"); + } + } + + nvs_close(handle); + + if (wiped) { + LOG_INFO("Restarting after NimBLE bond cleanup"); + ESP.restart(); + } +} +#endif + // Debugging options: careful, they slow things down quite a bit! // #define DEBUG_NIMBLE_ON_READ_TIMING // uncomment to time onRead duration // #define DEBUG_NIMBLE_ON_WRITE_TIMING // uncomment to time onWrite duration @@ -47,6 +102,11 @@ BLEServer *bleServer; static bool passkeyShowing; static std::atomic nimbleBluetoothConnHandle{BLE_HS_CONN_HANDLE_NONE}; // BLE_HS_CONN_HANDLE_NONE means "no connection" +// Set by onDisconnect to defer (re)starting advertising to the main task. A stale-bond reconnect +// triggers a MIC failure + NimBLE host reset; re-entering ble_gap_adv_* from the disconnect +// callback while the host is mid-reset crashes (LoadProhibited), so the main task does it instead. +static std::atomic pendingStartAdvertising{false}; + static void clearPairingDisplay() { if (!passkeyShowing) { @@ -155,6 +215,21 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread protected: virtual int32_t runOnce() override { + // Service a deferred advertising restart from onDisconnect, gated on ble_hs_synced() so we + // never re-enter the GAP API while the host is still mid-reset. + if (pendingStartAdvertising) { + if (checkIsConnected()) { + pendingStartAdvertising = false; // a new physical connection beat us to it; nothing to do + } else if (ble_hs_synced()) { + pendingStartAdvertising = false; + if (nimbleBluetooth) { + nimbleBluetooth->startAdvertising(); + } + } else { + return 200; // host still re-syncing after a reset; retry shortly + } + } + while (runOnceHasWorkToDo()) { /* PROCESS fromPhoneQueue BEFORE toPhoneQueue: @@ -592,6 +667,14 @@ class NimbleBluetoothSecurityCallback : public BLESecurityCallbacks } void onAuthenticationComplete(ble_gap_conn_desc *desc) override { + // Called on every BLE_GAP_EVENT_ENC_CHANGE, success or failure. A stale-bond reconnect + // yields a *failed* encryption change here -- don't latch a connected/authenticated state + // on a link that is actually being torn down. + if (desc == nullptr || !desc->sec_state.encrypted) { + LOG_WARN("BLE encryption change without an encrypted link; ignoring"); + return; + } + LOG_INFO("BLE authentication complete"); meshtastic::BluetoothStatus newStatus(meshtastic::BluetoothStatus::ConnectionState::CONNECTED); @@ -667,7 +750,13 @@ class NimbleBluetoothServerCallback : public BLEServerCallbacks nimbleBluetoothConnHandle = BLE_HS_CONN_HANDLE_NONE; - ble->startAdvertising(); + // Defer the advertising restart to runOnce (see pendingStartAdvertising): calling + // startAdvertising() here would crash if this disconnect was a host reset. + pendingStartAdvertising = true; + if (bluetoothPhoneAPI) { + bluetoothPhoneAPI->setIntervalFromNow(0); + } + concurrency::mainDelay.interrupt(); // wake the main loop to service the restart } }; @@ -761,6 +850,12 @@ void NimbleBluetooth::setup() LOG_INFO("Init the NimBLE bluetooth module"); +#ifdef ARCH_ESP32 + // Runs before BLEDevice::init() reads the bond store, but logs after the "Init" line above so + // any bond-cleanup output doesn't appear to precede the module init. + purgeIncompatibleBleBonds(); // wipe bonds left in an incompatible on-disk format (post-upgrade) +#endif + BLEDevice::init(getDeviceName()); BLEDevice::setPower(ESP_PWR_LVL_P9); @@ -889,6 +984,7 @@ void updateBatteryLevel(uint8_t level) void NimbleBluetooth::clearBonds() { LOG_INFO("Clearing bluetooth bonds!"); + ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_OUR_SEC, nullptr); ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_PEER_SEC, nullptr); ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, nullptr); } @@ -901,13 +997,4 @@ void NimbleBluetooth::sendLog(const uint8_t *logMessage, size_t length) logRadioCharacteristic->setValue(logMessage, length); logRadioCharacteristic->notify(); } - -void clearNVS() -{ - ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_PEER_SEC, nullptr); - ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, nullptr); -#ifdef ARCH_ESP32 - ESP.restart(); -#endif -} #endif diff --git a/variants/esp32/esp32-common.ini b/variants/esp32/esp32-common.ini index 75aa8fc63..fac7cc911 100644 --- a/variants/esp32/esp32-common.ini +++ b/variants/esp32/esp32-common.ini @@ -276,9 +276,23 @@ custom_sdkconfig = CONFIG_BT_NIMBLE_ROLE_CENTRAL=n CONFIG_BT_NIMBLE_ROLE_OBSERVER=n CONFIG_BT_CONTROLLER_ENABLED=y - CONFIG_BT_NIMBLE_HOST_TASK_STACK_SIZE=8192 - CONFIG_BT_NIMBLE_MAX_CCCDS=20 + # BLE RAM right-sizing for a single-phone peripheral. IDF-5.5/Arduino-3.x raised RAM use to where + # NimBLE bring-up no longer had enough contiguous heap (host task fails to allocate -> host never + # syncs -> BLEDevice::init() hangs; GATT/advertising then OOM). The node only ever has one + # connection and never scans, so trimming these over-provisioned controller/host buffers frees + # the heap. Keep the host-task stack at the IDF default 5120 (do NOT lower to 4096 -- #2618 raised + # it because 4096 overflows); the prior 8192 was an over-allocation that starved advertising. + CONFIG_BT_NIMBLE_HOST_TASK_STACK_SIZE=5120 + CONFIG_BT_NIMBLE_MAX_CCCDS=8 CONFIG_BT_NIMBLE_MAX_BONDS=6 + CONFIG_BT_NIMBLE_MAX_CONNECTIONS=1 + CONFIG_BT_CTRL_BLE_MAX_ACT=2 + CONFIG_BT_CTRL_SCAN_DUPL_CACHE_SIZE=10 + CONFIG_BT_NIMBLE_WHITELIST_SIZE=1 + CONFIG_BT_NIMBLE_MSYS_1_BLOCK_COUNT=8 + CONFIG_BT_NIMBLE_MSYS_2_BLOCK_COUNT=8 + CONFIG_BT_NIMBLE_TRANSPORT_ACL_FROM_LL_COUNT=8 + CONFIG_BT_NIMBLE_TRANSPORT_EVT_COUNT=12 CONFIG_BT_NIMBLE_ENABLE_PERIODIC_SYNC=n CONFIG_BT_NIMBLE_ENABLE_PERIODIC_ADV=n CONFIG_BT_NIMBLE_EXT_SCAN=n