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; }