otsaloma / poor-maps

Maps and navigation for Sailfish OS
https://openrepos.net/content/otsaloma/poor-maps
GNU General Public License v3.0
43 stars 10 forks source link

Navigation: Begin/Pause makes sense only when navigating from current location #45

Closed rinigus closed 7 years ago

rinigus commented 7 years ago

In NarrativePage, we have Begin and Pause controls. I think that they make sense only when using navigation from current location. Otherwise, we are just reviewing possible route and don't have anything to start.

In PR https://github.com/otsaloma/poor-maps/pull/43 I introduced app.navigationReroutable, but I guess it would be better to use an additional property like app.navigationFromCurrent and, depending on the state of this property, enable or disable Begin and Pause controls

otsaloma commented 7 years ago

I understand that when rerouting, we'll want to auto-start, but why would these buttons not make sense when rerouting? For example, if you stop for a cup of coffee on a long trip and want to examine the map, you'll want to press pause so that you can examine the map with auto-center, auto-rotate and without losing the route, which you'll resume after the coffee break. This should be valid when rerouting too?

Maybe it's the button names that need to be changed. Now that I think of it, begin and pause could be combined like the play/pause button on media players. It could initially say "Start", after that "Pause" and if paused, "Resume". What do you think?

otsaloma commented 7 years ago

Sorry, I read the condition hastily, but still, why would there be a case where the buttons should be disabled? Say, you have inaccurate positioning, and because of that route from an address? Or, you do the routing at home, but set it to begin from the parking lot a kilometer from your home?

rinigus commented 7 years ago

Let me try to explain the thought behind it.

Let's start with the simple part: Start/Pause/Resume would be great. Indeed, you can make a stop, pause navigation, look into the route and check all possible features that you wish, and then resume navigation.

In this new way, why do I think that Start should be used only if you start from "Current position"? Maybe its the simplest by examples:

When "Current position" is allowed only

This all works, assuming that you trust that the calculated route is fine.

When I can start navigation from address

As you could see, it boils down to making decision on when to recalculate the route. The both cases are probably fine assuming that if we start route which was found from address to address and then recalculate it as soon as we are too far from it (as given by 200 meters + positioning_precision). Originally, I didn't have positioning_precision component and then the difference between scenarios was significant. With it, the both cases are probably OK and you can pick and choose.

I can see positives in the both cases, so maybe you could pick the solution you prefer and then we''ll do it that way.

otsaloma commented 7 years ago

I somewhat understand the problem now, but still not the solution. Two things come to mind

  1. We need an option poor.conf.reroute anyway. That could default to false if routing from an address.

  2. Rerouting should only happen when positioning accuracy is below 40 meters or so, otherwise it doesn't really make sense.

What do you think?

rinigus commented 7 years ago

While replying last time, I realized that my position is not as polarized as it is represented by the title of the issue. You presented a good case, so, with the rerouting option it would be fine to reroute with the root based on addresses. I was considering the routing from current position and from address as two different modes, but it makes sense not to distinguish them and assume that user knows better than GPS sometimes.

As for questions:

  1. poor.conf.reroute is needed, indeed. However, we may cause large confusion if it will start to get enabled and disabled by "magic". Maybe we should assume that if the user pressed "Begin" (or Start) then we are, as soon as we get the GPS fix, rerouting if the user had selected poor.conf.reroute=true before.

  2. Taking into account that rerouting via online routers costs API calls, it would probably make sense to have some minimal accuracy. However, we actually have accuracy setting offsetting these 200 meters and maybe its sufficient. In my latest tests, rerouting started to work only when it got quite accurate position anyway. Have you tried yourself? Did it started to reroute too early? If it did then we have to add this cut-off, but otherwise, its not really needed. But note that I don't have strong opinion on this either. If you think that 40m is adequate, let's add that condition.

otsaloma commented 7 years ago

I was considering the routing from current position and from address as two different modes, but it makes sense not to distinguish them and assume that user knows better than GPS sometimes.

Indeed, I don't see them as different modes. One common use case I didn't mention was calculating the route while still at home, inside, with poor GPS accuracy, and thus using one's home address. Another thing I sometimes use myself is starting the route a couple blocks away from home to "force" the route to the alternative which I know is best (e.g. due to road works not accounted for by routers).

I haven't done any testing, so I have no opinion on the parameters yet, but anyway, I'd like you to update the PR and remove all the app.navigationCanStart stuff. After that I can start tweaking the details, do some testing and we can return the parameters after that.

rinigus commented 7 years ago

Its done in e728c7cdd1d8aac7cbb23a3e0ce7d17649b94ec3. The only condition where app.navigationReroutable = false is when the route was calculated to current position. I guess, we can close this issue/discussion and move on, right?

otsaloma commented 7 years ago

Yep.