Improve waypoints handling

[pini]

Hi,

I think waypoints with some improvements could be a really great feature. One very frequent use case I can think about is finding an alternative route in case of traffic jam or temporary closed routes.

In this use case, I'd like to be able to:

• select an existing waypoint
• add a new waypoint - after or before - the selected waypoint
• remove the selected waypoint

This requires to improve the way former destinations are stored (destination.txt) to begin with. Here is a first patch which keep waypoints consistent in destination.txt as the routing proceeds. Please review and comment...

Thanks.

[antiram]

Hi,

i had exact the same idea and made the other part with add waypoint before other waypoint and delete nth waypoint.

• Waypoints are shown as numbered circles.
• Click waypoint shows location info, click eg. "waypoint 2" here or in POI list
• then click "remove waypoint" or "add next waypoint before".

map needs

<itemgra item_types="waypoint">
    <circle color="#000000" radius="24" width="2" text_size="12"/>
</itemgra>

or so.

Added your patch to mine, tested, works, exept:

1. start ubuntu navit, position is not set, make route with 2 waypoints, then menu->Route->drop next waypoint. crash in route_remove_waypoint because path2 empty, fixed with "if (this->path2)" around code in route_remove_waypoint.

2. make route with 3 waypoints, then menu->Route->drop last waypoint, works, restart navit. 3rd waypoint was not deleted. it seems navit_remove_nth_waypoint needs this bookmarks_replace_destinations. i added it to navit_remove_nth_waypoint (for endless list of waypoints) but doesnt work for insert waypoint before other waypoint.

can only post both patches as one (mapitems.patch)

cheers

[pini]

Hi,

Many thanks for this patch! I've applied it on my source tree and done some quick and dirty fix. It seems functional now.

Attaching the resulting patch as mapitems.2.patch. Please note my source tree is from r5031.

[antiram]

Hi, it seems you have only changed navit_set_destination? I replaced yesterday 16 with endless list, changes in navit_set_destination, navit_add_former_destinations_from_file, textfile_coord_get, navit_remove_nth_waypoint. Added itemgra zoomlevels for car. apk here ​http://heiko.ploinger.de/navi/

no known bugs for now

cheers

[antiram]

replaced pcoord pc[16] with *pc

[pini]

Hi,

I'm trying to merge our patches. Please, could you give rationals for this hunk, from textfile_coord_get()?

