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>
2026-06-19 12:21:05 -05:00
parent 9358b2c549
commit f2f23f8978
3 changed files with 126 additions and 20 deletions
@@ -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
@@ -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 <nvs.h>
+#include <nvs_flash.h>
+#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<uint16_t> 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<bool> 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
@@ -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