Opened 11 years ago
Closed 10 years ago
#1040 closed enhancement/feature request (fixed)
Improve waypoints handling
Reported by: | pini | Owned by: | tryagain |
---|---|---|---|
Priority: | major | Milestone: | |
Component: | core | Version: | git master |
Severity: | Keywords: | waypoints, routing | |
Cc: |
Description
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.
Attachments (7)
Change History (23)
Changed 11 years ago by pini
comment:1 Changed 11 years ago by antiram
comment:2 Changed 11 years ago by 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.
Changed 11 years ago by pini
comment:3 Changed 11 years ago by 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
comment:4 Changed 11 years ago by 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.
comment:5 Changed 11 years ago by 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));
Changed 11 years ago by pini
Result of the merge. We have different implementations for navit_set_destinations() and I replaced route_append_destination() with route_insert_waypoint().
comment:6 Changed 11 years ago by 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.
comment:7 Changed 11 years ago by 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.
comment:8 Changed 11 years ago by 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
comment:9 Changed 11 years ago by 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.
comment:10 Changed 11 years ago by 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.
comment:11 Changed 10 years ago by 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
comment:12 Changed 10 years ago by tryagain
Hi!
- 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.
- 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
Changed 10 years ago by antiram
fix insert/delete wp while driving; show route_item selector only if route set
comment:13 Changed 10 years ago by antiram
hi,
1 fixed
2 good idea, less clicks and better to see. i add it.
comment:14 Changed 10 years ago by tryagain
- Owner changed from KaZeR to 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.
comment:15 Changed 10 years ago by 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.
comment:16 Changed 10 years ago by usul
- Keywords routing added
- Resolution set to fixed
- Status changed from new to closed
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.
Hi,
i had exact the same idea and made the other part with add waypoint before other waypoint and delete nth waypoint.
map needs
or so.
Added your patch to mine, tested, works, exept:
can only post both patches as one (mapitems.patch)
cheers