Feature: Add route planning support to foxtrotgps (Patch included)

[Dr. Tilmann Bubeck]

Please find attached a patch which is based upon the current bzr. It extends foxtrotGPS to be used as a route planing tool. You can get an idea by looking at the attached screen shot.

You start by setting a first waypoint (WP) using the menu "ways & routes/add WP". Then you move the mouse to set the next waypoint using the same menu entry. You construct a route by adding as many waypoints as you like.

A route can be changed by clicking on the flag icon and choosing "delete WP" or by dragging a flag icon around.

If you are satisfied with the route you can save it in either GPX or TomTom format to be used on a navigation system. It will then build a route by using the selected waypoints.

This is especially usefull for motor cycle tours or people you would like to drive a special route.

There is code to load the GPX tour for further use afterwards.

It would be very nice to see this patch accepted into foxtrotGPS. If you dislike the patch, then please let me know, so that I change whatever you want me to do. Put my patch under whatever license you like.

Feel free to contact me for any further details.

[Joshua Judson Rosen (rozzin)]

Til, this is fantastic new functionality.

There are a couple of issues with the patch, though--like forgotten function prototypes and missing #include directives, and some formatting inconsistencies.

I'll come up with some more feedback, shortly :)

[Dr. Tilmann Bubeck (t-bubeck)]

Thanks for your feedback! Glad to hear, that you like the idea. I will change anything you let me know.

BTW, I would like to tell the user how to use that feature. The manual page could be one place, but I expect many people to prefer a GUI help text. Are there any thoughts about that?

In any way I would like to write the help text in ASCIIdoc (http://www.methods.co.nz/asciidoc/) because I think that is the easiest and most flexible way to write such texts. Would you accept this or are there any other ideas/methods already in place?

Thanks for your help!

[Dr. Tilmann Bubeck (t-bubeck)]

Any news? I am excitingly awaiting your response and I am happy to improve the patch. :-)

[Joshua Judson Rosen (rozzin)]

