Add Back/Cancel rows to the four menus that had no way out - #145
Open
TheAngryRaven wants to merge 1 commit into
Open
Add Back/Cancel rows to the four menus that had no way out#145TheAngryRaven wants to merge 1 commit into
TheAngryRaven wants to merge 1 commit into
Conversation
Four menu pages offered only forward choices: the transfer menu (Bluetooth / USB), the replay session browser, and the course creator's track prompt and type picker. None of them is the main menu, and the idle-shutdown timer only runs there (and on the fault page), so none of them timed out either — opening one by mistake left the unlabelled Select + side-button 5 s reboot combo as the only escape. - Transfer menu gains a Back row. It also joins the reverseDirection group in displayLoop(): it renders top-to-bottom like the other static menus and always belonged there, but with two items the scroll direction was unobservable (either button wrapped to the other row). The third row makes it visible. - The replay browser gains a Back row after the last session, scrolling into view like any other row. replayItemCount() in replay.h is the single source of that layout for the renderer, the menu limit and the select handler, so the row cannot be drawn in one place and unreachable in another. The three near-identical render blocks collapse into one replayDrawEntry() helper. - The course creator's two entry screens each gain a Cancel, mapped to the Row::kCancel the model already had (select() returns kExit for it, so no new plumbing). The type picker's matters most: entering with no known track nearby skips the prompt and lands there, making it the first screen those users see. Also: cancelling manual camera-serial entry now returns to the camera page, where OK already landed, instead of dropping to the main menu. The course-type page drops the blank line under its title so three size-2 rows plus the hint line fit the panel, and the hint is blank on the Cancel row rather than describing a type the cursor is not on. Tests: two new course_creator cases cover the entry-screen Cancel rows and their out-of-range clamp. Sim goldens regenerated — transfer_menu and course_type_select are the only two fixtures whose hash moved, and both frames were eyeballed. Host suite 435/435, sim ctest 6/6, clang-tidy clean on course_creator.cpp (which also clears a pre-existing bugprone-branch-clone finding by merging the two identical rowCount branches). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BY4rxBEPYL9Zf3n7PCHovM
Coverage — host-testable units📂 Overall coverage
📄 File coverage
|
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
2 participants
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.
Summary
Four menu pages offered only forward choices, with no Back or Cancel row:
PAGE_TRANSFER_MENUPAGE_REPLAY_FILE_SELECTPAGE_COURSE_TRACKPAGE_COURSE_TYPENone of these is the main menu, and the idle-shutdown timer only runs on the main menu (and the fault page), so none of them timed out either. Opening one by mistake left the Select + side-button 5 s reboot combo — which is not labelled on any screen — as the only way out.
What changed:
reverseDirectiongroup indisplayLoop(): it renders top-to-bottom like the other static menus and always belonged there, but with two items the scroll direction was unobservable (either button wrapped to the other row). The third row makes it visible.replayItemCount()inreplay.his the single source of that layout for the renderer, the menu limit and the select handler, so the row can't be drawn in one place and unreachable in another. The three near-identical render blocks collapse into onereplayDrawEntry()helper.Row::kCancelthe model already had (select()returnskExitfor it, so no new plumbing). The type picker's matters most: entering the creator with no known track nearby skips the prompt and lands there, making it the first screen those users see.Layout note: the course-type page drops the blank line under its title so three size-2 rows plus the hint line fit the panel, and the hint is blank on the Cancel row rather than describing a course type the cursor isn't on.
Type of change
How it was verified
course_creatorcases covering the entry-screen Cancel rows and their out-of-range clampclang-tidyclean oncourse_creator.cpp. This also clears a pre-existingbugprone-branch-clonefinding by merging the two identicalrowCountbranches (the file isn't in CI's tidy list, so it was never surfaced)ctest6/6 (boot soak, determinism, goldens, both lap oracles, two-session carryover). The sim compiles the real.inosources, so the display changes are compile-checkedGolden fixtures regenerated:
transfer_menuandcourse_type_selectare the only two hashes that moved, which is exactly the two pages re-rendered. Both frames were eyeballed as PNGs — three rows plus the hint line fit the panel with nothing clipped.Not covered by goldens: the track-prompt page and the replay browser. The golden walk enters the creator with no track in range (so it never renders the prompt), and the sim's VFS has no
.dovexassets (so Review lands on the existingwarning_no_dovexfixture). Adding coverage would mean embedding a DOVEX asset and a parked-on-track creator walk — worth doing, but a separate change.Checklist
CHANGELOG.mdupdated under[Unreleased]CLAUDE.mdupdated — page list, the USB subsystem's UI flow, and the course-creator screen description; adds an explicit "every menu page carries a Back/Cancel row" rule with the three deliberate exceptions (PAGE_INTERNAL_FAULT,PAGE_SD_FORMAT, the speed-gated race rotation)tests/Generated by Claude Code