fix(ui): record the G peaks that were measured, not the ones the dial can draw

.gitignore

@@ -25,3 +25,8 @@ Thumbs.db
 
 # Core dumps, which a failing host test binary drops in the repository root.
 core.*
+
+# Blog drafts. Written here because the device is the subject and the facts have to come
+# from the repository, but they are Mark's words for caterham7diaries.com and are not part
+# of this project's history.
+blog/

CHANGELOG.md

@@ -264,6 +264,19 @@ The project follows Semantic Versioning once the first firmware release is tagge
 
 ### Fixed
 
+- recorded G peaks were bounded by what the radar can draw rather than by what the car did.
+  The display clamps a reading to 2 g so the dot stays on the dial, and that clamp was
+  applied before the peaks were taken - so a session summary recorded the limit as though
+  it were a measurement. A hand-shaken session filed `2577 milli-g`, which is two axes
+  pinned at the clamp rather than anything that happened.
+  It bites unevenly, which is why it survived: a car on road tyres reaches about 1.5 g
+  sideways and never approaches the limit, but a kerb strike passes 2 g vertically without
+  trying, and was being filed as exactly 2.00 with nothing to say it had been truncated.
+  The measurement now feeds the peaks and the display clamps its own copy. A credibility
+  bound remains at 10 g, because past that is a sensor fault rather than a lap and the
+  summary validator refuses the record anyway - but a peak that reaches it is now flagged
+  as truncated, since a bound nobody is told about is the defect itself rather than the fix
+
 - the carousel stopped accepting taps, which made every menu, mode, track and setting
   unselectable. `attach_gesture_input` keeps its bindings in a fixed table and refuses
   silently once it is full; Speed Only added a ninth screen to a table of eight, and the

firmware/components/ui/imu_meter.cpp

@@ -12,6 +12,13 @@ float clamp_g(const float value) noexcept
     return std::max(-kImuDisplayLimitG, std::min(kImuDisplayLimitG, value));
 }
 
+// The same shape, a different bound, and a different reason. This one exists to keep a
+// sensor fault out of the session record; the display's exists to keep a dot on screen.
+float clamp_recorded_g(const float value) noexcept
+{
+    return std::max(-kImuRecordedLimitG, std::min(kImuRecordedLimitG, value));
+}
+
 PlanarAcceleration rotate_sample(const ImuMeterInput& input) noexcept
 {
     const auto x = input.sample.acceleration_x_mps2 / kStandardGravityMps2;
@@ -33,8 +40,12 @@ PlanarAcceleration rotate_sample(const ImuMeterInput& input) noexcept
         break;
     }
 
-    point.lateral_g = point.lateral_valid ? clamp_g(point.lateral_g) : 0.0F;
-    point.longitudinal_g = point.longitudinal_valid ? clamp_g(point.longitudinal_g) : 0.0F;
+    // Deliberately unclamped. This is the measurement, and it feeds both the radar and the
+    // recorded peaks - clamping here is what made a session's peak bounded by what the
+    // dial could draw rather than by what the car did (#162). The display clamps its own
+    // copy below, where the limit belongs.
+    point.lateral_g = point.lateral_valid ? point.lateral_g : 0.0F;
+    point.longitudinal_g = point.longitudinal_valid ? point.longitudinal_g : 0.0F;
     return point;
 }
 
@@ -73,17 +84,24 @@ const ImuMeterSnapshot& ImuMeterController::update(const ImuMeterInput& input,
         return snapshot_;
     }
 
-    const auto point = rotate_sample(input);
-    snapshot_.current = point;
-    append(point);
+    const auto measured = rotate_sample(input);
     // 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;
+    const auto measured_vertical_g =
+        snapshot_.vertical_valid ? input.sample.acceleration_z_mps2 / kStandardGravityMps2
+                                 : 0.0F;
 
-    update_peaks(point);
+    // Peaks first, from the measurement, before anything is held back for the dial.
+    update_peaks(measured, measured_vertical_g);
+
+    // Now the drawn copy. The radar and the trail take the clamped values, because a dot
+    // beyond the outer ring is not a reading the display can place.
+    auto point = measured;
+    point.lateral_g = point.lateral_valid ? clamp_g(point.lateral_g) : 0.0F;
+    point.longitudinal_g = point.longitudinal_valid ? clamp_g(point.longitudinal_g) : 0.0F;
+    snapshot_.current = point;
+    append(point);
+    snapshot_.vertical_g = snapshot_.vertical_valid ? clamp_g(measured_vertical_g) : 0.0F;
     increment_saturated(snapshot_.accepted_samples);
 
     if (!point.lateral_valid || !point.longitudinal_valid) {
@@ -154,28 +172,45 @@ void ImuMeterController::append(const PlanarAcceleration point) noexcept
     }
 }
 
