[patch] Preserve Speed from Source Data (inside track_recompute)

[Chris Tracy-6]

Hello,

  I'm working on a couple patches to kml.c to get familiar with the
GPSBabel codebase.  One of these is a patch to add "speed as altitude"
functionality similar to that described here (not my idea, but I thought
it'd be nice to have in GPSBabel):

http://www.eoss.org/wbaltrak/Google_Earth_export.htm

In so doing, I've run into issues with track_recompute() in route.c,
specifically where it recomputes speed based on elapsed time and distance
for each waypoint.

  The source data I'm using is actually NMEA sentences generated
from MS Flight Simulator 2004.  When parsed with GPSBabel, all the
position, altitude, and time information is correct, but speed is
problematic.  The source data is generally updated once a second, but
occasionally there are one second gaps.  After running the NMEA through
GPSBabel going to KML, the speed data for sequential seconds isn't great,
but is close.  However, at the points where there's a missed second in the
original data, the speed is computed incorrect by basically 50%.

  I don't know enough about computing distance from lat/lon to say
where the exact issue is, but I'm willing to bet it's in MSFS.  However,
as this is the only issue, and as the source MSFS data contains "correct"
speeds, I created a patch to GPSBabel that adds a "-P" option to preserve
source speed data.  This lets track_recompute() be called (to do things
like figure out min/max speed/altitude, etc) while not changing the speeds
received as input data, solving my intermediate issue.

  I'm not sure I've done it the best way (or used the best CLI
flag), and would be happy to rework this such that it's more correct, if
someone would be willing to point me in the right direction.

  Thanks,

  Chris

If anyone's interested, I can provide source data from MSFS if someone
would like to check it against the distance/speed calculations in
track_recompute().

Index: defs.h
===================================================================
RCS file: /cvsroot/gpsbabel/gpsbabel/defs.h,v
retrieving revision 1.248
diff -u -r1.248 defs.h
--- defs.h 26 Apr 2009 04:13:07 -0000 1.248
+++ defs.h 12 Jul 2009 23:57:26 -0000
@@ -201,6 +201,7 @@
  cet_cs_vec_t *charset;
  char *charset_name;
  inifile_t *inifile;
+ int preserve_source_speed;
 } global_options;
 
 extern global_options global_opts;
Index: main.c
===================================================================
RCS file: /cvsroot/gpsbabel/gpsbabel/main.c,v
retrieving revision 1.86
diff -u -r1.86 main.c
--- main.c 2 Jul 2009 17:37:18 -0000 1.86
+++ main.c 12 Jul 2009 23:57:26 -0000
@@ -151,6 +151,7 @@
 "    -b               Process command file (batch mode)\n"
 "    -c               Character set for next operation\n"
 "    -N               No smart icons on output\n"
+"    -P               Preserve (don't recompute) speeds from original data\n"
 "    -x filtername    Invoke filter (placed between inputs and output) \n"
 "    -D level         Set debug level [%d]\n"
 "    -l               Print GPSBabel builtin character sets and exit\n"
@@ -505,6 +506,9 @@
  }
 
  break;
+ case 'P':
+ global_opts.preserve_source_speed = 1;
+ break;
  /*
  * Undocumented '-vs' option for GUI wrappers.
  */
Index: route.c
===================================================================
RCS file: /cvsroot/gpsbabel/gpsbabel/route.c,v
retrieving revision 1.45
diff -u -r1.45 route.c
--- route.c 6 Sep 2008 21:44:44 -0000 1.45
+++ route.c 12 Jul 2009 23:57:26 -0000
@@ -568,7 +568,9 @@
  * If we've moved as much as a meter, recompute speed.
  */
  if (timed && (dist > 1)) {
- WAYPT_SET(this, speed, dist / labs(timed));
+ if(!global_opts.preserve_source_speed) {
+ WAYPT_SET(this, speed, dist / labs(timed));
+ }
  if (this->speed > tdata->max_spd) {
  tdata->max_spd = this->speed;
  }

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge  
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize  
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Gpsbabel-code mailing list  http://www.gpsbabel.org
Gpsbabel-code@...
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code

[Robert Lipe-4]

On Sun, Jul 12, 2009 at 7:20 PM, Chris Tracy <gpsbabel@...> wrote:

Hello,

       I'm working on a couple patches to kml.c to get familiar with the GPSBabel codebase.  One of these is a patch to add "speed as altitude" functionality similar to that described here (not my idea, but I thought it'd be nice to have in GPSBabel):

