GPSBabel

I'm way behind on my mail queue, but...

Mark Hammond wrote:

> It seems that the problem is that when reading, gpx.c tries to save every
> tag it can.  When writing the gpx, it explicitly writes the '<name>' tag,

Bummer.

Without really thinking this through, I think we want to keep the tags
off the 'something else' queue.  While this patch might not be the final
words (there may be a need for a handful of 'case blah blah: break'
action) could you kick this approach around instead:

Index: gpx.c
===================================================================
RCS file: /cvsroot/gpsbabel/gpsbabel/gpx.c,v
retrieving revision 1.69
diff -p -u -r1.69 gpx.c
--- gpx.c       8 Jan 2004 21:47:51 -0000       1.69
+++ gpx.c       12 Jan 2004 07:10:59 -0000
@@ -380,7 +380,7 @@ gpx_start(void *data, const char *el, co
        case tt_trk_trkseg_trkpt:
                tag_wpt(attr);
                break;
-       default:
+       case tt_unknown:
                start_something_else(el, attr);
                break;
        }


I think this makes it more consistent with the old parser.

RJL

At 03:46 PM 1/12/2004 +1100, Mark Hammond wrote:
 ><wpt lat="50.937595" lon="6.883321">
 >  <name>ACHNR1</name>
 >  <cmt>ACHNR1</cmt>
 >  <desc>ACHNR1</desc>
 >  <sym>Waypoint</sym>
 ><name>ACHNR1<sym>Waypoint
 ></sym></name></wpt>
 >
 >Note that 'name' and 'sym' are specified twice.

There's something else here, and I wonder if it will affect
other "something elses": the duplicate name and sym are *nested*.
That shouldn't happen.

I'm of two minds on whether "something else" should include
absolutely everything.  As of now, we've basically been lucky
that the GPX files people actually use this functionality on
actually have all of the "something else" fields in the same
place in the <wpt> structure.  If that weren't the case, we
could see things getting rather ugly as the less-than-perfect
"something else" code shuffles the order around.  This might
take a bit of a rethink someday.

At 01:12 AM 1/12/2004 -0600, Robert Lipe wrote:
 >-       default:
 >+       case tt_unknown:

 >I think this makes it more consistent with the old parser.

Probably should do this for end_something_else too, eh?

At 08:28 AM 1/12/2004 -0500, Ron Parker wrote:
 >There's something else here, and I wonder if it will affect
 >other "something elses": the duplicate name and sym are *nested*.
 >That shouldn't happen.

I figured out why this is.  It's because "default" in the "start tag"
case and "default" in the "end tag" case don't do the same thing.
In particular, </name> has its own processing in the "end tag" case,
so end_something_else never sees it, and ends up nesting everything
inside of it.  Doing as I suggested in my previous email - replacing
default with case tt_unknown in the end tag case - will fix this
nesting problem.

> inside of it.  Doing as I suggested in my previous email - replacing
> default with case tt_unknown in the end tag case - will fix this
> nesting problem.

Yep.  Then we get into trouble with tags that we just sort of pass
through; if we just want to grope a tag on the way by but not
really explictly regenerate it on output, we lose the tag.  The
solution becomes much like what Ron originally did when he added the
*something_else brothers.


With this in place (ok, and an explicit modification of the header to
include the groundspeak namespace) I can babelize a groundspeak pocket
query and the output validates.

Ron, are you hip with this?

RJL