-void ImuMeterController::update_peaks(const PlanarAcceleration& point) noexcept
+void ImuMeterController::update_peaks(const PlanarAcceleration& point,
+                                      const float vertical_g) noexcept
 {
+    // Anything past the recorded bound is a sensor fault, not a lap. Recording it would
+    // put the fault in the session summary looking like a measurement, and the summary
+    // validator refuses the record outright beyond 10 g - so it is bounded here and the
+    // fact that it was bounded is kept alongside.
+    const auto record = [this](const float value) {
+        if (std::fabs(value) > kImuRecordedLimitG) {
+            snapshot_.peaks.truncated = true;
+        }
+        return clamp_recorded_g(value);
+    };
+
     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);
+        const auto vertical = record(vertical_g);
+        snapshot_.peaks.up_g = std::max(snapshot_.peaks.up_g, vertical);
+        snapshot_.peaks.down_g = std::max(snapshot_.peaks.down_g, -vertical);
     }
     if (point.longitudinal_valid) {
+        const auto longitudinal = record(point.longitudinal_g);
         snapshot_.peaks.acceleration_g =
-            std::max(snapshot_.peaks.acceleration_g, point.longitudinal_g);
-        snapshot_.peaks.braking_g =
-            std::max(snapshot_.peaks.braking_g, -point.longitudinal_g);
+            std::max(snapshot_.peaks.acceleration_g, longitudinal);
+        snapshot_.peaks.braking_g = std::max(snapshot_.peaks.braking_g, -longitudinal);
     }
     if (point.lateral_valid) {
-        snapshot_.peaks.left_g = std::max(snapshot_.peaks.left_g, -point.lateral_g);
-        snapshot_.peaks.right_g = std::max(snapshot_.peaks.right_g, point.lateral_g);
+        const auto lateral = record(point.lateral_g);
+        snapshot_.peaks.left_g = std::max(snapshot_.peaks.left_g, -lateral);
+        snapshot_.peaks.right_g = std::max(snapshot_.peaks.right_g, lateral);
     }
     if (point.lateral_valid && point.longitudinal_valid) {
-        const auto total = std::sqrt(point.lateral_g * point.lateral_g +
-                                     point.longitudinal_g * point.longitudinal_g);
+        const auto lateral = clamp_recorded_g(point.lateral_g);
+        const auto longitudinal = clamp_recorded_g(point.longitudinal_g);
+        const auto total = std::sqrt(lateral * lateral + longitudinal * longitudinal);
         if (total > snapshot_.peaks.total_g) {
             snapshot_.peaks.total_g = total;
-            snapshot_.peaks.total_position = point;
+            // The position this peak happened at, recorded as measured. It is a record of
+            // where the car was, not a point the radar has to be able to plot.
+            snapshot_.peaks.total_position = {lateral, longitudinal, true, true};
         }
     }
 }

firmware/components/ui/include/track_timer/ui/imu_meter.hpp