http://www.eoss.org/wbaltrak/Google_Earth_export.htm

I've seen this type of visualization in Earth.   It's handy, but it's hard for us to present a UI that scales well for this kind of control; what works great for a drag racer works poorly for a bicyclist.   So I've kind of left the custom KML to the tools that are really specialized KML writers.

 

information is correct, but speed is problematic.  The source data is generally updated once a second, but occasionally there are one second gaps.  After running the NMEA through GPSBabel going to KML, the speed data for sequential seconds isn't great, but is close.  However, at the points where there's a missed second in the original data, the speed is computed incorrect by basically 50%.

If there is a timespan of two seconds, wouldn't the distance also be 2x and the speed wash out?


 

"-P" option to preserve source speed data.  This lets track_recompute() be called (to do things like figure out min/max speed/altitude, etc) while not changing the speeds received

This probably shouldn't be a global flag like this.

I'd come closer to getting behind a patch to track_recompute() that just didn't overwrite the speed.   Something like replacing the WAYPT_SET(...speed...) with

 if WAYPT_UNSET(wpt, speed)
   WAYPT_SET(wpt, speed, blah)
 

Then we could retain the speed from your NMEA data instead of replacing it with our recomputed.

Does that get you where you want to be?

RJL

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge  
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize  
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Gpsbabel-code mailing list  http://www.gpsbabel.org
Gpsbabel-code@...
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code

[Chris Tracy-6]

Robert,

> I've seen this type of visualization in Earth.   It's handy, but it's hard
> for us to present a UI that scales well for this kind of control; what works
> great for a drag racer works poorly for a bicyclist.   So I've kind of left
> the custom KML to the tools that are really specialized KML writers.

  I was sort of afraid of that, but figured I'd give it a go and
submit a patch when I had it polished enough that I thought it was
generally useful.  Worst that happens is that you don't apply it.  I can
live with that.  :-)

> If there is a timespan of two seconds, wouldn't the distance also be 2x and
> the speed wash out?

  You would think so, but no, not in this case.  I don't fully
understand why, but I've not really looked into too deeply.  I can pass on
an example file (just a bunch of NMEA sentences) if you (or someone else)
wants to look at an example.

> I'd come closer to getting behind a patch to track_recompute() that just
> didn't overwrite the speed.   Something like replacing the
> WAYPT_SET(...speed...) with
>
>  if WAYPT_UNSET(wpt, speed)
>    WAYPT_SET(wpt, speed, blah)
>  
>
> Then we could retain the speed from your NMEA data instead of replacing it
> with our recomputed.
>
> Does that get you where you want to be?

  That would absolutely work for my needs.  I just figured it'd be
too much of a change to the fundamental functionality of
track_recompute().  However, if that's the direction you'd prefer, I'll
work up a new patch.

  Thanks,

  Chris
------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge  
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize  
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Gpsbabel-code mailing list  http://www.gpsbabel.org
Gpsbabel-code@...
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code

[Robert Lipe-4]

On Mon, Jul 20, 2009 at 9:00 AM, Chris Tracy <gpsbabel@...> wrote:

If there is a timespan of two seconds, wouldn't the distance also be 2x and
the speed wash out?

       You would think so, but no, not in this case.  I don't fully understand why, but I've not really looked into too

That sounds kind of bogus, but I kind of quit being surprised at the data I've seen in data files about a million users ago. :-)

I'd come closer to getting behind a patch to track_recompute() that just
didn't overwrite the speed.   Something like replacing the
WAYPT_SET(...speed...) with

 if WAYPT_UNSET(wpt, speed)
   WAYPT_SET(wpt, speed, blah)
 

