Add FasterRouteDetector to check for quicker routes while navigating #638
Conversation
danesfeder
force-pushed
the
dan-faster-route
branch
from
ee00f4f
to
0f74331
Compare
| private Location lastCheckedLocation; | ||
|
|
||
| @Override | ||
| public boolean shouldCheckFasterRoute(Location location, RouteProgress routeProgress) { |
There was a problem hiding this comment.
@mapbox/navigation-ios When you have a chance, can I get some 👀 on shouldCheckFasterRoute and isFasterRoute? The former determines when a new route request should fire, while the latter determines if the DirectionsRoute from the response is actually faster the current route. Thanks!
danesfeder
force-pushed
the
dan-faster-route
branch
from
8890a55
to
bf29a63
Compare
| double currentDurationRemaining = routeProgress.durationRemaining(); | ||
| DirectionsRoute newRoute = response.routes().get(0); | ||
|
|
||
| if (hasLegs(newRoute)) { |
There was a problem hiding this comment.
There was a problem hiding this comment.
I'm doing that check down on line 60 with https://github.com/mapbox/mapbox-navigation-android/pull/638/files#diff-eda094b0008667fdddf8a3bc30e37bd2R101
| } | ||
| // Check if the faster route time interval has been exceeded | ||
| if (secondsSinceLastCheck(location) > NAVIGATION_CHECK_FASTER_ROUTE_INTERVAL) { | ||
| lastCheckedLocation = location; |
There was a problem hiding this comment.
Should this only be set if the function returns true? If so, it needs to be moved into the next if statement.
There was a problem hiding this comment.
secondsSinceLastCheck() returns the how long in seconds it's been since lastCheckedLocation, so after two min, this will be set and essentially reset the timer
danesfeder
force-pushed
the
dan-faster-route
branch
3 times, most recently
from
15a7d62
to
5340c28
Compare
| this.routeProgress = routeProgress; | ||
|
|
||
| // Calculate remaining waypoints | ||
| List<Point> coordinates = new ArrayList<>(routeProgress.directionsRoute().routeOptions().coordinates()); |
There was a problem hiding this comment.
@ericrwolfe starting with this line, does this logic make sense? I've tested with multi-waypoint / origin + destination routes and is seems to be working fine
There was a problem hiding this comment.
Hrmm, @bsudekum might be a better person to review this logic. I'm not as versed on how we deal with multiple waypoints.
There was a problem hiding this comment.
@bsudekum RouteOptions#coordinates represents the list of points making up the route. This would be equivalent to your list of legs with destinations (which we currently don't have on Android)
danesfeder
force-pushed
the
dan-faster-route
branch
from
5340c28
to
e6fdfb4
Compare
danesfeder
force-pushed
the
dan-faster-route
branch
from
2868a1c
to
5e27d75
Compare
* master: Update Strings with Transifex Translations (mapbox#657) Added embedded navigation example (mapbox#654) Release 0.9.0 (mapbox#663) Update README.md for 0.9.0 (mapbox#664) Add Maneuver type exit rotary constant (mapbox#653) Update Maps and Services dependencies (mapbox#661) Add FasterRouteDetector to check for quicker routes while navigating (mapbox#638) Directions API Banner instructions (mapbox#582) NavigationTelemetry update cue for changing configurations (mapbox#648) Moved WaypointNavigationActivity from the SDK to the test app (mapbox#652) Exposes the MapboxMap in NavigationView with a getter method (mapbox#642) Add language to NavigationViewOptions with default from RouteOptions (mapbox#635) # Conflicts: # libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationEngine.java # libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationService.java
Closes #129
FasterRouteDetectorandRouteEngineto check for / retrieve faster routes while navigatingFasterRoutecan be subclassed to allow users to set exactly how quickly they'd like to check for a new, possibly faster route.RouteOptionstoNavigationRouteto create a route request mirroring the options from theDirectionsRoutethat provided theRouteOptionscc @ericrwolfe