@@ -15,7 +15,17 @@ namespace track_timer::ui {
 inline constexpr std::uint32_t kImuMeterSchemaVersion = 2;
 inline constexpr std::size_t kImuTrailCapacity = 24;
 inline constexpr float kStandardGravityMps2 = 9.80665F;
+// What the radar can draw. A dot has to stay on the display, so the drawn value is held
+// inside this - but a recorded peak is a different quantity with a different purpose, and
+// must not inherit a limit that exists for the dial.
 inline constexpr float kImuDisplayLimitG = 2.0F;
+
+// What a recorded peak is allowed to be. Deliberately the same figure as the summary
+// validator's kMaximumCrediblePeakG - beyond it is a sensor fault rather than a heroic
+// lap, and a peak past what the validator accepts would discard the entire session
+// record. The two are declared in different components, so screen_router static_asserts
+// the agreement where they meet rather than leaving it to coincidence.
+inline constexpr float kImuRecordedLimitG = 10.0F;
 inline constexpr std::uint64_t kImuRecoveredNoticeMs = 1'500;
 
 enum class ImuMeterState : std::uint8_t {
@@ -56,6 +66,13 @@ struct ImuPeakSummary {
     // 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};
+    // True when a reading was past kImuRecordedLimitG and the peak above is therefore the
+    // bound rather than the measurement.
+    //
+    // There is still a bound, so there is still a number that could be a clamp presented
+    // as a measurement - which is the whole defect. Saying so is what makes the remaining
+    // limit honest instead of simply moving the lie further out.
+    bool truncated{false};
 };
 
 struct ImuMeterSnapshot {
@@ -86,7 +103,7 @@ class ImuMeterController {
   private:
     void clear_measurements() noexcept;
     void append(PlanarAcceleration point) noexcept;
-    void update_peaks(const PlanarAcceleration& point) noexcept;
+    void update_peaks(const PlanarAcceleration& point, float vertical_g) noexcept;
 
     ImuMeterSnapshot snapshot_{};
     bool loss_seen_{false};

firmware/main/screen_router.cpp

@@ -2081,6 +2081,12 @@ class ScreenRouter {
         summary.peaks.up_g = peaks.up_g;
         summary.peaks.down_g = peaks.down_g;
         summary.peaks.total_g = peaks.total_g;
+        // The meter bounds a recorded peak at the same figure the summary validator calls
+        // credible. They are declared in different components, so the agreement is checked
+        // here, where the two actually meet: a meter bound above the validator's would let
+        // a peak through that silently discards the whole record.
+        static_assert(ui::kImuRecordedLimitG <= logger::kMaximumCrediblePeakG,
+                      "a recorded peak must not exceed what the summary will accept");
 
         // The card is not assumed to be there. A failed write costs the durability of this
         // record and nothing else: the session stays in memory and Review shows it either

simulator/CMakeLists.txt

@@ -283,7 +283,7 @@ if(BUILD_TESTING)
     set_tests_properties(
         simulator-imu-meter-model
         PROPERTIES
-            PASS_REGULAR_EXPRESSION "Bounded G-meter peaks, orientation, reset guard, IMU states, and session-scoped vertical peaks passed"
+            PASS_REGULAR_EXPRESSION "orientation, reset guard, IMU states, and session-scoped vertical peaks passed"
             TIMEOUT 20
     )
 

tests/cpp/test_imu_meter.cpp

@@ -166,7 +166,70 @@ int main()
     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 "
+    // #162. The radar clamps to 2 g so a dot stays on the dial, and that clamp used to be
+    // applied before the peaks were taken - so a session's recorded peak was bounded by
+    // what the display could draw rather than by what the car did. A hand-shaken session
+    // recorded 2577 milli-g, which is both horizontal axes pinned at the limit rather than
+    // a measurement.
+    {
+        ui::ImuMeterController recorder{};
+        auto hard = sample(2.6F, 1.4F, 10);
+        auto recorded = recorder.update(hard, false);
+
+        // Measured, not bounded by the dial.
+        assert(near(recorded.peaks.right_g, 2.6F));
+        assert(near(recorded.peaks.acceleration_g, 1.4F));
+        assert(!recorded.peaks.truncated);
+
+        // The drawn value still is bounded, because the dot has to stay on the display.
+        assert(near(recorded.current.lateral_g, ui::kImuDisplayLimitG));
+        assert(near(recorded.current.longitudinal_g, 1.4F));
+
+        // And the total comes from the measurement rather than from two clamped axes.
+        assert(near(recorded.peaks.total_g, std::sqrt(2.6F * 2.6F + 1.4F * 1.4F)));
+        assert(recorded.peaks.total_g > std::sqrt(2.0F * 2.0F + 1.4F * 1.4F));
+    }
+
+    // Vertical is the axis where this actually bites. A car on road tyres never reaches
+    // 2 g sideways, but a kerb strike passes it easily - and used to be filed as exactly
+    // 2.00 with nothing to say it had been truncated.
+    {
+        ui::ImuMeterController recorder{};
+        auto kerb = vertical_sample(3.4F, 10);
+        auto recorded = recorder.update(kerb, false);
+        assert(near(recorded.peaks.up_g, 3.4F));
+        assert(!recorded.peaks.truncated);
+        // The dial still shows what it can draw.
+        assert(near(recorded.vertical_g, ui::kImuDisplayLimitG));
+
+        auto compression = vertical_sample(-2.9F, 20);
+        recorded = recorder.update(compression, false);
+        assert(near(recorded.peaks.down_g, 2.9F));
+    }
+
+    // A bound remains, so there is still a number that could be a clamp wearing a
+    // measurement's clothes. Past 10 g is a sensor fault rather than a lap - the summary
+    // validator refuses the record beyond it - so it is bounded and says it was.
+    {
+        ui::ImuMeterController recorder{};
+        auto fault = sample(14.0F, 0.2F, 10);
+        auto recorded = recorder.update(fault, false);
+        assert(near(recorded.peaks.right_g, ui::kImuRecordedLimitG));
+        // The flag is the difference between a bound and a lie.
+        assert(recorded.peaks.truncated);
+    }
+
+    // An ordinary session never touches either bound and is never marked.
+    {
+        ui::ImuMeterController recorder{};
+        auto ordinary = sample(1.1F, 0.9F, 10);
+        const auto recorded = recorder.update(ordinary, false);
+        assert(near(recorded.peaks.right_g, 1.1F));
+        assert(!recorded.peaks.truncated);
+    }
+
+    std::cout << "G-meter peaks recorded as measured rather than bounded by the dial, "
+                 "orientation, reset guard, IMU states, and "
                  "session-scoped vertical peaks passed\n";
     return 0;
 }