JoystickUITask had two write-only fields after the recent refactors:
_msgcount (only reader getMsgCount() had no callers anywhere) and
_started_at (initialized, written in begin(), never read). Remove
both fields, their writes, and getMsgCount().
msgRead()'s auto-leave-Unread side-effect is preserved — it uses the
function parameter directly, not the field.
Also: update the loop() doc to drop the "calls poll()" reference and
the battery-cache comment to drop the "from housekeeping" wording —
both concepts gone since Phase C.
UIScreen gains onDisplayOff()/onDisplayOn() hooks. JoystickUITask tracks
display state and dispatches them on transition — at top of loop()
(catches display.c's auto-off, which fires behind our back) and right
after _display.turnOn() in the wake path (immediate resume).
Override in SnakeScreen and GPSSettingsScreen: stop their periodic
k_timers while the screen is off, restart on wake. Game state and
GPS-fix state are preserved across sleep. Snake doesn't crash into a
wall five seconds after the screen sleeps anymore.
Other screen timers (Countdown alarm, Contacts/Admin response timeout,
the global lock timer) intentionally keep running — their job is to
fire while the user is idle.
Also: wake-from-off now reschedules the lock timer if not already
locked, so a keypress near the end of the lock window gives you a
fresh LOCK_AFTER_MS instead of being immediately re-locked.
s_scan_sent (one-shot per boot) becomes s_last_scan_ms; RepeatersScreen
onEnter() re-runs the discover if there has never been a scan or the
last one is older than REPEATER_RESCAN_AFTER_MS (60s). Walking away
and returning now gives a fresh list instead of stale results.
Also fixes the manual KEY_ENTER_LONG rescan: after Phase C removed
the poll() that re-checked the flag, just unsetting it no longer
triggered anything. Now it calls doScan() directly.
Auto-off and auto-lock were the last polling-style deadline checks.
display.c already owns the auto-off (k_work_delayable rescheduled via
mc_display_reset_auto_off()), so its tracker in JoystickUITask was
fully redundant — remove _auto_off and the loop() check. Auto-lock
becomes a one-shot k_timer (_lock_timer) scheduled per activity; ISR
callback sets _locked + signals refresh.
The 2-second heartbeat timer in joystick_ui_hooks is gone, along with
the start/stopHeartbeatFns plumbing it served. The mesh thread now
wakes only on actual events: input, mesh, screen-owned timers, the
lock timer, and display.c's own auto-off work.
Also: screen member pointers in JoystickUITask are now their concrete
subclass types instead of UIScreen *; removes 14 static_cast<>s at
call sites. _curr stays UIScreen * (polymorphic).
UIScreen gains onEnter()/onExit() lifecycle hooks; poll() and the
_curr->poll() call in the main loop are removed. Each screen with
periodic or deadline-based work owns its own k_timer:
- one-shot timers: Splash dismiss, Countdown alarm, Contacts ping
timeout, RepeaterAdmin cmd/login timeout, Unread preview expiry
- periodic timers: Snake tick, GPSSettings sample
- onEnter()-only: Repeaters discover, Doom start
- deleted: Home, Stopwatch (were empty)
Timer ISR callbacks only signal _task->notify() — never mutate
screen state. Main-thread render() handles transitions. Setting
_curr now fires onExit on the outgoing screen and onEnter on the
incoming one, so timers are scoped to screen lifetime and can't
fire stale events on the wrong screen.
New 6th item in the repeater admin submenu (admin only). Sends
"clock sync" as a CLI command; the repeater reads sender_timestamp
from the packet metadata and sets its RTC to our companion epoch
if ours is ahead. Response lands in the existing admin history list.
- Lock overlay: show battery % + unread count between title and unlock
sequence; ui_invalidate_battery_cache() on screen wake forces a fresh
ADC sample so the user sees current data immediately
- UnreadScreen::addPreview gains initially_read; received msgs pass
_ble_connected (don't count unread when phone is syncing); sent msgs
pass true (you sent it, you know it)
- CompanionMesh::queueLocalSent{Contact,Channel}Message + PUSH_CODE_MSG_WAITING
on wio-originated sends so a connected phone app sees them via the
normal offline-queue flow (path_len = OUT_PATH_SENT marker)
- OUT_PATH_SENT moved back from joystick_defs.h to ContactInfo.h
(now a wire-format value, not UI-only)
- #ifdef-gate _pending_joystick_{ping,admin}_tag fields + setters
under CONFIG_ZEPHCORE_UI_DESIGN_JOYSTICK (saves 8 bytes per
CompanionMesh instance on button-UI builds)
- gate logTx ui_notify_packet_sent() to joystick builds only;
was firing on every TX for any UI variant (dead code on button UI)
- drop redundant _pending_login manual set in CMD_SEND_LOGIN;
BaseChatMesh::sendLogin's onLoginSent hook owns it now, just
clear the other pending fields explicitly
- src/Mesh.cpp Reverted #ifdef ZEPHCORE_COMPANION block → back to vanilla self_id.copyHashTo
- helpers/ContactInfo.h Removed OUT_PATH_SENT
- helpers/ui-joystick/joystick_defs.h Added OUT_PATH_SENT here (with comment clarifying it's UI-only)
- helpers/ui-joystick/joystick_ui_task.h Removed dead _next_batt_refresh field
- helpers/ui-joystick/joystick_ui_task.cpp Removed _next_batt_refresh(0) from ctor init list
- helpers/ui-joystick/joystick_screens.h MsgEntry::origin[80]→[32]; MAX_UNREAD_MSGS 32→16
- Kconfig DOOM help text now lists both UI activation paths
- ARCHITECTURE.md Same correction in §8.5
Stop doing UI work nobody asked for. The 5 s housekeeping tick was
reading env sensors (I2C, 10-50 ms), the battery ADC (regulator
toggle + 8 samples, every 60 s), and re-rendering the display
unconditionally — all while the display might be off and nothing
on-air had requested any of it.
Now:
- render_sensors() reads env sensors only when the user is on
that page (event-driven, never fires during idle)
- battery refresh is lazy on ui_pages_render() with a 30 s
freshness guard; explicit ui_set_battery() calls also count
- the unconditional OLED rerender from housekeeping is gone;
real state changes (messages, BLE, button press) still fire
schedule_render() directly
Telemetry / stats paths read fresh ADC + sensors on demand and
were never using the UI cache, so over-the-air consumers are
unaffected.
1. CONFIG_BT_DEVICE_NAME_GATT_WRITABLE=y removed. The default GAP
Device Name write permission is plain BT_GATT_PERM_WRITE — no
bonding required (Zephyr gap_svc.c:158). Any connected peer
(bonded or not) could rename the device. Worse, a GAP write
updates bt_get_name() but NOT prefs.node_name, so the advertised
name wouldn't track the renamed value. Rename now flows
exclusively through CMD_SET_ADVERT_NAME, which is NUS-protected
(AUTHEN required) and properly propagates via
zephcore_ble_update_name() to prefs + GATT + adv data.
2. CONFIG_BT_DIS_FW_REV_STR synced from "1.13.0" to "v1.15.1-zephyr"
to match CompanionMesh.cpp CMD_DEVICE_QUERY's version string.
Comment added requiring the two to stay in sync.
Three small correctness/polish improvements from BLE audit Phase 2F:
1. Five handlers (CMD_APP_START, CMD_GET_CHANNEL, CMD_SET_CHANNEL,
CMD_DEVICE_QUERY, CMD_SEND_CHANNEL_TXT_MSG) previously responded
with ERR_UNSUPPORTED on short-frame validation failure (because
they fell through to the dispatcher's default break, which the
caller converts to "unknown command"). They now explicitly
sendPacketError(ERR_ILLEGAL_ARG) — the semantically correct code
for "known cmd, bad frame".
2. CMD_SET_TUNING_PARAMS previously returned PACKET_OK on short
frames without applying any change. Now sends ERR_ILLEGAL_ARG so
the phone learns the change didn't take.
3. Added static_assert that CONFIG_ZEPHCORE_BOARD_NAME fits in 40
bytes including its null terminator, so a future too-long board
name fails at build time instead of producing an unterminated
wire-format response.
The wire format reserves a 32-byte name field; if the phone sends 32
non-null bytes, ContactInfo::name has no terminator. Subsequent
LOG_INF/LOG_DBG sites using %s with contact.name then read past the
field into adjacent struct bytes (type, flags, out_path_len, ...)
until the first null. No memory corruption — serializeContact uses
StrHelper::strzcpy which is length-bounded — but log output gets
garbage and a paired peer could probe a few bytes of the struct
through log capture.
Sibling handler CMD_SET_CHANNEL at :1593-1594 already does this
defensively. Match the pattern.
Two polish items from BLE audit Phase 2B:
1. CMD_SEND_TELEMETRY_REQ self-response buffer was uint8_t rsp[96]
with a comment claiming 70 B worst case. Actual worst case at
POWER_MAX_CHANNELS=4 is 82 B; if the channel cap ever grew the
buffer would silently overflow. Replaced with a sizeof-style
expression that tracks POWER_MAX_CHANNELS, plus an 8-byte safety
pad. No size change today (90 vs. 96) but the upper bound auto-
tracks any future bump.
2. CMD_GET_CUSTOM_VARS used `dp += snprintf(dp, 20, ...)` which
advances by the would-be-written length, not bytes actually
written. Currently safe only because gps_interval is capped
≤86400, but if either cap drifted or a new key was added the
length passed to writeFrame would include uninitialized stack
bytes between the truncation point and the (over-advanced) dp.
Now tracks rsp_end, computes remaining per snprintf, and only
advances dp on real progress.
Both are correctness polish, not exploitable today.
Both mesh::Packet::writePath and ::copyPath did a raw memcpy of the
decoded hash_count*hash_size bytes from src to dest with no bound on
src. Two call sites used phone-supplied or LoRa-anon-supplied buffers
where the path_len byte was attacker-controlled:
- CompanionMesh CMD_SEND_CHANNEL_DATA accepted len>=4 and called
writePath with no src bound; a paired phone could leak up to ~65
bytes of syswq stack into the outgoing LoRa channel-data frame.
- RepeaterMesh handleAnonRegionsReq / handleAnonOwnerReq /
handleAnonClockReq read reply_path_len from an unauthenticated
LoRa anon-request payload and called copyPath without any src
bound. Any LoRa neighbor could leak repeater stack into the
reply path.
Hardened the API: both functions now require an explicit src_len
and reject (return 0) when the decoded byte count exceeds it.
Updated all 14 call sites across Packet/Mesh/Dispatcher/BaseChatMesh/
CompanionMesh/RepeaterMesh. Trusted callers (internal MAX_PATH_SIZE
buffers) pass MAX_PATH_SIZE; untrusted callers pass real remaining
length. Added len-5 plumbing through the anon-handler signatures.
CMD_SEND_CHANNEL_DATA also gained a local len>=5 + path_bytes
sanity check for early rejection.
1. USB takeover opcode mismatch
ZephyrCompanionUSB.cpp checked payload[0] == 0x03 with a comment
claiming CMD_APP_START, but CMD_APP_START is 0x01 (0x03 is
CMD_SEND_CHANNEL_TXT_MSG). The USB handshake silently dropped the
companion app's first frame on every connection; the app appeared
broken over USB until the user happened to send a channel message.
2. CMD_SET_ADVERT_NAME didn't propagate to BLE adv data
Name changes were persisted to prefs but the advertising payload
and GATT device name kept the old value until reboot. Added
zephcore_ble_update_name() and called it from the handler.
3. No advertising-health watchdog
If bt_le_adv_start() ever failed transiently (HCI timeout,
controller pacing), the device would silently stop advertising
and stay undiscoverable until reboot. Added an adv_running flag
and a 5s watchdog in the companion housekeeping handler that
nudges adv back on if it stops outside a connection. Tracks
Arduino nrf52's equivalent 10s watchdog.
- calibrate_image: revert to datasheet band table. Narrow ±2 MHz
window had truncation bug placing 433/869 MHz operating freq
outside their own calibration windows. DS §9.2.1 confirms cal
is range-validity, not point-precision
- reset_agc: K_FOREVER mutex → K_MSEC(50). Was holding dispatcher
thread up to ~3 s if a long TX/RX held the SPI lock. Now bails
with WARN log; caller retries next maintenance interval.
every LBT-retried flood packet loses its priority (fixed)
witching between LBT and non-LBT mode (or any cad.mode change) could silently skip full reconfiguration and leave the radio in the wrong mode (fixed)
liquidraver