maplibre / maplibre-navigation-android

Maplibre Navigation SDK for Android
MIT License
86 stars 44 forks source link

Skip calculating step distance when too far away from the step #75

Closed boldtrn closed 1 year ago

boldtrn commented 1 year ago

This PR improves progress updates if the user is too far from the route. If the user is further away then 1km from the route, we don't progress along the step anymore.

Before this was done, independent of how far the user is away from the route.

This change should only be relevant if you have a fixed route, if you reroute, once the user if offroute, this should not be a relevant change.

wipfli commented 1 year ago

@Fabi755 would you be interested in reviewing this pull request?

Fabi755 commented 1 year ago

Yes, I will review this PR this week 👍

Fabi755 commented 1 year ago

The code looks good for me 👍

For my understanding only two questions:

  1. Without this changes, do we have some downsides? Or what is the reason for fixing this?
  2. I think the 1 km are a good value. How you select them? Can be use cases where we need a lower or higher value?
boldtrn commented 1 year ago

Without this changes, do we have some downsides? Or what is the reason for fixing this?

Quick explainer: with snapping in this context I mean that your location will be used for internal calculations on the route, not the visual snapping.

Yes, this is only relevant when you disable the route recalculation, so it is probably not super relevant for most users of the SDK.

You can be 50km away from the route, the SDK always snaps you to the closes point of the route, which makes no sense, if you are very far away from the route. The route progress will be increased and you skip ahead to a future part of the route. If you want to go back on the route at your original location, it can happen that the SDK does not recognises you to be back on the route, as you have skipped ahead already. I hope this makes sense? If not, let me know, I think it can better explained with a drawing :).

I think the 1 km are a good value. How you select them? Can be use cases where we need a lower or higher value?

I wanted to pick a reasonable distance where it makes sense that we actually consider a route snap. Right now I don't think we will need any higher values, if anything, maybe even lower values. While you are driving towards the route line, you will come closer than 1km to the route, and you will be snapped before you are on the route.

Fabi755 commented 1 year ago

Now I get this! Thanks for your explanation!

All things look good to me and we can merge this.

boldtrn commented 1 year ago

All things look good to me and we can merge this.

Thanks, maybe you can approve the PR (using the review option on Github)?

Fabi755 commented 1 year ago

Ahh interesting, I thought this is only possible as maintainer