Index: gpx.c
===================================================================
RCS file: /cvsroot/gpsbabel/gpsbabel/gpx.c,v
retrieving revision 1.69
diff -p -u -r1.69 gpx.c
--- gpx.c	8 Jan 2004 21:47:51 -0000	1.69
+++ gpx.c	12 Jan 2004 14:56:42 -0000
@@ -71,6 +71,7 @@ typedef enum {
 	tt_wpt_url,
 	tt_wpt_ele,
 	tt_wpt_time,
+	tt_wpt_type,
 	tt_wpt_urlname,
 	tt_cache_container,
 	tt_cache_difficulty,
@@ -105,8 +106,9 @@ typedef enum {
 } tag_type;
 
 typedef struct tag_mapping {
-	tag_type tag_type;
-	const char *tag_name;
+	tag_type tag_type;		/* enum from above for this tag */
+	int tag_passthrough;		/* true if we don't generate this */
+	const char *tag_name;		/* xpath-ish tag name */
 } tag_mapping;
 
 /*
@@ -116,53 +118,54 @@ typedef struct tag_mapping {
  */
 
 tag_mapping tag_path_map[] = {
-	{ tt_gpx, "/gpx" },
-	{ tt_time, "/gpx/time" },
-	{ tt_author, "/gpx/author" },
-	{ tt_email, "/gpx/email" },
-	{ tt_time, "/gpx/time" },
-	{ tt_desc, "/gpx/desc" },
-
-	{ tt_wpt, "/gpx/wpt" },
-	{ tt_wpt_ele, "/gpx/wpt/ele" },
-	{ tt_wpt_time, "/gpx/wpt/time" },
-	{ tt_wpt_name, "/gpx/wpt/name" },
-	{ tt_wpt_cmt, "/gpx/wpt/cmt" },
-	{ tt_wpt_desc, "/gpx/wpt/desc" },
-	{ tt_wpt_url, "/gpx/wpt/url" },
-	{ tt_wpt_urlname, "/gpx/wpt/urlname" },
-	{ tt_wpt_sym, "/gpx/wpt/sym" },
-	{ tt_cache_container, "/gpx/wpt/groundspeak:cache/groundspeak:container" },
-	{ tt_cache_difficulty, "/gpx/wpt/groundspeak:cache/groundspeak:difficulty" },
-	{ tt_cache_terrain, "/gpx/wpt/groundspeak:cache/groundspeak:terrain" },
-
-	{ tt_rte, "/gpx/rte" },
-	{ tt_rte_name, "/gpx/rte/name" },
-	{ tt_rte_desc, "/gpx/rte/desc" },
-	{ tt_rte_number, "/gpx/rte/number" },
-	{ tt_rte_rtept, "/gpx/rte/rtept" },
-	{ tt_rte_rtept_ele, "/gpx/rte/rtept/ele" },
-	{ tt_rte_rtept_time, "/gpx/rte/rtept/time" },
-	{ tt_rte_rtept_name, "/gpx/rte/rtept/name" },
-	{ tt_rte_rtept_cmt, "/gpx/rte/rtept/cmt" },
-	{ tt_rte_rtept_desc, "/gpx/rte/rtept/desc" },
-	{ tt_rte_rtept_url, "/gpx/rte/rtept/url" },
-	{ tt_rte_rtept_urlname, "/gpx/rte/rtept/urlname" },
-	{ tt_rte_rtept_sym, "/gpx/rte/rtept/sym" },
-
-	{ tt_trk, "/gpx/trk" },
-	{ tt_trk_name, "/gpx/trk/name" },
-	{ tt_trk_desc, "/gpx/trk/desc" },
-	{ tt_trk_trkseg, "/gps/trk/trkseg" },
-	{ tt_trk_trkseg_trkpt, "/gpx/trk/trkseg/trkpt" },
-	{ tt_trk_trkseg_trkpt_ele, "/gpx/trk/trkseg/trkpt/ele" },
-	{ tt_trk_trkseg_trkpt_time, "/gpx/trk/trkseg/trkpt/time" },
-	{ tt_trk_trkseg_trkpt_name, "/gpx/trk/trkseg/trkpt/name" },
-	{ tt_trk_trkseg_trkpt_cmt, "/gpx/trk/trkseg/trkpt/cmt" },
-	{ tt_trk_trkseg_trkpt_desc, "/gpx/trk/trkseg/trkpt/desc" },
-	{ tt_trk_trkseg_trkpt_url, "/gpx/trk/trkseg/trkpt/url" },
-	{ tt_trk_trkseg_trkpt_urlname, "/gpx/trk/trkseg/trkpt/urlname" },
-	{ tt_trk_trkseg_trkpt_sym, "/gpx/trk/trkseg/trkpt/sym" },
+	{ tt_gpx, 0, "/gpx" },
+	{ tt_time, 0, "/gpx/time" },
+	{ tt_author, 0, "/gpx/author" },
+	{ tt_email, 0, "/gpx/email" },
+	{ tt_time, 0, "/gpx/time" },
+	{ tt_desc, 0, "/gpx/desc" },
+
+	{ tt_wpt, 0, "/gpx/wpt" },
+	{ tt_wpt_ele, 0, "/gpx/wpt/ele" },
+	{ tt_wpt_time, 0, "/gpx/wpt/time" },
+	{ tt_wpt_name, 0, "/gpx/wpt/name" },
+	{ tt_wpt_cmt, 0, "/gpx/wpt/cmt" },
+	{ tt_wpt_desc, 0, "/gpx/wpt/desc" },
+	{ tt_wpt_url, 0, "/gpx/wpt/url" },
+	{ tt_wpt_urlname, 0, "/gpx/wpt/urlname" },
+	{ tt_wpt_sym, 0, "/gpx/wpt/sym" },
+	{ tt_wpt_type, 1, "/gpx/wpt/type" },
+	{ tt_cache_container, 1, "/gpx/wpt/groundspeak:cache/groundspeak:container" },
+	{ tt_cache_difficulty, 1, "/gpx/wpt/groundspeak:cache/groundspeak:difficulty" },
+	{ tt_cache_terrain, 1, "/gpx/wpt/groundspeak:cache/groundspeak:terrain" },
+
+	{ tt_rte, 0, "/gpx/rte" },
+	{ tt_rte_name, 0, "/gpx/rte/name" },
+	{ tt_rte_desc, 0, "/gpx/rte/desc" },
+	{ tt_rte_number, 0, "/gpx/rte/number" },
+	{ tt_rte_rtept, 0, "/gpx/rte/rtept" },
+	{ tt_rte_rtept_ele, 0, "/gpx/rte/rtept/ele" },
+	{ tt_rte_rtept_time, 0, "/gpx/rte/rtept/time" },
+	{ tt_rte_rtept_name, 0, "/gpx/rte/rtept/name" },
+	{ tt_rte_rtept_cmt, 0, "/gpx/rte/rtept/cmt" },
+	{ tt_rte_rtept_desc, 0, "/gpx/rte/rtept/desc" },
+	{ tt_rte_rtept_url, 0, "/gpx/rte/rtept/url" },
+	{ tt_rte_rtept_urlname, 0, "/gpx/rte/rtept/urlname" },
+	{ tt_rte_rtept_sym, 0, "/gpx/rte/rtept/sym" },
+
+	{ tt_trk, 0, "/gpx/trk" },
+	{ tt_trk_name, 0, "/gpx/trk/name" },
+	{ tt_trk_desc, 0, "/gpx/trk/desc" },
+	{ tt_trk_trkseg, 0, "/gps/trk/trkseg" },
+	{ tt_trk_trkseg_trkpt, 0, "/gpx/trk/trkseg/trkpt" },
+	{ tt_trk_trkseg_trkpt_ele, 0, "/gpx/trk/trkseg/trkpt/ele" },
+	{ tt_trk_trkseg_trkpt_time, 0, "/gpx/trk/trkseg/trkpt/time" },
+	{ tt_trk_trkseg_trkpt_name, 0, "/gpx/trk/trkseg/trkpt/name" },
+	{ tt_trk_trkseg_trkpt_cmt, 0, "/gpx/trk/trkseg/trkpt/cmt" },
+	{ tt_trk_trkseg_trkpt_desc, 0, "/gpx/trk/trkseg/trkpt/desc" },
+	{ tt_trk_trkseg_trkpt_url, 0, "/gpx/trk/trkseg/trkpt/url" },
+	{ tt_trk_trkseg_trkpt_urlname, 0, "/gpx/trk/trkseg/trkpt/urlname" },
+	{ tt_trk_trkseg_trkpt_sym, 0, "/gpx/trk/trkseg/trkpt/sym" },
 	{0}
 };
 
@@ -185,14 +188,16 @@ write_optional_xml_entity(FILE *ofd, con
 }
 
 static tag_type
-get_tag(const char *t)
+get_tag(const char *t, int *passthrough)
 {
 	tag_mapping *tm;
 	for (tm = tag_path_map; tm->tag_type != 0; tm++) {
 		if (0 == strcmp(tm->tag_name, t)) {
+			*passthrough = tm->tag_passthrough;
 			return tm->tag_type;
 		}
 	}
+	*passthrough = 1;
 	return tt_unknown;
 }
 
@@ -346,6 +351,8 @@ gpx_start(void *data, const char *el, co
 {
 	char *e;
 	char *ep;
+	int passthrough;
+
 	vmem_realloc(&current_tag, strlen(current_tag.mem) + 2 + strlen(el));
 	e = current_tag.mem;
 	ep = e + strlen(e);
@@ -359,7 +366,7 @@ gpx_start(void *data, const char *el, co
 	 */
 	memset(cdatastr.mem, 0, cdatastr.size);
 
-	switch (get_tag(current_tag.mem)) {
+	switch (get_tag(current_tag.mem, &passthrough)) {
 	case tt_gpx:
 		tag_gpx(attr);
 		break;
@@ -380,9 +387,14 @@ gpx_start(void *data, const char *el, co
 	case tt_trk_trkseg_trkpt:
 		tag_wpt(attr);
 		break;
-	default:
+	case tt_unknown:
+		fprintf(stderr, "Unknown Passing through %s\n", el);
+		start_something_else(el, attr);
+		return;
+	}
+	if (passthrough) {
+		fprintf(stderr, "%d Passing through %s\n", passthrough, el);
 		start_something_else(el, attr);
-		break;
 	}
 }
 
@@ -506,12 +518,13 @@ gpx_end(void *data, const char *el)
 	char *s = strrchr(current_tag.mem, '/');
 	float x;
 	char *cdatastrp = cdatastr.mem;
+	int passthrough;
 
 	if (strcmp(s + 1, el)) {
 		fprintf(stderr, "Mismatched tag %s\n", el);
 	}
 
-	switch (get_tag(current_tag.mem)) {
+	switch (get_tag(current_tag.mem, &passthrough)) {
 	/*
 	 * First, the tags that are file-global.
 	 */
@@ -619,9 +632,14 @@ gpx_end(void *data, const char *el)
 	case tt_rte_rtept_desc:
 		wpt_tmp->notes = xstrdup(cdatastrp);
 		break;
-	default:
+	case tt_unknown:
 		end_something_else();
+		*s = 0;
+		return;
 	}
+
+	if (passthrough)
+		end_something_else();
 
 	*s = 0;
 }

At 09:00 AM 1/12/2004 -0600, Robert Lipe wrote:
 >Ron, are you hip with this?

On a quick look-see, yeah, it looks perfect (other than
the potential problem we already had, which I mentioned
in my first email this morning and which we don't really
need to be in a big hurry to fix.)

Ron Parker wrote:
> At 09:00 AM 1/12/2004 -0600, Robert Lipe wrote:
> >Ron, are you hip with this?
> 
> On a quick look-see, yeah, it looks perfect (other than

OK.  I've committed this.  While it looks bulkier, it eliminates the
need for having another list of tags - the goal of this rewrite was to
eliminate such duplication.

"A man with two GPSes never knows where he is."  (Or something like that.)

> the potential problem we already had, which I mentioned
> in my first email this morning and which we don't really
> need to be in a big hurry to fix.)  

The reordering thing is an interesting one and probably does explain
why the something_else brothers don't handle the top-level tags  like
<bounds> and <keywords> but it's not worth fretting about right now.

RJL

At 09:35 AM 1/12/2004 -0600, Robert Lipe wrote:
 >The reordering thing is an interesting one and probably does explain
 >why the something_else brothers don't handle the top-level tags  like
 ><bounds> and <keywords> but it's not worth fretting about right now.

Well, that and the fact that they only handle things within <wpt> or
its moral equivalent; that is, since gpx_extras is a member of the
waypoint structure, there's nowhere for them to store such global values
anyway.

And <bounds> is something we really should recompute, since merges and
filters may change it significantly (to the point where the original
bounds may actually be too restrictive.)

> At 08:28 AM 1/12/2004 -0500, Ron Parker wrote:
>  >There's something else here, and I wonder if it will affect
>  >other "something elses": the duplicate name and sym are *nested*.
>  >That shouldn't happen.
>
> I figured out why this is.  It's because "default" in the "start tag"
> case and "default" in the "end tag" case don't do the same thing.
> In particular, </name> has its own processing in the "end tag" case,
> so end_something_else never sees it, and ends up nesting everything
> inside of it.  Doing as I suggested in my previous email - replacing
> default with case tt_unknown in the end tag case - will fix this
> nesting problem.

Yes, I'm looking at this, and believe I can fix the "extras" stuff, while at
the same time, cleany supporting "extra" tags on routes and tracks.

Mark.

At 08:24 AM 1/13/2004 +1100, Mark Hammond wrote:
>Yes, I'm looking at this, and believe I can fix the "extras" stuff, while at
>the same time, cleany supporting "extra" tags on routes and tracks.

Robert already fixed it, I think.  I wouldn't spend too much time
on routes and tracks; there aren't any known "extra" tags for those.

> Robert already fixed it, I think.

Yes, I noticed - excellent.  I got some way there last night, but all
experience with XML is good ;)  [I like projects that have a nice overlap
between "what I already know" and "what I want to know"]

> I wouldn't spend too much time
> on routes and tracks; there aren't any known "extra" tags for those.

What about tags from extra namespaces?

Eg, a route in a .gpx file saved by EasyGPS looks something like:

<rte>
 <name>bar</name>
 <desc>foo</desc>
 <number>1</number>
 <topografix:color>ff0000</topografix:color>
</rte>

And i've seen a number of other "topografix" tags.  Wouldn't it be a good
idea to maintain these, and any other data a tool may annotate these items
with?

I believe that for a whole class of gps users, routes (and the waypoints in
them) are more interesting than "top-level" waypoints.  I am one such user,
and thus whenever I see waypoints getting special treatment I want to extend
it to routes and tracks :)

Thanks!

Mark.