Index: navit/navit/map/textfile/textfile.c
===================================================================
--- navit.orig/navit/map/textfile/textfile.c	2012-03-08 22:06:38.000000000 +0100
+++ navit/navit/map/textfile/textfile.c	2012-05-20 19:33:27.000000000 +0200
@@ -98,9 +98,11 @@
 	dbg(1,"textfile_coord_get %d\n",count);
 	while (count--) {
 		if (mr->f && !feof(mr->f) && (!mr->item.id_hi || !mr->eoc) && parse_line(mr, mr->item.id_hi)) {
-			*c=mr->c;
-			dbg(1,"c=0x%x,0x%x\n", c->x, c->y);
-			c++;
+			if (c){
+				*c=mr->c;
+				dbg(1,"c=0x%x,0x%x\n", c->x, c->y);
+				c++;
+			}
 			ret++;		
 			get_line(mr);
 			if (mr->item.id_hi)

I don't think this function has any chance to be called with c==0.

Thanks.

[pini]

OK, found out it's because of the change in navit_add_former_destinations_from_file(). About his function, I think the chunck

        while (item=map_rect_get_item(mr))
                count=item_coord_get(item, NULL, INT_MAX);

should read instead

        while (item=map_rect_get_item(mr))
                count=max(count, item_coord_get(item, NULL, INT_MAX));

[pini]

Result of the merge. We have different implementations for navit_set_destinations() and I replaced route_append_destination() with route_insert_waypoint().

[tryagain]

Hi!

wp_displaynum = atoi(strndup(data+strlen(wp), strlen(data)-strlen(wp)));

should be read as

wp_displaynum = atoi(data+strlen(wp));

current code is memleaking pointer allocated by strdup. I've seen at least one similar chunk around.

[tryagain]

Another question, why do you use new csv map to display waypoints? Won't it be simplier to open destination.txt as textfile map and display bubbles from it?

You may store active waypoints as item_waypoint and leave inactive ones as item_former_destination in one textfile map and only waypoints would be displayed as bubbles.

[antiram]

Hi,

• renamed navit_append_map_waypoint_item to navit_insert_waypoint_item, also renamed navit_remove_nth_waypoint_item

• fixed in navit_set_destination map item display after toggle menu->settings->rules->plan with waypoints (4 lines "if (this_->waypoints){..}")

• fixed crash in navit_insert_waypoint_item (removed gfree)

• fixed crash in navit_remove_nth_waypoint_item (removed gfree)

• fixed crash navit_remove_nth_waypoint if no route set (from menu->route->drop last waypoint)

• fixed wp_displaynum leak

• added comments

@tryagain

its a merge of 2 different patches. I used textfile bookmark.txt before and all bookmarks=waypoints are shown on map, but this looks bad, bubbles everywhere. Only the current route should shown. destination.txt looks now so eg.: ​http://pastebin.com/ei9nxz1r

which would lead to the same result. Is there a easy way to show only mg's from last type=former_destination without copy everything to a new map? I used finally only numbers as labels, so the missing labels from the mg's don't care maybe.

For now all waypoints are shown until reaching the end of route. Numbering is consistent/unchanged also if i insert/delete a waypoint while driving. Current numbers are only lost if navit was restarted while driving, then the rest of the tour is shown.

"drop next waypoint" is renamed to "Ignore next waypoint", waypoint item stay on map because its no planning feature. Its only needed if i reach a waypoint with bad gps signal and navit tell me "go back".

waypoint and search_results maps is shown as "csv:Waypoints", "csv:search_results" in menu->settings->maps.

cheers

[antiram]

fixes and changes

[antiram]

@tryagain

i read your last paragraph not correctly. Maybe this is the solution. But was is if i plan a tour, then i change my mind, plan another tour, then i would see both undriven tours with destination.txt.

Or another textfile, which will deleted if destination is set to NULL. Active as item_waypoint and inactive ones as item_former_destination would also solve the changed numbering if navit was terminated from android, eg. if i move navit in background on android to change my music while driving, then sometimes navit restarts.

[antiram]

but then we have have to start again from ~0 and the only benefit is that reached waypoints can displayed with correct numbering after restart. nothing more, but current solution works now.

[antiram]

Hi,

• fixed: sometimes wrong wp numbering after insert waypoint.
• fixed: crash after "no route found" or "position blocked" or so
• added: selector for route items (waypoints, route_start, route_end) in POI list.
• added: new Menu item "Move waypoint" in waypoint context menu.
• added: itemgra's for all maps in navit-shipped.xml.
• changed: waypoints map renamed to route_items

makes currently no sense to me use former_destinations instead route_items as map for waypoints. Sometimes later the route_items map can store the planned route also as trackpoints to show planned route as track and current navigation to next waypoint as route. Export/import track or route as gpx, cvs or so would also nice.

cheers

[antiram]

fixes and wp move and poi selector

[tryagain]

Hi!

1. If you define, say, five waypoints, they will be given numbers 1,2,3,4,5. If you wait to pass two first of them, and after that add another three waypoints, you'll get duplicate waypoint numbers. So you'll see on the screen 1 and 2 (already passed), 3,4,5 and then 4,5,6. There will be two 4th and two 5th waypoints.

2. I think it's not very intuitive to have waypoint deleted when one select to move it and when a new waypoint is added, it's placed in the place of that deleted waypoint. Same for "insert before".

I'd better consider placing "insert waypoint" and "replace waypoint" actions inside Map Point context menu (not inside Waypoint one). When one clicks one of above actions, he would be presented a waypoint list to select before which waypoint (or at the place of which waypoint) current clicked (selected, found) point is to be paced. For example, if you want to replace third waypoint, you click the map, where you want it to be placed, then go to the "globe" icon, select "replace waypoint", then select third waypoint from the list.

tryagain

[antiram]

fix insert/delete wp while driving; show route_item selector only if route set

[antiram]

hi,

1 fixed
2 good idea, less clicks and better to see. i add it.

[tryagain]

Hi, I'm working on it.

Currently I have rewritten it to adhere discussion results from ​http://irclogs.navit.ie/%23navit-2012-10-10.log

Hope, we'll have this work commited in a week.

tryagain.

[tryagain]

Submitted as ​http://navit.svn.sourceforge.net/viewvc/navit?view=revision&revision=5264.

It's quite a big rewrite of your patches, but i think its more usable now.

Waypoint insertion is done in one gui enter, click point to visit on the map and then use the "Visit before..." item to insert it to waypoint list. Waypoints selector is moved from POI to "Route" menu (visible if you have enabled waypoints planning and have at least one waypoint defined). Waypoint can be deleted if you click it on the map or choose from the list. Though there's no way to "move" waypoint. It can currently be moved by deleting old and inserting new one in the same amount of clicks.

To simplify the code, I've got rid of additional csv map.

One note for translators: I have changed spelling for internal gui item "Replace Waypoints" to "Replace with waypoints". Looks like older spelling meaning is exactly opposite to what we were doing there.

Existing translations seem to follow the wrong phrase. So I left msgid "Replace Waypoints" and its translations unchanged for now to have some reference to do the changes needed.

tryagain.

[usul]

The patch has been submitted and I saw this enumation working. So I close this ticket for now. Feel free to open it at any time.