Opened 7 years ago

Closed 7 years ago

#980 closed defect/bug (fixed)

Navit starts with a route that has been deleted in previous run

Reported by: me.yahoo.com/a/t8pxanggmzrce9fscf2wgwk_ozmtbyifgarr#f3b4e Owned by: KaZeR
Priority: major Milestone:
Component: core Version: git master
Severity: Keywords:
Cc: sebastian.leske@…

Description

  • Navit version: navit-svn-4863
  • Device: Samsung Galaxy Tab P1000
  • OS: Android 2.2

Steps to reproduce:

  1. start navit
  2. wait for GPS fix
  3. specify some destination (using drive here, eventually)
  4. wait for route to be calculated
  5. click on screen in order to get the menu displayed
  6. go to Actions -> Stop navigation
  7. go back to map; as expected, there will be no routing information available
  8. quit navit (menu -> more -> exit navit)
  9. start navit

After GPS gets fixed, navit will recalculate previous route, even though it has been deleted before quitting at step 6

Attachments (1)

navi-stopped.diff (4.2 KB) - added by sleske 7 years ago.
Patch to remember if navigation was stopped.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 7 years ago by sleske

  • Cc sebastian.leske@… added

comment:2 Changed 7 years ago by sleske

Oops, just found out that is was me who caused this regression, through my patch implementing the list of former destinations (ticket #943).

Before the patch, Navit kept track of whether navigation to the last destination had been stopped by appending an emtpy line to destination.txt (rather sneaky, if I may say so). I missed that :-(.

I'm currently thinking about what the best fix is. I could re-implement the old behaviour, but to be honest, I don't really like this trick of using an empty line to say "don't restart navigation to this destination".

I'm torn between putting some comment line into destination.txt (and adapting the textfile reading code), or going all the way and having an extra file to record the last active destination, or the fact whether navigation was active when Navit was shut down.

Or maybe Navit really needs some file that records all the "current state" of Navit. There's already destination.txt and center.txt... maybe that could need some consolidation.

Changed 7 years ago by sleske

Patch to remember if navigation was stopped.

comment:3 Changed 7 years ago by sleske

The attached patch makes Navit remember if navigation was stopped. When navigation is stopped, a line

# navigation stopped

is appended to destination.txt. On startup Navit checks for this line, and will only restart navigation to the last destination if it is not found.

This is similar to how Navit worked before my patch in #943. Only then the marker was just an empty line at the end of destination.txt. I used a line with special text to make the feature more obvious.

To make this possible, the patch also adapts the textfile loading code to accept comment lines (lines starting with '#') between the points. That might be a useful feature of its own. I'll document this in the wiki once this patch is applied.

comment:4 Changed 7 years ago by me.yahoo.com/a/t8pxanggmzrce9fscf2wgwk_ozmtbyifgarr#f3b4e

I am not sure about your build system. Does this means that the code has been patched ? Will I find this fix in the next SVN build ?

Or this is just a patch that I need to apply manually on the source code and compile it in order to see it working.

Thanks. Ciprian.

comment:5 Changed 7 years ago by sleske

Yes, this is just a patch that you need to apply. However, I posted it here in the hope that someone from the Navit team will commit it.

When someone commits it, they'll note it in this ticket. Then it will be fixed in the next SVN build :-).

comment:6 Changed 7 years ago by sleske

  • Resolution set to fixed
  • Status changed from new to closed

Actually, I can commit the fix myself now :-).

Patch commited in rev. 4871, http://navit.svn.sourceforge.net/viewvc/navit?view=revision&revision=4871 .

comment:7 Changed 7 years ago by me.yahoo.com/a/t8pxanggmzrce9fscf2wgwk_ozmtbyifgarr#f3b4e

This bug is fixed. Verified in svn version 4871.

Thanks. Ciprian.

comment:8 Changed 7 years ago by tryagain

  • Resolution fixed deleted
  • Status changed from closed to reopened

Patch applied with r4871 is breaking code consistency. We shouldn't include map/textfile/textfile.h from outside of map/textfile/. It contains private definitions for textfile map type.

Map driver interaction should be done through existing map interface.

We should find better place for these two #defines used outside of mapdriver. Or, better, I'd use a different item type to signify if routing is active. This way everything can be done through map driver interface, and recent destinations will [not|less] depend on map type used to store them.

See also http://irclogs.navit.ie/%23navit-2012-01-01.log.

comment:9 Changed 7 years ago by sleske

Yes, I agree that including textfile.h breaks code consistency, by making the code depend on private details of the textfile code.

However, this dependency was already there before my changes; including textfile.h only makes it explicit. The bookmark/last destination code (bookmarks_append_coord etc. in bookmarks.c) contains code to output the bookmarks.txt and destination.txt files. This code has to output the files such that they can be read by the textfile map handling code - thus there is already an implicit dependency on the textfile format.

Map driver interaction should be done through existing map interface.

Yes, that would be ideal. However, the existing map interface does not handle writing, thus the bookmark code has to do it itself.

I also find this dependency between bookmark handling and the textfile format unfortunate, but I don't see an easy way around it. The cleanest solution would probably be to add writing methods to the map plugin interface, and move the writing code from bookmarks.c to textfile.c.

But frankly, with so many open bugs and so much upcoming work I'm not sure it's worth the hassle.

For now, the fact that the code includes textfile.h serves as a reminder that there is a dependency, so it is not forgotten, and I feel inclined to leave it at that. But maybe there's a solution I don't see...

comment:10 Changed 7 years ago by sleske

  • Resolution set to fixed
  • Status changed from reopened to closed

tryagain's remarks about code consistency is correct. Since this is no longer about the original bug, I opened a new ticket, #1003, and close this one.

Note: See TracTickets for help on using tickets.