fix errors handling optional Waypoint fields. (GPSBabel#948)

* fix errors handling optional Waypoint fields. 1. Hide wp_flags from formats by using WAYPT_SET. 2. Don't print values corresponding to Waypoint fields that aren't valid. a) if the field uses wp_flags, then the flag must be set. b) power, cadence heartrate must not be zero. This is in anticipation of use of std::optional instead of wp_flags to indicate an optional value is present. The use of invalid power, cadence, and heartrate values was found fortuitously. Note this changes kml output, using empty gx:value elements to correspond to missing data. gx:value elements are of type string, the empty string do not violate the schema (unlike empty when elements). However, it is unclear who is using this extra SchemaData so we cannot check to make sure they acutally handle empty string values. * don't use WAYPT_HAS with wp_flags that aren't optionals. * fix some bugs with our home grown optionals. mostly not using WAYPT_SET resulting in the value being lost.
jidanni · Jan 6, 2023 · e78e14a · e78e14a
1 parent aac3c51
commit e78e14a
Show file tree

Hide file tree

Showing 12 changed files with 2,590 additions and 2,571 deletions.
diff --git a/defs.h b/defs.h
@@ -319,10 +319,10 @@ struct bounds {
   double min_alt;	/* -unknown_alt => invalid */
 };
 
-#define WAYPT_SET(wpt,member,val) do { (wpt)->member = (val); wpt->wpt_flags.member = 1; } while (0)
-#define WAYPT_GET(wpt,member,def) ((wpt->wpt_flags.member) ? (wpt->member) : (def))
+#define WAYPT_SET(wpt,member,val) do { (wpt)->member = (val); (wpt)->wpt_flags.member = 1; } while (0)
+#define WAYPT_GET(wpt,member,def) (((wpt)->wpt_flags.member) ? ((wpt)->member) : (def))
 #define WAYPT_UNSET(wpt,member) wpt->wpt_flags.member = 0
-#define WAYPT_HAS(wpt,member) (wpt->wpt_flags.member)
+#define WAYPT_HAS(wpt,member) ((wpt)->wpt_flags.member)
 
 /*
  * This is a waypoint, as stored in the GPSR.   It tries to not
@@ -1083,7 +1083,7 @@ int color_to_bbggrr(const char* cname);
  * A constant for unknown altitude.   It's tempting to just use zero
  * but that's not very nice for the folks near sea level.
  */
-#define unknown_alt 	-99999999.0
-#define unknown_color	-1
+constexpr double unknown_alt = -99999999.0;
+constexpr int unknown_color = -1;
 
 #endif // DEFS_H_INCLUDED_
diff --git a/dg-100.cc b/dg-100.cc
@@ -228,8 +228,7 @@ Dg100Format::process_gpsfile(uint8_t data[], route_head** track) const
        * with a scaling factor of 100, in km/h.
        * The waypoint struct wants the speed as a
        * floating-point number, in m/s. */
