From c91bb8adca4e42a52bdacd6f20f6d503bfd487f1 Mon Sep 17 00:00:00 2001 From: mrsqr Date: Thu, 20 Aug 2026 17:01:48 +0100 Subject: [PATCH 1/2] fix(ui): refresh the start page when a setting changes Rest was changed to ten seconds on the device. The rest timer duly ran for ten seconds and the start page went on reading 20 MIN REST until a restart, so the device displayed one thing and did another, on the value a driver is most likely to check before going out. refresh_ready() is the only thing that rebuilds the dashboard, and it was called from exactly two places: once in build(), and again when a track is applied. Nothing called it when a setting was committed - neither the roller nor the value carousel. The snapshot reads the durations from settings_ correctly; it was simply never asked again after boot. The timing side was unaffected, which is why the ten second rest was honoured: SessionConfiguration is built from settings_ at session start, not from anything the dashboard holds. Refreshing from persist_settings() rather than at each call site, since every settings change already goes through it and the next screen that writes a setting cannot then forget to. Unconditional rather than only on a successful save: if the save fails, settings_ still holds what the device will use for the next session, and that is what the driver should be shown rather than the last value that reached flash. Closes #152 Co-Authored-By: Claude Opus 5 (1M context) --- firmware/main/screen_router.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/firmware/main/screen_router.cpp b/firmware/main/screen_router.cpp index 4c3824c..4f29039 100644 --- a/firmware/main/screen_router.cpp +++ b/firmware/main/screen_router.cpp @@ -196,9 +196,19 @@ class ScreenRouter { if (result != settings::SettingsApplyResult::applied) { ESP_LOGW("track_timer", "settings not saved (apply result %u)", static_cast(result)); - return; } - settings_ = settings_manager_.current(); + else { + settings_ = settings_manager_.current(); + } + // The dashboard renders the session and rest durations from these, and used to be + // rebuilt only at boot and on a track change. A duration edited in the menu was + // therefore obeyed by the timer but still shown at its old value until a restart. + // Refreshing here rather than at each call site means the next screen that writes + // a setting cannot forget to. + // + // Unconditional: if the save failed, settings_ still holds what the device will + // use for the next session, and that is what the driver should be shown. + refresh_ready(); } // Advances the session clock and refreshes whichever running-session screen is up. From d19783472eb7c34cf8fca3711a6cdf1995c38c79 Mon Sep 17 00:00:00 2001 From: mrsqr Date: Thu, 20 Aug 2026 17:01:48 +0100 Subject: [PATCH 2/2] fix(ui): write every duration as minutes and seconds The start page showed "1 MIN SESSION" beside "0:12 REST". format_duration_value printed a whole number of minutes as "20 MIN" and anything carrying seconds as "20:30", so two durations displayed together disagreed about how a time is written. The inconsistency has existed since the formatter was written, but was invisible until durations could hold seconds at all, which only became possible when the roller replaced the preset ladders. The moment a rest period could be twelve seconds, the two formats sat next to each other. Now one format everywhere: 20:00, 1:00, 0:12, 90:00. This also changes the settings editor, which formats its values through the same function, and that is the point - one way of writing a time across the device rather than one per screen. Worth noting how this got through: 35 host suites and 70 simulator tests all passed, because each asserts its own format in isolation. Nothing renders two durations together, which is the only place the disagreement shows. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 5 +++++ .../ui/include/track_timer/ui/time_roller.hpp | 4 ++-- firmware/components/ui/time_roller.cpp | 13 +++++-------- tests/cpp/test_application_screen.cpp | 2 +- tests/cpp/test_navigation.cpp | 4 ++-- tests/cpp/test_time_roller.cpp | 14 ++++++++++---- 6 files changed, 25 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b4fe0c..f3f2698 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ The project follows Semantic Versioning once the first firmware release is tagge ### Fixed +- the start page kept the durations it read at boot, so a session or rest period changed in + the menu was obeyed by the timer but still shown at its old value until a restart +- durations were written two ways at once, so a one minute session read "1 MIN SESSION" + beside a "0:12 REST"; every duration is now minutes and seconds + - a session that ran out simply stopped: the rest period was modelled and tested but never routed to a screen, and overtime rendered identically to a session still running diff --git a/firmware/components/ui/include/track_timer/ui/time_roller.hpp b/firmware/components/ui/include/track_timer/ui/time_roller.hpp index 095509d..e2ab90c 100644 --- a/firmware/components/ui/include/track_timer/ui/time_roller.hpp +++ b/firmware/components/ui/include/track_timer/ui/time_roller.hpp @@ -31,8 +31,8 @@ struct TimeFieldSpec { [[nodiscard]] bool apply_time_field(SettingsField field, std::uint32_t seconds, settings::DeviceSettings& draft) noexcept; -// "20 MIN" for a whole number of minutes, "20:30" otherwise. Whole minutes stay in the -// wording the device has always used, because that is what most values still are. +// Minutes and seconds, always: "20:00", "1:00", "0:12". One format everywhere, so two +// durations shown together cannot disagree about how a time is written. void format_duration_value(char* output, std::size_t size, std::uint32_t seconds) noexcept; // How far the finger travels to advance one step, and how a flick decays. Tuned against diff --git a/firmware/components/ui/time_roller.cpp b/firmware/components/ui/time_roller.cpp index 33d13b9..279a9de 100644 --- a/firmware/components/ui/time_roller.cpp +++ b/firmware/components/ui/time_roller.cpp @@ -87,14 +87,11 @@ void format_duration_value(char* const output, const std::size_t size, if (output == nullptr || size == 0) { return; } - const auto minutes = seconds / 60U; - const auto remainder = seconds % 60U; - if (remainder == 0U) { - std::snprintf(output, size, "%u MIN", static_cast(minutes)); - return; - } - std::snprintf(output, size, "%u:%02u", static_cast(minutes), - static_cast(remainder)); + // Always minutes and seconds. Printing whole minutes as "20 MIN" put two different + // formats side by side on the dashboard the moment a value carried seconds, so a one + // minute session read "1 MIN SESSION" next to "0:12 REST". + std::snprintf(output, size, "%u:%02u", static_cast(seconds / 60U), + static_cast(seconds % 60U)); } void HoldToSave::begin(const std::uint32_t now_ms) noexcept diff --git a/tests/cpp/test_application_screen.cpp b/tests/cpp/test_application_screen.cpp index 222af0d..d5ad002 100644 --- a/tests/cpp/test_application_screen.cpp +++ b/tests/cpp/test_application_screen.cpp @@ -131,7 +131,7 @@ int main() assert(std::strcmp(lv_label_get_text(screen.settings_screen().field_label_object()), "TRACK SESSION") == 0); assert(std::strcmp(lv_label_get_text(screen.settings_screen().value_label_object()), - "20 MIN") == 0); + "20:00") == 0); for (const auto action : {ui::SettingsScreenAction::previous_field, ui::SettingsScreenAction::next_field, ui::SettingsScreenAction::decrement, diff --git a/tests/cpp/test_navigation.cpp b/tests/cpp/test_navigation.cpp index 7d2cbdd..46ba3d6 100644 --- a/tests/cpp/test_navigation.cpp +++ b/tests/cpp/test_navigation.cpp @@ -60,8 +60,8 @@ int main() ready.logging_available = false; const auto degraded = present_ready(ready); assert(std::strcmp(degraded.selected_track.data(), "Synthetic Test Loop") == 0); - assert(std::strcmp(degraded.session_duration.data(), "30 MIN SESSION") == 0); - assert(std::strcmp(degraded.rest_duration.data(), "15 MIN REST") == 0); + assert(std::strcmp(degraded.session_duration.data(), "30:00 SESSION") == 0); + assert(std::strcmp(degraded.rest_duration.data(), "15:00 REST") == 0); assert(std::strcmp(degraded.timing_mode.data(), "TIMER ONLY - GPS UNAVAILABLE") == 0); assert(std::strcmp(degraded.storage.text.data(), "STORAGE DEGRADED") == 0); diff --git a/tests/cpp/test_time_roller.cpp b/tests/cpp/test_time_roller.cpp index 9553d35..b38bd06 100644 --- a/tests/cpp/test_time_roller.cpp +++ b/tests/cpp/test_time_roller.cpp @@ -213,19 +213,25 @@ void durations_round_trip_through_the_settings() } // Whole minutes keep the wording the device has always used, because most values still are. -void formatting_keeps_whole_minutes_readable() +void formatting_is_always_minutes_and_seconds() { std::array text{}; + // One format for every value, so a whole-minute duration and one carrying seconds can + // sit next to each other on the dashboard without disagreeing. ui::format_duration_value(text.data(), text.size(), 20 * 60); - assert(std::strcmp(text.data(), "20 MIN") == 0); + assert(std::strcmp(text.data(), "20:00") == 0); + ui::format_duration_value(text.data(), text.size(), 60); + assert(std::strcmp(text.data(), "1:00") == 0); ui::format_duration_value(text.data(), text.size(), 20 * 60 + 30); assert(std::strcmp(text.data(), "20:30") == 0); ui::format_duration_value(text.data(), text.size(), 107); assert(std::strcmp(text.data(), "1:47") == 0); ui::format_duration_value(text.data(), text.size(), 0); - assert(std::strcmp(text.data(), "0 MIN") == 0); + assert(std::strcmp(text.data(), "0:00") == 0); ui::format_duration_value(text.data(), text.size(), 5); assert(std::strcmp(text.data(), "0:05") == 0); + ui::format_duration_value(text.data(), text.size(), 90 * 60); + assert(std::strcmp(text.data(), "90:00") == 0); } // LVGL calls a press "long" at 400 ms, which committed a value while the driver was still @@ -293,7 +299,7 @@ int main() committing_clamps_into_the_validated_range(); clearing_the_average_lap_resets_the_lower_display(); durations_round_trip_through_the_settings(); - formatting_keeps_whole_minutes_readable(); + formatting_is_always_minutes_and_seconds(); a_save_needs_a_deliberate_hold(); a_hold_that_moves_is_not_a_save(); hold_progress_ramps_from_nothing_to_full();