feat: menu-driven airplane mode + WiFi/BLE coex - #1
Conversation
Replaces the earlier button-gesture design which crashed ESP32 on enter
when WiFi/BLE had live sessions. The gesture tore down radios
synchronously from the ButtonThread context; the menu version persists
radio-off config to NVS and reboots instead, using the same "reboot to
apply" pattern Meshtastic already uses for Reboot and Shutdown.
Design:
- AirplaneMode::toggle() is the single entry point.
- isActive() is derived from the live config (all 3 radio flags false)
rather than a runtime state machine.
- Saved pre-airplane radio state lives in ESP32 Preferences NVS so exit
restores faithfully; fallback defaults to all-radios-on if NVS was
wiped while active.
- No new protobuf fields; no build-flag gate; always-available menu
entry alongside Reboot and Shutdown.
Changes:
- src/modules/AirplaneMode.{h,cpp} rewritten (drops gesture state
machine; ~80 lines net from ~150+).
- src/graphics/draw/MenuHandler.h: add AirplaneModeMenu enum and
airplaneModeMenu() declaration.
- src/graphics/draw/MenuHandler.cpp: add airplaneModeMenu()
implementation, add entry to powerMenu() (label reflects state),
dispatch from handleMenuSwitch.
Resolves the crash reported on feat/portable-combined (gesture enter
crashed in ButtonThread due to synchronous esp_wifi_stop() /
nimbleBluetooth->deinit() on live connections).
Upstream Meshtastic intentionally excludes BLE initialization whenever WiFi is configured (src/platform/esp32/main-esp32.cpp setBluetoothEnable calls !isWifiAvailable()). isWifiAvailable() checks config, not runtime state, so a node with wifi_enabled=true will never advertise BLE even when WiFi has never successfully associated. This is the root cause of reports like meshtastic#1294 (bootloop on WiFi enable), meshtastic#7719 (dual mode feature request), meshtastic#3808 (BLE radio stays off), and meshtastic#8151 (S3-specific errors) — they are downstream of the XOR gate, not coexistence timing bugs. The gate made sense in the ESP32 + bluedroid era where coex was unreliable. On ESP32-S3 + NimBLE + ESP-IDF 5.x, the hardware coex arbiter handles dual-RF correctly when given reasonable inputs: - WiFi power save must allow the radio to release between DTIM beacons (WIFI_PS_NONE monopolizes the radio — line 309 in WiFiAPClient.cpp). - Coex preference should be set explicitly (defaults are version- dependent and historically WiFi-biased). This commit adds a MESHTASTIC_DUAL_RF build flag that: 1. src/platform/esp32/main-esp32.cpp — skips the !isWifiAvailable() gate in setBluetoothEnable(), always initializing NimBLE if config.bluetooth.enabled. 2. src/mesh/wifi/WiFiAPClient.cpp — switches to WIFI_PS_MIN_MODEM + WiFi.setSleep(true) and calls esp_coex_preference_set( ESP_COEX_PREFER_BALANCE) so the arbiter schedules BLE slots between WiFi windows. 3. variants/esp32s3/heltec_v4/platformio.ini — adds heltec-v4-coex env with -D MESHTASTIC_DUAL_RF=1 for opt-in testing. Default builds are unaffected; without the flag behavior is bit- identical to upstream. Verified root cause via direct code read + reproduced empirically on Heltec V4 running 2.7.20.6658ec2: factory-default firmware + WiFi enabled = BLE does not advertise; factory-default + WiFi disabled = BLE advertises immediately (same NVS, same binary).
When AirplaneMode::isActive() returns true (all three radio flags off in config), drawCommonHeader() replaces the per-screen title with "AIRPLANE MODE" (or "AIRPLANE" on ultra-low-res screens). Always visible on any main-screen frame the user navigates to, unambiguous at a glance, zero layout disruption. Addresses the gap identified during first field test: user could enter airplane mode via menu, but had no clear on-screen confirmation it was active — absence of WiFi/BLE icons isn't the same as an explicit indicator. Implementation: 6-line conditional in the existing title render block. No new assets, no new overlay, no new render pass.
…shtastic#3) Previous commit 6bfefb3 replaced the screen title with literal text "AIRPLANE MODE" (or "AIRPLANE" on ultra-low-res). That text was wide enough on the 128-pixel OLED to collide with the battery percentage on the left and the mail/mute/time cluster on the right. Swap the text replacement for a compact 8x8 XBM airplane icon drawn at the title's center position. Icon width is similar to a single glyph of the normal title font, so no possible overlap with any of the surrounding header elements at any supported screen resolution. Icon design: stylized top-down airplane silhouette (nose + wings + fuselage + tail) in 8 bytes, PROGMEM-resident.
Pre-review cleanup pass. Net -137 lines. - AirplaneMode: drop NVS persistence of pre-airplane radio state. Exit sets all three radios to enabled (the 99% expected outcome); users who had a radio manually disabled before entering can re-disable it after exit. Saves ~100 lines of Preferences boilerplate. - AirplaneMode.h: trim doc block, drop deleted copy/assign ctors, drop unused private helpers (enter/exitActive merged into toggle). - images.h: move the airplane icon into the neighbor-pattern location with airplane_width / airplane_height / airplane[] matching mail_width / mail[]. - SharedUIDisplay.cpp: drop inline bitmap + ASCII-art comments; use the shared icon from images.h. - WiFiAPClient.cpp / main-esp32.cpp: trim coex patch comments to 1-2 lines each. - MenuHandler.cpp: drop explanatory comment on always-available label.
Gemini Review (warn-only)GEMINI_API_KEY secret is not configured. Automated review did not run. |
Gemini Review (warn-only)SummaryThis pull request introduces an "Airplane Mode" feature to disable all radios via a menu option, and adds a build variant for ESP32-S3 boards to enable simultaneous WiFi and BLE operation. The implementation is mostly sound and follows project conventions, but a critical buffer overflow bug in the menu code must be fixed before merge. Issues
Upstream readinessThis PR is not ready to be sent upstream. The buffer overflow is a blocker that must be fixed. The Warn-only — this check never blocks merge. |
- images.h / SharedUIDisplay.cpp: rename airplane -> icon_airplane to match the newer icon_* convention (icon_mail, icon_compass, icon_system, icon_radio, etc.) and avoid a generic global name. - AirplaneMode.cpp: document isActive() derivation. State is deliberately inferred from live config rather than persisted, so any path that disables all three radios (menu, CLI, phone app) surfaces as airplane mode, and exit via the menu re-enables all three. Gemini flagged optionsArray[4] as a buffer overflow (BLOCKER); verified at MenuHandler.cpp:2407 it is already sized [5] — false positive, no action needed. Left the config.bluetooth.enabled == true comparisons alone for local consistency with the neighboring USE_WS5500 and HAS_WIFI branches in the same #if/#elif ladder.
|
Thanks for the review. Triage + actions in commit 44b44cd: BLOCKER —
|
Gemini Review (warn-only)SummaryThis pull request introduces a menu-driven "Airplane Mode" to disable all radios, and adds a build variant for ESP32-S3 to enable simultaneous WiFi and BLE operation. The WiFi/BLE coexistence fix is well-implemented, but the Airplane Mode feature has a major design flaw in how it restores state that would likely be blocked upstream. Issues
Upstream readinessThe WiFi/BLE coexistence portion is well-implemented with an opt-in build flag and is ready for an upstream PR. The Airplane Mode feature, however, will likely be rejected due to the destructive state-change behavior when exiting the mode. This should be fixed before upstreaming. It would be best to split this into two separate PRs: one for the coex fix, and one for the airplane mode feature after its design is revised. Warn-only — this check never blocks merge. |
…stic#3) Addresses Gemini review cycle 2 [MAJOR]: previous version unconditionally enabled WiFi, BLE, and LoRa TX on exit, overwriting any manual pre-airplane configuration. A user who had WiFi disabled for power reasons, or BLE disabled for privacy, would find them enabled after airplane-mode exit — a destructive behavior upstream would reject. Persist the pre-airplane state of config.network.wifi_enabled, config.bluetooth.enabled, and config.lora.tx_enabled to three ESP32 Preferences NVS keys on enter. On exit, load from NVS and apply; if NVS has no saved state (e.g. first run, or NVS wiped while active), fall back to all-radios-on with a LOG_WARN. Also tightens the header guard from #if HAS_SCREEN || ARCH_ESP32 to #if HAS_SCREEN (addresses Gemini's [QUESTION] — the feature is menu-driven only, the ARCH_ESP32 branch predates the simplified design and no longer adds value). Implementation keeps the NVS path behind #ifdef ARCH_ESP32 so non- ESP32 screen variants (nRF52 with screen, RP2040 with screen) still compile; they just fall back to the default-all-on exit behavior since Preferences is ESP32-only.
Gemini Review (warn-only)SummaryThis pull request introduces an "Airplane Mode" feature accessible from the power menu, which disables all radios and reboots the device. It also adds a new build variant for ESP32-S3 boards to enable simultaneous WiFi and BLE operation. The implementation is generally clean but has a significant state-management bug and a design issue that would block an upstream merge. Issues
Upstream readinessThis PR is not ready to be sent upstream. The state management bug is a blocker that must be fixed. The Warn-only — this check never blocks merge. |
) Addresses Gemini cycle 3 review. [BLOCKER] isActive() derived from live config introduces a race: if a radio is re-enabled externally (phone app, CLI) while airplane mode is active, isActive() flips false and the next toggle() treats the visit as ENTER rather than EXIT, overwriting the original pre-airplane saved state with a corrupted snapshot. Fix: store an explicit bool flag in NVS (apm_active) and consult it from isActive(); toggle() reads and writes it atomically with the save/restore of per-radio state. [MAJOR] The module was guarded by #if HAS_SCREEN which prevented headless builds from compiling the feature at all, blocking any future CLI or Admin API integration. The screen is only needed by callers (MenuHandler, SharedUIDisplay), not by the module itself. Move the guard to the call sites: the module now compiles on any platform, is fully functional on ARCH_ESP32 (Preferences NVS backend), and is a documented no-op elsewhere until an alternative persistence path is added. [MINOR] loadPreAirplaneState now LOG_ERRORs on Preferences::begin failure, matching the save-side pattern. Also added the same to readActiveFlag / writeActiveFlag. toggle() on non-ESP32 platforms logs a warning and returns — the module is callable but does nothing without NVS persistence available. Safe to compile everywhere; behaviorally inert where unsupported.
|
Cycle 3 fixes in commit 13e332c: BLOCKER — race condition in isActive()Fixed. Replaced config-derived isActive() with an explicit NVS flag (`apm_active`). toggle() reads/writes the flag atomically with the save/restore of per-radio state, so external config mutations mid-airplane-mode can't corrupt the enter/exit transition. MAJOR — HAS_SCREEN guard too tightFixed. Removed the guard from AirplaneMode.h/.cpp entirely. Module now compiles on all platforms. On ARCH_ESP32 it's fully functional via Preferences NVS. On non-ESP32 (nRF52, RP2040, etc.) toggle() logs a warning and returns — behaviorally inert until someone adds an alternative persistence path. This way a future admin/CLI integration on headless ESP32 is already unblocked. MINOR — silent Preferences::begin failureFixed. Added LOG_ERROR on every Preferences::begin() failure path (readActiveFlag, writeActiveFlag, savePreAirplaneState, loadPreAirplaneState). Consistent with the original save-side pattern. QUESTION — MESHTASTIC_DUAL_RF namingIntentional, keeping. The flag name is platform-agnostic because the intent is "allow WiFi and BLE to run together." The current implementation (WIFI_PS_MIN_MODEM + esp_coex_preference_set) is ESP32-specific, but a future nRF52 or similar implementation would use a different backend under the same intent. Renaming to MESHTASTIC_ESP32_DUAL_RF would lock the flag to one architecture and require yet another flag if we extend. The ESP32-only-today aspect can be documented in the platformio.ini env comment if reviewers want that. |
Gemini Review (warn-only)SummaryThis pull request introduces an "Airplane Mode" feature to disable all radios, and a related WiFi+BLE coexistence mode for ESP32-S3. The implementation is generally good and follows project conventions, but a critical performance bug in the UI path must be fixed. The PR should also be split into two smaller, more focused PRs before being sent upstream. Issues
Upstream readinessThis PR is not ready to be sent to Warn-only — this check never blocks merge. |
…stic#3) Cycle 4 Gemini review fixes. [BLOCKER] isActive() is called every frame from drawCommonHeader; the previous implementation did a blocking NVS read on each call. Added module-level static cache (s_cachedActive, s_cachedActiveLoaded) populated lazily on first access and kept in sync by toggle(). Hot path is now a pair of bool loads. [MINOR] Renamed NVS keys apm_sv_* to apm_save_* for readability. All keys remain within the 15-char NVS limit (apm_save_wifi = 13, apm_save_lora = 13, apm_save_bt = 11). MAJOR (commit shape: split into two PRs) is acknowledged and is the plan for upstream submission — this fork PR is a unified staging review. Upstream work is tracked as two separate issues/PRs (one for coex, one for airplane mode) so each gets independent maintainer review.
|
Cycle 4 fixes in commit 5c00ee0: BLOCKER — NVS read on every isActive() callFixed. isActive() is on the hot path (called every frame from drawCommonHeader). Added a module-level static cache populated lazily on first access and kept in sync by toggle(). Hot path is now two bool loads instead of a blocking NVS transaction. MINOR — cryptic NVS key namesFixed. Renamed apm_sv_wifi/bt/lora to apm_save_wifi/bt/lora. All keys stay under the 15-char NVS limit. MAJOR — split into two PRsAcknowledged, not splitting this PR. This fork PR is a unified staging review — the intent was to get one consolidated round of Gemini feedback on both features at once. Upstream submissions will be two separate PRs (tracked as issue meshtastic#4 for coex and issue meshtastic#5 for airplane-mode on Strycher/LoRa). Splitting the fork PR now would double the review overhead without changing the upstream outcome. If you'd prefer the split to happen at the fork level, I can do that as follow-up; otherwise the split happens naturally at the upstream/meshtastic/firmware step. |
Gemini Review (warn-only)SummaryThis pull request introduces a new "Airplane Mode" feature and separately enables WiFi/BLE coexistence for ESP32-S3 boards. The implementation is generally clean and follows project conventions, but the direct use of NVS for state management is a design choice that warrants discussion. The PR should be split into two before being sent upstream. Issues
Upstream readinessThis PR is not ready for an upstream submission in its current form. The primary issue is that it bundles two distinct features, which upstream maintainers will ask to be split. The direct use of NVS for state management is also a significant design deviation that will likely face pushback and require a strong justification. The minor issues should be addressed, but the major structural and design points are the most critical to resolve before this can be merged upstream. Warn-only — this check never blocks merge. |
Cycle 5 Gemini [MINOR]: moved the isActive() cache from file-static variables to mutable private members of the AirplaneMode singleton for proper encapsulation. mutable because isActive() is const. No behavior change; purely hygiene.
|
Cycle 5 fixes + holds in commit 32051ad: MAJOR — NVS Preferences vs protobuf ModuleConfigJustified, holding. The persisted data here is runtime state, not user configuration:
Protobuf `config` and `moduleConfig` are for user-configurable settings that should be visible and editable via phone app / CLI / Python API. Exposing runtime state there pollutes the user-facing namespace and invites accidental misuse (e.g. a user clearing `apm_active` via CLI while in airplane mode would desync state). Precedent: `main-esp32.cpp`'s `rebootCounter` uses Preferences directly for the same reason — it's runtime bookkeeping, not configuration. This module follows that pattern. MAJOR — Commit scope / split PRsAcknowledged in cycle 4; unchanged. This fork PR is a consolidated staging review. Upstream submissions are already tracked as two separate issues (meshtastic#4 coex, meshtastic#5 airplane-mode on the project board) and will be submitted as distinct PRs to meshtastic/firmware. MINOR — cache flags as file-staticsFixed. Moved `cachedActive` and `cachedActiveLoaded` to `mutable` private members of the singleton. Clean encapsulation. MINOR — static const array in header duplicates across TUsHolding for consistency. `icon_airplane[]` in images.h follows the same `static const unsigned char ...[] PROGMEM` form as `mail[]`, `hop[]`, `batteryBitmap_v[]`, and every other icon in the file. Fixing only the new one is inconsistent; a repo-wide refactor to extern-declared arrays with definitions in a .cpp is out of scope for this PR but would be a reasonable standalone cleanup PR upstream. QUESTION — non-ESP32 platform planNo concrete plan. The module compiles on all platforms; toggle() is a no-op on non-ESP32 until a platform-specific persistence backend is added. nRF52's `InternalFS` or similar could back an equivalent implementation if/when someone needs it. Keeping the ARCH_ESP32 guard at the backend call sites (not the module definition) means adding support is a pure-addition change. |
Gemini Review (warn-only)SummaryThis pull request introduces an ESP32-only "Airplane Mode" feature to disable all radios, accessible from the power menu. It also adds a new build variant for ESP32-S3 boards to enable WiFi and BLE to run concurrently. The implementation is generally sound and follows project conventions, but the Issues
Upstream readinessThis PR is close to being ready for upstream, but the Warn-only — this check never blocks merge. |
[MAJOR] platformio.ini: heltec-v4-coex now extends env:heltec-v4 instead of duplicating all pin defines and display flags. Cuts ~20 lines of duplication; adds only the COEX flag and display name override. Verifies the upstream-preferred pattern works with PlatformIO's env-to-env inheritance. [MINOR] Rename MESHTASTIC_DUAL_RF to MESHTASTIC_WIFI_BLE_COEX for clarity. Flag name now describes the specific behavior (WiFi and BLE running simultaneously) rather than the generic "dual RF" phrasing. Touches main-esp32.cpp, WiFiAPClient.cpp, platformio.ini. [MINOR] SharedUIDisplay.cpp: use local screenW parameter instead of SCREEN_WIDTH macro for title centering, matching the neighbor pattern in the same function.
|
Cycle 6 fixes in commit 38c90b3: MAJOR — platformio.ini duplicationFixed. `heltec-v4-coex` now `extends = env:heltec-v4` and only adds the COEX build flag and display name override. Cut ~20 lines of duplication. Verifying the build still works locally (PlatformIO env-to-env inheritance is documented but less common than extending a base section). MINOR — SCREEN_WIDTH vs screenWFixed. The title-centering math in the else branch now uses the local `screenW` parameter, matching the airplane-icon branch and the surrounding battery/time elements. The original `SCREEN_WIDTH` macro was a pre-existing inconsistency in the function. MINOR — MESHTASTIC_DUAL_RF namingFixed. Renamed to `MESHTASTIC_WIFI_BLE_COEX` — more descriptive of what the flag actually does. Touches main-esp32.cpp, WiFiAPClient.cpp, platformio.ini. QUESTION — mid-airplane config changes overwritten on exitIntentional. The saved pre-airplane radio state is the source of truth for the exit restore. A user who connects mid-airplane (via phone app / CLI) and toggles a radio would find that change transient — exit restores to the pre-airplane baseline, not to the mid-flight edit. This is deliberate because (a) exit is the natural "return to normal" boundary, and (b) treating mid-airplane edits as permanent would allow a confused UI to accumulate drift. If a user wants to permanently change a radio setting, they should exit airplane mode first, adjust, and re-enter — or simply re-enter after exit and the new state becomes the saved baseline. |
Gemini Review (warn-only)SummaryThis pull request adds an "Airplane Mode" feature to disable all radios via a menu option, persisting the state across reboots. It also introduces a build flag and configuration changes to properly support WiFi and BLE coexistence on ESP32-S3. The implementation is mostly sound, but a correctness issue in the Airplane Mode logic needs to be addressed. Issues
Upstream readinessUpstream maintainers will likely request that this PR be split into two: one for the WiFi/BLE coexistence support (a platform improvement) and another for the Airplane Mode feature. The LoRa disable logic is a functional bug that must be fixed before the Airplane Mode feature would be considered for merging. The coexistence changes appear ready for upstreaming once split out. Warn-only — this check never blocks merge. |
|
Cycle 7 — no code changes, evidence-based response only. MAJOR (repeated) — lora.tx_enabled vs lora.enabledHallucinated field, verified. This critique repeats cycle 1 and was already addressed there with evidence. The protobuf-generated `meshtastic_Config_LoRaConfig` struct in `src/mesh/generated/meshtastic/config.pb.h` has these bool/int fields and only these:
There is no `config.lora.enabled`. Using it would be a compile error. `tx_enabled=false` is the idiomatic Meshtastic "LoRa radio emits nothing" — the same field AdminModule's "TX disable" admin command uses. The receive path stays powered but emits no RF, which is what airplane mode requires for FCC/FAA (emissions, not reception). If a "radio fully powered off" flag is added to the schema in the future (meshtastic/protobufs PR), airplane mode can adopt it. That's a cross-repo coordination effort worth tracking separately; it doesn't block this PR. QUESTION — other radio-related flags for a complete kill-switchNo, these three are complete. The PR disables every TX-capable radio Meshtastic owns on this hardware:
Remaining radios on typical Meshtastic hardware are receive-only:
No other RF-capable subsystems exist in the config schema for this class of device. Kill-switch is complete from the emissions perspective, which is what FAA/FCC "airplane mode" rules target. |
1 participant
Summary
Two independent but co-delivered features for ESP32 (Heltec V4 specifically tested):
AIRPLANE_MODE_ENABLED) — menu entry under Power → disables LoRa TX + WiFi + BLE, persists to NVS, reboots clean. Symmetric exit. Airplane icon in the header when active.MESHTASTIC_DUAL_RF) — bypasses the WiFi-XOR-BLE gate insetBluetoothEnable, tunes WiFi power-save + coex arbiter preference so BLE advertising isn't starved.Both features gated behind build flags. Default behavior unchanged.
Why
isWifiAvailable()returning true (checks config, not runtime state) gatesNimbleBluetooth::setup()off entirely. This surfaces as reports like [Bug]: 1.3: Bootloop on ESP32 if Wifi enabled meshtastic/firmware#1294, [Bug]: ESP32 BLE Sleep Regression meshtastic/firmware#3808, [Feature Request]: Dual bluetooth and wifi mode meshtastic/firmware#7719, [Bug]: Wifi on esp32-s3 - causes an error and restarts the device meshtastic/firmware#8151. The fix explicitly routes both radios onto the hardware coex arbiter.Field verification (Heltec V4, 2.7.23)
Commits
feat: menu-driven airplane mode— state machine + menu integrationfeat(esp32): allow WiFi+BLE coexistence via MESHTASTIC_DUAL_RF— setBluetoothEnable gate + WiFi PS + esp_coex_preference_setfeat(airplane-mode): persistent header indicator— airplane icon in drawCommonHeaderfix(airplane-mode): replace overlong text indicator with 8x8 iconstyle(airplane-mode): simplify for upstream review— -137 lines of bloatReview focus
This is on a staging fork; Gemini warn-only review will run on this PR. Upstream PRs to
meshtastic/firmwareare tracked separately.