Repository navigation
feat(ui): open the gated menu on REVIEW - #161
Merged
Merged
Conversation
Opening the menu lands on the first item with no swipe at all, and everything else costs at least one, so the order is a statement about when each item is wanted. Review was last but one, four swipes from a hold - the furthest thing in the menu from the gesture that opens it - despite being wanted at the one moment the driver is definitely stopped and definitely reaching for the device. Mode, which is set once and rarely touched again, was free. The order now reads Review, Mode, Track, Trigger, Setup, Diagnostics: Review first, the three things decided at the circuit next in the order they are decided, and Diagnostics last because it is least often wanted. The trade is explicit rather than hidden. Review goes from a hold, four swipes and a press to a hold and a press; Mode gains one gesture, because it is no longer what a hold lands on. Both costs are asserted in tests so a later reorder has to face them. MenuItem is never persisted or serialised - menu_item() casts the carousel index straight to it - so nothing outside the UI depends on the values. The correspondence between the enum and the carousel array is positional and was maintained by hand, with a static_assert that only checked the count. It would have caught a missing entry but not a swapped pair, which is the easier mistake to make while reordering and the harder one to notice. Each entry's label is now checked against its enumerator at compile time. Closes #160 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Closes #160.
Why first position is the one that counts
Opening the menu sets
menu_index = 0, so the first item is what a hold lands on with noswipe at all. Everything else costs at least one. The order is therefore a statement about
when each item is wanted.
Review was second from last — four swipes from a hold, the furthest thing in the menu from the
gesture that opens it — despite being wanted at the one moment the driver is definitely stopped
and already reaching for the device. Mode, set once and rarely touched again, was free.
The rest keeps its logic: Mode, Track and Trigger are what get decided at the circuit before
going out, in that order, and Diagnostics stays last as the least often wanted.
The trade, stated rather than hidden
Mode gains a gesture, because it is no longer what a hold lands on. Both costs are asserted in
tests, so a later reorder has to face them rather than discover them.
Safe to reorder
MenuItemis never persisted or serialised —menu_item()casts the carousel index straightto it — so nothing outside the UI depends on the values.
A weakness found while moving them
The enum and
kMenuEntriescorrespond positionally, maintained by hand. Thestatic_assertadded with TRIGGER only checked the count: it would catch a missing entry butnot a swapped pair, which is the easier mistake to make while reordering and the harder one to
notice. Each entry's label is now checked against its enumerator at compile time via a
constexprstring compare, so the two cannot drift apart silently.A pattern in the test updates
Six tests broke, and every one of them navigated the menu by counting swipes. They were not
testing what they appeared to:
a_mode_is_three_gestures_from_the_dashboardpressed once fromthe menu and assumed it landed on Mode. Each is now name-based through the
to_menuhelper, soinserting or moving a menu item cannot silently point a test at a different one.
Verification
36 host suites, 70 simulator tests, and flashed to the panel: boots clean, no watchdog.