Fix Catmull-Rom interpolation bugs and clean up crossing point search

- Fix Bug B: Use Catmull-Rom only for lat/lng, linear for time/odometer
  to prevent spline overshoot producing garbage unsigned long values
- Fix Bug C: Store previous GPS fix snapshot and insert as pre-crossing
  buffer entry, giving Catmull-Rom its p0 control point on fast passes
- Clean up interpolateCrossingPoint: rename misleading "best" variables
  to "crossing", remove dead comparison (break makes it single-match),
  deduplicate t/distA/distB computation across interpolation branches
- Update CLAUDE.md known issues to reflect Catmull-Rom fix

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: TheAngryRaven

CLAUDE.md

@@ -139,7 +139,7 @@ DovesLapTimer/
 
 ## Known Issues & TODOs (from code comments)
 
-1. **Catmull-Rom interpolation bug**: basic_oled_example forces linear with comment "CURRENTLY REQUIRED: BUG IN CATMULROM INTERPOLATION METHOD"
+1. ~~**Catmull-Rom interpolation bug**~~: Fixed - spline now only used for lat/lng (not time/odometer), and previous GPS fix inserted as pre-crossing control point
 2. **Altitude messing up distance**: `loop()` line 27 has TODO: "I think alt is messing up, investigate more... maybe flag?"
 3. **Early abort bug**: checkStartFinish line 148-149 TODO about aborting early causing re-check immediately
 4. **`checkStartFinish` portability**: Line 106 TODO to make more portable for split timing

src/DovesLapTimer.cpp

@@ -102,6 +102,14 @@ int DovesLapTimer::loop(double currentLat, double currentLng, float currentAltit
     }
   }
 
+  // Save current fix as previous for next iteration's Catmull-Rom pre-crossing point
+  prevFixLat = currentLat;
+  prevFixLng = currentLng;
+  prevFixTime = millisecondsSinceMidnight;
+  prevFixOdometer = totalDistanceTraveled;
+  prevFixSpeedKmh = currentSpeedkmh;
+  hasPrevFix = true;
+
   return nearAnyLine ? 0 : -1;
 }
 
