diff --git a/CHANGELOG.md b/CHANGELOG.md index 47b1536..eb5b3b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ The project follows Semantic Versioning once the first firmware release is tagge - vertical G alongside the lateral and longitudinal pair, for kerbs and compressions - gyroscope zero-rate offset measured at rest and removed, 4.4 dps on this board - device settings persisted in NVS, so Mode survives a reboot +- the gated menu opens on REVIEW, which is wanted the moment a session ends - top-level TRIGGER selection: MANUAL starts on the button, IMU on a launch, GPS at the line - pending phase on the running screen for a session armed and waiting for its trigger - overrun timer counting up in deep purple once a session reaches 00:00, with the lap diff --git a/firmware/components/ui/include/track_timer/ui/shell_navigation.hpp b/firmware/components/ui/include/track_timer/ui/shell_navigation.hpp index 0377f58..8694f7f 100644 --- a/firmware/components/ui/include/track_timer/ui/shell_navigation.hpp +++ b/firmware/components/ui/include/track_timer/ui/shell_navigation.hpp @@ -25,14 +25,20 @@ enum class ShellLevel : std::uint8_t { // Track selection sits at the top level, between Mode and Setup: at a circuit it is the // thing most often changed, and burying it under Setup made it the deepest common task. +// Order is position, and position is a statement about when each item is wanted: opening +// the menu lands on the first one with no swipe at all, and everything else costs at least +// one. +// +// Review comes first because it is wanted at the one moment the driver is definitely +// stopped and definitely reaching for the device - just after a session. Mode, Track and +// Trigger follow, in the order they are decided at the circuit before going out. +// Diagnostics is last because it is the least often wanted. enum class MenuItem : std::uint8_t { + review, mode, track, - // What starts a session. Sits with the track because both are set at the circuit, - // before going out, rather than buried under Setup with the rarely-touched options. trigger, setup, - review, diagnostics, }; inline constexpr std::size_t kMenuItemCount = 6; diff --git a/firmware/main/screen_router.cpp b/firmware/main/screen_router.cpp index 14fc6d9..1d4801b 100644 --- a/firmware/main/screen_router.cpp +++ b/firmware/main/screen_router.cpp @@ -148,16 +148,40 @@ constexpr std::uint32_t kRuby = 0xFF8FA3; } } +// Must stay in MenuItem order: the shell casts the carousel index straight to the enum. constexpr ui::CarouselEntry kMenuEntries[] = { + {LV_SYMBOL_LIST, "REVIEW", kAzure}, {LV_SYMBOL_POWER, "MODE", kRuby}, {LV_SYMBOL_GPS, "TRACK", kAzure}, {LV_SYMBOL_CHARGE, "TRIGGER", kGreen}, {LV_SYMBOL_SETTINGS, "SETUP", kAmber}, - {LV_SYMBOL_LIST, "REVIEW", kAzure}, {LV_SYMBOL_EYE_OPEN, "DIAGNOSTICS", kGreen}, }; +// The shell casts the carousel index straight to MenuItem, so the array's order is the +// menu's order. Checking the count alone would catch a missing entry but not a swapped +// pair, which is the easier mistake to make and the harder one to notice. +constexpr bool same_text(const char* left, const char* right) noexcept +{ + while (*left != '\0' && *left == *right) { + ++left; + ++right; + } + return *left == *right; +} + +constexpr bool menu_entry_is(const ui::MenuItem item, const char* const label) noexcept +{ + return same_text(kMenuEntries[static_cast(item)].label, label); +} + static_assert(std::size(kMenuEntries) == ui::kMenuItemCount, "the carousel must offer exactly the items the shell can select"); +static_assert(menu_entry_is(ui::MenuItem::review, "REVIEW")); +static_assert(menu_entry_is(ui::MenuItem::mode, "MODE")); +static_assert(menu_entry_is(ui::MenuItem::track, "TRACK")); +static_assert(menu_entry_is(ui::MenuItem::trigger, "TRIGGER")); +static_assert(menu_entry_is(ui::MenuItem::setup, "SETUP")); +static_assert(menu_entry_is(ui::MenuItem::diagnostics, "DIAGNOSTICS")); // Manual first: it is what the device did before a trigger existed, and what a driver // falls back to when a trigger cannot arm. diff --git a/tests/cpp/test_shell_navigation.cpp b/tests/cpp/test_shell_navigation.cpp index dd8bc63..6519045 100644 --- a/tests/cpp/test_shell_navigation.cpp +++ b/tests/cpp/test_shell_navigation.cpp @@ -32,7 +32,7 @@ void only_a_hold_opens_the_menu() const auto open = shell.dispatch(InputAction::long_press); assert(open.outcome == ShellOutcome::menu_opened); assert(open.state.level == ShellLevel::menu); - assert(shell.menu_item() == MenuItem::mode); + assert(shell.menu_item() == MenuItem::review); } // Hops to a menu item by name. Counting swipes meant that inserting an item silently @@ -48,14 +48,16 @@ void to_menu(ShellNavigation& shell, const MenuItem item) assert(false && "menu item not reachable"); } -// Track and Trigger sit between Mode and Setup: at a circuit both are set before going -// out, so neither should be buried under Setup with the rarely-touched options. -void the_menu_offers_mode_then_track_and_wraps() +// Opening the menu costs no swipe to reach the first item and at least one for everything +// else, so Review leads: it is wanted the moment a session ends, when the driver is +// stopped and already reaching for the device. Mode, Track and Trigger follow in the order +// they are decided at the circuit, and Diagnostics is last. +void the_menu_opens_on_review_and_wraps() { auto shell = opened(); - assert(shell.menu_item() == MenuItem::mode); - for (const auto expected : {MenuItem::track, MenuItem::trigger, MenuItem::setup, - MenuItem::review, MenuItem::diagnostics, MenuItem::mode}) { + assert(shell.menu_item() == MenuItem::review); + for (const auto expected : {MenuItem::mode, MenuItem::track, MenuItem::trigger, + MenuItem::setup, MenuItem::diagnostics, MenuItem::review}) { (void)shell.dispatch(InputAction::swipe_left); assert(shell.menu_item() == expected); } @@ -67,8 +69,7 @@ void the_menu_offers_mode_then_track_and_wraps() void the_track_list_is_sized_by_the_caller() { auto shell = opened(); - (void)shell.dispatch(InputAction::swipe_left); // TRACK - assert(shell.menu_item() == MenuItem::track); + to_menu(shell, MenuItem::track); auto result = shell.dispatch(InputAction::press); assert(result.outcome == ShellOutcome::entered); assert(result.state.level == ShellLevel::section); @@ -102,22 +103,35 @@ void a_shorter_track_list_resets_an_out_of_range_index() assert(shell.state().section_index == 0); } -// Choosing a mode is the entire interaction: enter Mode, swipe to it, press. -void a_mode_is_three_gestures_from_the_dashboard() +// Choosing a mode is still the whole interaction, and still shallow. Putting Review first +// costs Mode one gesture - it is no longer what a hold lands on - which is the trade the +// order makes: Review is wanted right after a session, Mode is set once and left. +void a_mode_is_a_handful_of_gestures_from_the_dashboard() { - auto shell = opened(); // 1: hold - auto result = shell.dispatch(InputAction::press); // 2: enter Mode + auto shell = opened(); // 1: hold + to_menu(shell, MenuItem::mode); // 2: swipe + auto result = shell.dispatch(InputAction::press); // 3: enter Mode assert(result.outcome == ShellOutcome::entered); assert(result.state.level == ShellLevel::section); - assert(!result.emits_action); // Mode has no destination screen + assert(!result.emits_action); // Mode has no destination screen - result = shell.dispatch(InputAction::swipe_left); // 3: to Race + result = shell.dispatch(InputAction::swipe_left); // 4: to Race assert(result.state.section_index == 1); - result = shell.dispatch(InputAction::press); // 4: select + result = shell.dispatch(InputAction::press); // 5: select assert(result.outcome == ShellOutcome::mode_selected); assert(result.state.section_index == 1); } +// The other half of that trade: Review is now a hold and a press, where it used to be a +// hold, four swipes and a press. +void review_is_two_gestures_from_the_dashboard() +{ + auto shell = opened(); + assert(shell.menu_item() == MenuItem::review); + const auto result = shell.dispatch(InputAction::press); + assert(result.emits_action && result.action == NavigationAction::open_review); +} + void review_and_diagnostics_hand_over_to_the_destination_model() { auto shell = opened(); @@ -128,7 +142,7 @@ void review_and_diagnostics_hand_over_to_the_destination_model() assert(result.emits_action && result.action == NavigationAction::open_review); assert(result.state.level == ShellLevel::menu); - (void)shell.dispatch(InputAction::swipe_left); + to_menu(shell, MenuItem::diagnostics); result = shell.dispatch(InputAction::press); assert(result.emits_action && result.action == NavigationAction::open_diagnostics); } @@ -201,6 +215,9 @@ void a_live_session_blocks_and_closes_the_menu() shell.synchronize_session(false); (void)shell.dispatch(InputAction::long_press); + // Review is a destination and stays at menu level, so descend into one that has + // children to prove a live session closes the menu from depth. + to_menu(shell, MenuItem::mode); (void)shell.dispatch(InputAction::press); assert(shell.state().level == ShellLevel::section); @@ -278,10 +295,11 @@ void an_out_of_range_section_falls_back_to_the_first() int main() { only_a_hold_opens_the_menu(); - the_menu_offers_mode_then_track_and_wraps(); + the_menu_opens_on_review_and_wraps(); the_track_list_is_sized_by_the_caller(); a_shorter_track_list_resets_an_out_of_range_index(); - a_mode_is_three_gestures_from_the_dashboard(); + a_mode_is_a_handful_of_gestures_from_the_dashboard(); + review_is_two_gestures_from_the_dashboard(); review_and_diagnostics_hand_over_to_the_destination_model(); a_setting_value_is_reachable_by_descending_three_levels(); every_level_climbs_back_out_one_at_a_time();