feat(settings): hub-and-spoke reorg + fix(send): off-lock persistence + fix(propagation): endpoint hot-path read gate - #96
Merged
Conversation
… by use Settings held live status (GPS fix, storage/RAM/identity) that belongs on the Status screen, plus a per-second tick() doing SPI flash stat reads and label churn mid-scroll — the main cause of laggy scrolling. - GPS section (sats/location/altitude/HDOP/time) -> StatusScreen - System Info (firmware build, storage, RAM) -> StatusScreen, with storage/RAM stat reads throttled to ~5s and stack-buffer snprintfs instead of Arduino String concatenation - Settings gains a Status link row (trackball-reachable) that opens Route::STATUS; the per-second SettingsScreen tick/refresh is deleted - Reordered sections by frequency of use: General (name/brightness/ timeout/kb-light), Notifications, Network (now includes the TCP/Auto/BLE interface switches), Radio (LoRa + params), Delivery, Advanced, DANGER: Transport Mode (still final) - Identity/LXMF hashes shown in Settings were truncated duplicates of the Status screen's full display; removed - main.cpp publishes firmware build + GPS to the Status screen - Contract test for the storage readout follows the code to StatusScreen
LVGL re-focuses the default input group when the screen is shown; the focused member is the last focusable object (the transport switch), so the view landed at the bottom of the list. A one-shot 50ms LVGL timer after show() scrolls the content back to the top without touching the focus state, and leaves subsequent user scrolling alone.
The one-shot LVGL timer's callback deleted the timer but left _entry_scroll_reset_timer dangling. On the 2nd+ screen entry show() took the 'else' path and called lv_timer_reset() on freed memory, so the view was never reset to the top — it stayed wherever the group re-focus had auto-scrolled it (the bottom). Null the pointer before lv_timer_del so each entry recreates a fresh timer.
…t-entry movement Temporary [SCROLLDIAG] build: show() logs _content scroll_y before and after the explicit reset, and a 30ms LVGL timer logs the first deviation from 0 after entry. Captures the actual mechanism behind the bottom-open instead of guessing. Remove before PR.
…onfirmation The explicit scroll-to-top in show() was confirmed working on the device (open-at-top on every entry, including repeat entries after scrolling to the bottom). Remove the temporary Serial logging and the 30ms deviation watcher.
Replace the flat Settings screen with a card hub + dedicated sub-views. - Hub: 8 navigation cards (Status, Network, Identity, Radio, Delivery, Appearance, Advanced, Transport) reusing the Network-screen card widget; Transport stays last (danger invariant preserved). - Tap a card -> dedicated sub-view holding that area's controls; no accordion. - Network sub-view gains a LoRa interface toggle alongside TCP/Auto/BLE; it two-way-mirrors the Radio page's canonical lora_enabled switch. - Save model: simple controls apply immediately; a Save button appears only on the form sub-views (Network, Radio, Identity). - Identity sub-view adds a View Identity row routing to the existing lxma:// QR screen (the Status Share button already reached it). - Focus group rebuilt per view (only-visible objects) so the auto-scroll-to- bottom class of bug cannot recur; each sub-view entry scrolls to top. Contracts updated for the new structure; 170/170 build-script tests pass. tdeck build green: RAM 23.1% (75,848 B, unchanged), Flash 92.7% (2,914,713 B, +2,640 B). Not flashed, not PR'd.
Boot-loop regression from the hub-and-spoke rewrite: lv_label_set_text() on a plain lv_obj object wrote label internals into a non-label and freed a bogus pointer (heap_caps_free assert 'outside heap areas') during UIManager::init -> SettingsScreen::create_advanced_view, crashing every boot at 'Initializing UIManager'. Verified by decoding the on-device backtrace (ELF 62cf1d0e...) against the exact flashed firmware.
Pre-building all seven settings sub-views at construction created ~100 LVGL objects that (being <256B) allocated into internal DRAM via the hybrid allocator. On the T-Deck that fragmented internal heap to an 8180B largest block at LVGL task-creation time, and the 8KB task (needs ~8.5KB with TCB) failed: xTaskCreatePinnedToCore returned errQUEUE_FULL, the device parked on the boot splash, and confirm_running_firmware never ran. Root cause proven with an on-device heap probe (largest internal block 8180B, 8192-stack task FAIL, 4096-stack OK), then fixed by building each sub-view on first navigation. Boot-time internal DRAM rises to a 47KB largest block and the task starts cleanly. Signed-off-by: Torlando <tyler@torlando.tech>
'TX Power:' in Montserrat 14 is ~78px wide but the slider track started at x=75, tucking the colon under the track. Start the track at x=86 (8px clear; still 51px from the right-aligned value label). Signed-off-by: Torlando <tyler@torlando.tech>
…n clarity - Move the LoRa TX Power slider from x=75 to x=86: the 'TX Power:' label in Montserrat 14 is ~78px wide, so the slider track started on top of the colon. - Network sub-view: the Reticulum interface toggles (TCP/AUTO/LoRa/BLE) are now row 1, relabeled 'Reticulum Interfaces:', with WiFi SSID, WiFi password, TCP host and port following in their original order. - Rename the ambiguous 'Reconnect' button to 'WiFi Reconnect' — it re-applies the WiFi SSID/password fields to the running stack and has nothing to do with the TCP host. Sized 110px to fit the label. Signed-off-by: Torlando <tyler@torlando.tech>
Contributor
Greptile SummaryThis PR reorganizes Settings into a lazy hub-and-spoke interface, moves slow message persistence and conversation reads outside the LVGL lock, updates the patched Reticulum dependencies, and expands build contracts and diagnostics.
Confidence Score: 5/5The PR appears safe to merge; no new actionable failures were found in the changes since the previous review. The only post-review production change replaces conversation-row CLICKED handling with SHORT_CLICKED to prevent navigation after long-press deletion, while retaining ordinary touch and keypad activation. The six earlier findings are reflected as fixed in the current code and their threads were manually resolved without explanatory replies. Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as LVGL task
participant MB as Send mailbox
participant Main as Main loop
participant Store as Message store
participant Router as LXMF router
UI->>MB: Publish destination and exact composer text
Main->>MB: Take pending request
Main->>Store: Persist outgoing message off LVGL lock
Main->>Router: Admit message under router lock
Main->>UI: Commit result under brief LVGL lock
alt Composer still matches submitted text
UI->>UI: Clear composer
else User edited composer
UI->>UI: Preserve new draft
end
Reviews (7): Last reviewed commit: "fix(ui): conversation-row selection on S..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Address the three P1 findings from the Greptile review of PR #96: 1. Network LoRa toggle reverted on save. on_interface_switch_changed saved without propagating the Network switch state, so the save path read the stale Radio-page twin and wrote it back over the mirror. The handler now mirrors the flipped switch to its twin (and toggles the LoRa params visibility) before saving, mirroring on_lora_enabled_changed in the other direction. 2. Immediate controls committed unsaved drafts from other sub-views. Every immediate handler ran the full-screen update_settings_from_ui, so typing a TCP host and then adjusting brightness silently persisted the host. update_settings_from_ui is now view-scoped: save_settings reads only the sub-view that just changed (LVGL events only fire for visible widgets, so _view is always the touched view). The lora handle merges as Radio-page switch wins, Network mirror when the Radio page is unbuilt. 3. Hidden controls retained focus. hide() removed only the header buttons and hub cards, and the focus_group_for cleanup list was missing the three Radio dropdowns — so hidden sub-view controls stayed in the default group (and Radio dropdowns lingered after leaving the Radio view). Extracted the full removal list into remove_all_from_focus_group (now including _dropdown_lora_bandwidth/ _sf/_cr) and use it from both focus_group_for and hide. Verification: tdeck build SUCCESS (RAM 23.1%, Flash 92.7%), tests/build_scripts 170/170. All save_settings() call sites confirmed event-driven so _view always matches the touched control. Signed-off-by: Torlando <tyler@torlando.tech>
Every message send on the device was deterministically rebooting it: send_message() ran the full pipeline (identity recall, message construction, RouterLock-scoped router admission, and LittleFS persistence) synchronously on LVGL's 8 KiB task while holding the LVGL mutex. On this device's degraded filesystem a single save takes ~7s of 400ms-2s per-op gaps, tripping the 5s LVGL deadlock guard and asserting at LVGLLock.h:45 (assert failed: LVGL mutex timeout (5s)). The receive path already carries the fix pattern for exactly this failure class (see on_message_received); the send path never got it. Restructure the send path as a mailbox handoff, following the existing CallStartMailbox / LocationShareCommandMailbox precedent: - send_message() (LVGL task) now only validates and publishes (destination, content, source) into a mutex-guarded single-slot OutgoingSendMailbox. No router lock, no I/O, no message construction. - update() services the mailbox in service_pending_sends() on the main loop, before the big LVGL_LOCK() — the only place in the send path that may take the router lock, block on admission, or wait on LittleFS. - On acceptance, a brief LVGL_LOCK in apply_outbound_result() commits the UI (add_message / clear_composer / compose->chat navigation, route-guarded). The admitted packed form is unpacked for display with incoming/state flags restored. - On rejection (storage error, router busy, queue full) the user's input is retained for retry, matching the old behavior. The 500-char UI cap bounds the mailbox payload. Build tdeck SUCCESS, 170/170 contract tests pass.
Opening a conversation crashed the same way sending did. load_conversation() (LVGL task, under the LVGL lock held by replace_route) ran the full open pipeline synchronously: identity recall (ustore), display-name read, the message-index read, and the per-message metadata reads. On this device's degraded LittleFS each op is 0.4-2s, so a cold open of a dozen-message conversation held the LVGL mutex past the 5s deadlock guard and asserted at LVGLLock.h:45. The send path already got the mailbox fix; the open path never did. Restructure with the same pattern: - load_conversation() (LVGL task) now only navigates + resets the list and shows the truncated hash in the header. Same-peer re-opens return early with zero store I/O (rows are still built). - prepare_conversation() (main loop, called from update()) does the store I/O between a short guard lock and a short commit lock, then commits the header name + initial bubbles + background-fill arming under a brief LVGL_LOCK. A generation counter discards a stale in-flight prepare when the conversation changes mid-I/O. - refresh() re-arms the prepare instead of re-reading under the lock. The 1Hz store 'not found in index' fetch is pre-existing (present on 2527c6d) and is being tracked separately as a flash-wear follow-up. Build tdeck SUCCESS, 170/170 contract tests pass.
load_more_messages() (background fill, main loop) ran each batch's metadata reads while holding the LVGL lock. A cold batch is 2+ LittleFS reads; on the degraded device a fill batch measured ~4s in earlier captures, within a hair of the 5s deadlock guard. The fill starts immediately after any conversation open (first page > INITIAL_RENDER), so an open triggered a second under-lock stall right after the first was fixed. Same restructure: guard lock copies the batch's hashes, metadata I/O runs off-lock, the bubble prepend commits under a brief lock. A _fill_generation counter (bumped on every list rebuild: open, prepare commit, refresh) discards a batch whose conversation changed mid-I/O. Display order is preserved (newest-first read order + push_front gives oldest-to-newest, matching the old loop).
[SENDT] marks: queue_wait (mailbox wait), identity_recall, router_lock, save_message (isolated), admission_done, ui_commit_done. Instrumentation only; to be removed before merge. Loosens one contract pin to the save call expression (invariant preserved: save runs in persistOutgoingMessage, not service_pending_sends).
…ump) Bumps microLXMF d7e05fd -> 82d2e54 (fork branch fix/sync-path-wait-backoff): LXMRouter::process_sync() PR_PATH_REQUESTED now re-polls Transport::has_path() at most once per PATH_REQUEST_WAIT instead of every main-loop iteration. In the CBA microStore-backed microReticulum fork has_path() is a FileStore exists() = a LittleFS read, so the old code issued a path-table read every ~2s for the whole 60s sync window while the propagation node was unreachable. Measured on the T-Deck: one destination fetched 293x in 600s (~every 2s), all on the same 2MB partition as message storage, with [DISP] flush stalls interleaved. Worst-case sync cycle cost drops from ~30 reads to ~3-4. The periodic message sync was already 4h default with a user setting (Settings 'Prop Sync Interval (hrs)', sync_int=14400) — unchanged. Adds tests/build_scripts/test_propagation_sync_poll_contract.py: pins the re-poll gate ordering, the PATH_REQUEST_WAIT window, the untouched one-shot check in request_messages_from_propagation_node, and enforces platformio.ini <-> audit-tool pin agreement.
TEMPORARY DIAGNOSTIC PIN. ef07187 = cd0338e + per-call-site _new_path_table.get() counters ([PG] summary every 30s on serial). Purpose: identify which Transport call site drives the ~1.5s full FileStore get() on the offline propagation node (6b9f6601...). Revert this commit (back to cd0338e) after the capture.
…ulum bump) Bumps microReticulum pin to e2c9d4d (diag/path-get-caller on cd0338e): Transport::inbound() and path_request() no longer perform a full microStore get() (flash write + read) for every inbound packet / path request on endpoint-only nodes. The read is gated on the exact conditions where destination_entry is consumed, and the local-destination path-request answer uses the in-memory _destinations table, so the device stays discoverable. The build still carries the temporary [PG] counters for the live before/after capture; counters are stripped in a follow-up before anything merges.
[SENDT] send-pipeline timing and the microReticulum [PG] path-store call-site counters now compile to no-ops unless explicitly enabled: - DPYXIS_SEND_DIAG / -DRNS_PATHGET_DIAG added to env:tdeck base flags - both removed by env:tdeck-release build_unflags (no-op in release) So production tdeck/tdeck-release builds are merge-clean, while the instrumented build stays one env/flag away for the path-request spammer hunt. Bumps microReticulum pin to 921b3aa (same endpoint hot-path read gate, counters gated behind RNS_PATHGET_DIAG). Contract suite: 176/176.
torlando-tech
changed the title
The pyxis-pytest job checks out the microReticulum reference for the NomadNet x86 client, whose CMake manifest attests the exact commit (921b3aa, endpoint hot-path read gate). The workflow still pointed at cd0338e, so the client configure aborted on the attestation check and the pytest job failed before running tests. The native17 job (submodule) is unaffected and already green.
…y (Greptile P1s) Greptile P1 remediation on the exact head (round: 1a34c55): 1. Send Completion Erases Draft (UIManager.cpp:1780). The async send deferral (1c68860) leaves the composer un-cleared between the send click and the main-loop's ADDED commit, so input typed into the composer while persistence/admission is in flight was wiped by the unconditional clear_composer(). ChatScreen now captures the exact submitted text when the send is accepted into the mailbox, and clear_composer() only clears when the composer still holds that text. A rejected send still retains input (unchanged), and a fresh draft can no longer be erased by a late completion. 2. Same-Peer History Stays Stale (ChatScreen.cpp:177). The same-peer early-return (ce92e80) skipped the store re-read, so a message for this peer that persisted while the chat was hidden ( on_message_received only appends to the visible chat) never surfaced on re-open. The early-return now compares the store's in-memory conversation count (get_messages_for_conversation — pure slot lookup, no LittleFS, safe under the LVGL lock) against the count at the last prepare commit and falls through to the peer-change path on a mismatch, which resets the list and re-arms prepare so the main loop re-gathers off-lock and rebuilds with the new message. Verification: 181/181 build-script contracts (5 new pins), tdeck + tdeck-release green. Compose path audited and unaffected: the single send slot makes a second send a no-op until the first commits, and its clear rides on the route replacement (render_route).
… race) Greptile round on ba5af76 (4/5) correctly rejected the first attempt: the submitted-text marker was assigned in ChatScreen::on_send_clicked AFTER the mailbox publish returned, so the main loop could take() + admit the send and enter clear_composer() while the marker was still empty — neither clearing the submitted text nor associating the commit with its submission. The marker is now recorded by the send callback itself (UIManager::on_send_message_from_chat) immediately after the mailbox accept, in the same LVGL lock section as the publish. The click handler runs on the LVGL task with the LVGL mutex held (LVGLInit.cpp:160-179 wraps the whole lv_task_handler in the recursive mutex), so the marker is visible to the main loop only after the mailbox entry is — the take() + admit + clear sequence can never observe an empty marker for an accepted send. clear_composer() additionally no-ops on an empty marker, which is the retained-text path for rejected/retry sends. The contract test is tightened to assert the marker is NOT assigned in the click handler and IS assigned in the callback, so the race cannot silently regress. Verification: 181/181 contracts, tdeck + tdeck-release green.
Long-press to delete a conversation, then release: the row fired on_conversation_long_pressed (confirm dialog) AND, on release, on_conversation_clicked, navigating into the conversation and hiding the dialog. Root cause: LVGL 8.4 lv_indev.c indev_proc_release() (lines 973-980) sends LV_EVENT_CLICKED on every pointer release without scrolling, including long-press release — only LV_EVENT_SHORT_CLICKED is gated on long_pr_sent == 0. The row had both CLICKED and LONG_PRESSED bound. Selection is now bound to LV_EVENT_SHORT_CLICKED. Trackball selection is unaffected: the keypad path sends SHORT_CLICKED + CLICKED on a plain enter release and suppresses both when long_pr_sent is set (lv_indev.c:526-531, 678-692). No programmatic CLICKED sends exist in the app. This row is the only object with the CLICKED + LONG_PRESSED double-bind (bubble/textarea long-press sites are single-bound, verified). Contract pins: selection must bind SHORT_CLICKED, no CLICKED binding on the row, long-press delete binding unchanged. 184/184 contracts, tdeck + tdeck-release green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scope (grew beyond settings during this branch's lifetime)
1. Settings hub-and-spoke reorg (original scope — physically accepted)
lv_obj_scroll_to_y(0)inshow(); no timer workarounds).2. Send-persistence threading (crash class — physically proven)
Three under-LVGL-lock LittleFS sites moved off the LVGL task:
fix(lxmf): move outgoing-send persistence off the LVGL task(1c68860)fix(lxmf): move conversation-open store I/O off the LVGL task(ce92e80)fix(lxmf): move background-fill metadata reads off the LVGL lock(7bd6e67)3. Propagation sync polling (microLXMF)
fix(propagation): stop 2s path-table polling during sync— bounded re-poll ofPR_PATH_REQUESTED(microLXMF pin82d2e54).4. Endpoint hot-path path-table read gate (microReticulum) — the big one
Root cause (proven on-device with per-call-site counters):
Transport::inbound()andpath_request()in the CBA microStore fork performed a full_new_path_table.get()—flush_buffer()flash write + read — for every non-announce radio packet, before thetransport_enabled()gate. A mesh-wide path-request storm (stuck node retransmitting to offline propagation node6b9f6601…, ~1.4s cadence, 392 relaying peers) produced 792 flash transactions / 600s, saturating the main loop (300s+ display stalls) and making sends feel slow.Fix (microReticulum pin
921b3aa, branchdiag/path-get-caller): when transport is disabled and no local client interfaces are attached (the Pyxis configuration), the flashget()is skipped entirely in both sites. The in-memory_destinations.find()local-destination answer is preserved, so the device still answers path requests for its own destinations (inbound discovery per the reference implementation,alephRNS/Transport.py:3188-3192). Transport-enabled devices are unaffected.On-device before/after (same storm, 600s idle):
get()readsReference-comparison evidence: the Python RNS keeps the path table in-memory (zero per-packet I/O) and carries Atterman's own TODO to short-circuit path requests on non-transport nodes. Upstream
attermann/microReticulumv0.5.0 (40fa628) still has the unguarded read — this is a new fix, not a backport.5. Diagnostics gated behind build flags
[SENDT](send-pipeline timing) and[PG](per-call-site path-store counters) compile to no-ops unlessPYXIS_SEND_DIAG/RNS_PATHGET_DIAGare defined. Both are set inenv:tdeck(dev/physical-test) and removed byenv:tdeck-releasebuild_unflags— production artifacts are instrument-free (verified by building both envs; release contract suite green). New contracttest_endpoint_hotpath_read_gate_contract.pypins the gate logic against the fork source.Verification at head 1a34c55
176/176 build-script contract tests (host)
tdeck+tdeck-releasebuilds greenDevice: app0 flash with read-back + 6-partition preservation, boot clean (9 conversations, System Ready, 0 panics), gate counters live-verified
Known pre-existing (not introduced here):
[DISP] STALLED/flush_count=1health-monitor artifact (present in all pre-fix captures); upstream rebase of the fork (24 commits behind, overlaps patch region) is a scheduled follow-upCI workflow microReticulum reference checkout advanced to the same attested pin (1a34c55) so the NomadNet x86 client attestation passes