-      wpt->speed = KPH_TO_MPS(be_read32(data + i + 16) / 100.0);
-      wpt->wpt_flags.speed = 1;
+      WAYPT_SET(wpt, speed, KPH_TO_MPS(be_read32(data + i + 16) / 100.0));
     }
 
     if (style >= 2) {

diff --git a/garmin.cc b/garmin.cc
@@ -640,10 +640,8 @@ pvt2wpt(GPS_PPvt_Data pvt, Waypoint* wpt)
   wpt->altitude = pvt->alt;
   wpt->latitude = pvt->lat;
   wpt->longitude = pvt->lon;
-  WAYPT_SET(wpt,course,1);
-  WAYPT_SET(wpt,speed,1);
 
-  wpt->course = 180 + DEG(std::atan2(-pvt->east, -pvt->north));
+  WAYPT_SET(wpt, course, 180 + DEG(std::atan2(-pvt->east, -pvt->north)));
 
   /* velocity in m/s */
   WAYPT_SET(wpt,speed, std::sqrt(pvt->north*pvt->north + pvt->east*pvt->east));

diff --git a/garmin_fit.h b/garmin_fit.h
@@ -100,12 +100,12 @@ class GarminFitFormat : public Format
       : lat(wpt.latitude),
         lon(wpt.longitude),
         altitude(wpt.altitude),
-        speed(WAYPT_HAS((&wpt), speed) ? wpt.speed : -1),
+        speed(WAYPT_GET(&wpt, speed, -1)),
         odometer_distance(wpt.odometer_distance),
         creation_time(wpt.creation_time),
         shortname(wpt.shortname),
         is_course_point(is_course_point),
-        course_point_type(course_point_type) { }
+        course_point_type(course_point_type) {}
     double lat, lon, altitude;
     double speed, odometer_distance;
     gpsbabel::DateTime creation_time;

diff --git a/globalsat_sport.cc b/globalsat_sport.cc
@@ -644,7 +644,7 @@ GlobalsatSportFormat::track_read()
               wpt->longitude = ((int32_t) point.Longitude) / 1000000.0;
               wpt->latitude = ((int32_t) point.Latitude) / 1000000.0;
               wpt->altitude = point.Altitude;
-              wpt->speed = ((double) point.Speed / 100.0) * 1000.0 / 3600.0;
+              WAYPT_SET(wpt, speed, ((double) point.Speed / 100.0) * 1000.0 / 3600.0);
               wpt->heartrate = point.HeartRate;
               wpt->cadence = point.Cadence;	//TODO convert in any way??
               wpt->power = point.Power;	//TODO convert in any way??

diff --git a/kml.cc b/kml.cc
@@ -846,7 +846,8 @@ void KmlFormat::kml_output_point(const Waypoint* waypointp, kml_point_type pt_ty
     } else {
       if (trackdirection && (pt_type == kmlpt_track)) {
         QString value;
-        if (waypointp->speed < 1) {
+        if (!WAYPT_HAS(waypointp, speed) || !WAYPT_HAS(waypointp, course) ||
+            (waypointp->speed < 1.0f)) {
           value = QStringLiteral("%1-none").arg(style);
         } else {
           value = QStringLiteral("%1-%2").arg(style)
@@ -1471,19 +1472,24 @@ void KmlFormat::kml_mt_simple_array(const route_head* header,
 
     switch (member) {
     case fld_power:
-      writer->writeTextElement(QStringLiteral("gx:value"), QString::number(wpt->power, 'f', 1));
+      writer->writeTextElement(QStringLiteral("gx:value"), wpt->power?
+        QString::number(wpt->power, 'f', 1) : QString());
       break;
     case fld_cadence:
-      writer->writeTextElement(QStringLiteral("gx:value"), QString::number(wpt->cadence));
+      writer->writeTextElement(QStringLiteral("gx:value"), wpt->cadence?
+       QString::number(wpt->cadence) : QString());
       break;
     case fld_depth:
-      writer->writeTextElement(QStringLiteral("gx:value"), QString::number(wpt->depth, 'f', 1));
+      writer->writeTextElement(QStringLiteral("gx:value"), WAYPT_HAS(wpt, depth)?
+        QString::number(wpt->depth, 'f', 1) : QString());
       break;
     case fld_heartrate:
-      writer->writeTextElement(QStringLiteral("gx:value"), QString::number(wpt->heartrate));
+      writer->writeTextElement(QStringLiteral("gx:value"), wpt->heartrate?
+       QString::number(wpt->heartrate) : QString());
       break;
     case fld_temperature:
-      writer->writeTextElement(QStringLiteral("gx:value"), QString::number(wpt->temperature, 'f', 1));
+      writer->writeTextElement(QStringLiteral("gx:value"), WAYPT_HAS(wpt, temperature)?
+        QString::number(wpt->temperature, 'f', 1) : QString());
       break;
     default:
       fatal("Bad member type");

diff --git a/qstarz_bl_1000.cc b/qstarz_bl_1000.cc
@@ -31,7 +31,7 @@
 #include <QDebug>              // for QDebug
 #include <QFile>               // for QFile
 #include <QIODevice>           // for QIODevice, QIODevice::ReadOnly
-#include "defs.h"              // for Waypoint, ddmm2degrees, route_head, track_add_head, track_add_wpt, waypt_add, waypt_count, wp_flags, fix_unknown, fix_2d, fix_3d, fix_dgps, fix_none, fix_pps, fix_type, global_options, global_opts
+#include "defs.h"
 #include "src/core/logging.h"  // for Fatal
 
 
@@ -233,11 +233,9 @@ QstarzBL1000Format::qstarz_bl_1000_read_record(QDataStream& stream, route_head*
   waypoint->fix = fix;
   waypoint->sat = satelliteCountUsed;
 
-  waypoint->speed = KPH_TO_MPS(speed);
-  waypoint->wpt_flags.speed = 1;
+  WAYPT_SET(waypoint, speed, KPH_TO_MPS(speed));
 
-  waypoint->course = heading;
-  waypoint->wpt_flags.course = 1;
+  WAYPT_SET(waypoint, course, heading);
   waypoint->SetCreationTime(time, milliseconds);
 
   auto* fsdata = new qstarz_bl_1000_fsdata;

diff --git a/reference/realtime.kml b/reference/realtime.kml
@@ -187,40 +187,40 @@
           <SchemaData schemaUrl="#schema">
             <gx:SimpleArrayData name="cadence">
               <gx:value>245</gx:value>
-              <gx:value>0</gx:value>
-              <gx:value>0</gx:value>
-              <gx:value>0</gx:value>
-              <gx:value>0</gx:value>
+              <gx:value></gx:value>
+              <gx:value></gx:value>
+              <gx:value></gx:value>
+              <gx:value></gx:value>
               <gx:value>146</gx:value>
               <gx:value>204</gx:value>
               <gx:value>108</gx:value>
-              <gx:value>0</gx:value>
+              <gx:value></gx:value>
               <gx:value>120</gx:value>
               <gx:value>165</gx:value>
               <gx:value>1</gx:value>
               <gx:value>186</gx:value>
               <gx:value>156</gx:value>
-              <gx:value>0</gx:value>
+              <gx:value></gx:value>
               <gx:value>33</gx:value>
               <gx:value>40</gx:value>
             </gx:SimpleArrayData>
             <gx:SimpleArrayData name="depth">
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
               <gx:value>201.2</gx:value>
               <gx:value>76.0</gx:value>
-              <gx:value>0.0</gx:value>
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
+              <gx:value></gx:value>
               <gx:value>249.4</gx:value>
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
               <gx:value>978.7</gx:value>
               <gx:value>511.7</gx:value>
               <gx:value>509.5</gx:value>
               <gx:value>407.5</gx:value>
               <gx:value>136.0</gx:value>
               <gx:value>275.3</gx:value>
               <gx:value>680.8</gx:value>
-              <gx:value>0.0</gx:value>
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
+              <gx:value></gx:value>
               <gx:value>630.0</gx:value>
             </gx:SimpleArrayData>
             <gx:SimpleArrayData name="heartrate">
@@ -229,9 +229,9 @@
               <gx:value>151</gx:value>
               <gx:value>4</gx:value>
               <gx:value>168</gx:value>
-              <gx:value>0</gx:value>
+              <gx:value></gx:value>
               <gx:value>145</gx:value>
-              <gx:value>0</gx:value>
+              <gx:value></gx:value>
               <gx:value>55</gx:value>
               <gx:value>247</gx:value>
               <gx:value>119</gx:value>
@@ -240,45 +240,45 @@
               <gx:value>77</gx:value>
               <gx:value>2</gx:value>
               <gx:value>96</gx:value>
-              <gx:value>0</gx:value>
+              <gx:value></gx:value>
             </gx:SimpleArrayData>
             <gx:SimpleArrayData name="temperature">
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
               <gx:value>8.2</gx:value>
               <gx:value>5.2</gx:value>
               <gx:value>13.7</gx:value>
               <gx:value>12.6</gx:value>
               <gx:value>19.8</gx:value>
               <gx:value>24.7</gx:value>
               <gx:value>10.2</gx:value>
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
               <gx:value>2.7</gx:value>
               <gx:value>2.7</gx:value>
               <gx:value>24.6</gx:value>
               <gx:value>16.9</gx:value>
               <gx:value>23.9</gx:value>
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
               <gx:value>19.0</gx:value>
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
             </gx:SimpleArrayData>
             <gx:SimpleArrayData name="power">
               <gx:value>88.9</gx:value>
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
               <gx:value>81.3</gx:value>
               <gx:value>378.5</gx:value>
               <gx:value>85.3</gx:value>
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
               <gx:value>384.3</gx:value>
               <gx:value>61.9</gx:value>
               <gx:value>262.2</gx:value>
-              <gx:value>0.0</gx:value>
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
+              <gx:value></gx:value>
               <gx:value>493.0</gx:value>
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
               <gx:value>350.7</gx:value>
               <gx:value>114.8</gx:value>
               <gx:value>402.7</gx:value>
-              <gx:value>0.0</gx:value>
+              <gx:value></gx:value>
             </gx:SimpleArrayData>
           </SchemaData>
         </ExtendedData>

diff --git a/reference/track/gpx_garmin_extensions-kml_track.kml b/reference/track/gpx_garmin_extensions-kml_track.kml
@@ -160,8 +160,8 @@
                 <gx:value>153</gx:value>
                 <gx:value>153</gx:value>
                 <gx:value>153</gx:value>
-                <gx:value>0</gx:value>
-                <gx:value>0</gx:value>
+                <gx:value></gx:value>
+                <gx:value></gx:value>
                 <gx:value>76</gx:value>
                 <gx:value>124</gx:value>
               </gx:SimpleArrayData>