Then we could retain the speed from your NMEA data instead of replacing it
with our recomputed.

Does that get you where you want to be?

       That would absolutely work for my needs.  I just figured it'd be too much of a change to the fundamental functionality of track_recompute().  However, if that's the direction you'd prefer, I'll work up a new patch.

track_recompute() has a history that's hard to feel good about.   When we were working on the playback animation for Earth 4.0, I needed to compute the heading/course so we could set the camera angle.  I also wanted to collect some simple statistics to put in the balloon and left hand panel summary.   Most of the data didn't have speed and I wanted to display that, so I just always calculated it.  (Most of the things GPSBabel support don't *really* have a concept of speed.)   When I did the contract work for Franson's gpssim target about the same time, it needed similar functionality, so I hoisted this code from the KML writer into common code and didn't really scrub it of Earth-isms.   Since that time, a few other modules have started using track_recompute() and sometimes get hosed by some of the original decisions.

I'd say that t_r's goal is to ensure that course, speed, and friends have *something* in them upon return.   If you have a device/format that actually records course or speed that's somehow better than what we'd compute (and our computations aren't broken) it doesn't seem unreasonable to me to use the existing data instead of clobbering it with our own.

RJL

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge  
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize  
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Gpsbabel-code mailing list  http://www.gpsbabel.org
Gpsbabel-code@...
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code

[Chris Tracy-6]

Robert,

  Attached is a patch that mods track_recompute() as you specified.
It will compute a speed for a waypoint iff that waypoint doesn't already
have a speed defined.

  If it needs to be changed, just let me know.

  Chris


Note that this change causes testo to fail for my VPL reference data.
Seems the gpx I sent you didn't include speed data.  (because of the bug
in vpl.c I sent you a fix for earlier, I only just now got around to
figuring out how to run testo...I was overthinking it :-)  Will send an
updated VPL reference shortly.  (Otherwise, testo seems happy with
this patch...I think)
Index: route.c
===================================================================
RCS file: /cvsroot/gpsbabel/gpsbabel/route.c,v
retrieving revision 1.45
diff -u -r1.45 route.c
--- route.c 6 Sep 2008 21:44:44 -0000 1.45
+++ route.c 21 Jul 2009 04:26:46 -0000
@@ -565,10 +565,15 @@
  }
 
  /*
- * If we've moved as much as a meter, recompute speed.
+ * If we've moved as much as a meter,
+ * conditionally recompute speeds.
  */
  if (timed && (dist > 1)) {
- WAYPT_SET(this, speed, dist / labs(timed));
+ if(!WAYPT_HAS(this, speed)) {
+ // Only recompute speed if the waypoint
+ // didn't already have a speed
+ WAYPT_SET(this, speed, dist / labs(timed));
+ }
  if (this->speed > tdata->max_spd) {
  tdata->max_spd = this->speed;
  }

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge  
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize  
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Gpsbabel-code mailing list  http://www.gpsbabel.org
Gpsbabel-code@...
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code

[Robert Lipe-4]

On Mon, Jul 20, 2009 at 11:37 PM, Chris Tracy <gpsbabel@...> wrote:

Robert,

       Attached is a patch that mods track_recompute() as you specified. It will compute a speed for a waypoint iff that waypoint doesn't already have a speed defined.

Thanx.  Applied.

 

       If it needs to be changed, just let me know.

Since it's basically the very two-liner that I suggested, I'm not sensing an argument. :-)
 

Note that this change causes testo to fail for my VPL reference data. Seems the gpx I sent you didn't include speed data.  (because of the bug in vpl.c I sent you a fix for earlier, I only just now got around to figuring out how to run testo...I was overthinking it :-)  Will send an updated VPL reference shortly.  (Otherwise, testo seems happy with this patch...I think)

It looks like the committed VPL reference dat has speed.  Will investigate this.

RJL

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge  
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize  
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Gpsbabel-code mailing list  http://www.gpsbabel.org
Gpsbabel-code@...
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code