* Bound remote-initiated key verification sessions
Opening a session raises a 30 second banner and a client notification and
occupies the only verification slot, all before the peer has authenticated
anything, and nothing limited how often that could happen.
Adds an absolute session cap that incoming packets cannot refresh, a
cooldown between remote-initiated sessions measured from when the previous
one ended, and refreshes the idle deadline only when the protocol actually
advances rather than on any arriving packet.
The busy path now sets ignoreRequest so it no longer answers with a NAK.
Also replaces the getTime() - 60 timeout comparison, which underflowed
before the clock passed 60.
* Use elapsed-time comparison for the session timeout and condense comments
perhapsDecode fell back to the not-yet-verified key held during a key
verification handshake for any incoming PKI unicast. That key is supplied
by whoever opened the handshake and proves only that they hold it, not that
they are the node they claim to be, so until the session ended they could
send DMs on any port that decrypted and were marked pki_encrypted.
perhapsEncode already restricts the pending key to KEY_VERIFICATION_APP.
The receive path now applies the same rule.
The send_bell append writes bytes[size] and bytes[size+1] while the guard only checked size < DATA_PAYLOAD_LEN, overrunning the 233-byte payload array by one when size == DATA_PAYLOAD_LEN - 1. Guard on size + 1.
writeSecret is a setter, so calling it on the NETWORK_CONFIG get path was a no-op: the buffer
already holds the stored psk, never the sentinel. MQTT_CONFIG returned the broker password
verbatim.
Both get paths now return secretReserved when req.from != 0, and the matching set paths call
writeSecret so a read-modify-write round trip keeps the stored value.
* Null-check packet allocations in allocForSending and its callers
Sibling of 0ae44d701.
* Null-check allocDataProtobuf, allocAckNak and allocErrorResponse callers
Second tier of the same nullable contract.
* Keep telemetry sleep scheduling on allocation failure
Allocation failure now marks the telemetry invalid instead of returning
early, so power-saving SENSOR nodes still arm deep sleep.
setPassKey drew the session passkey outside cryptLock, unlike the signing
and key verification paths. Also replaces random() for the key verification
nonce with the hardware RNG.
* Initial plan
* fix: declare sa as const pointer in t5s3_epaper variant.cpp
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
splitConfiguredMessages capped the split loop at
CANNED_MESSAGE_MODULE_MESSAGE_MAX_COUNT and then appended [Exit]
unconditionally, so messages[] could be written one past its last index
and messagesCount could exceed the array size.
A client's "regenerate keys" action sends a blank SecurityConfig carrying
only the new private key rather than the config it read from the device,
so assigning it wholesale cleared admin_key, is_managed, serial_enabled,
debug_log_api_enabled, admin_channel_enabled and packet_signature_policy.
Losing the admin keys locks the owner out of remote admin with no recourse
but a physical connection to the node.
Detect that bare-rotation shape and swap in just the keypair, leaving the
rest of the security config intact. Deliberately clearing admin keys still
works through a SET that leaves the private key alone.
Fixes#11073
- t5s3-epaper: the board's 3.3V octal (AP_3v3) PSRAM is unreliable at the qio_opi
default of 80MHz (Total PSRAM reads 0, display dead / boot-loop); clock it at 40MHz.
- EInkParallelDisplay: skip EInk init and no-op display ops when the PSRAM framebuffer
is unavailable, so the node boots headless instead of hard-faulting on a NULL buffer.
- src/platform/esp32: weak software __atomic_*_{1,2,4} so esp32s3 links on toolchains
(e.g. macOS) that lack the sized libcalls GCC emits under -mdisable-hardware-atomics.
Removed:
- MessageStore::addFromPacket and addFromString, superseded by
tryAddFromPacket
- GeoCoord rangeRadiansToMeters, distanceTo, bearingTo
- Router::rawSend, declared virtual with no override and no caller
- ContentHandler handleHotspot, handleFs, handleAdminSettings,
handleAdminSettingsApply, handleDeleteFsContent and their commented
route registrations, plus the now unreachable htmlDeleteDir and the
handleUpdateFs declaration that had no definition
- ContentHelper replaceAll
- OnScreenKeyboardModule popup chain: showPopup, clearPopup, drawPopup,
drawPopupOverlay and their state, unreachable since the frame based UI
was replaced by baseUI
- DebugRenderer drawDebugInfoTrampoline, drawDebugInfoSettingsTrampoline
and the orphaned drawFrameSettings
- NodeListRenderer calculateMaxScroll, drawColumns and a stale extern
haveGlyphs declaration with no definition
- UIRenderer::haveGlyphs, Screen::blink,
NotificationRenderer::showKeyboardMessagePopupWithTitle,
VirtualKeyboard::getInputText
- InkHUD touchNavLeft, touchNavRight, Applet::getActiveNodeCount,
ThreadedMessageApplet::saveMessagesToFlash
- TwoButton::setHandlerUp, TwoButtonExtended setHandlerUp,
setJoystickDownHandlers, setJoystickUpHandlers
- CannedMessageModule LaunchRepeatDestination, isCharInputAllowed,
hasMessages
- TrafficManagementModule resetStats, recordRouterHopPreserved,
saturatingIncrement
- UnitConversions::MetersPerSecondToMilesPerHour
- EncryptedStorage getSessionRemainingSeconds
- BMI270Sensor::writeRegisters, GPS::hasFlow, FSCommon copyFile,
SerialConsole consolePrintf, buzz playLongPressLeadUp,
memGet displayPercentHeapFree
* Stop accelerometer thread when double-tap/wake-on-motion disabled at runtime
double_tap_as_button_press and wake_on_tap_or_motion are applied live: the
OFF->ON edge calls accelerometerThread->start(), but there was no ON->OFF
branch, so turning either flag off left the sensor thread running (polling
I2C, drawing power) until reboot. Worse, because enabled stayed true, a later
OFF->ON edge was a no-op (the enabled==false guard blocked re-start), leaving
the feature un-restartable without a reboot.
Add the symmetric ON->OFF branch in both handlers. When a flag goes true->off
and the other consumer of the shared thread is also off, call
accelerometerThread->disable() (stops runOnce polling and clears enabled so a
later re-enable can start() again). Each branch checks the other flag first so
disabling one feature never stops the sensor while the other still needs it.
* Keep accelerometer thread running when its sensor drives the compass
* Guard accelerometer thread config toggles against a null thread pointer
* AdminModule: factor shared accelerometer start/stop into a helper
The device and display config handlers had mirror-image blocks reconciling the
shared accelerometer thread. Extract reconcileAccelerometerThread(wasOn, nowOn,
otherFeatureOn) so the null guard, edge logic, compass (providesHeading) guard,
and rationale live in one place; each call site is now a single call. Behavior
is unchanged. Also drops the redundant per-field assignment that the
whole-struct `config.device = ...` / `config.display = ...` overwrites anyway.
---------
Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>
* Use hardware RNG for session passkey and PKC extra nonce
The admin session passkey and the Curve25519 extra nonce were drawn
from Arduino random(), which is not a CSPRNG. Source them from
HardwareRNG::fill, mirroring the signing path, and fall back to the
seeded CSPRNG (CryptRNG) only when no hardware source is available.
* AdminModule: make session passkey expiry rollover-safe
session_time was compared as millis()/1000 seconds with additive
thresholds, which breaks across the millis() wrap and could keep a stale
admin session key valid. Store session_time in millis() and use
Throttle::isWithinTimespanMs for the 150s refresh and 300s validity
windows.
* AdminModule: track session passkey validity with an explicit flag
session_time == 0 was used as the uninitialized sentinel, but millis()
is legitimately 0 in the first millisecond of uptime, so a passkey
issued then would be treated as no session. Use a dedicated
session_passkey_valid flag instead.
* AdminModule: camelCase the session passkey validity flag
---------
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
* TrafficManagement: gate role/NodeInfo cache writes on signer authenticity
The tier-3 role cache and the PSRAM NodeInfo response cache were updated
from any received NodeInfo with no authenticity check, so a spoofed
NodeInfo could set a node's cached role (granting dedup exceptions) or
poison the cached user served in direct responses. Skip both cache
writes when a known signer's NodeInfo arrives unsigned, matching the
identity-update gate on the direct-response path.
* TrafficManagement: hoist shared NodeInfo signer lookup
Compute the sender node lookup and unauthenticated-signer check once per
NodeInfo packet and reuse it for both the cache-refresh gate and the
direct-response identity gate, avoiding a second O(N) getMeshNode scan.
---------
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
decryptForHash accepted chIndex == getNumChannels() before reading
getHash(chIndex), which indexes one past hashes[MAX_NUM_CHANNELS].
Use >= so an out-of-range index is rejected before the array read.
* AdminModule: only accept admin responses to requests we sent
An admin *_response short-circuited the auth and session-passkey checks that gate
every other admin message, so any node could deliver one. On a channel the module
listens to unauthenticated, a get_module_config_response drives the remote-hardware
pin handler with attacker-supplied values.
Track the destination of outgoing admin requests (per remote, with the pinned PKC
key when there is one) and accept a response only from a node with a matching
outstanding request, inside the same window as the session passkey. Local (from == 0)
admin is unchanged; PhoneAPI already gates it.
Also fix the response dispatch: get_module_config_response.which_payload_variant is a
ModuleConfig oneof tag, but it was compared against the AdminMessage ModuleConfigType
enum (different numbering), so the handler never ran. Compare against the oneof tag.
* AdminModule: rollover-safe request window, bind response to request type
Two review refinements to the request/response pairing:
Use Throttle::isWithinTimespanMs for the outstanding-request expiry instead of
comparing millis()/1000 sums, which mis-expired across the millis() rollover.
Bind each accepted response to a request type actually sent to that node. Each
outstanding record now carries a bitmask of the response variants its requests
authorize, so a get_owner request no longer admits a get_module_config response.
The mask accumulates per remote, so a client may still pipeline several request
types to one node and have every answer accepted.
* AdminModule: track admin requests per-request, not per-node
Reworks the outstanding-request table so each request is its own entry with its own
expiry window and pinned key, replacing the per-node bitmask that shared one timestamp
and one key across every response variant.
That sharing let a later request to the same node extend an earlier one's window and,
worse, clear its PKC pin: an unpinned request cleared keyValid, so a plaintext response
to an earlier PKC-pinned request was then accepted. Per-request entries keep each pin
intact. Identical requests are de-duplicated (a client may fetch several config subtypes,
all answered by one response variant) and eviction compares elapsed time, which is
rollover-safe.
Test: a pinned request's response still requires its key after an unpinned request to the
same node.
* AdminModule: match module-config subtype and consume answered requests
Two refinements to the request/response pairing:
Only remote_hardware get_module_config_response mutates state (the pin table), so it
must answer a request for that exact ModuleConfigType, not just any module-config
request. Each entry records the requested subtype and the gate checks it.
A matched request is now consumed on accept, so a node cannot replay a state-mutating
response within the window. Because one request yields one response, request de-dup is
dropped (a client's N indexed get_channel requests are N entries, each consumed once).
Tests: a non-remote-hardware request does not admit a remote_hardware response, and a
second copy of an answered response is rejected.
On lockdown builds (MESHTASTIC_PHONEAPI_ACCESS_CONTROL, nRF52) the SerialConsole
is a process-lifetime singleton, so the per-connection admin-auth slot keyed by
its inherited PhoneAPI* is reused for every USB/serial client for the whole boot.
An operator's admin unlock stayed latched across serial client swaps: an attacker
plugging into the USB/serial port before the prior session's 15-minute inactivity
timeout inherited admin authorization -- the serial analog of the BLE stale-session
reuse bug closed by resetting state in onConnect()/onDisconnect().
Sample the USB-CDC host link (DTR/mount) state each runOnce(); on the link-drop
edge call close(), which frees the auth slot and resets PhoneAPI state so whoever
connects next re-locks via handleStartConfig()'s !isConnected() branch on their
first want_config -- the same physical-link boundary BLE enforces in onConnect().
On the nRF52 TinyUSB (Adafruit) core, (bool)Port == tud_cdc_n_connected(), which
goes false on cable unplug or host port-close. Console transports without a real
DTR line fall back to the existing inactivity timeout, no worse than before.
Entirely nRF52-lockdown-gated; non-lockdown builds are byte-identical.
The SOH/STX control frame carries a client-supplied filename that was passed
straight to FSCom open/remove/exists, so a ".." component could write, read, or
delete outside the filesystem root. On embedded LittleFS this is largely inert
(no parent of the partition root); on the Portduino daemon FSCom is the host
filesystem under a mountpoint, so it is a real arbitrary-path write/read/delete.
Validate the filename before any FS access: reject empty and any ".." path
component, and NAK the transfer. Absolute and subdirectory paths are still
accepted - the file manager transfers them from the manifest and PortduinoFS
confines them to its mountpoint - so only traversal out of the root is blocked.
Reachable only from a local client connection (PhoneAPI: BLE/USB/serial/TCP),
not over the RF mesh; on the daemon the TCP API makes it network-reachable.
native-suite-count goes to 34: +1 for the new test_xmodem suite and +1 correcting
a pre-existing miscount (it read 32 for 33 suite directories).
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
* stm32wl(rak3172): implement cpuDeepSleep()/shutdown() via STM32LowPower
cpuDeepSleep() was an empty stub, so the SDS deep-sleep PowerFSM state
and low-battery shutdown did nothing but leave the CPU running at full
power. Power::shutdown() also didn't include STM32WL in its arch list,
so an explicit shutdown command just logged a FIXME warning.
Adds STM32LowPower as a lib_dep (rak3172 only, mirroring STM32RTC) and
implements cpuDeepSleep() using it. Standby mode is used for both the
finite-wake (SDS/low battery) and forever (shutdown) paths, via
LowPower.shutdown(), with or without an RTC alarm.
If the LSE-backed hardware RTC never came up (stm32wlRtcAvailable()
false), this is a no-op - safer than sleeping without a confirmed wake
source. LowPower.shutdown() resets the MCU on wake and should never
return; if it somehow does, force a reset via HAL_NVIC_SystemReset()
rather than hanging the device silently forever.
Gated behind the existing HAS_LSE flag, so this is inert on every
variant but rak3172.
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Yong <me@ndoo.sg>
* feat(stm32wl): Warn and reset when trying to deepSleep without RTC
This is to prevent leaving the firmware catanonic when firmware has run
its shutdown routine but doesn't actually shutdown.
Signed-off-by: Andrew Yong <me@ndoo.sg>
---------
Signed-off-by: Andrew Yong <me@ndoo.sg>
* PhoneAPI: gate local admin on the connection, not the wire from
The lockdown admin check in handleToRadioPacket only ran when p.from == 0. from is
a client-supplied wire field, and MeshService::handleToRadio rewrites it to 0 before
AdminModule sees the packet. A client could therefore set from != 0 to skip the
!getAdminAuthorized() drop, then have the packet normalized back to a local-admin
identity and executed - unauthorized admin from an unauthorized connection.
Every packet in handleToRadioPacket already comes from the local connection, so
locality is a property of the connection, not of from. Move the decision into
classifyLocalAdminPacket(), which ignores from and keys only on the admin variant and
the connection's authorization: lockdown_auth is delivered inline, any other admin
from an unauthorized connection is dropped, authorized admin passes through.
The classifier is compiled unconditionally and unit-tested; the guarded caller (built
only in the nRF52 lockdown config) calls it. Test: an unauthorized connection's
ADMIN_APP packet with from != 0 is classified DropUnauthorized.
* PhoneAPI: wipe the encoded lockdown passphrase, shorten comments
---------
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
decoded.payload.bytes is a 233-byte protobuf field that is not NUL-terminated.
Printing it with a plain "%s" reads until a NUL, which for a full-length payload
with no NUL runs past the field. Use "%.*s" with payload.size, matching the write
form already used a few lines up in SerialModule.
Live sites: RangeTestModule appendFile, SerialModule text output. The same fix is
applied to the commented-out debug lines in RangeTestModule and Router so the
pattern is consistent if they are re-enabled.
* Build fix
* update
* Update platformio.ini
* Update SerialConsole.cpp
* Remove Network provisioning for this build
* Update platformio.ini
Per Vid, this is not needed once his PR is merged
* fix: resolve merge conflict in SerialConsole.cpp - use setHostDraining(false) from develop
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
* AudioModule: validate codec2 header and bound the RX decode reads
Two issues reachable from a crafted AUDIO_APP payload:
The RX path built a temp codec2 from rx_encode_frame[3] whenever the frame header
did not match ours. codec2_create returns NULL for an invalid mode byte, and the
next call dereferenced it. Only decode frames that carry our own header (magic +
mode) and drop the rest, so the untrusted mode byte never reaches codec2_create.
The decode loop advanced by the frame size while testing only i < rx_encode_frame_index,
so a payload length that was not a multiple of the frame size read past the received
data and could read past rx_encode_frame. Bound each read to i + frameSize <= the
received length clamped to the buffer, and clamp the receive memcpy to the buffer.
Behavior change: audio frames whose codec2 mode differs from this node's configured
mode are dropped instead of decoded with a temporary codec.
* AudioModule: pass byte count (not sample count) to i2s_write
* AudioModule: fix the mirror sample/byte bug on the i2s_read capture path
* stm32wl: add hardware RTC support infrastructure
Wires the STM32WL chip's internal RTC (running off the LSE 32.768kHz
crystal) into meshtastic's existing time-of-day framework
(perhapsSetRTC()/readFromRTC()), following the same pattern already
used for I2C RTC chips (RV3028, PCF8563/85063, RX8130CE).
LSE is started and polled manually before ever calling into the
STM32RTC library, with our own bounded timeout - the library's own
internal LSE startup path has no bounded fallback and hangs forever
via Error_Handler() if the crystal never locks, so this is required
for a board with a missing/faulty crystal to boot normally rather
than hang.
Gated behind a new HAS_LSE variant flag (currently unset everywhere,
so this is inert until a variant opts in - see follow-up commit).
Signed-off-by: Andrew Yong <me@ndoo.sg>
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
* gps: qualify RTC.h includes to avoid case-insensitive filesystem collision with STM32RTC
The stm32duino STM32RTC library (added to lib_deps in a follow-up
commit) ships its own src/rtc.h. On case-insensitive filesystems
(the macOS default), an unqualified #include "RTC.h"/<RTC.h> from
any file outside src/gps/ resolves to the library's rtc.h instead of
src/gps/RTC.h, since PlatformIO's LDF puts lib_deps include paths
ahead of the project's own -Isrc/gps.
Qualify every include as gps/RTC.h so it can't collide with any
same-named header a future dependency might ship, regardless of
filesystem case sensitivity. Purely mechanical, no behavior change.
Signed-off-by: Andrew Yong <me@ndoo.sg>
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
* stm32wl(rak3172): enable hardware RTC support
Opts rak3172 into the HAS_LSE infrastructure added previously: sets
STM32WL_LSE_DRIVE to a conservative default and pulls in the
STM32RTC library. rak3172 has ~63KB flash headroom going in;
build-verified at 76.7% flash usage after this change (up from a
73.8% baseline), well within budget.
wio-e5 is not opted in here despite sharing the same STM32WLE5 chip
- it's already at 96.8% flash usage today (GPS + I2C sensor support
compiled in, unlike rak3172), leaving too little headroom to safely
add STM32RTC without first trimming something else.
Signed-off-by: Andrew Yong <me@ndoo.sg>
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
* stm32wl: add docstrings for LSE/RTC setup functions
Addresses CodeRabbit's docstring coverage check on PR #10961.
Signed-off-by: Andrew Yong <me@ndoo.sg>
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
* stm32wl: address CodeRabbit nitpicks on PR #10961
- Brace the single-statement HAS_LSE branch in perhapsSetRTC() to
match the sibling readFromRTC() branch's style.
- Quote the RTC.h include in PhoneAPI.cpp for consistency with every
other qualified include site.
Signed-off-by: Andrew Yong <me@ndoo.sg>
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
---------
Signed-off-by: Andrew Yong <me@ndoo.sg>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
* Snake!
* Add spiLock to snake score saving
* Check fixes
* More careful locking
* WIP: Big Display Node
* Update src/graphics/HUB75Display.cpp
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Add HUB75 Native
* Add Tetris game module with core logic and UI integration
- Implement TetrisGame class for game logic, including piece movement, rotation, and line clearing.
- Create TetrisModule class to manage game state, input handling, and rendering on OLED display.
- Introduce high score tracking with persistence and optional mesh broadcasting.
- Define UI states for title, playing, paused, game over, and high scores.
- Implement input handling for game controls and state transitions.
- Add rendering functions for the game board, high scores, and title screen.
* feat(snake): add snake graphics and update display logic in SnakeModule
* Prompt for Initials for Tetris, too
* refactor games module
* Games refactor
* hub75 native double buffer
* Games tuning
* Make joystick repeat events on held button
* Add clouds and colors
* Fix breakout and more color
* difficulty tuning
* trunk
* refactor game announcements, etc
* Scale chirpy gravity as game speed increases
* Portduino, check for hub75 display before reading in hub75 options
* Final(?) games tuning
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Properly ignore input when games screen not shown
* Fail gracefully when HUB75 is selected but not supported.
---------
Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Ixitxachitl <kramerfm@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
A SECURITY_CONFIG get_config response copied config.security verbatim, so a remote
request was answered with the device identity private_key and sent over the air.
Only the local owner needs it, for backup.
Zero private_key in the SECURITY_CONFIG response when the request is remote
(from != 0); the local BLE/USB/TCP path (from == 0) still receives it. public_key
and admin_key are public and stay as they were.
* fix(display): don't leak a TFT driver instance on every screen wake
On variants whose Screen::handleSetOn() re-runs ui->init() on each display
wake (Heltec Tracker V1.x/V2, VTFT_LEDA and ST7796 boards, MUZI/Cardputer
via dispdev->init()), OLEDDisplay::init() unconditionally re-invokes
connect(), and TFTDisplay::connect() allocated a fresh driver instance
each time without freeing the old one. Each OFF->ON transition orphaned a
full LGFX device (measured: exactly 1,260 bytes per wake on a Heltec
Wireless Tracker V2). Since PowerFSM wakes the screen on every received
text message, a device on a busy channel leaked tens of KB per day -
matching field reports of heap climbing from 78% to 92% within a day.
Null-guard the driver allocations (matching the existing linePixelBuffer/
repaintChunkBuffer guards below them) so re-entry re-runs tft->init() on
the existing instance. Bench-validated on a Tracker V2: free heap now flat
across 13 consecutive wake cycles, with the panel still re-initializing
and rendering on every wake.
* fix(ble): don't leak BluetoothPhoneAPI and callbacks on BLE re-enable
NimbleBluetooth::setup()/setupService() re-run when Bluetooth is
re-enabled after deinit(), but BLEDevice::deinit() only frees the GATT
objects - the caller-owned allocations were re-created unguarded on every
cycle:
- bluetoothPhoneAPI: ~3.5KB PhoneAPI object per cycle, and the orphaned
instance is an OSThread that stays registered with the scheduler, so a
duplicate thread kept servicing the same static queues
- toRadioCallbacks / fromRadioCallbacks / security + server callbacks:
one small object each per cycle
- the BLESecurity shim was heap-allocated and never freed even on first
boot; it only forwards to static setters, so use a stack instance
Reuse the existing objects on re-setup; the setCallbacks() calls still
run every time since the characteristics themselves are new.
* chore: remove unused PacketCache
PacketCache landed in #8341 but no consumer was ever wired up - repo-wide,
the only references to PacketCache/packetCache are in its own two files.
As designed it also malloc()s per cached packet with no eviction or size
cap, so it should be re-reviewed for bounds before any future use. Remove
the dead code; git history preserves it if a bounded revival is wanted.
* fix(ble): reset stale session state when reusing BluetoothPhoneAPI
Review follow-up: reusing bluetoothPhoneAPI across BLE enable cycles could
hand the next session the previous session's dirty state. The only full
cleanup path (onDisconnect) is skipped when deinit()'s bounded disconnect
wait expires before the event lands (its 2s cap matches the connection
supervision timeout, so a phone that walked out of range makes this a
coin flip): the reused object then enters the next session mid-config,
with stale queue contents served to the new phone and a stale connection
handle that defeats the checkConnectionTimeout self-heal.
Factor onDisconnect's cleanup into resetBleSessionState() and run it from
setupService() when reusing the instance, restoring the old fresh-object
invariant. Also switch the four stateless callback objects to function-
local statics (the resolved framework BLE wrapper stores plain pointers
and never frees them, so static instances are safe and avoid the guarded
heap allocations), correct the comment that claimed deinit() frees the
GATT objects (it deletes only the BLEServer itself; the services and
characteristics it created remain a small library-side leak per cycle),
and fix the inverted connection check in getRssi() that made BLE RSSI
always read 0 on ESP32-S3/C6.
* refactor(display): construct-once guard around the whole driver ladder
Review follow-up: one if (!tft) around the #if/#elif/#else construction
block instead of a guard per branch, so a future display family can't
reintroduce the per-wake leak by copying an unguarded branch. Make the
HACKADAY bus pointer local to the construction (it was a write-only
global), and point the comment at the Screen::handleSetOn gates instead
of hand-listing boards that would go stale.
* fix(ble): use-after-free notifying a freed characteristic after deinit()
deinit() nulled bleServer and BatteryCharacteristic but left
fromNumCharacteristic dangling after BLEDevice::deinit(true) freed the
GATT graph, and it never detached the PhoneAPI fromNumChanged observer.
When deinit()'s bounded 2s disconnect wait expires before onDisconnect
runs (the same stale-bond / host-reset race resetBleSessionState was
built for), close() is skipped, so the observer stays attached with
state == STATE_SEND_PACKETS. After BLE is off, the next mesh packet
drives MeshService fromNumChanged -> PhoneAPI::onNotify -> onNowHasData
-> fromNumCharacteristic->notify() on freed memory (the framework BLE
wrapper's notify() dereferences the freed server via getConnectedCount).
Unlike sendLog(), onNowHasData() had no isConnected guard.
deinit() now calls resetBleSessionState() to detach the observer and
reset session state unconditionally (also forcing the conn handle to
NONE so checkConnectionTimeout can't be fooled by the stale handle),
nulls fromNumCharacteristic/logRadioCharacteristic like the other freed
pointers, and onNowHasData() bails on a null characteristic.
Reachable on all ESP32/S3/C6 NimBLE boards via admin disable-bluetooth
or a sleep transition while a phone is connected. Found by a follow-up
lifecycle audit of the re-enable changes in this PR.
Thomas GöttgensandGitHub
Jason PandGitHub
HarukiToredaandGitHub
Ben MeadorsandGitHub
Jonathan BennettandGitHub
TomandGitHub
oscgonferandGitHub
Andrew YongandGitHub
JorropoandGitHub