OK, I've read through it all, and have just a few requests; some of these are addressed in my branch at <https://code.launchpad.net/~rozzin/foxtrotgps/route-planning.cleanup>:

    * You forgot to include some necessary prototypes, so it didn't build :)
      (see my commit #208)
    * There are some (it looks like) superfluous diffs that make it a little confusing to someone reading the changeset (commits #209 & #210)
    * The use of whitespace and comment-styles are confused (see my commit #211; this is really
      my fault for not documenting the existing situation better...)
    * We're not tangoGPS anymore ;), and w e probably want to include the program's version in output
      that we produce, like we do for the track-saving code in tracks.c (see my commit #213)

I'd like to squash those all down into your changes and just attribute them to you (they're separate commits on a branch for the sake of this discussion, but after that it doesn't really make much sense to keep them that way).

After fixing that, there's just a few issues and questions that I see:

    * Since we just capture the bzr log as our ChangeLog file, it's helpful to split the changes out into a more digestible series: I see that you've made several changes to refactor the existing codebase so that you don't have to copy+paste as much to implement your feature (thanks! we need a lot of refactoring :)), but can you do the refactoring as its own set of commits, and then add the new features as commits on top of that (split into whatever chunks make sense to you)?

    * You've introduced a couple of minor memory-leaks, and included comments that acknowledge them. But it looks like it's straightforward to just include the relevant g_free() calls to avoid having the leaks--so please do.

    * There's a commented-out call in routes.c that looks like this:

        gtk_file_chooser_set_current_folder (GTK_FILE_CHOOSER (dialog), default_folder_for_saving);

    Do we want to do that, or not? Why or why not?

    * Semantically, you're loading/saving `waypoints', but the GPX elements your using are `trackpoints'. Is this intentional?

    * There's a hard-coded timestamp in routes.c:

        xmlTextWriterWriteElement (writer, BAD_CAST "time",
                                           BAD_CAST "2010-08-02T21:48:37Z");

[Dr. Tilmann Bubeck (t-bubeck)]

Thanks for your valuable feedback. I have no problems with:
 - fixing memory leaks
 - commented-out call
 - timestamp

But I do not know, how to split the changes in some other direction (refactor first, then change). You already improved my patch (Thanks!) but how should I now reorder the changes? This is more a kind of a bzr like question. What I can do is branch from your repo (https://code.launchpad.net/~rozzin/foxtrotgps/route-planning.cleanup) and generate a patch of the remaining issues listed above. Is that what you wanted?

Thanks!

[Joshua Judson Rosen (rozzin)]

"Dr. Tilmann Bubeck" <email address hidden> writes:
>
> Thanks for your valuable feedback. I have no problems with:
> - fixing memory leaks
> - commented-out call
> - timestamp
>
> But I do not know, how to split the changes in some other direction
> (refactor first, then change). You already improved my patch (Thanks!)
> but how should I now reorder the changes? This is more a kind of a bzr
> like question.

It is indeed really a bzr question. The useful commands for revising
a series of commits are mainly "uncommit", "shelve", "commit", "unshelve".
It can be really useful to understand how to do to that.

However...:

> What I can do is branch from your repo
> (https://code.launchpad.net/~rozzin/foxtrotgps/route-planning.cleanup)
> and generate a patch of the remaining issues listed above. Is that
> what you wanted?

I think that would actually be fine, yes; the refactoring that you did
here is pretty self-evident, so I can split it out myself without
much difficulty. If you can just fix the remaining few issues,
and supply answers to the questions, that will be great :)

--
"Don't be afraid to ask (λf.((λx.xx) (λr.f(rr))))."

[Dr. Tilmann Bubeck (t-bubeck)]

I made all requested changes and uploaded the attached patch which is based on lp:~rozzin/foxtrotgps/route-planning.cleanup

* All known memory leaks are fixed.
* gtk_file_chooser_set_current_folder was not needed and is removed now
* waypoints/trackpoints/routepoints? Well, I think we are planing a route consisting of routepoints (rtept). However, some
  web pages speak of routes consisting of waypoints, so I think this is not very clear by definition. Regardless of what we
  write into our generated GPX files we are able to read "rtept" and also "waypt", but not "trackpt" which is for tracking
  a route which you already travelled. So I did not change anything, because I do think that it is consistent. If you disagree,
  feel free to change in any way.
* hardcoded timestamp is now created dynamically.

In addition I reordered the entries of the "route & wp" menu. I moved all entries dealing with single points at the top of the menu and all entries dealing with all waypoints (=routes) to the end.

So I would consider this patch as final and would appreciate inclusion very much.

[Joshua Judson Rosen (rozzin)]

"Dr. Tilmann Bubeck" <email address hidden> writes:
>
> I made all requested changes and uploaded the attached patch which is
> based on lp:~rozzin/foxtrotgps/route-planning.cleanup

Great--thanks!

> * waypoints/trackpoints/routepoints? Well, I think we are planing a
> route consisting of routepoints (rtept). However, some web pages speak
> of routes consisting of waypoints, so I think this is not very clear
> by definition. Regardless of what we write into our generated GPX
> files we are able to read "rtept" and also "waypt", but not "trackpt"
> which is for tracking a route which you already travelled. So I did
> not change anything, because I do think that it is consistent.

Agreed.

> In addition I reordered the entries of the "route & wp" menu. I moved
> all entries dealing with single points at the top of the menu and all
> entries dealing with all waypoints (=routes) to the end.

Regardless of the ordering of the menu-items, I think it ends up being
confusing because each of "route" and "WP" both now mean multiple,
different things:

    * The pre-existing "get route" just draws a non-editable route
      (like you'd get when loading a *track* from a GPX file).
    * Your new "load route from GPX" loads a series of editable
      routepoints, and has nothing to do with "get route"
      (likewise "save route as GPX" has nothing to do with the `route'
       generated by "get route"; and "clear route" clears only
       your `route of waypoints', not routes from "get route").

    * The pre-existing "set WP" and "unset WP" do something
      orthogonal to your "add WP", "delete WP", and "insert WP before"
      operations.

One thought I had on this was we could try to disambiguate the different
features by moving them to different menus (e.g.: "route planning"
vs. "route finding").... But, based upon your explanation above,
I wonder if the best option is actually just to label your new operations
as being `routepoint' operations rather than `route' or `waypoint'
operations. Something like:

    * add routepoint
    * insert routepoint before this
    * delete routepoint
    * clear routepoints
    * save routepoints to GPX route
    * save routepoints to TomTom ITN

Or is that also confusing? Would it be better to do something like:

    * clear planned route
    * save planned route to GPX
    * save planned route to TomTom ITN

We could also resolve the "WP" conflict by renaming the historic WP
menu-items, e.g.:

    * set target
    * unset target

Thoughts?

I guess I should pose the question to the foss-gps list, as well....

--
"Don't be afraid to ask (λf.((λx.xx) (λr.f(rr))))."

[Dr. Tilmann Bubeck (t-bubeck)]

Yes, you are right: it is very unclear. When I started hacking on tangoGPS, then "set WP" and "unset WP" and also "get route" had no function at all. I though about removing them, but let them stay because I did not know what they are used for. However, I would prefer if we put the different things in different menus, like your first suggestion:

route finding > find route

route planning > add routepoint
               > load routepoints from GPX
               > save routepoints as GPX
               > save routepoints as TomTom ITN
               > delete all routepoints // previously "clear route"

route > set target
               > unset target

I never found out, what "set target" and "unset target" are used for. Do you know? Otherwise delete the "route" menu with its entries "set target" and "unset target".

A better alternative may be, to change the "get route" stuff to use "my" routepoints. The "get route" dialog would disappear and the "get route" menu entry would use all route points which were set by "route planning" and send them to "yournavigation.org" (or other servers). The result would be displayed like before.

So you could think of a two step route planing:
 (1) set route points and save them to let a vehicle navigation system do the route finding. STOP here.
 (2) or CONTINUE and let "yournavigation.org" find a route through the points with no further vehicle navigation system.

But this would be a bigger change. Maybe release often and early in two releases?
 * release 1.2.0: menus changed like the discussion above
 * release 1.2.1: integrate route finding like "alternative" above.

What do you think?

[Joshua Judson Rosen (rozzin)]

"Dr. Tilmann Bubeck" <email address hidden> writes:
>
> Yes, you are right: it is very unclear. When I started hacking on
> tangoGPS, then "set WP" and "unset WP" and also "get route" had no
> function at all. I though about removing them, but let them stay because
> I did not know what they are used for.

In addition to positioning a flag icon, `set WP' causes a targetting-
arrow to be added to the GPS location-indicator (if you have a GPS
attached and the GPS has a fix); so, while the GPS position indicator
will normally show you in which direction you're moving, `set WP'
will cause it to *also* show the direction in which your WP lies.
Useful in navigating toward a destination without having a specific
route planned/plotted.

> However, I would prefer if we put the different things in different
> menus
[...]
> A better alternative may be, to change the "get route" stuff to use "my"
> routepoints. The "get route" dialog would disappear and the "get route"
> menu entry would use all route points which were set by "route planning"
> and send them to "yournavigation.org" (or other servers). The result
> would be displayed like before.
>
> So you could think of a two step route planing:
> (1) set route points and save them to let a vehicle navigation system
> do the route finding. STOP here.
> (2) or CONTINUE and let "yournavigation.org" find a route through
> the points with no further vehicle navigation system.

Oh, *that* is a neat idea!

> But this would be a bigger change. Maybe release often and early in two releases?
> * release 1.2.0: menus changed like the discussion above
> * release 1.2.1: integrate route finding like "alternative" above.
>
> What do you think?

I think that sounds like a good plan.

--
"Don't be afraid to ask (λf.((λx.xx) (λr.f(rr))))."

[Joshua Judson Rosen (rozzin)]

Joshua Judson Rosen <email address hidden> writes:
>
> "Dr. Tilmann Bubeck" <email address hidden> writes:
> >
> > * waypoints/trackpoints/routepoints? Well, I think we are planing a
> > route consisting of routepoints (rtept). However, some web pages speak
> > of routes consisting of waypoints, so I think this is not very clear
> > by definition. Regardless of what we write into our generated GPX
> > files we are able to read "rtept" and also "waypt", but not "trackpt"
> > which is for tracking a route which you already travelled. So I did
> > not change anything, because I do think that it is consistent.
>
> Agreed.

Actually, re-reading the GPX schemas, it looks like GPX uses `waypt'
more akin to how we use POIs: `waypt' elements are defined as being
an *unordered set* of points, and the cannot occur inside a `rte'
element. The documentation strings in the schema also seem agreeable
with this:

    "wpt represents a waypoint, point of interest,
     or named feature on a map."

    "rte represents route - an ordered list of waypoints
     representing a series of turn points leading to
     a destination."

(I believe the word, "waypoints", in the `rte' documentation
 refers to the `wptType' type, rather than the `wpt' element-name).

So I think I'm going to just have the route-loader load the rtepts,
for the time being.

--
"Don't be afraid to ask (λf.((λx.xx) (λr.f(rr))))."

[Joshua Judson Rosen (rozzin)]

"Dr. Tilmann Bubeck" <email address hidden> writes:
>
> Yes, you are right: it is very unclear. When I started hacking on
n> tangoGPS, then "set WP" and "unset WP" and also "get route" had no
> function at all. I though about removing them, but let them stay because
> I did not know what they are used for. However, I would prefer if we put
> the different things in different menus, like your first suggestion:

I split the menu up as you described, but kept the labels
mostly unchanged, in this branch:

    https://code.launchpad.net/~rozzin/foxtrotgps/route-planning

--
"Don't be afraid to ask (λf.((λx.xx) (λr.f(rr))))."

[Dr. Tilmann Bubeck (t-bubeck)]

Thanks!

[Dr. Tilmann Bubeck (t-bubeck)]

I would like to document that feature in some kind of user manual or help text. Do you plan to implement this? What else can we do?

[Joshua Judson Rosen (rozzin)]

"Dr. Tilmann Bubeck" <email address hidden> writes:
>
> I would like to document that feature in some kind of user manual or
> help text. Do you plan to implement this? What else can we do?

At present, we just have the man page in doc/foxtrotgps.1.

Daniel Baumann initially wrote it for Debian, and I've
been adding to it; we can certainly include a section
in there for the route-planner.

It might be helpful to have a manual in some format that could
include screenshots, but I'm not sure what that would be--
or what the right way to manage it would be. Your input
is welcome :)

[Dr. Tilmann Bubeck (t-bubeck)]

I see two options:

 a) write DocBook(xml) and convert it into what ever you want (typically HTML, PDF, ...)
     This is how GNOME and Fedora are writing their docs.
     However, DocBook is not easy to use and some people do not like the way it is made.

  b) use AsciiDoc (http://www.methods.co.nz/asciidoc/) and convert it into what ever you want (typically HTML, PDF, ...)

AsciiDoc and DocBook are both declarative languages to write documentations. They both are able to generate HTML, PDF and various output formats.

They are different in the way you write. DocBook is a full featured XML text, annotating anything. E.g.

<article>
  <title>Sample article</title>
  <para>This is a really short article.</para>
</article>

Whereas AsciiDoc is more like a standard ASCII text, where it derives the intended annotation by looking at formatting of the ASCII text itself, e.g.

DESCRIPTION
-----------
The asciidoc(1) command translates the AsciiDoc text file 'FILE' to
DocBook or HTML. If 'FILE' is '-' then the standard input is used.

So AsciiDoc looks at the Ascii text which normally does not contain any tags. Tags are only needed for special cases, like including image files.

I personally used both methods and found both working well. AsciiDoc is easier but not as professional standardized as DocBook. On the other hand, DocBook is harder to use.

You are the package owner and therefore you are the one using the tool more than anyone else. So you give the direction.
:-)