From f4452749d35476e5092e49461ea837913e910922 Mon Sep 17 00:00:00 2001 From: mrsqr Date: Thu, 20 Aug 2026 20:19:56 +0100 Subject: [PATCH] fix(settings): put the launch ladder inside what a car can actually pull The IMU trigger reads forward acceleration, which a car reaches at roughly 0.3 to 1.0 g. The ladder ran 0, 0.5, 1.0, 1.25 up to 4.0 g, so six of its ten choices could never fire, and nothing was offered below 0.5 g where a deliberate pit exit actually sits. The setting was stored and edited but read by nothing until the trigger landed, so the values had never been checked against a measurement. The new ladder spans 0.15 to 1.0 g. Zero is gone: it meant "off", which as a trigger threshold is a device that never starts a session, and TRIGGER is the on/off now. Stored settings are snapped onto the ladder rather than rejected. Refusing a value that is no longer offered would fail the whole blob and take every unrelated setting back to defaults with it, which is a far worse outcome than a threshold moving one step. Zero maps to the default rather than to the lowest value, which would be the most trigger-happy setting of all. The ladder existed in three places - the picker, the stepping editor and the validator - which is how it drifted away from anything measurable without anyone noticing. There is one definition now, and the editor's test reads it rather than restating it, so a test can no longer describe a ladder the device does not offer. Verified on the device: settings written by the previous firmware migrated in place and the selected circuit was still restored afterwards, which a reset to defaults would have lost. Closes #156 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 5 ++ firmware/components/settings/component.cpp | 69 +++++++++++++++---- .../include/track_timer/settings/settings.hpp | 26 ++++++- firmware/components/ui/settings_editor.cpp | 17 ++--- firmware/components/ui/value_picker.cpp | 19 ++--- tests/cpp/test_settings.cpp | 56 ++++++++++++++- tests/cpp/test_settings_editor.cpp | 8 ++- 7 files changed, 158 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c81efd1..47b1536 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 launch sensitivity ladder ran to 4 g, which a car cannot reach as forward + acceleration, so six of its ten choices could never fire and nothing was offered below + 0.5 g where a deliberate pit exit sits; stored values are snapped onto the new ladder + rather than failing validation and resetting every other setting + - section menus opened on their first item rather than the one in force, so Mode, Track and Trigger could not tell the driver what was set, only let them change it - the menu carousel was told there were four items while the shell offered five, leaving diff --git a/firmware/components/settings/component.cpp b/firmware/components/settings/component.cpp index 333931a..b0d29cc 100644 --- a/firmware/components/settings/component.cpp +++ b/firmware/components/settings/component.cpp @@ -18,6 +18,8 @@ constexpr std::size_t kLegacyV4PayloadSize = 66; // v5 ignores them in favour of the appended values. constexpr std::size_t kLegacyV5PayloadSize = 74; // v6 appends the session trigger, which nothing before it had. +// v7 changed what the launch field may hold, not the shape of the payload. +constexpr std::size_t kLegacyV6PayloadSize = 75; constexpr std::size_t kCurrentPayloadSize = 75; inline constexpr std::uint32_t kMaximumDurationSeconds = 24U * 60U * 60U; @@ -28,13 +30,6 @@ bool valid_brightness(const std::uint8_t percent) noexcept return percent == 25 || percent == 50 || percent == 75 || percent == 100; } -bool valid_launch_sensitivity(const std::uint16_t milli_g) noexcept -{ - constexpr std::array values{0, 500, 1'000, 1'250, 1'500, - 1'750, 2'000, 2'500, 3'500, 4'000}; - return std::find(values.begin(), values.end(), milli_g) != values.end(); -} - bool valid_track_identifier( const std::array& identifier) noexcept { @@ -136,6 +131,32 @@ bool valid_legacy_settings(const LegacySettingsV1& settings) noexcept } // namespace +bool valid_launch_sensitivity(const std::uint16_t milli_g) noexcept +{ + return std::find(kLaunchSensitivityMilliG.begin(), kLaunchSensitivityMilliG.end(), + milli_g) != kLaunchSensitivityMilliG.end(); +} + +std::uint16_t nearest_launch_sensitivity(const std::uint16_t milli_g) noexcept +{ + if (milli_g == 0) { + // Zero meant "off", which as a trigger threshold is a device that never starts a + // session. The trigger itself is the on/off now, so off becomes the default rather + // than the lowest setting, which would be the most trigger-happy of all. + return kDefaultLaunchSensitivityMilliG; + } + auto best = kLaunchSensitivityMilliG.front(); + auto best_distance = milli_g > best ? milli_g - best : best - milli_g; + for (const auto candidate : kLaunchSensitivityMilliG) { + const auto distance = milli_g > candidate ? milli_g - candidate : candidate - milli_g; + if (distance < best_distance) { + best = candidate; + best_distance = distance; + } + } + return best; +} + bool valid_settings(const DeviceSettings& settings) noexcept { return settings.session_duration_seconds >= kMinimumSessionSeconds && @@ -238,15 +259,26 @@ SettingsBlob encode_legacy_settings_v5(const DeviceSettings& settings) noexcept return make_blob(5, payload.data(), payload.size()); } -SettingsBlob encode_settings(const DeviceSettings& settings) noexcept +SettingsBlob encode_legacy_settings_v6(const DeviceSettings& settings) noexcept { const auto legacy = encode_legacy_settings_v5(settings); if (legacy.size == 0) { return {}; } - std::array payload{}; + std::array payload{}; std::copy_n(legacy.bytes.data() + kHeaderSize, kLegacyV5PayloadSize, payload.data()); payload[74] = static_cast(settings.session_trigger); + return make_blob(6, payload.data(), payload.size()); +} + +SettingsBlob encode_settings(const DeviceSettings& settings) noexcept +{ + const auto legacy = encode_legacy_settings_v6(settings); + if (legacy.size == 0) { + return {}; + } + std::array payload{}; + std::copy_n(legacy.bytes.data() + kHeaderSize, kLegacyV6PayloadSize, payload.data()); return make_blob(kCurrentSettingsVersion, payload.data(), payload.size()); } @@ -289,7 +321,7 @@ DecodeResult decode_settings(const SettingsBlob& blob, DeviceSettings& settings) settings = candidate; return DecodeResult::migrated_v1; } - if (version != 2 && version != 3 && version != 4 && version != 5 && + if (version != 2 && version != 3 && version != 4 && version != 5 && version != 6 && version != kCurrentSettingsVersion) { return DecodeResult::unsupported_version; } @@ -298,6 +330,7 @@ DecodeResult decode_settings(const SettingsBlob& blob, DeviceSettings& settings) : version == 3 ? kLegacyV3PayloadSize : version == 4 ? kLegacyV4PayloadSize : version == 5 ? kLegacyV5PayloadSize + : version == 6 ? kLegacyV6PayloadSize : kCurrentPayloadSize; if (payload_size != expected_payload_size) { return DecodeResult::corrupt; @@ -329,9 +362,16 @@ DecodeResult decode_settings(const SettingsBlob& blob, DeviceSettings& settings) candidate.session_duration_seconds = get_u32(payload + 66); candidate.rest_duration_seconds = get_u32(payload + 70); } - if (version == kCurrentSettingsVersion) { + if (version >= 6) { candidate.session_trigger = static_cast(payload[74]); } + if (version < kCurrentSettingsVersion) { + // Before v7 the ladder ran to 4 g and offered zero as "off". Snapping keeps the + // rest of the blob rather than failing validation over one field and taking every + // unrelated setting back to defaults. + candidate.launch_sensitivity_milli_g = + nearest_launch_sensitivity(candidate.launch_sensitivity_milli_g); + } // Everything before v6 predates the trigger, so it defaults to manual, which is what // those devices were doing. if (payload[12] > 1 || @@ -339,8 +379,7 @@ DecodeResult decode_settings(const SettingsBlob& blob, DeviceSettings& settings) (version >= 4 && (payload[63] > static_cast(LapBoundaryMode::finish) || payload[64] > 1 || payload[65] > 1)) || - (version == kCurrentSettingsVersion && - payload[74] > static_cast(SessionTrigger::gps)) || + (version >= 6 && payload[74] > static_cast(SessionTrigger::gps)) || !valid_settings(candidate)) { return DecodeResult::corrupt; } @@ -349,6 +388,7 @@ DecodeResult decode_settings(const SettingsBlob& blob, DeviceSettings& settings) : version == 3 ? DecodeResult::migrated_v3 : version == 4 ? DecodeResult::migrated_v4 : version == 5 ? DecodeResult::migrated_v5 + : version == 6 ? DecodeResult::migrated_v6 : DecodeResult::current; } @@ -402,6 +442,9 @@ SettingsLoadReport SettingsManager::load() noexcept case DecodeResult::migrated_v5: current_ = decoded; return {SettingsSource::migrated_v5, persist(current_)}; + case DecodeResult::migrated_v6: + current_ = decoded; + return {SettingsSource::migrated_v6, persist(current_)}; case DecodeResult::corrupt: current_ = {}; return {SettingsSource::defaults_corrupt, persist(current_)}; diff --git a/firmware/components/settings/include/track_timer/settings/settings.hpp b/firmware/components/settings/include/track_timer/settings/settings.hpp index 22cf7ca..6237dd1 100644 --- a/firmware/components/settings/include/track_timer/settings/settings.hpp +++ b/firmware/components/settings/include/track_timer/settings/settings.hpp @@ -7,7 +7,7 @@ namespace track_timer::settings { -inline constexpr std::uint16_t kCurrentSettingsVersion = 6; +inline constexpr std::uint16_t kCurrentSettingsVersion = 7; inline constexpr std::size_t kSettingsBlobCapacity = 128; inline constexpr std::size_t kTrackIdentifierCapacity = 48; @@ -41,12 +41,30 @@ enum class LapBoundaryMode : std::uint8_t { finish, }; +// The IMU session trigger reads forward acceleration, which a car reaches at roughly 0.3 +// to 1.0 g. The previous ladder ran to 4.0 g, so six of its ten choices could never fire, +// and it offered nothing below 0.5 g where a deliberate pit exit actually sits. +// +// One definition, consumed by the picker, the stepping editor and the validator alike. +// Three copies of it were how the values drifted away from anything measurable without +// anyone noticing. +inline constexpr std::array kLaunchSensitivityMilliG{ + 150, 200, 250, 300, 350, 400, 500, 600, 800, 1'000}; +// A deliberate pit exit, without tripping on a bump. +inline constexpr std::uint16_t kDefaultLaunchSensitivityMilliG = 300; + +// Snaps any stored value onto the ladder. Settings written before v7 hold values that are +// no longer offered, and rejecting them would fail the whole blob and take every unrelated +// setting back to defaults with it. +[[nodiscard]] std::uint16_t nearest_launch_sensitivity(std::uint16_t milli_g) noexcept; +[[nodiscard]] bool valid_launch_sensitivity(std::uint16_t milli_g) noexcept; + struct DeviceSettings { // Seconds, not minutes: the roller sets these as minutes and seconds, and 24 hours of // seconds does not fit a uint16. std::uint32_t session_duration_seconds{20 * 60}; std::uint32_t rest_duration_seconds{20 * 60}; - std::uint16_t launch_sensitivity_milli_g{0}; + std::uint16_t launch_sensitivity_milli_g{kDefaultLaunchSensitivityMilliG}; std::uint16_t average_lap_seconds{0}; std::uint8_t day_brightness_percent{100}; std::uint8_t night_brightness_percent{50}; @@ -98,6 +116,7 @@ enum class DecodeResult : std::uint8_t { migrated_v3, migrated_v4, migrated_v5, + migrated_v6, corrupt, unsupported_version, }; @@ -109,6 +128,7 @@ enum class SettingsSource : std::uint8_t { migrated_v3, migrated_v4, migrated_v5, + migrated_v6, defaults_missing, defaults_corrupt, defaults_unsupported, @@ -159,6 +179,8 @@ struct FeatureAvailability { const LegacySettingsV1& settings) noexcept; [[nodiscard]] SettingsBlob encode_legacy_settings_v2( const DeviceSettings& settings) noexcept; +[[nodiscard]] SettingsBlob encode_legacy_settings_v6( + const DeviceSettings& settings) noexcept; [[nodiscard]] SettingsBlob encode_legacy_settings_v5( const DeviceSettings& settings) noexcept; [[nodiscard]] SettingsBlob encode_legacy_settings_v4( diff --git a/firmware/components/ui/settings_editor.cpp b/firmware/components/ui/settings_editor.cpp index 1b2311d..5c1dddb 100644 --- a/firmware/components/ui/settings_editor.cpp +++ b/firmware/components/ui/settings_editor.cpp @@ -13,8 +13,6 @@ namespace { // coarse preset ladder; the roller in the gated menu is what reaches every value. constexpr std::array kDurationSeconds{60, 300, 600, 900, 1200, 1500, 1800, 2400, 3000, 3600}; -constexpr std::array kLaunchMilliG{0, 500, 1'000, 1'250, 1'500, - 1'750, 2'000, 2'500, 3'500, 4'000}; constexpr std::array kBrightnessPercent{25, 50, 75, 100}; template @@ -95,15 +93,10 @@ void format_value(const SettingsField field, const settings::DeviceSettings& set format_duration_value(output.data(), output.size(), settings.rest_duration_seconds); break; case SettingsField::launch_sensitivity: - if (settings.launch_sensitivity_milli_g == 0) { - std::snprintf(output.data(), output.size(), "OFF"); - } - else { - std::snprintf(output.data(), output.size(), "%u.%02u g", - static_cast(settings.launch_sensitivity_milli_g / 1000), - static_cast((settings.launch_sensitivity_milli_g % 1000) / - 10)); - } + // Always a real threshold now; TRIGGER decides whether it is consulted. + std::snprintf(output.data(), output.size(), "%u.%02u g", + static_cast(settings.launch_sensitivity_milli_g / 1000), + static_cast((settings.launch_sensitivity_milli_g % 1000) / 10)); break; case SettingsField::day_brightness: std::snprintf(output.data(), output.size(), "%u%%", @@ -320,7 +313,7 @@ bool SettingsEditor::adjust(const bool forward) noexcept adjusted = step_choice(draft_.rest_duration_seconds, kDurationSeconds, forward); break; case SettingsField::launch_sensitivity: - adjusted = step_choice(draft_.launch_sensitivity_milli_g, kLaunchMilliG, forward); + adjusted = step_choice(draft_.launch_sensitivity_milli_g, settings::kLaunchSensitivityMilliG, forward); break; case SettingsField::day_brightness: adjusted = step_choice(draft_.day_brightness_percent, kBrightnessPercent, forward); diff --git a/firmware/components/ui/value_picker.cpp b/firmware/components/ui/value_picker.cpp index f7c0382..f4250af 100644 --- a/firmware/components/ui/value_picker.cpp +++ b/firmware/components/ui/value_picker.cpp @@ -6,8 +6,6 @@ namespace track_timer::ui { namespace { -constexpr std::array kLaunchMilliG{0, 500, 1'000, 1'250, 1'500, - 1'750, 2'000, 2'500, 3'500, 4'000}; constexpr std::array kBrightnessPercent{25, 50, 75, 100}; @@ -67,15 +65,12 @@ ValueChoiceList choices_for(const SettingsField field, case SettingsField::average_lap: return {}; case SettingsField::launch_sensitivity: - return from_list(kLaunchMilliG, current.launch_sensitivity_milli_g, + return from_list(settings::kLaunchSensitivityMilliG, current.launch_sensitivity_milli_g, [](ValueChoice& c, std::uint16_t v) { - if (v == 0) { - set_text(c, "OFF"); - } - else { - std::snprintf(c.text.data(), c.text.size(), "%u.%02u G", - v / 1000U, (v % 1000U) / 10U); - } + // No "off" any more: the threshold is always a real value and + // TRIGGER is what decides whether it is consulted. + std::snprintf(c.text.data(), c.text.size(), "%u.%02u G", + v / 1000U, (v % 1000U) / 10U); }); case SettingsField::day_brightness: return from_list(kBrightnessPercent, current.day_brightness_percent, @@ -119,10 +114,10 @@ bool apply_choice(const SettingsField field, const std::size_t index, case SettingsField::average_lap: return false; case SettingsField::launch_sensitivity: - if (index >= kLaunchMilliG.size()) { + if (index >= settings::kLaunchSensitivityMilliG.size()) { return false; } - draft.launch_sensitivity_milli_g = kLaunchMilliG[index]; + draft.launch_sensitivity_milli_g = settings::kLaunchSensitivityMilliG[index]; return true; case SettingsField::day_brightness: if (index >= kBrightnessPercent.size()) { diff --git a/tests/cpp/test_settings.cpp b/tests/cpp/test_settings.cpp index b2d9dd8..3806719 100644 --- a/tests/cpp/test_settings.cpp +++ b/tests/cpp/test_settings.cpp @@ -47,7 +47,7 @@ DeviceSettings customized_settings() DeviceSettings settings{}; settings.session_duration_seconds = 30 * 60 + 30; // exercises the seconds v5 added settings.rest_duration_seconds = 10 * 60; - settings.launch_sensitivity_milli_g = 1'250; + settings.launch_sensitivity_milli_g = 600; settings.average_lap_seconds = 103; settings.day_brightness_percent = 75; settings.night_brightness_percent = 25; @@ -92,6 +92,46 @@ void test_validation_and_codec() assert(encode_settings(invalid).size == 0); } +// The ladder changed under stored settings, and rejecting a value no longer offered would +// fail the whole blob and take every unrelated setting back to defaults with it. +void test_launch_sensitivity_snapping() +{ + // Every value the old ladder offered maps onto the new one rather than being refused. + for (const std::uint16_t old_value : {500, 1'000, 1'250, 1'500, 1'750, 2'000, 2'500, + 3'500, 4'000}) { + const auto snapped = nearest_launch_sensitivity(old_value); + assert(valid_launch_sensitivity(snapped)); + } + + // Everything above the top of the new ladder lands on it rather than somewhere odd. + assert(nearest_launch_sensitivity(4'000) == 1'000); + assert(nearest_launch_sensitivity(2'500) == 1'000); + assert(nearest_launch_sensitivity(1'250) == 1'000); + + // Zero meant "off", which as a trigger threshold is a device that never starts. The + // trigger is the on/off now, so off becomes the default rather than the lowest value, + // which would be the most trigger-happy setting of all. + assert(nearest_launch_sensitivity(0) == kDefaultLaunchSensitivityMilliG); + assert(nearest_launch_sensitivity(0) != kLaunchSensitivityMilliG.front()); + + // Nearest really is nearest, either side. + assert(nearest_launch_sensitivity(160) == 150); + assert(nearest_launch_sensitivity(190) == 200); + assert(nearest_launch_sensitivity(700) == 600); + assert(nearest_launch_sensitivity(750) == 800); + + // A value already on the ladder is left exactly alone. + for (const auto value : kLaunchSensitivityMilliG) { + assert(nearest_launch_sensitivity(value) == value); + } + + // The ladder itself has to be reachable as forward acceleration, which is the defect + // this replaced: a car manages roughly 0.3 to 1.0 g, and the old top was 4.0. + assert(kLaunchSensitivityMilliG.front() < 500); // below a deliberate pit exit + assert(kLaunchSensitivityMilliG.back() <= 1'000); // within what a car can pull + assert(valid_launch_sensitivity(kDefaultLaunchSensitivityMilliG)); +} + void test_defaults_restart_and_deferred_apply() { MemorySettingsStore store; @@ -194,6 +234,19 @@ void test_migration_corruption_and_storage_errors() // devices were doing. assert(version_five.current().session_trigger == SessionTrigger::manual); + MemorySettingsStore version_six_store; + version_six_store.found = true; + version_six_store.blob = encode_legacy_settings_v6(customized_settings()); + SettingsManager version_six{version_six_store}; + const auto version_six_migration = version_six.load(); + assert(version_six_migration.source == SettingsSource::migrated_v6); + assert(version_six_migration.current_format_persisted); + // v6 carried the trigger, so it survives; the launch value was already on the new + // ladder, so it is untouched. + assert(version_six.current().session_trigger == SessionTrigger::imu); + assert(version_six.current().launch_sensitivity_milli_g == 600); + assert(version_six.current().session_duration_seconds == 30 * 60 + 30); + MemorySettingsStore corrupt_store; corrupt_store.found = true; corrupt_store.blob = encode_settings(customized_settings()); @@ -289,6 +342,7 @@ void test_file_store_restart() int main() { test_validation_and_codec(); + test_launch_sensitivity_snapping(); test_defaults_restart_and_deferred_apply(); test_migration_corruption_and_storage_errors(); test_degraded_feature_matrix(); diff --git a/tests/cpp/test_settings_editor.cpp b/tests/cpp/test_settings_editor.cpp index 52c00d5..6406a1d 100644 --- a/tests/cpp/test_settings_editor.cpp +++ b/tests/cpp/test_settings_editor.cpp @@ -92,8 +92,12 @@ int main() assert(editor.draft().rest_duration_seconds == durations.back()); select_field(editor, ui::SettingsField::launch_sensitivity); - constexpr std::array launch_values{0, 500, 1'000, 1'250, 1'500, - 1'750, 2'000, 2'500, 3'500, 4'000}; + // Read from the one definition, so this test cannot describe a ladder the device no + // longer offers - which is exactly how the old values went unchecked. + const auto& launch_values = settings::kLaunchSensitivityMilliG; + // The default no longer sits at the bottom of this ladder, so wind down to it first. + while (editor.decrement()) { + } for (const auto expected : launch_values) { assert(editor.draft().launch_sensitivity_milli_g == expected); if (expected != launch_values.back()) {