Skip to content

Comments

Change reroute radius threshold calculation#228

Merged
bsudekum merged 3 commits intomasterfrom
reroute-snapping
May 26, 2017
Merged

Change reroute radius threshold calculation#228
bsudekum merged 3 commits intomasterfrom
reroute-snapping

Conversation

@bsudekum
Copy link
Contributor

@bsudekum bsudekum commented May 15, 2017

We've had reports of constant rerouting on highways. Our current thinking that this is a combination of a few factors:

  • user has very high gps accuracy
  • user is traveling on large highway with many lanes
  • user has high horizontal distance from the route geometry (I.E. in first lane)

These combinations could produce a rerouting event since their horizontalAccuracy is < 10m and the RouteControllerUserLocationSnappingDistance is `10m. This produces a radius threshold of only 20m.

This change now looks at the maximum value between RouteControllerMaximumDistanceBeforeRecalculating (50m) and the horizontalAccuracy + RouteControllerUserLocationSnappingDistance (10m).

Here are the situations that will change:

  • 👍 fewer rerouting on a highway with good accuracy
  • 👎 possibly longer time to reroute while in urban areas (with worse horizontal accuracy)

/cc @mapbox/navigation @danpat @willwhite

@bsudekum bsudekum requested review from 1ec5 and frederoni May 15, 2017 19:54
@bsudekum bsudekum added the ⚠️ DO NOT MERGE PR should not be merged! label May 15, 2017
@bsudekum
Copy link
Contributor Author

In f2c9e69, I changed when we snap. Previously we:

  • if snapping was enabled, we always returned a snapped location
  • even if we errored while calculating the snapping course, we still returned a snapped location
  • if a user was not snapped, we rerouted

Now:

  • if anything errors while calculating the snapped location or location we return a non snapped location
  • We check if the snapped location is greater than RouteControllerUserLocationSnappingDistance, we return the users actual location and course
  • This allows for more wiggle room and less aggressive snapping and rerouting

@bsudekum bsudekum removed the ⚠️ DO NOT MERGE PR should not be merged! label May 16, 2017
@bsudekum bsudekum force-pushed the reroute-snapping branch 2 times, most recently from 0e1bb3e to 273b7e6 Compare May 17, 2017 20:56
@bsudekum bsudekum force-pushed the reroute-snapping branch from 273b7e6 to c0cea02 Compare May 18, 2017 17:08
@1ec5 1ec5 mentioned this pull request May 22, 2017
@bsudekum
Copy link
Contributor Author

@frederoni @1ec5 after testing this for a bit, I think it's makes for a good addition. 👍 on merging?

@bsudekum bsudekum merged commit e994b87 into master May 26, 2017
@bsudekum bsudekum deleted the reroute-snapping branch May 26, 2017 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants