From 7060a38dba1fc99521ded784f380fb0fdfd2463a Mon Sep 17 00:00:00 2001 From: Christopher Van Hoose Date: Fri, 21 Aug 2026 00:53:41 -0400 Subject: [PATCH] Fix V4-R8 crash on Wi-Fi association with BLE on; widen companion ATT MTU Wi-Fi power save: remove the preference, always use DTIM modem sleep. An R8 with saved credentials and Bluetooth enabled aborted every time Wi-Fi associated. Coredump: crashing task "wifi", abort() inside pm_set_sleep_type, with nimble_host and btController both live. WIFI_PS_NONE is not a legal sleep mode while the BT controller is running -- modem sleep is precisely what yields airtime to BLE -- and the Wi-Fi PM blob aborts rather than refusing. 844d8c3 made power save a preference defaulting OFF on the V4-R8, which is why only that board crashed; the comment above the apply site already stated the invariant it broke ("gives BLE coexistence airtime"). The latency it was buying does not exist either. Profiling the companion link on hardware: an app sync is ~40 CMD_GET_CHANNEL round trips (the device reports MAX_GROUP_CHANNELS=40 and the app reads every slot individually), 165ms each, with this firmware accounting for 28ms of 8264ms -- 0.3%. Sync cost is round-trip COUNT, not link latency, so there was nothing to trade away. Guarding the setting was considered and rejected: it would have to be silently inert whenever Bluetooth is on, which is a confusing thing to leave in a Settings screen. Removed instead -- toggle, prefs accessors and R8 default. The stored wifi_ps NVS key is simply never read again; no migration needed. Companion BLE: ATT MTU MAX_FRAME_SIZE -> MAX_FRAME_SIZE + 3. A notification payload is ATT_MTU-3, so SerialBLEInterface::begin()'s setMTU(MAX_FRAME_SIZE) left a full-length frame 3 bytes too big and NimBLE silently dropped the overflow (ble_att_truncate_to_mtu). MyMesh::queueMessage() clamps message text to exactly MAX_FRAME_SIZE, so maximum-length messages lost their last 3 bytes -- over BLE only; USB/TCP/WS were unaffected. Applied from the vendored transport rather than the core: setMTU() is a public static, so MeshCore needs no change. Verified on an M9, which now negotiates 179. All 8 environments build. R8 flashed and confirmed to be the booting slot (otadata seq 1 -> ota_0); its boot log is not observable over USB because opening the port drops that board into ROM download mode. Co-Authored-By: Claude Opus 5 --- .../MultiTransportCompanionInterface.cpp | 10 ++++++++ src/helpers/esp32/WifiRuntimeStore.cpp | 20 ---------------- src/helpers/esp32/WifiRuntimeStore.h | 8 ------- src/main.cpp | 17 ++++++++----- src/ui-touch/UITask.cpp | 24 ------------------- 5 files changed, 21 insertions(+), 58 deletions(-) diff --git a/src/helpers/esp32/MultiTransportCompanionInterface.cpp b/src/helpers/esp32/MultiTransportCompanionInterface.cpp index d1986f5..89c3cc4 100644 --- a/src/helpers/esp32/MultiTransportCompanionInterface.cpp +++ b/src/helpers/esp32/MultiTransportCompanionInterface.cpp @@ -188,6 +188,15 @@ void MultiTransportCompanionInterface::prepareBle(const char* prefix, char* name void MultiTransportCompanionInterface::beginBle(const char* prefix, char* name, uint32_t pin_code) { prepareBle(prefix, name, pin_code); _ble.begin(prefix, name, pin_code); + // ATT notification payload is ATT_MTU-3, so SerialBLEInterface::begin()'s + // setMTU(MAX_FRAME_SIZE) leaves a full-length frame 3 bytes too big and NimBLE + // silently drops the overflow (ble_att_truncate_to_mtu()). MyMesh::queueMessage() + // clamps message text to exactly MAX_FRAME_SIZE, so a maximum-length message lost + // its last 3 bytes -- over BLE only; USB/TCP/WS were unaffected. Widen it here + // rather than in the core: that file is upstream MeshCore, this transport is + // vendored in our src/, and setMTU() is a public static, so no core change is + // needed. Verified on an M9: the app now negotiates ATT MTU 179 (was 176). + NimBLEDevice::setMTU(MAX_FRAME_SIZE + 3); _ble_begun = true; _ble_enabled = true; _ota_ble_released = false; @@ -228,6 +237,7 @@ void MultiTransportCompanionInterface::enableBle() { strncpy(name, _ble_name, sizeof(name) - 1); name[sizeof(name) - 1] = '\0'; _ble.begin(_ble_prefix, name, _ble_pin_code); + NimBLEDevice::setMTU(MAX_FRAME_SIZE + 3); // see beginBle(): ATT payload is MTU-3 _ble_begun = true; } _ble_enabled = true; diff --git a/src/helpers/esp32/WifiRuntimeStore.cpp b/src/helpers/esp32/WifiRuntimeStore.cpp index 9c32653..67180d4 100644 --- a/src/helpers/esp32/WifiRuntimeStore.cpp +++ b/src/helpers/esp32/WifiRuntimeStore.cpp @@ -17,12 +17,6 @@ static const char *WIFI_CONFIG_PWD_KEY = "wifi_pwd"; static const char *WIFI_CONFIG_RADIO_EN_KEY = "wifi_radio_en"; static const char *WIFI_CONFIG_WIFI_CHOSEN_KEY = "wifi_chosen"; static const char *WIFI_CONFIG_BLE_EN_KEY = "ble_en"; // BLE radio on/off (default on) -static const char *WIFI_CONFIG_PS_KEY = "wifi_ps"; // modem power save once associated -#if defined(HELTEC_LORA_V4_R8) - #define WIFI_CONFIG_PS_DEFAULT 0 // USB-powered Expansion Kit: latency + link stability over mA -#else - #define WIFI_CONFIG_PS_DEFAULT 1 // battery boards keep the DTIM sleep they always had -#endif static SdNvsPrefs s_prefs; static bool s_begun = false; @@ -149,20 +143,6 @@ void wifiConfigSetBleEnabled(bool enabled) { s_begun = s_prefs.begin(WIFI_CONFIG_NAMESPACE, true); } -bool wifiConfigGetPowerSave() { - if (!s_begun) wifiConfigBegin(); - return s_prefs.getUChar(WIFI_CONFIG_PS_KEY, WIFI_CONFIG_PS_DEFAULT) != 0; -} - -void wifiConfigSetPowerSave(bool enabled) { - if (!s_begun) wifiConfigBegin(); - s_prefs.end(); - if (!s_prefs.begin(WIFI_CONFIG_NAMESPACE, false)) return; - s_prefs.putUChar(WIFI_CONFIG_PS_KEY, enabled ? 1 : 0); - s_prefs.end(); - s_begun = s_prefs.begin(WIFI_CONFIG_NAMESPACE, true); -} - #if defined(TLORA_PAGER) void wifiConfigSetPagerWifiBlePhase(PagerWifiBlePhase phase) { s_pager_wifi_ble_phase = phase; diff --git a/src/helpers/esp32/WifiRuntimeStore.h b/src/helpers/esp32/WifiRuntimeStore.h index cebf6e8..2cf3462 100644 --- a/src/helpers/esp32/WifiRuntimeStore.h +++ b/src/helpers/esp32/WifiRuntimeStore.h @@ -27,14 +27,6 @@ void wifiConfigSetRadioEnabled(bool enabled); bool wifiConfigGetBleEnabled(); void wifiConfigSetBleEnabled(bool enabled); -/* Wi-Fi modem power save (DTIM sleep) once associated. ON saves power and gives BLE - * coexistence airtime; OFF keeps the RX chain up for a lower-latency companion-TCP / - * web-mirror link and holds a marginal association better. Default ON everywhere - * except the V4-R8 (USB-powered Expansion Kit form factor; perf pass 2026-08-20). - * Applied by the main loop after association and live by the Wi-Fi settings toggle. */ -bool wifiConfigGetPowerSave(); -void wifiConfigSetPowerSave(bool enabled); - #if defined(TLORA_PAGER) /* Pager coexistence ownership. Wi-Fi intent alone cannot answer whether a * cold NimBLE start is safe: touch builds keep the STA scannable even with no diff --git a/src/main.cpp b/src/main.cpp index b0fec9f..c203a44 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1691,14 +1691,19 @@ void loop() { // BLE coexistence airtime). Deferred to here on purpose: enabling it on the // unassociated STA naps the radio through a scan dwell and breaks the setup // wizard's WiFi.scanNetworks() ("no networks found"). One-shot. - // Persisted preference (Wi-Fi settings -> "Power save"): default ON, except the V4-R8 - // where it defaults OFF (perf pass 2026-08-20 — lower-latency app link, steadier - // association on its weak 2.4 GHz path). The toggle also applies live once associated. + // + // Unconditional, and deliberately NOT a user preference. This was briefly + // configurable (default off on the V4-R8, for a lower-latency app link), but + // WIFI_PS_NONE is not a legal sleep mode while the BT controller is running -- + // modem sleep is exactly what yields airtime to BLE, and the Wi-Fi PM blob + // aborts rather than refusing. An R8 with saved credentials and Bluetooth on + // crashed on association every time: task "wifi", abort() in pm_set_sleep_type. + // The latency it was buying did not exist either: profiling the companion link + // showed sync cost is round-trip COUNT (~40 CMD_GET_CHANNEL), with the firmware + // accounting for 0.3% of it. So there is nothing to trade away here. static bool modem_sleep_set = false; if (!modem_sleep_set) { - const bool ps = wifiConfigGetPowerSave(); - WiFi.setSleep(ps); - Serial.printf("[wifi] modem power save %s\n", ps ? "on" : "off"); + WiFi.setSleep(true); modem_sleep_set = true; } if (!sntp_kicked) { diff --git a/src/ui-touch/UITask.cpp b/src/ui-touch/UITask.cpp index d35aaaf..e4f1691 100644 --- a/src/ui-touch/UITask.cpp +++ b/src/ui-touch/UITask.cpp @@ -14943,18 +14943,6 @@ static void buildMqttSettings() { #endif } -#if defined(ESP32) && defined(MULTI_TRANSPORT_COMPANION) -// Wi-Fi modem power save: persist + apply live. Only touches the driver once associated — -// WiFi.setSleep() on an unassociated STA naps the radio through scan dwells (see main.cpp). -// esp_wifi_set_ps is a plain setter (no disconnect/begin), so it is safe from the LVGL ctx. -static void wifiPowerSaveToggleCb(lv_event_t* e) { - if (lv_event_get_code(e) != LV_EVENT_VALUE_CHANGED) return; - const bool on = lv_obj_has_state(lv_event_get_target(e), LV_STATE_CHECKED); - wifiConfigSetPowerSave(on); - if (WiFi.status() == WL_CONNECTED) WiFi.setSleep(on); -} -#endif - static void buildWifiSettings() { lv_obj_t* body = createSettingsModal("", SettingsModalKind::Wifi); // no group header — the top bar already says "Wi-Fi" int y = 0; @@ -14975,18 +14963,6 @@ static void buildWifiSettings() { lv_label_set_text(g_set_modal.wifi_sta_status_l, TR("Loading...")); y += SC(30); - // Modem power save (DTIM sleep) — see wifiPowerSaveToggleCb. Default ON; OFF on the V4-R8. - { - int rh = settingsRowLabel(body, y, 4, TR("Power save (modem sleep)"), COLOR_TEXT, &g_font_12, 56); - lv_obj_t* sw = lv_switch_create(body); - lv_obj_align(sw, LV_ALIGN_TOP_RIGHT, 0, y); - if (wifiConfigGetPowerSave()) lv_obj_add_state(sw, LV_STATE_CHECKED); - lv_obj_add_event_cb(sw, wifiPowerSaveToggleCb, LV_EVENT_VALUE_CHANGED, nullptr); - y += LV_MAX(34, rh + 10); - y += settingsRowLabel(body, y, 0, TR("Off: snappier app/TCP link, steadier on a weak signal. On: less power, kinder to Bluetooth."), - COLOR_SUB, &g_font_12, 0) + 2; - } - // Make sure the network we're currently using shows up as saved, and on first // run import any legacy 3-slot profiles into the new known-networks store. {