nmea: reject stale-valid fixes on poor HDOP or non-3D type

gp_gga_parse() only skipped *promoting* location_valid to
LOC_VALID_NEW on a low satellite count, but never demoted it - so a
low-quality epoch could coast on LOC_VALID left over from the last
good fix and still get accepted downstream. Now also gates on HDOP
(read fresh from GGA's own field, since GSA - which normally supplies
System.hdop_x100 - is parsed later in the epoch) and on fix_type
being 3D, since a 2D fix can't solve for altitude and reports a
held-over or drifting value instead.

soft/nmea.c

@@ -209,9 +209,19 @@ static time_t gp_rmc_parse(const char *str) {
 	return utc;
 }
 
+/* A fix is only trusted (for acceptance into gpx_process_point(), the speed
+ * average, and the auto-pause speed-based unpause check) when it is valid,
+ * of a known type, and reasonably accurate: a single sample from a poor fix
+ * (e.g. HDOP 16 with only a few satellites) would otherwise distort the
+ * whole-session average, corrupt the track with a wild position/altitude
+ * outlier, or - worse - talk the auto-pause logic into resuming a stationary
+ * logger. */
+#define FIX_QUALITY_HDOP_MAX_X100	500	/* a fix is untrustworthy once HDOP > 5.00 */
+
 static void gp_gga_parse(const char *str) {
 	const char *p;
-	double tmp;
+	double tmp, hdop;
+	unsigned int hdop_x100;
 
 	p = gp_col(str, 7); /* satellites used */
 	if (!p)
@@ -225,6 +235,15 @@ static void gp_gga_parse(const char *str) {
 		System.sat_count_low = 1;
 	}
 
+	/* HDOP, from this same sentence: GSA (which also reports HDOP, into
+	 * System.hdop_x100) is parsed after GGA within the epoch, so its value
+	 * is still last epoch's here - read GGA's own HDOP field instead. */
+	p = gp_col(str, 8);
+	if (!p)
+		return;
+	xatof(&p, &hdop);
+	hdop_x100 = (unsigned int)(hdop*100.0 + 0.5);
+
 	/* check validity */
 	p = gp_col(str, 6);
 	if (!p)
@@ -234,8 +253,24 @@ static void gp_gga_parse(const char *str) {
 		return;
 	}
 
-	if (!System.sat_count_low)
-		System.location_valid = LOC_VALID_NEW; /* don't accept the coordinates otherwise, even if reported valid */
+	/* Reject (rather than just not-promote) on a low satellite count, poor
+	 * HDOP, or anything short of a 3D fix, so a bad fix can't coast on
+	 * LOC_VALID left over from the last good epoch - otherwise a
+	 * low-quality point (wrong position and/or altitude) would still be
+	 * accepted downstream. A 2D fix in particular can't solve for altitude
+	 * at all (only 3 satellites: 2 horizontal DOF + time, no vertical) and
+	 * reports a held-over or drifting one instead - seen after a real
+	 * reacquisition, where a 2D fix reported an altitude a few hundred
+	 * meters off before enough satellites came in for a real 3D fix.
+	 * System.fix_type is set by GSA, parsed after GGA within the epoch, so
+	 * it's last epoch's value here - same staleness as HDOP above, but a
+	 * bad 2D fix reliably spans several consecutive epochs, so a one-epoch
+	 * lag isn't a practical gap here (unlike a true point fix). */
+	if (!System.sat_count_low && hdop_x100 <= FIX_QUALITY_HDOP_MAX_X100
+			&& System.fix_type == FIX_TYPE_3D)
+		System.location_valid = LOC_VALID_NEW;
+	else
+		System.location_valid = LOC_INVALID;
 
 	/* parse location */
 	p = gp_col(str, 2);		/* latitude */
@@ -285,13 +320,6 @@ static void gp_gga_parse(const char *str) {
 	location.time = utc; /* parsed from RMC */
 }
 
-/* A fix is only trusted (for the speed average, and for the auto-pause
- * speed-based unpause check below) when it is valid, of a known type, and
- * reasonably accurate: a single sample from a poor fix (e.g. HDOP 16 with
- * only a few satellites) would otherwise distort the whole-session average,
- * or - worse - talk the auto-pause logic into resuming a stationary logger. */
-#define FIX_QUALITY_HDOP_MAX_X100	500	/* a fix is untrustworthy once HDOP > 5.00 */
-
 unsigned char gps_fix_trustworthy(void) {
 	return (FLAGS & F_GPSOK) && System.fix_type != FIX_TYPE_UNKNOWN
 		&& System.hdop_x100 <= FIX_QUALITY_HDOP_MAX_X100;