From 8cff436055f51b629e9277387764463fc2887b5b Mon Sep 17 00:00:00 2001 From: mrsqr Date: Thu, 20 Aug 2026 23:12:29 +0100 Subject: [PATCH] feat: give a finished session a record, and show it in Review The device ran a full session - countdown, overrun, rest, launch trigger - and forgot all of it the moment the driver double-tapped. Nothing was recorded and Review showed nothing, which is the difference between a timer and something worth taking to a track day. Most of this was connecting parts that already existed. SessionReviewController takes a SessionSummaryProvider and SessionSummaryV1 already carried duration, overrun, completion reason and integrity; nothing implemented the provider, so the router passed nullptr. MemorySummaryStore is that provider, holding the last eight sessions newest first and refusing anything that fails the summary validator, so a malformed record cannot reach Review and be rendered as though it were real. Three gaps were wiring rather than logic. The meter was told session_active was false, hardcoded - correct when written, because no session existed on the device, and stale for three merges since - so peaks described everything since boot and were scoped to nothing. The router computed vertical G and dropped it before the meter saw it. And the summary record had no G fields, so even a scoped peak had nowhere to live. Up and down peaks are held apart because a kerb throws the car up and a compression loads it down, and those are different events. Peaks reset when a session starts rather than when a trigger arms: a driver waiting on a launch has not started, and carrying the device to the car is not the session. Review was then found to be rendering its summary at 0.82 mm, a twelfth the height of the countdown, which is why a session that had been recorded correctly read as an empty screen. The rows are 1.65 mm now with a 2.82 mm peak headline. The peaks block occupies the space lap rows will take once there is a receiver, so only one of them is ever up. Sessions are marked degraded_gnss, which is true and which Review should say rather than present a lap-less session as complete. Peaks recorded before the gravity reference was gyro-tracked latched tilt as acceleration and had left and right swapped, so both schema versions move and older records are refused rather than migrated: they are wrong, not old. This is deliberately in RAM. Nothing survives a power cycle, and the card is next. Part of #137 and #142 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 8 + Makefile | 17 +- firmware/components/logger/CMakeLists.txt | 2 +- firmware/components/logger/formats.cpp | 13 ++ .../include/track_timer/logger/formats.hpp | 20 ++- .../logger/memory_summary_store.hpp | 42 +++++ .../logger/memory_summary_store.cpp | 63 +++++++ firmware/components/ui/imu_meter.cpp | 13 ++ .../ui/include/track_timer/ui/imu_meter.hpp | 14 +- .../include/track_timer/ui/session_review.hpp | 6 + .../track_timer/ui/session_review_screen.hpp | 5 + firmware/components/ui/session_review.cpp | 20 +++ .../components/ui/session_review_screen.cpp | 80 ++++++--- firmware/main/screen_router.cpp | 68 +++++++- simulator/CMakeLists.txt | 2 +- tests/cpp/test_imu_meter.cpp | 52 +++++- tests/cpp/test_summary_store.cpp | 158 ++++++++++++++++++ 17 files changed, 553 insertions(+), 30 deletions(-) create mode 100644 firmware/components/logger/include/track_timer/logger/memory_summary_store.hpp create mode 100644 firmware/components/logger/memory_summary_store.cpp create mode 100644 tests/cpp/test_summary_store.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index eb5b3b2..02269be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ The project follows Semantic Versioning once the first firmware release is tagge ### Fixed +- G peaks were never scoped to a session, so they described everything since boot; the meter + was still being told no session was ever running +- the vertical axis was computed and then dropped before it reached the meter +- Review rendered its summary rows at 0.82 mm, which on this panel reads as an empty screen + - 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 @@ -68,6 +73,9 @@ The project follows Semantic Versioning once the first firmware release is tagge - vertical G alongside the lateral and longitudinal pair, for kerbs and compressions - gyroscope zero-rate offset measured at rest and removed, 4.4 dps on this board - device settings persisted in NVS, so Mode survives a reboot +- a finished session leaves a record: duration, overrun and peak G on every axis, shown in + Review +- vertical G recorded alongside the horizontal pair, with kerbs and compressions kept apart - the gated menu opens on REVIEW, which is wanted the moment a session ends - top-level TRIGGER selection: MANUAL starts on the button, IMU on a launch, GPS at the line - pending phase on the running screen for a session armed and waiting for its trigger diff --git a/Makefile b/Makefile index 6de17a1..117983f 100644 --- a/Makefile +++ b/Makefile @@ -43,15 +43,16 @@ IMU_CALIBRATION_TEST_BINARY := build/host/imu_calibration_test SESSION_URGENCY_TEST_BINARY := build/host/session_urgency_test TIME_ROLLER_TEST_BINARY := build/host/time_roller_test SESSION_TRIGGER_TEST_BINARY := build/host/session_trigger_test +SUMMARY_STORE_TEST_BINARY := build/host/summary_store_test GATE_CAPTURE_TEST_BINARY := build/host/gate_capture_test GATE_SESSION_AUTOMATION_TEST_BINARY := build/host/gate_session_automation_test SIMULATOR_BUILD_DIR ?= build/simulator SIMULATOR_IMAGE ?= track-session-timer-simulator:lvgl-9.5.0 CMAKE ?= cmake -.PHONY: check test track-validate uk-track-pack track-pack-test simulator-track-catalog-test simulator-fixture-validate repo-check host-test simulator-model-test session-state-test settings-test settings-editor-test projection-test intersection-test crossing-validation-test crossing-time-test lap-state-machine-test timing-engine-test gate-event-engine-test track-definition-test track-capture-test track-matching-test track-selection-test log-format-test async-logger-test session-review-test diagnostics-test active-session-test display-policy-test imu-meter-test rest-session-test track-catalog-test shell-navigation-test device-mode-test imu-calibration-test session-urgency-test time-roller-test session-trigger-test gate-capture-test gate-session-automation-test ui-foundation-test navigation-test simulator-configure simulator-build simulator-test simulator-run simulator-container-image simulator-container-test simulator-clean firmware-build firmware-container-build firmware-container-flash firmware-container-monitor firmware-container-flash-monitor firmware-container-erase firmware-device-info firmware-clean issue-preview label-preview +.PHONY: check test track-validate uk-track-pack track-pack-test simulator-track-catalog-test simulator-fixture-validate repo-check host-test simulator-model-test session-state-test settings-test settings-editor-test projection-test intersection-test crossing-validation-test crossing-time-test lap-state-machine-test timing-engine-test gate-event-engine-test track-definition-test track-capture-test track-matching-test track-selection-test log-format-test async-logger-test session-review-test diagnostics-test active-session-test display-policy-test imu-meter-test rest-session-test track-catalog-test shell-navigation-test device-mode-test imu-calibration-test session-urgency-test time-roller-test session-trigger-test summary-store-test gate-capture-test gate-session-automation-test ui-foundation-test navigation-test simulator-configure simulator-build simulator-test simulator-run simulator-container-image simulator-container-test simulator-clean firmware-build firmware-container-build firmware-container-flash firmware-container-monitor firmware-container-flash-monitor firmware-container-erase firmware-device-info firmware-clean issue-preview label-preview -check: test track-validate track-pack-test simulator-track-catalog-test simulator-fixture-validate repo-check host-test simulator-model-test session-state-test settings-test settings-editor-test projection-test intersection-test crossing-validation-test crossing-time-test lap-state-machine-test timing-engine-test gate-event-engine-test track-definition-test track-capture-test track-matching-test track-selection-test log-format-test async-logger-test session-review-test diagnostics-test active-session-test display-policy-test imu-meter-test rest-session-test track-catalog-test shell-navigation-test device-mode-test imu-calibration-test session-urgency-test time-roller-test session-trigger-test gate-capture-test gate-session-automation-test ui-foundation-test navigation-test +check: test track-validate track-pack-test simulator-track-catalog-test simulator-fixture-validate repo-check host-test simulator-model-test session-state-test settings-test settings-editor-test projection-test intersection-test crossing-validation-test crossing-time-test lap-state-machine-test timing-engine-test gate-event-engine-test track-definition-test track-capture-test track-matching-test track-selection-test log-format-test async-logger-test session-review-test diagnostics-test active-session-test display-policy-test imu-meter-test rest-session-test track-catalog-test shell-navigation-test device-mode-test imu-calibration-test session-urgency-test time-roller-test session-trigger-test summary-store-test gate-capture-test gate-session-automation-test ui-foundation-test navigation-test test: $(PYTHON) -B -m unittest discover -s tests -p 'test_*.py' @@ -424,6 +425,18 @@ gate-session-automation-test: -o $(GATE_SESSION_AUTOMATION_TEST_BINARY) $(GATE_SESSION_AUTOMATION_TEST_BINARY) +summary-store-test: + mkdir -p build/host + $(CXX) -std=c++17 -Wall -Wextra -Werror -pedantic \ + -Ifirmware/components/domain/include \ + -Ifirmware/components/settings/include \ + -Ifirmware/components/logger/include \ + firmware/components/settings/component.cpp \ + firmware/components/logger/formats.cpp \ + firmware/components/logger/memory_summary_store.cpp \ + tests/cpp/test_summary_store.cpp -o $(SUMMARY_STORE_TEST_BINARY) + $(SUMMARY_STORE_TEST_BINARY) + session-trigger-test: mkdir -p build/host $(CXX) -std=c++17 -Wall -Wextra -Werror -pedantic \ diff --git a/firmware/components/logger/CMakeLists.txt b/firmware/components/logger/CMakeLists.txt index 006ebfb..2571ad0 100644 --- a/firmware/components/logger/CMakeLists.txt +++ b/firmware/components/logger/CMakeLists.txt @@ -1,5 +1,5 @@ idf_component_register( - SRCS "component.cpp" "formats.cpp" "async_logger.cpp" "task_esp.cpp" + SRCS "component.cpp" "formats.cpp" "memory_summary_store.cpp" "async_logger.cpp" "task_esp.cpp" INCLUDE_DIRS "include" REQUIRES domain board settings esp_timer freertos ) diff --git a/firmware/components/logger/formats.cpp b/firmware/components/logger/formats.cpp index 9f5793e..4a652c9 100644 --- a/firmware/components/logger/formats.cpp +++ b/firmware/components/logger/formats.cpp @@ -290,8 +290,21 @@ bool valid_event_record(const EventRecordV1& record) noexcept record.session_overrun_ms <= record.session_elapsed_ms; } +// A peak is a magnitude, so it cannot be negative, and anything beyond what a car can +// physically pull says the reading is wrong rather than remarkable. +bool valid_peak(const float value) noexcept +{ + return value >= 0.0F && value <= kMaximumCrediblePeakG; +} + bool valid_summary(const SessionSummaryV1& record) noexcept { + if (!valid_peak(record.peaks.acceleration_g) || !valid_peak(record.peaks.braking_g) || + !valid_peak(record.peaks.left_g) || !valid_peak(record.peaks.right_g) || + !valid_peak(record.peaks.up_g) || !valid_peak(record.peaks.down_g) || + !valid_peak(record.peaks.total_g)) { + return false; + } if (record.schema_version != kLogFormatVersion || record.record_size_bytes != sizeof(SessionSummaryV1) || !non_empty(record.session_id) || record.session_duration_ms < 0 || record.session_overrun_ms < 0 || diff --git a/firmware/components/logger/include/track_timer/logger/formats.hpp b/firmware/components/logger/include/track_timer/logger/formats.hpp index 3d53d2a..d0d2e66 100644 --- a/firmware/components/logger/include/track_timer/logger/formats.hpp +++ b/firmware/components/logger/include/track_timer/logger/formats.hpp @@ -14,9 +14,14 @@ namespace track_timer::logger { // 2: session and rest durations widened from uint16 minutes to uint32 seconds, which // changes both the meaning and the size of every meta record. Logs written before this // are rejected by their version rather than silently misread as very short sessions. -inline constexpr std::uint16_t kLogFormatVersion = 2; +// 3: peak G on the session summary. Peaks recorded before the gravity reference was +// tracked with the gyroscope latched tilt as acceleration and had left and right swapped, +// so older summaries are refused by version rather than migrated - they are wrong, not old. +inline constexpr std::uint16_t kLogFormatVersion = 3; inline constexpr std::int64_t kUnavailableUtcNs = -1; inline constexpr std::size_t kSessionIdentifierCapacity = 32; +// Well beyond a road car on a circuit, and far short of anything a working sensor reports. +inline constexpr float kMaximumCrediblePeakG = 10.0F; inline constexpr std::size_t kFirmwareCommitCapacity = 41; inline constexpr std::size_t kProfileNameCapacity = 32; inline constexpr std::size_t kTrackFingerprintCapacity = 17; @@ -155,6 +160,18 @@ struct EventRecordV1 { std::int64_t session_overrun_ms{domain::kUnavailableTime}; }; +// What the car pulled during a session. Vertical is kept apart from the horizontal pair, +// and up from down, because a kerb strike and a compression are different events. +struct SummaryPeakG { + float acceleration_g{0.0F}; + float braking_g{0.0F}; + float left_g{0.0F}; + float right_g{0.0F}; + float up_g{0.0F}; + float down_g{0.0F}; + float total_g{0.0F}; +}; + struct SessionSummaryV1 { std::uint16_t schema_version{kLogFormatVersion}; std::uint16_t record_size_bytes{0}; @@ -175,6 +192,7 @@ struct SessionSummaryV1 { std::uint32_t source_event_record_count{0}; std::uint32_t logger_dropped_record_count{0}; std::uint32_t logger_write_failure_count{0}; + SummaryPeakG peaks{}; }; struct SummaryLapRecordV1 { diff --git a/firmware/components/logger/include/track_timer/logger/memory_summary_store.hpp b/firmware/components/logger/include/track_timer/logger/memory_summary_store.hpp new file mode 100644 index 0000000..a38cffc --- /dev/null +++ b/firmware/components/logger/include/track_timer/logger/memory_summary_store.hpp @@ -0,0 +1,42 @@ +#pragma once + +#include "track_timer/logger/summary_provider.hpp" + +#include +#include + +namespace track_timer::logger { + +// Holds the last few session summaries in RAM so Review has something to show the moment a +// session ends. +// +// Deliberately not the durable answer: nothing here survives a power cycle, and the card is +// where these belong. It exists so the record's shape is settled and visible before the +// write path is built on top of it, and so Review stops being wired to nullptr. +class MemorySummaryStore final : public SessionSummaryProvider { + public: + static constexpr std::size_t kCapacity = 8; + + // Newest first. Returns false if the summary would not survive its own validator, so a + // malformed record cannot reach Review and be rendered as though it were real. + bool record(const SessionSummaryV1& summary) noexcept; + void clear() noexcept; + + [[nodiscard]] SummaryReadResult session_count(std::size_t& count) noexcept override; + [[nodiscard]] SummaryReadResult read_summary(std::size_t history_index, + SessionSummaryV1& summary) noexcept override; + // No laps without a receiver. Reported as an empty page rather than an error, because + // a session genuinely having no laps is not a fault. + [[nodiscard]] SummaryReadResult read_lap_page( + const std::array& session_id, std::size_t offset, + SummaryLapPage& page) noexcept override; + + [[nodiscard]] std::size_t rejected_count() const noexcept; + + private: + std::array summaries_{}; + std::size_t count_{0}; + std::size_t rejected_{0}; +}; + +} // namespace track_timer::logger diff --git a/firmware/components/logger/memory_summary_store.cpp b/firmware/components/logger/memory_summary_store.cpp new file mode 100644 index 0000000..344326c --- /dev/null +++ b/firmware/components/logger/memory_summary_store.cpp @@ -0,0 +1,63 @@ +#include "track_timer/logger/memory_summary_store.hpp" + +#include + +namespace track_timer::logger { + +bool MemorySummaryStore::record(const SessionSummaryV1& summary) noexcept +{ + if (!valid_summary(summary)) { + ++rejected_; + return false; + } + // Newest first, so history_index 0 is the session that just finished. The oldest falls + // off the end rather than the newest being refused: a driver wants the last session far + // more than the eighth one back. + const auto keep = std::min(count_, kCapacity - 1); + for (std::size_t index = keep; index > 0; --index) { + summaries_[index] = summaries_[index - 1]; + } + summaries_[0] = summary; + count_ = std::min(count_ + 1, kCapacity); + return true; +} + +void MemorySummaryStore::clear() noexcept +{ + summaries_.fill({}); + count_ = 0; + rejected_ = 0; +} + +SummaryReadResult MemorySummaryStore::session_count(std::size_t& count) noexcept +{ + count = count_; + return count_ == 0 ? SummaryReadResult::empty : SummaryReadResult::ready; +} + +SummaryReadResult MemorySummaryStore::read_summary(const std::size_t history_index, + SessionSummaryV1& summary) noexcept +{ + if (count_ == 0) { + return SummaryReadResult::empty; + } + if (history_index >= count_) { + return SummaryReadResult::corrupt; + } + summary = summaries_[history_index]; + return SummaryReadResult::ready; +} + +SummaryReadResult MemorySummaryStore::read_lap_page( + const std::array& session_id, const std::size_t offset, + SummaryLapPage& page) noexcept +{ + (void)session_id; + page = {}; + page.offset = offset; + return SummaryReadResult::ready; +} + +std::size_t MemorySummaryStore::rejected_count() const noexcept { return rejected_; } + +} // namespace track_timer::logger diff --git a/firmware/components/ui/imu_meter.cpp b/firmware/components/ui/imu_meter.cpp index ade4c3b..ef3d264 100644 --- a/firmware/components/ui/imu_meter.cpp +++ b/firmware/components/ui/imu_meter.cpp @@ -76,6 +76,13 @@ const ImuMeterSnapshot& ImuMeterController::update(const ImuMeterInput& input, const auto point = rotate_sample(input); snapshot_.current = point; append(point); + // Vertical does not rotate with the display: up is up however the unit is mounted. + snapshot_.vertical_valid = input.sample_available && input.z_axis_valid; + snapshot_.vertical_g = + snapshot_.vertical_valid + ? clamp_g(input.sample.acceleration_z_mps2 / kStandardGravityMps2) + : 0.0F; + update_peaks(point); increment_saturated(snapshot_.accepted_samples); @@ -130,6 +137,8 @@ PlanarAcceleration ImuMeterController::trail_point( void ImuMeterController::clear_measurements() noexcept { snapshot_.current = {}; + snapshot_.vertical_g = 0.0F; + snapshot_.vertical_valid = false; snapshot_.peaks = {}; snapshot_.trail.fill({}); snapshot_.trail_count = 0; @@ -147,6 +156,10 @@ void ImuMeterController::append(const PlanarAcceleration point) noexcept void ImuMeterController::update_peaks(const PlanarAcceleration& point) noexcept { + if (snapshot_.vertical_valid) { + snapshot_.peaks.up_g = std::max(snapshot_.peaks.up_g, snapshot_.vertical_g); + snapshot_.peaks.down_g = std::max(snapshot_.peaks.down_g, -snapshot_.vertical_g); + } if (point.longitudinal_valid) { snapshot_.peaks.acceleration_g = std::max(snapshot_.peaks.acceleration_g, point.longitudinal_g); diff --git a/firmware/components/ui/include/track_timer/ui/imu_meter.hpp b/firmware/components/ui/include/track_timer/ui/imu_meter.hpp index 7645a58..96176c4 100644 --- a/firmware/components/ui/include/track_timer/ui/imu_meter.hpp +++ b/firmware/components/ui/include/track_timer/ui/imu_meter.hpp @@ -9,7 +9,10 @@ namespace track_timer::ui { -inline constexpr std::uint32_t kImuMeterSchemaVersion = 1; +// 2: the vertical axis. Peaks recorded before the gravity reference was tracked with the +// gyroscope latched tilt as acceleration and had left and right swapped, so they are wrong +// rather than merely old and are not migrated. +inline constexpr std::uint32_t kImuMeterSchemaVersion = 2; inline constexpr std::size_t kImuTrailCapacity = 24; inline constexpr float kStandardGravityMps2 = 9.80665F; inline constexpr float kImuDisplayLimitG = 2.0F; @@ -31,6 +34,8 @@ struct ImuMeterInput { bool x_axis_valid{false}; bool y_axis_valid{false}; bool calibrating{false}; + // Appended: the vertical axis arrives on the sample's z component. + bool z_axis_valid{false}; }; struct PlanarAcceleration { @@ -47,6 +52,10 @@ struct ImuPeakSummary { float right_g{0.0F}; float total_g{0.0F}; PlanarAcceleration total_position{}; + // Kept apart from the horizontal pair and from each other: a kerb strike throws the car + // up and a compression loads it down, and they are different events to a driver. + float up_g{0.0F}; + float down_g{0.0F}; }; struct ImuMeterSnapshot { @@ -54,6 +63,9 @@ struct ImuMeterSnapshot { ImuMeterState state{ImuMeterState::unavailable}; board::DisplayOrientation orientation{board::DisplayOrientation::degrees_0}; PlanarAcceleration current{}; + // Unaffected by the display rotation: up is up however the unit is mounted. + float vertical_g{0.0F}; + bool vertical_valid{false}; ImuPeakSummary peaks{}; std::array trail{}; std::size_t trail_count{0}; diff --git a/firmware/components/ui/include/track_timer/ui/session_review.hpp b/firmware/components/ui/include/track_timer/ui/session_review.hpp index 409f2bd..312cecf 100644 --- a/firmware/components/ui/include/track_timer/ui/session_review.hpp +++ b/firmware/components/ui/include/track_timer/ui/session_review.hpp @@ -35,6 +35,12 @@ struct SessionReviewViewModel { std::array completion{}; std::array integrity{}; std::array message{}; + // What the car pulled. Recorded on the summary since the peaks landed there, and shown + // here because a session without lap times still has this much to say about itself. + std::array peak_total{}; + std::array peak_longitudinal{}; + std::array peak_lateral{}; + std::array peak_vertical{}; std::array laps{}; bool newer_session_enabled{false}; bool older_session_enabled{false}; diff --git a/firmware/components/ui/include/track_timer/ui/session_review_screen.hpp b/firmware/components/ui/include/track_timer/ui/session_review_screen.hpp index f884323..2845507 100644 --- a/firmware/components/ui/include/track_timer/ui/session_review_screen.hpp +++ b/firmware/components/ui/include/track_timer/ui/session_review_screen.hpp @@ -52,6 +52,11 @@ class SessionReviewScreen { lv_obj_t* completion_{nullptr}; lv_obj_t* integrity_{nullptr}; lv_obj_t* message_{nullptr}; + lv_obj_t* peak_caption_{nullptr}; + lv_obj_t* peak_total_{nullptr}; + lv_obj_t* peak_longitudinal_{nullptr}; + lv_obj_t* peak_lateral_{nullptr}; + lv_obj_t* peak_vertical_{nullptr}; std::array lap_panels_{}; std::array lap_labels_{}; std::array lap_durations_{}; diff --git a/firmware/components/ui/session_review.cpp b/firmware/components/ui/session_review.cpp index 2a75459..30bcc0f 100644 --- a/firmware/components/ui/session_review.cpp +++ b/firmware/components/ui/session_review.cpp @@ -237,6 +237,10 @@ void SessionReviewController::update_view() noexcept view_.completion.fill('\0'); view_.integrity.fill('\0'); view_.message.fill('\0'); + view_.peak_total.fill('\0'); + view_.peak_longitudinal.fill('\0'); + view_.peak_lateral.fill('\0'); + view_.peak_vertical.fill('\0'); for (auto& row : view_.laps) { row = {}; } @@ -289,6 +293,22 @@ void SessionReviewController::update_view() noexcept std::snprintf(view_.completion.data(), view_.completion.size(), "ENDED %s", completion_name(summary_.completion_reason)); + // Two decimals, because the difference between 0.94 and 1.02 g is the difference + // between a good corner and a very good one. + const auto& peaks = summary_.peaks; + std::snprintf(view_.peak_total.data(), view_.peak_total.size(), "%.2f", + static_cast(peaks.total_g)); + std::snprintf(view_.peak_longitudinal.data(), view_.peak_longitudinal.size(), + "ACC %.2f BRK %.2f", static_cast(peaks.acceleration_g), + static_cast(peaks.braking_g)); + std::snprintf(view_.peak_lateral.data(), view_.peak_lateral.size(), + "LEFT %.2f RIGHT %.2f", static_cast(peaks.left_g), + static_cast(peaks.right_g)); + // Up and down kept apart: a kerb and a compression are different events. + std::snprintf(view_.peak_vertical.data(), view_.peak_vertical.size(), + "UP %.2f DOWN %.2f", static_cast(peaks.up_g), + static_cast(peaks.down_g)); + if (view_.status == SessionReviewStatus::partial_log) { set_text(view_.integrity, "PARTIAL LOG - RESULTS MAY BE INCOMPLETE"); } diff --git a/firmware/components/ui/session_review_screen.cpp b/firmware/components/ui/session_review_screen.cpp index 8f71ec1..7fd445e 100644 --- a/firmware/components/ui/session_review_screen.cpp +++ b/firmware/components/ui/session_review_screen.cpp @@ -34,31 +34,56 @@ SessionReviewScreen::SessionReviewScreen(lv_obj_t* root, buttons_[1] = make_button(1, SessionReviewAction::older_session, "OLDER " LV_SYMBOL_RIGHT, 460, 8, 120, 56, color::surface); - duration_ = create_label(root_, Typography::caption, color::text_primary, + duration_ = create_label(root_, Typography::timer_secondary, color::text_primary, LV_TEXT_ALIGN_LEFT); - lv_obj_set_pos(duration_, 20, 72); - lv_obj_set_size(duration_, 185, 24); - overrun_ = create_label(root_, Typography::caption, color::caution_bright); - lv_obj_set_pos(overrun_, 205, 72); - lv_obj_set_size(overrun_, 175, 24); - completion_ = create_label(root_, Typography::caption, color::text_primary, - LV_TEXT_ALIGN_RIGHT); - lv_obj_set_pos(completion_, 380, 72); - lv_obj_set_size(completion_, 200, 24); - - integrity_ = create_label(root_, Typography::caption, color::text_secondary, - LV_TEXT_ALIGN_LEFT); - lv_obj_set_pos(integrity_, 20, 102); - lv_obj_set_size(integrity_, 560, 24); - message_ = create_label(root_, Typography::caption, color::caution_bright, + lv_obj_set_pos(duration_, 20, 62); + lv_obj_set_size(duration_, 300, 34); + overrun_ = create_label(root_, Typography::timer_secondary, color::caution_bright, + LV_TEXT_ALIGN_RIGHT); + lv_obj_set_pos(overrun_, 320, 62); + lv_obj_set_size(overrun_, 260, 34); + completion_ = create_label(root_, Typography::body, color::text_primary, + LV_TEXT_ALIGN_LEFT); + lv_obj_set_pos(completion_, 20, 100); + lv_obj_set_size(completion_, 300, 26); + + integrity_ = create_label(root_, Typography::body, color::text_secondary, + LV_TEXT_ALIGN_RIGHT); + lv_obj_set_pos(integrity_, 320, 100); + lv_obj_set_size(integrity_, 260, 26); + message_ = create_label(root_, Typography::body, color::caution_bright, LV_TEXT_ALIGN_LEFT); - lv_obj_set_pos(message_, 20, 128); - lv_obj_set_size(message_, 560, 24); + lv_obj_set_pos(message_, 20, 288); + lv_obj_set_size(message_, 560, 26); + + // The peaks occupy the space the lap rows will take once there is a receiver, so they + // are shown only when a session has no laps to list - which is every session today. + peak_caption_ = create_label(root_, Typography::body, color::text_secondary, + LV_TEXT_ALIGN_LEFT); + lv_obj_set_pos(peak_caption_, 20, 138); + lv_obj_set_size(peak_caption_, 200, 26); + lv_label_set_text(peak_caption_, "PEAK G"); + peak_total_ = create_label(root_, Typography::timer_primary, color::text_primary, + LV_TEXT_ALIGN_LEFT); + lv_obj_set_pos(peak_total_, 20, 160); + lv_obj_set_size(peak_total_, 250, 56); + peak_longitudinal_ = create_label(root_, Typography::body, color::text_primary, + LV_TEXT_ALIGN_LEFT); + lv_obj_set_pos(peak_longitudinal_, 290, 152); + lv_obj_set_size(peak_longitudinal_, 290, 26); + peak_lateral_ = create_label(root_, Typography::body, color::text_primary, + LV_TEXT_ALIGN_LEFT); + lv_obj_set_pos(peak_lateral_, 290, 180); + lv_obj_set_size(peak_lateral_, 290, 26); + peak_vertical_ = create_label(root_, Typography::body, color::text_primary, + LV_TEXT_ALIGN_LEFT); + lv_obj_set_pos(peak_vertical_, 290, 208); + lv_obj_set_size(peak_vertical_, 290, 26); for (std::size_t index = 0; index < lap_panels_.size(); ++index) { auto* panel = lv_obj_create(root_); style_flat_panel(panel, color::surface, 8); - lv_obj_set_pos(panel, 20, 156 + static_cast(index * 40)); + lv_obj_set_pos(panel, 20, 152 + static_cast(index * 40)); lv_obj_set_size(panel, 560, 36); lap_panels_[index] = panel; lap_labels_[index] = create_label(panel, Typography::caption, color::text_primary, @@ -100,6 +125,23 @@ void SessionReviewScreen::update(const SessionReviewViewModel& model) noexcept : color::text_secondary), 0); + lv_label_set_text(peak_total_, model.peak_total.data()); + lv_label_set_text(peak_longitudinal_, model.peak_longitudinal.data()); + lv_label_set_text(peak_lateral_, model.peak_lateral.data()); + lv_label_set_text(peak_vertical_, model.peak_vertical.data()); + + // Peaks and lap rows share the same space, so only one of them is ever up. + const auto show_peaks = model.lap_count == 0 && model.peak_total[0] != '\0'; + for (auto* label : {peak_caption_, peak_total_, peak_longitudinal_, peak_lateral_, + peak_vertical_}) { + if (show_peaks) { + lv_obj_remove_flag(label, LV_OBJ_FLAG_HIDDEN); + } + else { + lv_obj_add_flag(label, LV_OBJ_FLAG_HIDDEN); + } + } + for (std::size_t index = 0; index < model.laps.size(); ++index) { const auto& row = model.laps[index]; if (!row.visible) { diff --git a/firmware/main/screen_router.cpp b/firmware/main/screen_router.cpp index 1d4801b..cf3b6c7 100644 --- a/firmware/main/screen_router.cpp +++ b/firmware/main/screen_router.cpp @@ -23,6 +23,7 @@ #include "track_timer/ui/navigation.hpp" #include "track_timer/ui/presenter.hpp" #include "track_timer/ui/ready_screen.hpp" +#include "track_timer/logger/memory_summary_store.hpp" #include "track_timer/ui/rest_session.hpp" #include "track_timer/ui/session_trigger.hpp" #include "track_timer/ui/session_review.hpp" @@ -39,6 +40,7 @@ #include #include +#include #include #include #include @@ -345,15 +347,21 @@ class ScreenRouter { resolved.lateral_g * imu::kStandardGravityMps2; resolved_sample.acceleration_y_mps2 = resolved.longitudinal_g * imu::kStandardGravityMps2; + // Kerbs and compressions are the events this axis exists for, and it was being + // computed and thrown away here. + resolved_sample.acceleration_z_mps2 = + resolved.vertical_g * imu::kStandardGravityMps2; resolved_sample.valid = resolved.valid; input.sample = resolved_sample; input.sample_available = resolved.valid; input.x_axis_valid = resolved.valid; input.y_axis_valid = resolved.valid; - // No session is wired on the device yet, so peaks are not session-scoped here. - // Session-scoped peaks for Review are tracked in #137. + input.z_axis_valid = resolved.valid; service_session(); - const auto& snapshot = imu_meter_.update(input, false); + // Peaks are scoped to the session so a summary can report what happened during it + // rather than everything since boot. Arming counts: a driver waiting on a launch + // has not started yet, and whatever they did getting to the line is not the session. + const auto& snapshot = imu_meter_.update(input, session_was_active_); // Only when the radar is the visible screen. Repositioning 26 objects and // reformatting five labels on every LVGL iteration is wasted work off-screen, // and it invalidates areas nobody is looking at. @@ -445,7 +453,8 @@ class ScreenRouter { // Restore the track that was selected before the last reboot. restore_selected_track(); refresh_ready(); - review_controller_.begin(nullptr); // no storage backend yet + // In memory for now: the card is where these belong, and that is the next step. + review_controller_.begin(&summaries_); review_->update(review_controller_.view_model()); diagnostics_controller_.begin(device_snapshot()); diagnostics_->update(diagnostics_controller_.view_model()); @@ -966,10 +975,59 @@ class ScreenRouter { // A double tap ends whatever clock is running: the session or its overrun give way to // the rest period, and rest gives way to the dashboard. Two taps rather than one // because this ends a session, and a stray touch on a moving car must not. + // What the car actually did, taken at the moment the session ends and before anything + // resets. Peaks come from the meter, which has been scoped to the session since it + // started, so they describe this session rather than everything since boot. + void capture_summary() noexcept + { + const auto snapshot = session_.snapshot(); + const auto& peaks = imu_meter_.snapshot().peaks; + + logger::SessionSummaryV1 summary{}; + summary.record_size_bytes = static_cast(sizeof(summary)); + std::snprintf(summary.session_id.data(), summary.session_id.size(), "S%03u", + static_cast(++session_ordinal_)); + // Duration is the whole elapsed session, overrun the part of it past the configured + // end, which is why one is never greater than the other. + summary.session_duration_ms = std::max(0, snapshot.session_elapsed_ms); + summary.session_overrun_ms = std::max(0, snapshot.session_overrun_ms); + summary.completion_reason = logger::SessionCompletionReason::driver_stop; + summary.integrity = logger::SummaryIntegrity::complete; + // Honest rather than flattering: there is no receiver, so every session so far is + // recorded without the lap data a driver would expect. + summary.degraded_subsystems = logger::degraded_gnss; + summary.peaks.acceleration_g = peaks.acceleration_g; + summary.peaks.braking_g = peaks.braking_g; + summary.peaks.left_g = peaks.left_g; + summary.peaks.right_g = peaks.right_g; + summary.peaks.up_g = peaks.up_g; + summary.peaks.down_g = peaks.down_g; + summary.peaks.total_g = peaks.total_g; + + if (!summaries_.record(summary)) { + ESP_LOGW("track_timer", "session summary refused by its own validator"); + return; + } + ESP_LOGI("track_timer", + "session summary %s: ran %lld ms (%lld overrun), peak %d milli-g", + summary.session_id.data(), + static_cast(summary.session_duration_ms), + static_cast(summary.session_overrun_ms), + static_cast(summary.peaks.total_g * 1000.0F)); + review_controller_.begin(&summaries_); + if (review_ != nullptr) { + review_->update(review_controller_.view_model()); + } + } + void advance_session_phase() noexcept { const auto now_ms = static_cast(esp_timer_get_time() / 1000); const auto state = session_.snapshot().state; + if (state == session::SessionState::running || + state == session::SessionState::overtime) { + capture_summary(); + } if (session_.request_stop(now_ms) != session::TransitionResult::accepted) { return; } @@ -1212,6 +1270,8 @@ class ScreenRouter { session::SessionController session_{}; ui::ActiveSessionController active_session_{}; bool session_was_active_{false}; + logger::MemorySummaryStore summaries_{}; + std::uint32_t session_ordinal_{0}; bool armed_{false}; bool armed_ready_{false}; settings::SessionTrigger armed_trigger_{settings::SessionTrigger::manual}; diff --git a/simulator/CMakeLists.txt b/simulator/CMakeLists.txt index 44a2ded..0e76817 100644 --- a/simulator/CMakeLists.txt +++ b/simulator/CMakeLists.txt @@ -274,7 +274,7 @@ if(BUILD_TESTING) set_tests_properties( simulator-imu-meter-model PROPERTIES - PASS_REGULAR_EXPRESSION "Bounded G-meter peaks, orientation, reset guard, and IMU states passed" + PASS_REGULAR_EXPRESSION "Bounded G-meter peaks, orientation, reset guard, IMU states, and session-scoped vertical peaks passed" TIMEOUT 20 ) diff --git a/tests/cpp/test_imu_meter.cpp b/tests/cpp/test_imu_meter.cpp index 869d2bf..856bc64 100644 --- a/tests/cpp/test_imu_meter.cpp +++ b/tests/cpp/test_imu_meter.cpp @@ -27,6 +27,17 @@ track_timer::ui::ImuMeterInput sample(const float lateral_g, const float longitu return input; } +// Vertical arrives on the sample's z component and, unlike the horizontal pair, does not +// rotate with the display: up is up however the unit is mounted. +track_timer::ui::ImuMeterInput vertical_sample(const float vertical_g, + const std::uint64_t now_ms) +{ + auto input = sample(0.0F, 0.0F, now_ms); + input.sample.acceleration_z_mps2 = vertical_g * track_timer::ui::kStandardGravityMps2; + input.z_axis_valid = true; + return input; +} + } // namespace int main() @@ -117,6 +128,45 @@ int main() assert(rotated.snapshot().trail_count == 0); assert(near(rotated.snapshot().peaks.total_g, 0.0F)); - std::cout << "Bounded G-meter peaks, orientation, reset guard, and IMU states passed\n"; + // A kerb throws the car up and a compression loads it down. Recording only a magnitude + // would lose which happened, so the two are held apart. + ui::ImuMeterController vertical; + (void)vertical.update(vertical_sample(0.8F, 0), false); + (void)vertical.update(vertical_sample(-1.3F, 100), false); + (void)vertical.update(vertical_sample(0.2F, 200), false); + assert(near(vertical.snapshot().peaks.up_g, 0.8F)); + assert(near(vertical.snapshot().peaks.down_g, 1.3F)); + assert(near(vertical.snapshot().vertical_g, 0.2F)); + assert(vertical.snapshot().vertical_valid); + + // Without the axis the peaks must stay put rather than collapsing to zero. + (void)vertical.update(sample(0.0F, 0.0F, 300), false); + assert(!vertical.snapshot().vertical_valid); + assert(near(vertical.snapshot().peaks.up_g, 0.8F)); + assert(near(vertical.snapshot().peaks.down_g, 1.3F)); + + // Peaks belong to a session. Starting one clears what came before, so a summary reports + // what the car did on track rather than everything since boot - including whatever + // happened carrying the device to the car. + ui::ImuMeterController scoped; + (void)scoped.update(sample(1.5F, 0.0F, 0), false); + (void)scoped.update(vertical_sample(2.0F, 100), false); + assert(scoped.snapshot().peaks.left_g > 1.0F || scoped.snapshot().peaks.right_g > 1.0F); + assert(near(scoped.snapshot().peaks.up_g, 2.0F)); + + (void)scoped.update(sample(0.4F, 0.0F, 200), true); // session starts + assert(near(scoped.snapshot().peaks.up_g, 0.0F)); + assert(!scoped.snapshot().reset_allowed); // and cannot be cleared by hand + + (void)scoped.update(vertical_sample(0.5F, 300), true); + assert(near(scoped.snapshot().peaks.up_g, 0.5F)); + + // Ending the session leaves the peaks standing, because that is when they are read. + (void)scoped.update(sample(0.0F, 0.0F, 400), false); + assert(near(scoped.snapshot().peaks.up_g, 0.5F)); + assert(scoped.snapshot().reset_allowed); + + std::cout << "Bounded G-meter peaks, orientation, reset guard, IMU states, and " + "session-scoped vertical peaks passed\n"; return 0; } diff --git a/tests/cpp/test_summary_store.cpp b/tests/cpp/test_summary_store.cpp new file mode 100644 index 0000000..87145ef --- /dev/null +++ b/tests/cpp/test_summary_store.cpp @@ -0,0 +1,158 @@ +#include "track_timer/logger/memory_summary_store.hpp" + +#include +#include +#include +#include + +namespace { + +using namespace track_timer; + +logger::SessionSummaryV1 summary_of(const char* const id, const std::int64_t duration_ms, + const float total_g = 0.9F) +{ + logger::SessionSummaryV1 summary{}; + summary.record_size_bytes = static_cast(sizeof(summary)); + std::snprintf(summary.session_id.data(), summary.session_id.size(), "%s", id); + summary.session_duration_ms = duration_ms; + summary.session_overrun_ms = 0; + summary.completion_reason = logger::SessionCompletionReason::driver_stop; + summary.integrity = logger::SummaryIntegrity::complete; + summary.degraded_subsystems = logger::degraded_gnss; + summary.peaks.total_g = total_g; + return summary; +} + +// A driver wants the session they just finished, so it has to be the one Review opens on. +void the_newest_session_is_first() +{ + logger::MemorySummaryStore store; + std::size_t count = 0; + assert(store.session_count(count) == logger::SummaryReadResult::empty); + assert(count == 0); + + assert(store.record(summary_of("S001", 20 * 60'000))); + assert(store.record(summary_of("S002", 15 * 60'000))); + assert(store.session_count(count) == logger::SummaryReadResult::ready); + assert(count == 2); + + logger::SessionSummaryV1 read{}; + assert(store.read_summary(0, read) == logger::SummaryReadResult::ready); + assert(std::strcmp(read.session_id.data(), "S002") == 0); + assert(store.read_summary(1, read) == logger::SummaryReadResult::ready); + assert(std::strcmp(read.session_id.data(), "S001") == 0); +} + +// The oldest falls off rather than the newest being refused: the last session matters far +// more than the ninth one back. +void the_oldest_session_is_the_one_lost() +{ + logger::MemorySummaryStore store; + for (std::size_t index = 0; index < logger::MemorySummaryStore::kCapacity + 3; ++index) { + char id[8]{}; + std::snprintf(id, sizeof(id), "S%03u", static_cast(index)); + assert(store.record(summary_of(id, 60'000))); + } + std::size_t count = 0; + assert(store.session_count(count) == logger::SummaryReadResult::ready); + assert(count == logger::MemorySummaryStore::kCapacity); + + logger::SessionSummaryV1 read{}; + assert(store.read_summary(0, read) == logger::SummaryReadResult::ready); + assert(std::strcmp(read.session_id.data(), "S010") == 0); // the most recent + assert(store.read_summary(logger::MemorySummaryStore::kCapacity, read) == + logger::SummaryReadResult::corrupt); +} + +// A malformed record must not reach Review and be rendered as though it were real. +void a_summary_that_fails_its_own_validator_is_refused() +{ + logger::MemorySummaryStore store; + + auto no_identifier = summary_of("", 60'000); + assert(!store.record(no_identifier)); + + auto overrun_exceeds_duration = summary_of("S001", 60'000); + overrun_exceeds_duration.session_overrun_ms = 120'000; + assert(!store.record(overrun_exceeds_duration)); + + // A peak beyond anything a car can pull says the reading is wrong, not remarkable. + auto impossible_peak = summary_of("S002", 60'000, 40.0F); + assert(!store.record(impossible_peak)); + + // A negative peak is not a magnitude at all. + auto negative_peak = summary_of("S003", 60'000); + negative_peak.peaks.braking_g = -1.0F; + assert(!store.record(negative_peak)); + + assert(store.rejected_count() == 4); + std::size_t count = 0; + assert(store.session_count(count) == logger::SummaryReadResult::empty); +} + +// Every axis survives the round trip, which is the whole point of recording them. +void the_peaks_are_carried_through() +{ + logger::MemorySummaryStore store; + auto summary = summary_of("S001", 60'000); + summary.peaks.acceleration_g = 0.62F; + summary.peaks.braking_g = 1.14F; + summary.peaks.left_g = 0.98F; + summary.peaks.right_g = 1.02F; + summary.peaks.up_g = 0.44F; + summary.peaks.down_g = 0.71F; + summary.peaks.total_g = 1.21F; + assert(store.record(summary)); + + logger::SessionSummaryV1 read{}; + assert(store.read_summary(0, read) == logger::SummaryReadResult::ready); + assert(read.peaks.acceleration_g == 0.62F); + assert(read.peaks.braking_g == 1.14F); + assert(read.peaks.left_g == 0.98F); + assert(read.peaks.right_g == 1.02F); + assert(read.peaks.up_g == 0.44F); + assert(read.peaks.down_g == 0.71F); + assert(read.peaks.total_g == 1.21F); +} + +// No receiver means no laps, which is a session with nothing to list rather than a fault. +void a_session_without_laps_reads_as_an_empty_page() +{ + logger::MemorySummaryStore store; + assert(store.record(summary_of("S001", 60'000))); + + logger::SessionSummaryV1 read{}; + assert(store.read_summary(0, read) == logger::SummaryReadResult::ready); + + logger::SummaryLapPage page{}; + assert(store.read_lap_page(read.session_id, 0, page) == logger::SummaryReadResult::ready); + assert(page.count == 0); + assert(page.total_count == 0); +} + +void clearing_empties_it() +{ + logger::MemorySummaryStore store; + assert(store.record(summary_of("S001", 60'000))); + store.clear(); + std::size_t count = 0; + assert(store.session_count(count) == logger::SummaryReadResult::empty); + assert(store.rejected_count() == 0); +} + +} // namespace + +int main() +{ + the_newest_session_is_first(); + the_oldest_session_is_the_one_lost(); + a_summary_that_fails_its_own_validator_is_refused(); + the_peaks_are_carried_through(); + a_session_without_laps_reads_as_an_empty_page(); + clearing_empties_it(); + + std::cout << "Session summary store: newest first, bounded history, refusal of records " + "that fail their own validator, and peaks carried on every axis passed\n"; + return 0; +}