use snapped location in check faster route, fix voice instructions logic, add instruction loader null checks to prevent potential npes and opt-in mocking of final classes/methods and fix voice instruction milestone tests (#1318)

Guardiola31337 · danesfeder · commit 65380e29bd95 · 2018-09-27T13:04:39.000-04:00
diff --git a/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/instruction/InstructionView.java b/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/instruction/InstructionView.java
@@ -750,15 +750,22 @@ private void updateDataFromInstruction(InstructionModel model) {
   private void updateDataFromBannerInstruction(InstructionModel model) {
     updateManeuverView(model);
     if (model.getPrimaryBannerText() != null) {
-      createInstructionLoader(upcomingPrimaryText, model.getPrimaryBannerText()).loadInstruction();
+      InstructionLoader instructionLoader = createInstructionLoader(upcomingPrimaryText, model.getPrimaryBannerText());
+      if (instructionLoader != null) {
+        instructionLoader.loadInstruction();
+      }
     }
     if (model.getSecondaryBannerText() != null) {
       if (upcomingSecondaryText.getVisibility() == GONE) {
         upcomingSecondaryText.setVisibility(VISIBLE);
         upcomingPrimaryText.setMaxLines(1);
         adjustBannerTextVerticalBias(0.65f);
       }
-      createInstructionLoader(upcomingSecondaryText, model.getSecondaryBannerText()).loadInstruction();
+      InstructionLoader instructionLoader = createInstructionLoader(upcomingSecondaryText,
+        model.getSecondaryBannerText());
+      if (instructionLoader != null) {
+        instructionLoader.loadInstruction();
+      }
     } else {
       upcomingPrimaryText.setMaxLines(2);
       upcomingSecondaryText.setVisibility(GONE);
diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/milestone/VoiceInstructionMilestone.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/milestone/VoiceInstructionMilestone.java
@@ -1,6 +1,7 @@
 package com.mapbox.services.android.navigation.v5.milestone;
 
 import com.mapbox.api.directions.v5.models.DirectionsRoute;
+import com.mapbox.navigator.VoiceInstruction;
 import com.mapbox.services.android.navigation.v5.instruction.Instruction;
 import com.mapbox.services.android.navigation.v5.navigation.VoiceInstructionLoader;
 import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress;
@@ -104,10 +105,10 @@ private void cacheInstructions(RouteProgress routeProgress, boolean isFirst) {
   }
 
   private boolean updateCurrentAnnouncement(RouteProgress routeProgress) {
-    String currentAnnouncement = routeProgress.currentAnnouncement();
-    if (!currentAnnouncement.isEmpty() && !announcement.equals(currentAnnouncement)) {
-      announcement = currentAnnouncement;
-      ssmlAnnouncement = routeProgress.currentSsmlAnnouncement();
+    VoiceInstruction currentVoiceInstruction = routeProgress.voiceInstruction();
+    if (currentVoiceInstruction != null) {
+      announcement = currentVoiceInstruction.getAnnouncement();
+      ssmlAnnouncement = currentVoiceInstruction.getSsmlAnnouncement();
       cacheInstructions(routeProgress, false);
       return true;
     }
diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRouteProcessor.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRouteProcessor.java
@@ -124,9 +124,6 @@ private void addUpcomingStepPoints(RouteProgress.Builder progressBuilder) {
 
   private void addVoiceInstructions(NavigationStatus status, RouteProgress.Builder progressBuilder) {
     VoiceInstruction voiceInstruction = status.getVoiceInstruction();
-    if (voiceInstruction != null) {
-      progressBuilder.currentAnnouncement(voiceInstruction.getAnnouncement());
-      progressBuilder.currentSsmlAnnouncement(voiceInstruction.getSsmlAnnouncement());
-    }
+    progressBuilder.voiceInstruction(voiceInstruction);
   }
 }
diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/RouteProcessorRunnable.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/RouteProcessorRunnable.java
@@ -59,7 +59,8 @@ private void process() {
     NavigationEngineFactory engineFactory = navigation.retrieveEngineFactory();
     final boolean userOffRoute = isUserOffRoute(options, status, rawLocation, routeProgress, engineFactory);
     final Location snappedLocation = findSnappedLocation(status, rawLocation, routeProgress, engineFactory);
-    final boolean checkFasterRoute = checkFasterRoute(options, rawLocation, routeProgress, engineFactory, userOffRoute);
+    final boolean checkFasterRoute = checkFasterRoute(options, snappedLocation, routeProgress, engineFactory,
+      userOffRoute);
     final List<Milestone> milestones = findTriggeredMilestones(navigation, routeProgress);
 
     sendUpdateToResponseHandler(userOffRoute, milestones, snappedLocation, checkFasterRoute, routeProgress);
diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/routeprogress/RouteProgress.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/routeprogress/RouteProgress.java
@@ -9,6 +9,7 @@
 import com.mapbox.api.directions.v5.models.RouteLeg;
 import com.mapbox.api.directions.v5.models.StepIntersection;
 import com.mapbox.geojson.Point;
+import com.mapbox.navigator.VoiceInstruction;
 
 import java.util.List;
 
@@ -28,8 +29,6 @@
 @AutoValue
 public abstract class RouteProgress {
 
-  private static final String EMPTY_ANNOUNCEMENT = "";
-
   /**
    * Get the route the navigation session is currently using. When a reroute occurs and a new
    * directions route gets obtained, with the next location update this directions route should
@@ -161,16 +160,13 @@ public int remainingWaypoints() {
   public abstract boolean inTunnel();
 
   /**
-   * @return current announcement
-   * @since 0.19.0
-   */
-  public abstract String currentAnnouncement();
-
-  /**
-   * @return current announcement with SSML markup
-   * @since 0.19.0
+   * Current voice instruction.
+   *
+   * @return current voice instruction
+   * @since 0.20.0
    */
-  public abstract String currentSsmlAnnouncement();
+  @Nullable
+  public abstract VoiceInstruction voiceInstruction();
 
   public abstract RouteProgress.Builder toBuilder();
 
@@ -251,9 +247,7 @@ public abstract Builder intersectionDistancesAlongStep(
 
     public abstract Builder inTunnel(boolean inTunnel);
 
-    public abstract Builder currentAnnouncement(String announcement);
-
-    public abstract Builder currentSsmlAnnouncement(String ssmlAnnouncement);
+    public abstract Builder voiceInstruction(@Nullable VoiceInstruction voiceInstruction);
 
     abstract RouteProgress autoBuild(); // not public
 
@@ -279,8 +273,6 @@ public RouteProgress build() {
   }
 
   public static Builder builder() {
-    return new AutoValue_RouteProgress.Builder()
-      .currentAnnouncement(EMPTY_ANNOUNCEMENT)
-      .currentSsmlAnnouncement(EMPTY_ANNOUNCEMENT);
+    return new AutoValue_RouteProgress.Builder();
   }
 }
diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/snap/SnapToRoute.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/snap/SnapToRoute.java
@@ -24,6 +24,7 @@ private Location buildSnappedLocation(Location location, NavigationStatus status
     snappedLocation.setLatitude(status.getLocation().latitude());
     snappedLocation.setLongitude(status.getLocation().longitude());
     snappedLocation.setBearing(status.getBearing());
+    snappedLocation.setTime(status.getTime().getTime());
     return snappedLocation;
   }
 }
diff --git a/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/milestone/VoiceInstructionMilestoneTest.java b/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/milestone/VoiceInstructionMilestoneTest.java
@@ -1,13 +1,15 @@
 package com.mapbox.services.android.navigation.v5.milestone;
 
 import com.mapbox.api.directions.v5.models.DirectionsRoute;
+import com.mapbox.navigator.VoiceInstruction;
 import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress;
 
 import org.junit.Test;
 
 import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertFalse;
 import static junit.framework.Assert.assertNotNull;
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -23,26 +25,25 @@ public void sanity() {
   @Test
   public void onSameInstructionOccurring_milestoneDoesNotTriggerTwice() {
     RouteProgress firstProgress = mock(RouteProgress.class);
-    when(firstProgress.currentAnnouncement()).thenReturn("instruction");
+    VoiceInstruction voiceInstruction = mock(VoiceInstruction.class);
+    when(firstProgress.voiceInstruction()).thenReturn(voiceInstruction, null);
+    when(voiceInstruction.getAnnouncement()).thenReturn("instruction");
     when(firstProgress.directionsRoute()).thenReturn(mock(DirectionsRoute.class));
-    RouteProgress secondProgress = mock(RouteProgress.class);
-    when(secondProgress.directionsRoute()).thenReturn(mock(DirectionsRoute.class));
-    when(secondProgress.currentAnnouncement()).thenReturn("instruction");
     VoiceInstructionMilestone milestone = buildVoiceInstructionMilestone();
 
     milestone.isOccurring(firstProgress, firstProgress);
-    boolean shouldNotBeOccurring = milestone.isOccurring(firstProgress, secondProgress);
+    boolean shouldNotBeOccurring = milestone.isOccurring(firstProgress, firstProgress);
 
     assertFalse(shouldNotBeOccurring);
   }
 
   @Test
   public void onOccurringMilestone_voiceSsmlInstructionsAreReturned() {
-    RouteProgress routeProgress = mock(RouteProgress.class);
+    RouteProgress routeProgress = mock(RouteProgress.class, RETURNS_DEEP_STUBS);
     when(routeProgress.directionsRoute()).thenReturn(mock(DirectionsRoute.class));
-    when(routeProgress.currentAnnouncement()).thenReturn("current announcement");
+    when(routeProgress.voiceInstruction().getAnnouncement()).thenReturn("current announcement");
     String currentSsmlAnnouncement = "current SSML announcement";
-    when(routeProgress.currentSsmlAnnouncement()).thenReturn(currentSsmlAnnouncement);
+    when(routeProgress.voiceInstruction().getSsmlAnnouncement()).thenReturn(currentSsmlAnnouncement);
     VoiceInstructionMilestone milestone = buildVoiceInstructionMilestone();
 
     milestone.isOccurring(routeProgress, routeProgress);
@@ -52,12 +53,12 @@ public void onOccurringMilestone_voiceSsmlInstructionsAreReturned() {
 
   @Test
   public void onOccurringMilestone_voiceInstructionsAreReturned() {
-    RouteProgress routeProgress = mock(RouteProgress.class);
+    RouteProgress routeProgress = mock(RouteProgress.class, RETURNS_DEEP_STUBS);
     when(routeProgress.directionsRoute()).thenReturn(mock(DirectionsRoute.class));
     String currentAnnouncement = "current announcement";
-    when(routeProgress.currentAnnouncement()).thenReturn(currentAnnouncement);
+    when(routeProgress.voiceInstruction().getAnnouncement()).thenReturn(currentAnnouncement);
     String currentSsmlAnnouncement = "current SSML announcement";
-    when(routeProgress.currentSsmlAnnouncement()).thenReturn(currentSsmlAnnouncement);
+    when(routeProgress.voiceInstruction().getSsmlAnnouncement()).thenReturn(currentSsmlAnnouncement);
     VoiceInstructionMilestone milestone = buildVoiceInstructionMilestone();
 
     milestone.isOccurring(routeProgress, routeProgress);
@@ -67,12 +68,12 @@ public void onOccurringMilestone_voiceInstructionsAreReturned() {
 
   @Test
   public void onOccurringMilestone_instructionsAreReturned() {
-    RouteProgress routeProgress = mock(RouteProgress.class);
+    RouteProgress routeProgress = mock(RouteProgress.class, RETURNS_DEEP_STUBS);
     when(routeProgress.directionsRoute()).thenReturn(mock(DirectionsRoute.class));
     String currentAnnouncement = "current announcement";
-    when(routeProgress.currentAnnouncement()).thenReturn(currentAnnouncement);
+    when(routeProgress.voiceInstruction().getAnnouncement()).thenReturn(currentAnnouncement);
     String currentSsmlAnnouncement = "current SSML announcement";
-    when(routeProgress.currentSsmlAnnouncement()).thenReturn(currentSsmlAnnouncement);
+    when(routeProgress.voiceInstruction().getSsmlAnnouncement()).thenReturn(currentSsmlAnnouncement);
     VoiceInstructionMilestone milestone = buildVoiceInstructionMilestone();
 
     milestone.isOccurring(routeProgress, routeProgress);
diff --git a/libandroid-navigation/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/libandroid-navigation/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker
@@ -0,0 +1 @@
+mock-maker-inline

Original file line number	Diff line number	Diff line change
`@@ -124,9 +124,6 @@ private void addUpcomingStepPoints(RouteProgress.Builder progressBuilder) {`
`124`	`124`
`125`	`125`	`private void addVoiceInstructions(NavigationStatus status, RouteProgress.Builder progressBuilder) {`
`126`	`126`	`VoiceInstruction voiceInstruction = status.getVoiceInstruction();`
`127`		`- if (voiceInstruction != null) {`
`128`		`- progressBuilder.currentAnnouncement(voiceInstruction.getAnnouncement());`
`129`		`- progressBuilder.currentSsmlAnnouncement(voiceInstruction.getSsmlAnnouncement());`
`130`		`- }`
	`127`	`+ progressBuilder.voiceInstruction(voiceInstruction);`
`131`	`128`	`}`
`132`	`129`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@ private Location buildSnappedLocation(Location location, NavigationStatus status`
`24`	`24`	`snappedLocation.setLatitude(status.getLocation().latitude());`
`25`	`25`	`snappedLocation.setLongitude(status.getLocation().longitude());`
`26`	`26`	`snappedLocation.setBearing(status.getBearing());`
	`27`	`+ snappedLocation.setTime(status.getTime().getTime());`
`27`	`28`	`return snappedLocation;`
`28`	`29`	`}`
`29`	`30`	`}`