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/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. 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();