Re: [Gpsbabel-code] [Gpsbabel-misc] Gpsbabel and 1.4.2 and 1.4.4 incorrectly handling gpxtpx extens
Brought to you by:
robertl
From: Robert L. <rob...@gp...> - 2013-02-24 21:15:12
|
>From a real computer where I can more easily talk in code before my next nap. The test file I was working with: roberts-imac:gpsbabel-gpx robertlipe$ cat testo.d/garmin_tpx.test # # Test garmin <extensions> in GPX writer # rm -f ${TMPDIR}/tpx-* gpsbabel -i gpx -f ${REFERENCE}/track/gpx_garmin_extensions.gpx -o gpx,garminextensions -F ${TMPDIR}/tpx-sample.gpx compare ${REFERENCE}/track/gpx_garmin_extensions.gpx ${TMPDIR}/tpx-sample.gpx This simple change over what you've checked in (thank you) demonstrates that tension I spoke of. Index: gpx.cc =================================================================== --- gpx.cc (revision 4325) +++ gpx.cc (working copy) @@ -391,8 +391,8 @@ { tt_garmin_wpt_phone_nr, 0, GARMIN_WPT_EXT "/gpxx:PhoneNumber", 0UL }, // In Garmin space, but in core of waypoint. - { tt_trk_trkseg_trkpt_heartrate, 0, GARMIN_TRK_EXT "/gpxtpx:hr", 0UL }, - { tt_trk_trkseg_trkpt_cadence, 0, GARMIN_TRK_EXT "/gpxtpx:cad", 0UL }, + { tt_trk_trkseg_trkpt_heartrate, 1, GARMIN_TRK_EXT "/gpxtpx:hr", 0UL }, + { tt_trk_trkseg_trkpt_cadence, 1, GARMIN_TRK_EXT "/gpxtpx:cad", 0UL }, { tt_humminbird_wpt_depth, 0, "/gpx/wpt/extensions/h:depth", 0UL }, // in centimeters. { tt_humminbird_wpt_status, 0, "/gpx/wpt/extensions/h:status", 0UL }, Changing that (completely underdocumented) flag tells our GPX writer, "no, don't bother to write that tag; we'll just let it get sucked into the fs_print_chain stuff." Oddly, it does that somewhat by cooperating with the GPX reader - you can't write what you didn't remember on read. Of course, we have code that DOES write those tags so we then end up with two complete <extensions><gpx:TrackPointExtension>" blocks in each trackpoint. We also end up losing the <extensions> block in the <wpt>, but I think that's because we have an explicit match on that in the reader { tt_wpt_extensions, 0, "/gpx/wpt/extensions", 0UL }, so it's not falling into the passthrough. It looks like we have three fundamental schemes at play in the GPX writer and they just interact badly. In the order they were implemented, we have: 1) The original "don't write what you don't know" scheme was mine from the days before GPX 1.0 was finalized and probably like the third or fourth format in GPSBabel. That's the explicit "write this tag with this value" stuff. 2) Ron's "I don't know what this is, but I know it came between <foo> and <bar>, so I'll smuggle it along. That's the original fprint_xml_chain() stuff. 3) Olaf's hybrid of the above + the format-specific data which you can see in FS_GPX and garmin_fs.cc. This seems to basically subclass struct waypoint and adds a couple of dozen things that are present across several Garmin formats like GDB, MPS, FIT, and their GPX extensions, but only marginally Garmin-specific. (Associating a phone number or a street address with a position isn't exactly zany, though I'm not sure I want it in the base element - it's a lovely reason to make waypoint smaller and start subclassing it, though...) I think we "just" have formalize the boundaries between these. I'm not sure it's right, but as a starting point, let's consider this proposal. If we're not using the garmin or humminbird flags from the command line, just pass everything through blindly as per Ron's model. If either of those are present, we take control and explicitly write what we know. Someone might legitimately want a GPX 1.1 file with <extensions> for both Humminbird and Garmin, right? (That's kind of nutty...maybe we don't support that. But publishing a marine plot with depths that can be read by both kinds of devices might actually be reasonable) so we'd have to sandwich them up <extensions> <gpxtpx:tpe> <gpx:depth>117</gpx:depth> </gpxtpx:tpe> <h:depth>117</h:depth> </extensions> ..and that's a level of ordering and control we won't get from passthrough - we'd have to manually pull out the <extensions> and such and write all those manually. Triggering the "explicitly generate this stuff if garminextensions" path also lets us write this if the data came into our data structures via some non-GPX read path, such as Unicsv. We still have to find interactions with other Garmin extensions and be sure we don't screw up the GPX 1.0 stuff (geocaching.com pocket query users are a big user base of ours) It turns out there's a surprising amount of "Oh, Dear" exposed by this seemingly simple bugreport. On Sun, Feb 24, 2013 at 1:44 PM, Robert Lipe <rob...@gp...>wrote: > > On Sun, Feb 24, 2013 at 12:38 PM, tsteven4 <tst...@gm...> wrote: > >> I fixed a number of little issues with the new gpx writer, but the case >> Angel is interested in still fails as it did in 1.4.4. To this point I >> have been concentrating on restoring missing functionality with the new gpx >> writer as opposed to fixing long standing bugs. >> > > Sorry you had to see that. There's a lot of "slash and burn" in there > right now, so I appreciate the extra eyes on it. Some of the stuff I did > on Thursday was really dicey. I really was just powering through to get > testo passing and then planned a manual review of all the old/new cases to > handle the rest before tossing the OLDGPX stuff. I won't miss manually > formatting indenting and remembering to manually entity encode things. > > > >> We do have, and have had for some time, a conflict with unrecognized >> elements we attempt to pass through and the existing partial support for >> some recognized elements and their children, e.g. Garmin >> TrackPointExtensions. >> > > That is indeed the root of this specific problem. > passthrough/fs_print_chain and "I know how to format that tag" are clearly > at war with each other in this regard. I had some work in progress > yesterday (between naps) that made things better, but a fundamental rethink > is probably in order. It's probably not crazy to think about things like > converting a garmin HR extension to a humminbird HR extension (or depth or > whatever) and for that, passthrough just doesn't work. But without > passthrough, we only write what we know. I don't know what the winning > move here is yet. > > > >> I can manually construct sample gpx files to test from the schema, but >> perhaps it would be better to have sample files from a real device. Can >> you provide short sample(s) containing elements form the Garmin extensions >> for test? It would be nice to have samples that demonstrate the Garmin >> WaypointExtensions, RouteExtensions, TrackExtensions and >> TrackPointExtensions. >> > > There is a reference file in our tree, used for read only (conversion to > KML, I think), that has :hr and :cad in our test suite that I was using for > experimentation. I wrote a test case to prove we could reproduce it but > have committed none of that as it didn't actually work very well and I was > just moving problems around. I looked in our reference for something that > used the county/address/color stuff and couldn't find that it was tested at > all. > > >> The TrackPointExtension in >> http://www8.garmin.com/xmlschemas/GpxExtensionsv3.xsd seems to have been >> replaced by http://www8.garmin.com/xmlschemas/TrackPointExtensionv1.xsd. >> Our existing code refers to the later schema for TrackPointExtensions and >> the former for the other extensions. >> > > The reference file actually refers to both. From Garmin's doc, I can't > figure out what's in and what's out. TPE says it replaces GE, but that's > clearly not the case; it's a subset of that one field, at best. TPE > doesn't seem to in/clude things like cadence, which is definitely in the > reference file I was looking at. (Which isn't of clear heritage...) > Since Garmin has always been terrible about documenting even their "open" > formats, the thing to do is probably grab some files from representative > devices, running the latest firmware, and subset them to see what's > actually in play. Basecamp and/or their sporting thingy may add > additional kinks. It's also weird to have TRACKpoint extensions appearing > in WAYpoints, but our sample file clearly has it. > > Figuring out what is "right" will probably be harder than actually making > our writer do it. > > > >> P.S. Robert, best wishes for a speedy and complete recovery. >> > > Thanx. The story is long, but I have about 12 inches of incisions on the > bottom and a giant boot on my left foot (and a head full of > morphene/oxycodone) for a while. > > > > >> >> >> On 2/21/2013 11:12 PM, Robert Lipe wrote: >> >> Moved to list instead of list administrator. PLEASE, people, quit doing >> that. The list administrator is an audience of one that doesn't have >> access to every configuration, sometimes travels, sometimes disappears for >> a while without a lot of notice (I'm having surgery to remove my Plantar >> Fascia and the associated Fibroma that's residing on it in the morning, I >> can't personally support all the many millions of GPSBabel users, etc....) >> >> We've always placed more effort in READING proprietary extensions over >> writing proprietary extensions. Maybe that's a Stallman-like act of >> passive aggression, but it just makes more sense to move your data OUT of >> proprietary formats than to help put them in. But a case of reading GPX, >> filtering, and then writing GPX doesn't really hold up to that ethics test. >> The reality is that it looks like we've screwed up the >> gpxtpx:TrackPointExtension and have *never* actually written it correctly. >> Garmin's GPX (at least used to) change a lot from device to device and >> from firmware rev to firmware rev and we really never got on the treadmill >> of *which* garmin extensions were safe to use. >> >> Additionally, there's all kinds of stuff like >> http://www8.garmin.com/xmlschemas/GpxExtensionsv3.xsd (which isn't >> actually the XSD referenced in the sample file I have...) saying that >> temperature is stored in a different tag for a trackpoint than a waypoint >> (what!?!?!) and our writer just doesn't cope with that. >> >> It looks like Garmin as at least slowed down on cranking out the GPX >> mutations, but the doc I'm looking at doesn't match the sample files I'm >> looking at which, for example, contain hear rate. Our GPX writer try to >> warp around Garmin extensions, but frankly, they do a poor job of capturing >> the current versions via a combination of neglect and just incompetence. >> I'm working on making the GPX writer WAY easier to work on, but this is an >> area for someone that's meticulous about staring at sample files, writing >> tests, and making largely mechanical changes in gpx.cc to improve things. >> Any takers? For example, there's a TrackPointExtension in >> http://www8.garmin.com/xmlschemas/GpxExtensionsv3.xsd >> >> This would be a good area for someone with attention to detail, willing >> to collect a few sample files to deterimine what's actually relevant, and >> work through test cases, etc. could contribute a lot to the project. The >> *code* isn't actually that hard and a partnership between someone taking >> the time to figure out "yeah, they don't actually discuss heart rate and >> cadence in the spec, but the following market leading units spit them into >> trackpoints and waypoint" could really help a code guy sling those bits. >> >> >> I've made a horrible mess out of gpx.cc trying to make this work better >> (I've succeeded in some ways, but failed in others, so I'm not committing >> it) but I have to leave it for a while right now and I should shift my >> focus to rearranging my body so I can walk effectively again. >> >> If any of you are really interested in driving the state of our Garmin >> Extensions writer - and you don't actually have to BE a coder, though the >> ability to read code would help - this is an invitation to step forward. >> >> >> On Thu, Feb 21, 2013 at 5:07 AM, Angel J. Garcia Adeva < >> ang...@eh...> wrote: >> >>> Hi all, >>> >>> I've noticed recently that gpsbabel behaves weird when dealing with some >>> garmin extensions (heart rate in my case) in gpx files. Typically, each >>> node in my input gpx file looks like this: >>> >>> <trkpt lat="42.86463333333333" lon="-2.7025183333333334"> >>> <ele>573.6</ele> >>> <time>2013-02-20T20:55:18.16</time> >>> <extensions> >>> <gpxtpx:TrackPointExtension> >>> <gpxtpx:hr>92</gpxtpx:hr> >>> </gpxtpx:TrackPointExtension> >>> </extensions> >>> </trkpt> >>> >>> Since my gps unit has a very high acquisition rate (that's a matter for >>> another question to the forum some other day...), I usually filter them >>> to get more manageable files. The command I use is: >>> >>> gpsbabel -i gpx -f input.gpx -x position,distance=1.5m -o gpx -F >>> output.gpx >>> >>> The problem is that the resulting node looks like this: >>> >>> <trkpt lat="42.864633333" lon="-2.702518333"> >>> <ele>573.600000</ele> >>> <time>2013-02-20T20:55:18.160Z</time> >>> <extensions> >>> <gpxtpx:TrackPointExtension> >>> 92 >>> </gpxtpx:TrackPointExtension> >>> </extensions> >>> </trkpt> >>> >>> I guess everybody notices the missing <gpxtpx:hr></gpxtpx:hr> pair >>> surrounding the heart rate extension. >>> >>> I have tried directly converting a gpx file into another gpx file >>> without any filtering and all the options of the gpx module that I >>> thought were relevant (garminextensions, gpxver, and even >>> humminbirdextensions); the result, unfortunately, is always the same. >>> >>> I'd really appreciate if anybody could shed some light on this issue. >>> >>> Thanks in advance. >>> >>> Best, >>> >>> Angel >>> >>> >>> ------------------------------------------------------------------------------ >>> Everyone hates slow websites. So do we. >>> Make your web apps faster with AppDynamics >>> Download AppDynamics Lite for free today: >>> http://p.sf.net/sfu/appdyn_d2d_feb >>> _______________________________________________ >>> Gpsbabel-misc mailing list http://www.gpsbabel.org >>> Gps...@li... >>> To unsubscribe, change list options, or see archives, visit: >>> https://lists.sourceforge.net/lists/listinfo/gpsbabel-misc >>> >> >> >> >> ------------------------------------------------------------------------------ >> Everyone hates slow websites. So do we. >> Make your web apps faster with AppDynamics >> Download AppDynamics Lite for free today:http://p.sf.net/sfu/appdyn_d2d_feb >> >> >> >> _______________________________________________ >> Gpsbabel-code mailing list http://www...@li...https://lists.sourceforge.net/lists/listinfo/gpsbabel-code >> >> >> > |