Quellcode durchsuchen

main: fix stale current_pause_start corrupting stopped/moving time

current_pause_start was only ever set inside gpx_write(), deferred
until the next accepted point after a pause began. A near-stationary
point - exactly what triggers a pause - is also exactly what the
min-distance/Kalman/distdiff rejects in gpx_process_point() would
drop, so it could be a long time (or never, before the pause ends)
before that next point arrived. Meanwhile get_pause_time() would add
elapsed time against whatever stale value current_pause_start was
left holding from a previous, unrelated pause - producing an
impossible drop in stopped time (and corresponding spike in moving
time) between consecutive summaries, and absurd average speeds.

Add pause_time_start()/pause_time_end(), called exactly once at the
actual pause/resume transition (auto_pause_activate()/auto_unpause()
in autopause.c, tracking_pause() in working_modes.c) instead of being
tied to GPX point acceptance. Mirrored in gps-test-tool/main.h, which
keeps its own copy of these helpers since soft/main.c isn't linked
into the PC build.

Verified against a real log where the bug produced alternating
moving=Xh59m/stopped=0h00m and moving=0h00m/stopped=Xh59m summaries;
after the fix, moving/stopped are monotonic across every summary.
k4be vor 1 Woche
Ursprung
Commit
ea60116417
6 geänderte Dateien mit 61 neuen und 9 gelöschten Zeilen
  1. 16 3
      gps-test-tool/main.h
  2. 3 0
      soft/autopause.c
  3. 0 3
      soft/gpx.c
  4. 34 3
      soft/main.c
  5. 2 0
      soft/main.h
  6. 6 0
      soft/working_modes.c

+ 16 - 3
gps-test-tool/main.h

@@ -290,11 +290,24 @@ static inline unsigned char tracking_is_paused(void) {
     return System.tracking_paused || System.tracking_auto_paused;
 }
 
+/* Kept in sync with soft/main.c's pause_time_start()/pause_time_end()/
+ * get_pause_time(): call *_start()/*_end() exactly once, at the moment a
+ * pause actually begins/ends (not deferred to the next accepted point,
+ * which may never come before the pause ends, or may come from a much
+ * later, unrelated pause) - see soft/main.c for the full rationale. */
+static inline void pause_time_start(void) {
+    System.current_pause_start = utc;
+}
+
+static inline void pause_time_end(void) {
+    if (System.current_pause_start && System.current_pause_start >= System.time_start)
+        System.pause_time += utc - System.current_pause_start;
+    System.current_pause_start = 0;
+}
+
 static inline time_t get_pause_time(void) {
     time_t res = System.pause_time;
-    if (System.current_pause_start < System.time_start)
-        System.current_pause_start = System.time_start;
-    if (is_paused() && System.current_pause_start)
+    if (is_paused() && System.current_pause_start && System.current_pause_start >= System.time_start)
         res += utc - System.current_pause_start;
     return res;
 }

+ 3 - 0
soft/autopause.c

@@ -35,12 +35,15 @@ static void auto_unpause(void) {
 	if (!System.tracking_auto_paused)
 		return;
 	System.tracking_auto_paused = 0;
+	if (!System.tracking_paused)
+		pause_time_end();
 	log_pause_event(0);
 	beep(50, 4);
 }
 
 static void auto_pause_activate(void) {
 	System.tracking_auto_paused = 1;
+	pause_time_start();
 	/* Otherwise, speed-counter progress from just before this decision (e.g.
 	 * 2 of the 3 consecutive above-threshold samples the resume check below
 	 * wants) could carry over and complete on the very next sample,

+ 0 - 3
soft/gpx.c

@@ -164,7 +164,6 @@ unsigned char gpx_write(struct location_s *loc, FIL *file) {
 			strcpy_P(buf, xml_trkseg_end);
 			gpx.paused = 1;
 			gpx.point_count = 0;
-			System.current_pause_start = utc;
 		} else {
 			return 0; /* nothing to store */
 		}
