Skip to content

Commit

Permalink
Remove last location check from location validation (#788)
Browse files Browse the repository at this point in the history 
* Remove last location check from location validation

* Replace magic number

* Update constant

* Fix crash with force increased indices

* Extract duplicate logic
  • Loading branch information
@danesfeder
danesfeder authored Mar 23, 2018
1 parent be38961 commit 2975aea
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 50 deletions.
2 changes: 1 addition & 1 deletion .../src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationEngine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ private void handleRequest(final NewLocationModel newLocationModel) {

final boolean userOffRoute = isUserOffRoute(newLocationModel, routeProgress, routeProcessor);

routeProcessor.checkIncreaseStepIndex(mapboxNavigation);
routeProcessor.checkIncreaseIndex(mapboxNavigation);

RouteProgress previousRouteProgress = routeProcessor.getPreviousRouteProgress();
final List<Milestone> milestones = checkMilestones(previousRouteProgress, routeProgress, mapboxNavigation);
Expand Down
23 changes: 16 additions & 7 deletions .../src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,23 @@ static boolean checkBearingForStepCompletion(Location userLocation, RouteProgres
*/
static NavigationIndices increaseIndex(RouteProgress routeProgress,
NavigationIndices previousIndices) {
// Check if we are in the last step in the current routeLeg and iterate it if needed.
if (previousIndices.stepIndex()
>= routeProgress.directionsRoute().legs().get(routeProgress.legIndex())
.steps().size() - 2
&& previousIndices.legIndex() < routeProgress.directionsRoute().legs().size() - 1) {
return NavigationIndices.create((previousIndices.legIndex() + 1), 0);
DirectionsRoute route = routeProgress.directionsRoute();
int previousStepIndex = previousIndices.stepIndex();
int previousLegIndex = previousIndices.legIndex();
int routeLegSize = route.legs().size();
int legStepSize = route.legs().get(routeProgress.legIndex()).steps().size();

boolean isOnLastLeg = previousLegIndex == routeLegSize - 1;
boolean isOnLastStep = previousStepIndex == legStepSize - 1;

if (isOnLastStep && !isOnLastLeg) {
return NavigationIndices.create((previousLegIndex + 1), 0);
}

if (isOnLastStep) {
return previousIndices;
}
return NavigationIndices.create(previousIndices.legIndex(), (previousIndices.stepIndex() + 1));
return NavigationIndices.create(previousLegIndex, (previousStepIndex + 1));
}

static List<Milestone> checkMilestones(RouteProgress previousRouteProgress,
Expand Down
32 changes: 16 additions & 16 deletions ...n/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRouteProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,23 @@ class NavigationRouteProcessor implements OffRouteCallback {
private RouteProgress previousRouteProgress;
private List<Point> stepPoints;
private NavigationIndices indices;
private boolean shouldIncreaseStepIndex;
private boolean shouldIncreaseIndex;

NavigationRouteProcessor() {
indices = NavigationIndices.create(FIRST_LEG_INDEX, FIRST_STEP_INDEX);
}

@Override
public void onShouldIncreaseIndex() {
shouldIncreaseStepIndex = true;
shouldIncreaseIndex = true;
}

/**
* Will take a given location update and create a new {@link RouteProgress}
* based on our calculations of the distances remaining.
* <p>
* Also in charge of detecting if a step / leg has finished and incrementing the
* indices if needed ({@link NavigationRouteProcessor#advanceStepIndex(MapboxNavigation)} handles
* indices if needed ({@link NavigationRouteProcessor#advanceIndices(MapboxNavigation)} handles
* the decoding of the next step point list).
*
* @param mapboxNavigation for the current route / options
Expand All @@ -68,10 +68,10 @@ RouteProgress buildNewRouteProgress(MapboxNavigation mapboxNavigation, Location
boolean bearingMatchesManeuver = checkBearingForStepCompletion(
location, previousRouteProgress, stepDistanceRemaining, completionOffset
);
boolean forceIncreaseStepIndex = stepDistanceRemaining == 0 && !bearingMatchesManeuver;
boolean forceIncreaseIndices = stepDistanceRemaining == 0 && !bearingMatchesManeuver;

if ((bearingMatchesManeuver && withinManeuverRadius) || forceIncreaseStepIndex) {
advanceStepIndex(mapboxNavigation);
if ((bearingMatchesManeuver && withinManeuverRadius) || forceIncreaseIndices) {
advanceIndices(mapboxNavigation);
stepDistanceRemaining = calculateStepDistanceRemaining(location, directionsRoute);
}

Expand Down Expand Up @@ -109,21 +109,21 @@ private boolean hasInvalidLegs(List<RouteLeg> legs) {
return legs == null || legs.isEmpty();
}

private boolean hasInvalidSteps(int stepIndex, List<LegStep> steps) {
return steps == null || steps.isEmpty() || stepIndex >= steps.size();
private boolean hasInvalidSteps(List<LegStep> steps) {
return steps == null || steps.isEmpty();
}

/**
* If the {@link OffRouteCallback#onShouldIncreaseIndex()} has been called by the
* {@link com.mapbox.services.android.navigation.v5.offroute.OffRouteDetector}, shouldIncreaseStepIndex
* will be true and the {@link NavigationIndices} step index needs to be increased by one.
* {@link com.mapbox.services.android.navigation.v5.offroute.OffRouteDetector}, shouldIncreaseIndex
* will be true and the {@link NavigationIndices} index needs to be increased by one.
*
* @param navigation to get the next {@link LegStep#geometry()} and off-route engine
*/
void checkIncreaseStepIndex(MapboxNavigation navigation) {
if (shouldIncreaseStepIndex) {
advanceStepIndex(navigation);
shouldIncreaseStepIndex = false;
void checkIncreaseIndex(MapboxNavigation navigation) {
if (shouldIncreaseIndex) {
advanceIndices(navigation);
shouldIncreaseIndex = false;
}
}

Expand All @@ -146,7 +146,7 @@ Location buildSnappedLocation(MapboxNavigation mapboxNavigation, boolean snapToR
*
* @param mapboxNavigation to get the next {@link LegStep#geometry()} and {@link OffRoute}
*/
private void advanceStepIndex(MapboxNavigation mapboxNavigation) {
private void advanceIndices(MapboxNavigation mapboxNavigation) {
indices = increaseIndex(previousRouteProgress, indices);

stepPoints = decodeStepPoints(mapboxNavigation.getRoute(), indices.legIndex(), indices.stepIndex());
Expand All @@ -171,7 +171,7 @@ private List<Point> decodeStepPoints(DirectionsRoute directionsRoute, int legInd
return stepPoints;
}
List<LegStep> steps = legs.get(legIndex).steps();
if (hasInvalidSteps(stepIndex, steps)) {
if (hasInvalidSteps(steps)) {
return stepPoints;
}

Expand Down
11 changes: 4 additions & 7 deletions ...src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public class NavigationService extends Service implements LocationEngineListener

// Message id used when a new location update occurs and we send to the thread.
private static final int MSG_LOCATION_UPDATED = 1001;
private static final int HORIZONTAL_ACCURACY_THRESHOLD_IN_METERS = 100;

private final IBinder localBinder = new LocalBinder();

Expand Down Expand Up @@ -286,13 +287,9 @@ private void startForegroundNotification(Notification notification, int notifica
*/
@SuppressWarnings("MissingPermission")
private boolean isValidLocationUpdate(Location location) {
if (location == null) {
return false;
}
// If the locations the same as previous, no need to recalculate things
return !(location.equals(locationEngine.getLastLocation())
|| (location.getSpeed() <= 0 && location.hasSpeed())
|| location.getAccuracy() >= 100);
return location != null
&& location.hasSpeed()
&& location.getAccuracy() <= HORIZONTAL_ACCURACY_THRESHOLD_IN_METERS;
}

/**
Expand Down
11 changes: 11 additions & 0 deletions libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/BaseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,17 @@
import com.mapbox.api.directions.v5.DirectionsAdapterFactory;
import com.mapbox.api.directions.v5.models.DirectionsResponse;
import com.mapbox.api.directions.v5.models.DirectionsRoute;
import com.mapbox.api.directions.v5.models.LegStep;
import com.mapbox.core.constants.Constants;
import com.mapbox.geojson.LineString;
import com.mapbox.geojson.Point;
import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress;
import com.mapbox.turf.TurfConstants;
import com.mapbox.turf.TurfMeasurement;

import java.io.IOException;
import java.io.InputStream;
import java.util.List;
import java.util.Scanner;

import static junit.framework.Assert.assertEquals;
Expand Down Expand Up @@ -92,4 +96,11 @@ protected Point buildPointAwayFromLocation(Location location, double distanceAwa
protected Point buildPointAwayFromPoint(Point point, double distanceAway, double bearing) {
return TurfMeasurement.destination(point, distanceAway, bearing, TurfConstants.UNIT_METERS);
}

@NonNull
protected List<Point> createCoordinatesFromCurrentStep(RouteProgress progress) {
LegStep currentStep = progress.currentLegProgress().currentStep();
LineString lineString = LineString.fromPolyline(currentStep.geometry(), Constants.PRECISION_6);
return lineString.coordinates();
}
}
55 changes: 36 additions & 19 deletions ...va/com/mapbox/services/android/navigation/v5/navigation/NavigationRouteProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
import android.content.Context;
import android.location.Location;

import com.mapbox.api.directions.v5.models.LegStep;
import com.mapbox.core.constants.Constants;
import com.mapbox.geojson.LineString;
import com.mapbox.geojson.Point;
import com.mapbox.services.android.navigation.v5.BaseTest;
import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress;
Expand Down Expand Up @@ -53,9 +50,11 @@ public void onShouldIncreaseStepIndex_indexIsIncreased() throws Exception {
RouteProgress progress = routeProcessor.buildNewRouteProgress(navigation, mock(Location.class));
int currentStepIndex = progress.currentLegProgress().stepIndex();
routeProcessor.onShouldIncreaseIndex();
routeProcessor.checkIncreaseStepIndex(navigation);
routeProcessor.checkIncreaseIndex(navigation);

RouteProgress secondProgress = routeProcessor.buildNewRouteProgress(navigation, mock(Location.class));
int secondStepIndex = secondProgress.currentLegProgress().stepIndex();

assertTrue(currentStepIndex != secondStepIndex);
}

Expand All @@ -64,9 +63,7 @@ public void onSnapToRouteEnabledAndUserOnRoute_snappedLocationReturns() throws E
RouteProgress progress = routeProcessor.buildNewRouteProgress(navigation, mock(Location.class));
boolean snapEnabled = true;
boolean userOffRoute = false;
LegStep currentStep = progress.currentLegProgress().currentStep();
LineString lineString = LineString.fromPolyline(currentStep.geometry(), Constants.PRECISION_6);
List<Point> coordinates = lineString.coordinates();
List<Point> coordinates = createCoordinatesFromCurrentStep(progress);
Point lastPointInCurrentStep = coordinates.remove(coordinates.size() - 1);
Location rawLocation = buildDefaultLocationUpdate(
lastPointInCurrentStep.longitude(), lastPointInCurrentStep.latitude()
Expand All @@ -84,9 +81,7 @@ public void onSnapToRouteDisabledAndUserOnRoute_rawLocationReturns() throws Exce
RouteProgress progress = routeProcessor.buildNewRouteProgress(navigation, mock(Location.class));
boolean snapEnabled = false;
boolean userOffRoute = false;
LegStep currentStep = progress.currentLegProgress().currentStep();
LineString lineString = LineString.fromPolyline(currentStep.geometry(), Constants.PRECISION_6);
List<Point> coordinates = lineString.coordinates();
List<Point> coordinates = createCoordinatesFromCurrentStep(progress);
Point lastPointInCurrentStep = coordinates.remove(coordinates.size() - 1);
Location rawLocation = buildDefaultLocationUpdate(
lastPointInCurrentStep.longitude(), lastPointInCurrentStep.latitude()
Expand All @@ -104,9 +99,7 @@ public void onSnapToRouteEnabledAndUserOffRoute_rawLocationReturns() throws Exce
RouteProgress progress = routeProcessor.buildNewRouteProgress(navigation, mock(Location.class));
boolean snapEnabled = false;
boolean userOffRoute = false;
LegStep currentStep = progress.currentLegProgress().currentStep();
LineString lineString = LineString.fromPolyline(currentStep.geometry(), Constants.PRECISION_6);
List<Point> coordinates = lineString.coordinates();
List<Point> coordinates = createCoordinatesFromCurrentStep(progress);
Point lastPointInCurrentStep = coordinates.remove(coordinates.size() - 1);
Location rawLocation = buildDefaultLocationUpdate(
lastPointInCurrentStep.longitude(), lastPointInCurrentStep.latitude()
Expand All @@ -123,9 +116,7 @@ public void onSnapToRouteEnabledAndUserOffRoute_rawLocationReturns() throws Exce
public void onStepDistanceRemainingZeroAndNoBearingMatch_stepIndexForceIncreased() throws Exception {
RouteProgress firstProgress = routeProcessor.buildNewRouteProgress(navigation, mock(Location.class));
int firstProgressIndex = firstProgress.currentLegProgress().stepIndex();
LegStep currentStep = firstProgress.currentLegProgress().currentStep();
LineString lineString = LineString.fromPolyline(currentStep.geometry(), Constants.PRECISION_6);
List<Point> coordinates = lineString.coordinates();
List<Point> coordinates = createCoordinatesFromCurrentStep(firstProgress);
Point lastPointInCurrentStep = coordinates.remove(coordinates.size() - 1);
Location rawLocation = buildDefaultLocationUpdate(
lastPointInCurrentStep.longitude(), lastPointInCurrentStep.latitude()
Expand All @@ -137,13 +128,39 @@ public void onStepDistanceRemainingZeroAndNoBearingMatch_stepIndexForceIncreased
assertTrue(firstProgressIndex != secondProgressIndex);
}

@Test
public void onInvalidNextLeg_indexIsNotIncreased() throws Exception {
routeProcessor.buildNewRouteProgress(navigation, mock(Location.class));
int legSize = navigation.getRoute().legs().size();

for (int i = 0; i < legSize; i++) {
routeProcessor.onShouldIncreaseIndex();
routeProcessor.checkIncreaseIndex(navigation);
}
RouteProgress progress = routeProcessor.buildNewRouteProgress(navigation, mock(Location.class));

assertTrue(progress.legIndex() == legSize - 1);
}

@Test
public void onInvalidNextStep_indexIsNotIncreased() throws Exception {
routeProcessor.buildNewRouteProgress(navigation, mock(Location.class));
int stepSize = navigation.getRoute().legs().get(0).steps().size();

for (int i = 0; i < stepSize; i++) {
routeProcessor.onShouldIncreaseIndex();
routeProcessor.checkIncreaseIndex(navigation);
}
RouteProgress progress = routeProcessor.buildNewRouteProgress(navigation, mock(Location.class));

assertTrue(progress.currentLegProgress().stepIndex() == stepSize - 1);
}

@Test
public void withinManeuverRadiusAndBearingMatches_stepIndexIsIncreased() throws Exception {
RouteProgress firstProgress = routeProcessor.buildNewRouteProgress(navigation, mock(Location.class));
int firstProgressIndex = firstProgress.currentLegProgress().stepIndex();
LegStep currentStep = firstProgress.currentLegProgress().currentStep();
LineString lineString = LineString.fromPolyline(currentStep.geometry(), Constants.PRECISION_6);
List<Point> coordinates = lineString.coordinates();
List<Point> coordinates = createCoordinatesFromCurrentStep(firstProgress);
Point lastPointInCurrentStep = coordinates.remove(coordinates.size() - 2);
Location rawLocation = buildDefaultLocationUpdate(
lastPointInCurrentStep.longitude(), lastPointInCurrentStep.latitude()
Expand Down

0 comments on commit 2975aea

Please sign in to comment.