From 01d1c3c654ce35afe2885c458ea7b202c6e88c2d Mon Sep 17 00:00:00 2001 From: PurpleSentinel <199351535+PurpleSentinel@users.noreply.github.com> Date: Wed, 21 Oct 2026 11:07:40 +0100 Subject: [PATCH] fix(ui): make Race's running-screen rule testable and drop the delta it never drew Issue #219 and #221 are the same road test: a Race session started on pit exit, timed laps, and showed none of them. #221 was fixed in #224, which gave Race its own screen. Three of #219's acceptance criteria were still open. The routing was the important one. Track Day withholding live lap times is a compliance rule, and the screen it draws has no lap labels anywhere in it - so the mode comparison inside the router's screen lookup was the only thing enforcing that rule, and nothing asserted it. ui::running_screen_for states it once, defaults to the screen that cannot show a lap time, and the tests pin it for every mode plus the invariant that ties the two rules together: a mode is drawn on the Race screen exactly when it is permitted lap times. Grant laps to a mode routed to TrackdayScreen and they would never appear, which is the bug this issue reported. lap_delta is now false everywhere. Race was granted it when its running screen had no lap fields at all; the screen it has shows three figures and no delta, and the reference mapping a trustworthy delta needs is still #58. A permission for something nothing draws describes a display that does not exist. The Race screen's track name is written only when it changes. The router handed it over every display tick while the armed track cannot change during a session, and lv_label_set_text invalidates whether or not the text differs - a repaint of the widest label on the screen 25 times a second, beside three lap rows already change-gated for that reason (#213). Closes #219 Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 20 +++++++ firmware/components/ui/device_mode.cpp | 15 ++++- .../ui/include/track_timer/ui/device_mode.hpp | 22 +++++++ .../ui/include/track_timer/ui/race_screen.hpp | 8 +++ firmware/components/ui/race_screen.cpp | 7 ++- firmware/main/screen_router.cpp | 3 +- tests/cpp/test_device_mode.cpp | 57 +++++++++++++++++-- 7 files changed, 124 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13fc0b7..9ba3e2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -364,6 +364,26 @@ The project follows Semantic Versioning once the first firmware release is tagge ### Fixed +- the rule sending a running session to **Race's screen rather than Track Day's is now a + host-tested function** instead of a comparison inside the router's screen lookup. Track + Day withholding live lap times is a compliance rule, and the screen it is drawn on has + no lap labels anywhere in it - so that mode check was the only thing enforcing the rule, + and nothing asserted it. `ui::running_screen_for` states it once, the default is the + screen that *cannot* show a lap time, and the tests pin both halves for every mode plus + the invariant tying them together: a mode is drawn on the Race screen exactly when it is + permitted lap times (#219) +- **no mode claims a live lap delta any more.** Race was granted `lap_delta` back when its + running screen had no lap fields at all; the screen it has now shows three figures and + no delta, and the reference mapping a trustworthy one needs is still #58. A permission + for something nothing draws describes a display that does not exist, which is the same + mistake as a plausible reading - so the flag is false everywhere until a screen draws + one, and a test holds it there (#219) +- the Race screen's **track name is written only when it changes.** The router handed it + over on every display tick while the one thing it names - the armed track - cannot + change during a session, and `lv_label_set_text` invalidates whether or not the text + differs. That was a repaint of the widest label on the screen 25 times a second, next to + three lap rows that were already change-gated for exactly this reason (#213) + - `SettingsField` gained `acceleration_target` without `kSettingsFieldCount` or the name array following it, so naming that field read past the end of the array. Found while removing the two pit entries from the same list diff --git a/firmware/components/ui/device_mode.cpp b/firmware/components/ui/device_mode.cpp index f6e0e43..bd8cd30 100644 --- a/firmware/components/ui/device_mode.cpp +++ b/firmware/components/ui/device_mode.cpp @@ -9,7 +9,11 @@ ModeVisibility visibility_for(const DeviceMode mode) noexcept // Session countdown only. No lap time, no delta, on circuit. return {true, false, false, false, false}; case DeviceMode::race: - return {true, true, true, false, false}; + // Laps and the session clock, and no delta: the screen shows the lap in progress, + // the fastest lap and the time left, which is the design #219 asked for. The + // delta flag goes back to false until #58 gives a delta worth trusting and a + // screen draws it. + return {true, true, false, false, false}; case DeviceMode::g_only: // The G meter owns the whole panel; no timer, no laps. return {false, false, false, true, false}; @@ -42,6 +46,15 @@ ModeVisibility visibility_for(const DeviceMode mode) noexcept return {true, false, false, false, false}; } +RunningScreen running_screen_for(const DeviceMode mode) noexcept +{ + // Only Race has lap figures to draw. Everything else runs a session as a countdown, + // which is all TrackdayScreen has ever had on it, and the default is the withholding + // one on purpose: a mode added without a thought for this rule gets the screen that + // cannot show a lap time rather than the one that can. + return mode == DeviceMode::race ? RunningScreen::race : RunningScreen::trackday; +} + DeviceMode mode_from_settings(const settings::DeviceSettings& settings) noexcept { // The operating mode wins over the trackday flag, so the ambiguous combinations that diff --git a/firmware/components/ui/include/track_timer/ui/device_mode.hpp b/firmware/components/ui/include/track_timer/ui/device_mode.hpp index 9a25e41..78358c6 100644 --- a/firmware/components/ui/include/track_timer/ui/device_mode.hpp +++ b/firmware/components/ui/include/track_timer/ui/device_mode.hpp @@ -51,11 +51,33 @@ inline constexpr std::size_t kDeviceModeCount = 9; struct ModeVisibility { bool session_timer{true}; bool lap_times{false}; + // A live gap to the best lap. Granted by no mode yet, and deliberately so: the + // reference mapping and the validity rules a trustworthy delta needs are #58, and + // until a screen draws one, a mode claiming the permission describes a display that + // does not exist. Race asked for and was given it when its screen had no lap fields + // at all (#219); the screen it has now shows three figures and no delta. bool lap_delta{false}; bool g_meter{false}; bool gnss_data{false}; }; +// Which running-session screen a mode is drawn on. Two exist, and they differ in what +// they are able to show rather than in what they are configured to show: TrackdayScreen +// has no lap labels anywhere in it, so Track Day cannot leak a lap time through a mode +// check that is wrong, and RaceScreen is the one with the lap figures on it. +// +// A rule rather than a lookup inside the router, so the choice is host-testable: the +// compliance rule #132 established is only as good as the routing that enforces it. +enum class RunningScreen : std::uint8_t { + trackday, + race, +}; + +// Invariant, asserted in test_device_mode: a mode is drawn on RaceScreen exactly when it +// is permitted lap times. Granting laps to a mode routed to TrackdayScreen would show +// nothing; routing a mode without them to RaceScreen would show what it must not. +[[nodiscard]] RunningScreen running_screen_for(DeviceMode mode) noexcept; + // Track Day withholds live lap times deliberately. Many track days run under regulations // that prohibit timing on circuit, so this is a compliance rule, not a preference. Laps // are still recorded and remain available in Review once the session has ended. diff --git a/firmware/components/ui/include/track_timer/ui/race_screen.hpp b/firmware/components/ui/include/track_timer/ui/race_screen.hpp index f352086..7e66935 100644 --- a/firmware/components/ui/include/track_timer/ui/race_screen.hpp +++ b/firmware/components/ui/include/track_timer/ui/race_screen.hpp @@ -45,6 +45,9 @@ class RaceScreen { static constexpr std::size_t kLapCells = 9; // m m : s s static constexpr std::size_t kSessionCells = 5; + // Long enough for the catalogue's longest name; a longer one is compared and drawn + // truncated rather than rewritten every tick. + static constexpr std::size_t kTrackNameCapacity = 48; lv_obj_t* root_{nullptr}; lv_obj_t* track_label_{nullptr}; @@ -61,6 +64,11 @@ class RaceScreen { std::array shown_current_{}; std::array shown_fastest_{}; std::array shown_session_{}; + // The same gate for the title, which the router hands over on every display tick + // while the one thing it names - the armed track - cannot change during a session. + // lv_label_set_text invalidates whether or not the text differs, so writing it + // unconditionally was a repaint of the widest label on the screen 25 times a second. + std::array shown_track_{}; }; } // namespace track_timer::ui diff --git a/firmware/components/ui/race_screen.cpp b/firmware/components/ui/race_screen.cpp index 5cd9bff..d9623b8 100644 --- a/firmware/components/ui/race_screen.cpp +++ b/firmware/components/ui/race_screen.cpp @@ -4,6 +4,7 @@ #include "track_timer/ui/lvgl_visual_system.hpp" #include "track_timer/ui/timer_font.hpp" +#include #include namespace track_timer::ui { @@ -226,7 +227,11 @@ void RaceScreen::set_track_name(const char* const name) noexcept if (track_label_ == nullptr || name == nullptr) { return; } - lv_label_set_text(track_label_, name); + if (std::strncmp(shown_track_.data(), name, shown_track_.size() - 1) == 0) { + return; + } + std::snprintf(shown_track_.data(), shown_track_.size(), "%s", name); + lv_label_set_text(track_label_, shown_track_.data()); } lv_obj_t* RaceScreen::root() const noexcept { return root_; } diff --git a/firmware/main/screen_router.cpp b/firmware/main/screen_router.cpp index 8941636..450d68e 100644 --- a/firmware/main/screen_router.cpp +++ b/firmware/main/screen_router.cpp @@ -1669,7 +1669,8 @@ class ScreenRouter { // times and Track Day must not; every other mode keeps the countdown. [[nodiscard]] lv_obj_t* running_screen() noexcept { - if (ui::mode_from_settings(settings_) == ui::DeviceMode::race && + if (ui::running_screen_for(ui::mode_from_settings(settings_)) == + ui::RunningScreen::race && race_root_ != nullptr) { return race_root_; } diff --git a/tests/cpp/test_device_mode.cpp b/tests/cpp/test_device_mode.cpp index cc7d45c..4368d98 100644 --- a/tests/cpp/test_device_mode.cpp +++ b/tests/cpp/test_device_mode.cpp @@ -176,15 +176,59 @@ void track_day_shows_the_timer_only() assert(!visibility.g_meter); } -void race_shows_laps_delta_and_remaining() +// Three figures and no fourth: the lap in progress, the fastest lap and the time left. +// The delta flag was granted when Race's screen had no lap fields at all, and is now +// false on purpose - a permission for something nothing draws describes a display that +// does not exist, and a delta worth trusting is #58 (#219). +void race_shows_laps_and_remaining_but_no_delta() { const auto visibility = ui::visibility_for(ui::DeviceMode::race); assert(visibility.session_timer); assert(visibility.lap_times); - assert(visibility.lap_delta); + assert(!visibility.lap_delta); assert(!visibility.g_meter); } +// No mode may claim a delta while no screen draws one. +void no_mode_claims_a_delta_nothing_draws() +{ + for (std::size_t index = 0; index < ui::kDeviceModeCount; ++index) { + assert(!ui::visibility_for(static_cast(index)).lap_delta); + } +} + +// The routing half of the compliance rule. Track Day's screen has no lap labels in it, +// so the mode check is the only thing standing between a Track Day session and a lap +// time on the panel - and that check had never been asserted anywhere (#219). +void a_running_session_goes_to_the_screen_its_mode_allows() +{ + assert(ui::running_screen_for(ui::DeviceMode::race) == ui::RunningScreen::race); + assert(ui::running_screen_for(ui::DeviceMode::track_day) == + ui::RunningScreen::trackday); + + // Stated for every mode rather than the two that matter, because the default is what + // a mode added without a thought for this rule inherits, and it must be the screen + // that cannot show a lap time. + for (std::size_t index = 0; index < ui::kDeviceModeCount; ++index) { + const auto mode = static_cast(index); + const auto expected = mode == ui::DeviceMode::race ? ui::RunningScreen::race + : ui::RunningScreen::trackday; + assert(ui::running_screen_for(mode) == expected); + } +} + +// The two rules have to agree. Lap times granted to a mode drawn on TrackdayScreen would +// simply never appear, which is the bug #219 reported; a mode without them routed to +// RaceScreen would show exactly what it is forbidden to. +void the_screen_a_mode_gets_matches_the_laps_it_is_allowed() +{ + for (std::size_t index = 0; index < ui::kDeviceModeCount; ++index) { + const auto mode = static_cast(index); + const auto on_race_screen = ui::running_screen_for(mode) == ui::RunningScreen::race; + assert(on_race_screen == ui::visibility_for(mode).lap_times); + } +} + void g_only_shows_nothing_but_the_meter() { const auto visibility = ui::visibility_for(ui::DeviceMode::g_only); @@ -290,7 +334,10 @@ int main() navigation_shows_three_readouts_and_no_timer(); every_mode_has_a_name_a_label_and_a_summary(); track_day_shows_the_timer_only(); - race_shows_laps_delta_and_remaining(); + race_shows_laps_and_remaining_but_no_delta(); + no_mode_claims_a_delta_nothing_draws(); + a_running_session_goes_to_the_screen_its_mode_allows(); + the_screen_a_mode_gets_matches_the_laps_it_is_allowed(); g_only_shows_nothing_but_the_meter(); every_field_offers_its_values_directly(); any_average_lap_is_reachable_on_the_roller(); @@ -299,7 +346,7 @@ int main() turning_off_average_lap_resets_the_dependent_field(); mode_owned_settings_are_not_offered_as_fields(); - std::cout << "Mode derivation, per-mode visibility rules, and one-press direct value " - "selection passed\n"; + std::cout << "Mode derivation, per-mode visibility rules, running-screen routing, " + "and one-press direct value selection passed\n"; return 0; }