@@ -173,8 +172,6 @@ unsigned char gpx_write(struct location_s *loc, FIL *file) {
 			strcpy_P(buf, xml_trkseg_start);
 			f_write(file, buf, strlen(buf), &bw);
 			gpx.paused = 0;
-			if (System.current_pause_start)
-				System.pause_time += utc - System.current_pause_start;
 		}
 		time = get_iso_time(loc->time, 0);
 		xsprintf(buf, PSTR("\t\t\t<trkpt lat=\"%.8f\" lon=\"%.8f\">\n\t\t\t\t<ele>%.2f</ele>\n\t\t\t\t<time>%s</time>\n"), loc->lat, loc->lon, loc->alt, time);

+ 34 - 3
soft/main.c

@@ -400,11 +400,42 @@ unsigned int get_dist_avg_speed_x100(void) {
 	return (unsigned int)((unsigned long int)System.distance * 36UL / (10UL * moving));
 }
 
+/* Call exactly once, at the moment tracking_paused/tracking_auto_paused
+ * actually transitions to true (auto_pause_activate(), tracking_pause()) -
+ * not deferred until the next accepted point, which may never come before
+ * the pause ends (a near-stationary point, which is exactly what triggers a
+ * pause, is also exactly what a min-distance/Kalman/distdiff reject in
+ * gpx_process_point() would drop) or may come from a much later, unrelated
+ * pause, corrupting current_pause_start into a stale value that then leaks
+ * into get_pause_time() and the pause_time carried forward on resume. */
+void pause_time_start(void) {
+	System.current_pause_start = utc;
+}
+
+/* Call exactly once, at the moment tracking_paused/tracking_auto_paused
+ * actually transitions to false (auto_unpause(), tracking_pause()) - folds
+ * this pause's duration into the permanent accumulator and clears the
+ * start, so a later summary/query while not paused never reuses it. */
+void pause_time_end(void) {
+	/* Same time_start floor as get_pause_time() - a pause that started (and
+	 * possibly also ended) before the first fix contributes no pause time,
+	 * since logging itself hadn't started yet either. */
+	if (System.current_pause_start && System.current_pause_start >= System.time_start)
+		System.pause_time += utc - System.current_pause_start;
+	System.current_pause_start = 0;
+}
+
 time_t get_pause_time(void) {
 	time_t res = System.pause_time;
-	if (System.current_pause_start < System.time_start)
-		System.current_pause_start = System.time_start; /* disallow negative pause time */
-	if (is_paused() && System.current_pause_start)
+	/* current_pause_start is 0 whenever there's no in-progress pause to add -
+	 * either genuinely not paused, or paused but reset_counters() cleared it
+	 * (along with time_start) after pause_time_start() ran for this pause.
+	 * The time_start floor guards the one remaining case reset_counters()
+	 * doesn't cover: pause_time_start() ran before the first fix (e.g. the
+	 * boot-time "don't log until moving" pause), leaving current_pause_start
+	 * at an earlier (possibly 0) utc than time_start - only time since
+	 * logging actually started counts as pause time. */
+	if (is_paused() && System.current_pause_start && System.current_pause_start >= System.time_start)
 		res += utc - System.current_pause_start;
 	return res;
 }

+ 2 - 0
soft/main.h

@@ -279,6 +279,8 @@ void reset_counters(void);
 time_t get_pause_time(void);
 unsigned int get_logging_time(void);
 unsigned char tracking_is_paused(void);
+void pause_time_start(void);
+void pause_time_end(void);
 unsigned int get_avg_speed_x100(void);
 unsigned int get_dist_avg_speed_x100(void);
 

+ 6 - 0
soft/working_modes.c

@@ -1,6 +1,8 @@
 #include "main.h"
 
 void tracking_pause(unsigned char cmd, unsigned char display) {
+	unsigned char was_paused = System.tracking_paused;
+
 	switch (cmd) {
 		case TRACKING_PAUSE_CMD_TOGGLE:
 			System.tracking_paused = !System.tracking_paused;
@@ -13,10 +15,14 @@ void tracking_pause(unsigned char cmd, unsigned char display) {
 			break;
 	}
 	if (System.tracking_paused) {
+		if (!was_paused && !System.tracking_auto_paused)
+			pause_time_start();
 		LEDB_ON();
 		if (display)
 			display_event(DISPLAY_EVENT_TRACKING_PAUSED);
 	} else {
+		if (was_paused && !System.tracking_auto_paused)
+			pause_time_end();
 		LEDB_OFF();
 		if (display)
 			display_event(DISPLAY_EVENT_TRACKING_RESUMED);