@@ -256,9 +264,18 @@ bool DovesLapTimer::checkStartFinish(double currentLat, double currentLng) {
       debugln();
       debugln(F("we are possibly crossing"));
       crossing = true;
-      // crossingStartedLineSide = pointOnSideOfLine(currentLat, currentLng, startFinishPointALat, startFinishPointALng, startFinishPointBLat, startFinishPointBLng);
 
-      // Capture this first point - it's important context for Catmull-Rom interpolation
+      // Insert previous GPS fix as pre-crossing point (gives Catmull-Rom its p0 control point)
+      if (hasPrevFix) {
+        crossingPointBuffer[crossingPointBufferIndex].lat = prevFixLat;
+        crossingPointBuffer[crossingPointBufferIndex].lng = prevFixLng;
+        crossingPointBuffer[crossingPointBufferIndex].time = prevFixTime;
+        crossingPointBuffer[crossingPointBufferIndex].odometer = prevFixOdometer;
+        crossingPointBuffer[crossingPointBufferIndex].speedKmh = prevFixSpeedKmh;
+        crossingPointBufferIndex = (crossingPointBufferIndex + 1) % crossingPointBufferSize;
+      }
+
+      // Capture current point
       crossingPointBuffer[crossingPointBufferIndex].lat = currentLat;
       crossingPointBuffer[crossingPointBufferIndex].lng = currentLng;
       crossingPointBuffer[crossingPointBufferIndex].time = millisecondsSinceMidnight;
@@ -449,16 +466,15 @@ double DovesLapTimer::catmullRom(double p0, double p1, double p2, double p3, dou
 void DovesLapTimer::interpolateCrossingPoint(double& crossingLat, double& crossingLng, unsigned long& crossingTime, double& crossingOdometer, double pointALat, double pointALng, double pointBLat, double pointBLng) {
   int numPoints = crossingPointBufferFull ? crossingPointBufferSize : crossingPointBufferIndex;
 
-  // Variables to store the best pair of points
-  int bestIndexA = -1;
-  int bestIndexB = -1;
-  double bestSumDistances = INFINITY;
+  // Find the first pair of consecutive buffer points on opposite sides of the crossing line.
+  // In a normal pass there is exactly one such pair - the two GPS fixes that straddle the line.
+  int crossingIndexA = -1;
+  int crossingIndexB = -1;
+  double crossingSumDistances = INFINITY;
 
-  // Iterate through the crossingPointBuffer, comparing the sum of distances from the start/finish line of each pair of consecutive points
   for (int i = 0; i < numPoints - 1; i++) {
     double distA = pointLineSegmentDistance(crossingPointBuffer[i].lat, crossingPointBuffer[i].lng, pointALat, pointALng, pointBLat, pointBLng);
     double distB = pointLineSegmentDistance(crossingPointBuffer[i + 1].lat, crossingPointBuffer[i + 1].lng, pointALat, pointALng, pointBLat, pointBLng);
-    double sumDistances = distA + distB;
 
     int sideA = pointOnSideOfLine(crossingPointBuffer[i].lat, crossingPointBuffer[i].lng, pointALat, pointALng, pointBLat, pointBLng);
     int sideB = pointOnSideOfLine(crossingPointBuffer[i + 1].lat, crossingPointBuffer[i + 1].lng, pointALat, pointALng, pointBLat, pointBLng);
@@ -474,71 +490,64 @@ void DovesLapTimer::interpolateCrossingPoint(double& crossingLat, double& crossi
     debug(F(" sideB: "));
     debug(sideB);
     debug(F(" sum: "));
-    debugln(sumDistances, 2);
-
-    // Update the best pair of points if the current pair has a smaller sum of distances and the points are on opposite sides of the line
-    // todo: investigate if accuracy "on line" is good enough to avoid interpolation
-    if (sumDistances < bestSumDistances && sideA != sideB) {
-      debug(F("new best sum: "));
-      debugln(sumDistances, 2);
-      bestSumDistances = sumDistances;
-      bestIndexA = i;
-      bestIndexB = i + 1;
-      // this seems safe, as soon as we cross the line, no two points can be closer...
+    debugln(distA + distB, 2);
+
+    // First pair on opposite sides of the line = the crossing pair
+    if (sideA != sideB) {
+      crossingIndexA = i;
+      crossingIndexB = i + 1;
+      crossingSumDistances = distA + distB;
+      debug(F("crossing pair found, sum: "));
+      debugln(crossingSumDistances, 2);
       break;
     }
   }
-  debug(F("bestSumDistances: "));
-  debugln(bestSumDistances);
+  debug(F("crossingSumDistances: "));
+  debugln(crossingSumDistances);
 
-  // Make sure we found a valid pair of points
-  if (bestSumDistances < crossingThresholdMeters && bestIndexA != -1 && bestIndexB != -1) {
+  // Validate: the crossing pair must exist and be close enough to the line
+  if (crossingSumDistances < crossingThresholdMeters && crossingIndexA != -1 && crossingIndexB != -1) {
     debugln(F("~~~ VALID CROSSING ~~~"));
 
+    // Compute the interpolation factor (t) from distances and speeds at the crossing pair
+    double distA = pointLineSegmentDistance(crossingPointBuffer[crossingIndexA].lat, crossingPointBuffer[crossingIndexA].lng, pointALat, pointALng, pointBLat, pointBLng);
+    double distB = pointLineSegmentDistance(crossingPointBuffer[crossingIndexB].lat, crossingPointBuffer[crossingIndexB].lng, pointALat, pointALng, pointBLat, pointBLng);
+    double t = interpolateWeight(distA, distB, crossingPointBuffer[crossingIndexA].speedKmh, crossingPointBuffer[crossingIndexB].speedKmh);
+
+    // Time and odometer are always interpolated linearly (monotonic values that
+    // should not overshoot), regardless of the interpolation mode for position.
+    double deltaOdometer = crossingPointBuffer[crossingIndexB].odometer - crossingPointBuffer[crossingIndexA].odometer;
+    double deltaTime = crossingPointBuffer[crossingIndexB].time - crossingPointBuffer[crossingIndexA].time;
+    crossingOdometer = crossingPointBuffer[crossingIndexA].odometer + t * deltaOdometer;
+    crossingTime = crossingPointBuffer[crossingIndexA].time + t * deltaTime;
+
     if (forceLinear) {
-      // Linear interpolation
-      double distA = pointLineSegmentDistance(crossingPointBuffer[bestIndexA].lat, crossingPointBuffer[bestIndexA].lng, pointALat, pointALng, pointBLat, pointBLng);
-      double distB = pointLineSegmentDistance(crossingPointBuffer[bestIndexB].lat, crossingPointBuffer[bestIndexB].lng, pointALat, pointALng, pointBLat, pointBLng);
-      double t = interpolateWeight(distA, distB, crossingPointBuffer[bestIndexA].speedKmh, crossingPointBuffer[bestIndexB].speedKmh);
-
-      double deltaLat = crossingPointBuffer[bestIndexB].lat - crossingPointBuffer[bestIndexA].lat;
-      double deltaLon = crossingPointBuffer[bestIndexB].lng - crossingPointBuffer[bestIndexA].lng;
-      double deltaOdometer = crossingPointBuffer[bestIndexB].odometer - crossingPointBuffer[bestIndexA].odometer;
-      double deltaTime = crossingPointBuffer[bestIndexB].time - crossingPointBuffer[bestIndexA].time;
-
-      crossingLat = crossingPointBuffer[bestIndexA].lat + t * deltaLat;
-      crossingLng = crossingPointBuffer[bestIndexA].lng + t * deltaLon;
-      crossingOdometer = crossingPointBuffer[bestIndexA].odometer + t * deltaOdometer;
-      crossingTime = crossingPointBuffer[bestIndexA].time + t * deltaTime;
+      // Linear interpolation for position
+      double deltaLat = crossingPointBuffer[crossingIndexB].lat - crossingPointBuffer[crossingIndexA].lat;
+      double deltaLon = crossingPointBuffer[crossingIndexB].lng - crossingPointBuffer[crossingIndexA].lng;
+
+      crossingLat = crossingPointBuffer[crossingIndexA].lat + t * deltaLat;
+      crossingLng = crossingPointBuffer[crossingIndexA].lng + t * deltaLon;
     } else {
-      // Catmull-Rom spline interpolation requires 4 control points:
-      // index0 (before A), index1 (A), index2 (B), index3 (after B)
-      // Check bounds: we need bestIndexA >= 1 and bestIndexB <= numPoints - 2
-      bool canUseCatmullRom = (bestIndexA >= 1) && (bestIndexB <= numPoints - 2);
+      // Catmull-Rom spline interpolation for position only.
+      // Requires 4 control points: p0 (before A), p1 (A), p2 (B), p3 (after B)
+      bool canUseCatmullRom = (crossingIndexA >= 1) && (crossingIndexB <= numPoints - 2);
 
       if (!canUseCatmullRom) {
-        // Not enough points for Catmull-Rom, fall back to linear interpolation
+        // Not enough points for Catmull-Rom, fall back to linear
         debugln(F("Catmull-Rom: insufficient control points, using linear fallback"));
 
-        double distA = pointLineSegmentDistance(crossingPointBuffer[bestIndexA].lat, crossingPointBuffer[bestIndexA].lng, pointALat, pointALng, pointBLat, pointBLng);
-        double distB = pointLineSegmentDistance(crossingPointBuffer[bestIndexB].lat, crossingPointBuffer[bestIndexB].lng, pointALat, pointALng, pointBLat, pointBLng);
-        double t = interpolateWeight(distA, distB, crossingPointBuffer[bestIndexA].speedKmh, crossingPointBuffer[bestIndexB].speedKmh);
-
-        double deltaLat = crossingPointBuffer[bestIndexB].lat - crossingPointBuffer[bestIndexA].lat;
-        double deltaLon = crossingPointBuffer[bestIndexB].lng - crossingPointBuffer[bestIndexA].lng;
-        double deltaOdometer = crossingPointBuffer[bestIndexB].odometer - crossingPointBuffer[bestIndexA].odometer;
-        double deltaTime = crossingPointBuffer[bestIndexB].time - crossingPointBuffer[bestIndexA].time;
+        double deltaLat = crossingPointBuffer[crossingIndexB].lat - crossingPointBuffer[crossingIndexA].lat;
+        double deltaLon = crossingPointBuffer[crossingIndexB].lng - crossingPointBuffer[crossingIndexA].lng;
 
-        crossingLat = crossingPointBuffer[bestIndexA].lat + t * deltaLat;
-        crossingLng = crossingPointBuffer[bestIndexA].lng + t * deltaLon;
-        crossingOdometer = crossingPointBuffer[bestIndexA].odometer + t * deltaOdometer;
-        crossingTime = crossingPointBuffer[bestIndexA].time + t * deltaTime;
+        crossingLat = crossingPointBuffer[crossingIndexA].lat + t * deltaLat;
+        crossingLng = crossingPointBuffer[crossingIndexA].lng + t * deltaLon;
       } else {
         // We have 4 valid control points for Catmull-Rom
-        int index0 = bestIndexA - 1;
-        int index1 = bestIndexA;
-        int index2 = bestIndexB;
-        int index3 = bestIndexB + 1;
+        int index0 = crossingIndexA - 1;
+        int index1 = crossingIndexA;
+        int index2 = crossingIndexB;
+        int index3 = crossingIndexB + 1;
 
         debugln(F("Catmull-Rom: using spline interpolation"));
         debug(F("  indices: "));
@@ -550,22 +559,14 @@ void DovesLapTimer::interpolateCrossingPoint(double& crossingLat, double& crossi
         debug(F(", "));
         debugln(index3);
 
-        // Compute the interpolation factor based on distance and speed
-        double distA = pointLineSegmentDistance(crossingPointBuffer[index1].lat, crossingPointBuffer[index1].lng, pointALat, pointALng, pointBLat, pointBLng);
-        double distB = pointLineSegmentDistance(crossingPointBuffer[index2].lat, crossingPointBuffer[index2].lng, pointALat, pointALng, pointBLat, pointBLng);
-        double t = interpolateWeight(distA, distB, crossingPointBuffer[index1].speedKmh, crossingPointBuffer[index2].speedKmh);
-
-        // Perform Catmull-Rom spline interpolation for latitude, longitude, time, and odometer
+        // Catmull-Rom for lat/lng only - spline smoothing helps with curved paths
         crossingLat = catmullRom(crossingPointBuffer[index0].lat, crossingPointBuffer[index1].lat, crossingPointBuffer[index2].lat, crossingPointBuffer[index3].lat, t);
         crossingLng = catmullRom(crossingPointBuffer[index0].lng, crossingPointBuffer[index1].lng, crossingPointBuffer[index2].lng, crossingPointBuffer[index3].lng, t);
-        crossingTime = catmullRom(crossingPointBuffer[index0].time, crossingPointBuffer[index1].time, crossingPointBuffer[index2].time, crossingPointBuffer[index3].time, t);
-        crossingOdometer = catmullRom(crossingPointBuffer[index0].odometer, crossingPointBuffer[index1].odometer, crossingPointBuffer[index2].odometer, crossingPointBuffer[index3].odometer, t);
       }
     }
   } else {
     debugln(F("~~~ INVALID CROSSING ~~~ INVALID CROSSING ~~~ INVALID CROSSING ~~~ INVALID CROSSING ~~~"));
   }
-  return;
 }
 
 /////////// sector timing helper methods
@@ -736,7 +737,17 @@ bool DovesLapTimer::checkSectorLine(double currentLat, double currentLng, double
       debugln(F(" crossing zone"));
       crossingFlag = true;
 
-      // Capture this first point - it's important context for Catmull-Rom interpolation
+      // Insert previous GPS fix as pre-crossing point (gives Catmull-Rom its p0 control point)
+      if (hasPrevFix) {
+        crossingPointBuffer[crossingPointBufferIndex].lat = prevFixLat;
+        crossingPointBuffer[crossingPointBufferIndex].lng = prevFixLng;
+        crossingPointBuffer[crossingPointBufferIndex].time = prevFixTime;
+        crossingPointBuffer[crossingPointBufferIndex].odometer = prevFixOdometer;
+        crossingPointBuffer[crossingPointBufferIndex].speedKmh = prevFixSpeedKmh;
+        crossingPointBufferIndex = (crossingPointBufferIndex + 1) % crossingPointBufferSize;
+      }
+
+      // Capture current point
       crossingPointBuffer[crossingPointBufferIndex].lat = currentLat;
       crossingPointBuffer[crossingPointBufferIndex].lng = currentLng;
       crossingPointBuffer[crossingPointBufferIndex].time = millisecondsSinceMidnight;
@@ -789,6 +800,12 @@ void DovesLapTimer::reset() {
   positionPrevLng = 0;
   positionPrevAlt = 0;
   firstPositionReceived = false;
+  prevFixLat = 0;
+  prevFixLng = 0;
+  prevFixTime = 0;
+  prevFixOdometer = 0;
+  prevFixSpeedKmh = 0;
+  hasPrevFix = false;
 
   // Reset the crossingPointBuffer index and full status
   crossing = false;

src/DovesLapTimer.h

@@ -496,6 +496,14 @@ class DovesLapTimer {
   double positionPrevLng = 0.00;
   bool firstPositionReceived = false;  // Explicit flag for first GPS fix detection
 
+  // Previous GPS fix snapshot (used as Catmull-Rom pre-crossing control point)
+  double prevFixLat = 0;
+  double prevFixLng = 0;
+  unsigned long prevFixTime = 0;
+  float prevFixOdometer = 0;
+  float prevFixSpeedKmh = 0;
+  bool hasPrevFix = false;
+
   double startFinishPointALat;
   double startFinishPointALng;
   double startFinishPointBLat;