Repository navigation
fix(ui): refresh the start page on a settings change, and write every duration one way - #153
Merged
Merged
Conversation
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #152.
Two defects on the same screen, found on the device within minutes of each other.
1. The start page kept the values it read at boot
Rest was changed to ten seconds. The rest timer ran for ten seconds; the start page went on
reading
20 MIN RESTuntil a restart. The device displayed one thing and did another, onthe 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 fromexactly two places:
build()apply_selected_track()Nothing called it when a setting was committed — neither the roller nor the value carousel.
ready_snapshot()reads the durations fromsettings_correctly; it was simply never askedagain.
The timing side was unaffected, which is why the ten second rest was honoured:
SessionConfigurationis built fromsettings_at session start, not from anything thedashboard holds.
Fixed in
persist_settings(), which every settings change already goes through, so the nextscreen that writes a setting cannot forget it. Unconditional rather than only on a successful
save: if the save fails,
settings_still holds what the device will use for the nextsession, and that is what the driver should see.
2. Durations were written two ways at once
format_duration_valueprinted whole minutes as20 MINand anything carrying seconds as20:30, so the page read1 MIN SESSIONbeside0:12 REST.The inconsistency dates from when the formatter was written, but was invisible until
durations could hold seconds at all — only possible since the roller replaced the preset
ladders. The moment a rest period could be twelve seconds, the two formats sat side by side.
One format everywhere now:
20:00,1:00,0:12,90:00. This also changes the settingseditor, which formats through the same function — that is the point, one way of writing a
time across the device rather than one per screen.
How both got through
35 host suites and 70 simulator tests pass, and passed throughout.
ScreenRouter, which has no host test harnessat all, so nothing exercises "a setting was committed" → "the dashboard was redrawn".
test asserts its own format in isolation.
This is the third defect of the first shape recently: #142 had rest modelled and tested but
never routed to a screen, and #144 had the vehicle frame correct in every host test while the
axis reaching the display was inverted. The logic keeps being right; the wiring to the panel
keeps being what nothing checks. Making
ScreenRoutertestable off-device would catch allthree — noted on #152 rather than attempted here.
Verification
35 host suites, 70 simulator tests, and confirmed on the physical panel: a changed rest
duration is now reflected on the start page before the next session, and session and rest are
written the same way.