diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt index 42a3f2ae851..66de05699b7 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt @@ -619,6 +619,9 @@ class MapboxNavigation( rerouteController?.interrupt() @RoutesExtra.RoutesUpdateReason val reason = when { + routeAlternativesController.areAlternatives(routes) -> { + RoutesExtra.ROUTES_UPDATE_REASON_ALTERNATIVE + } routes.isEmpty() -> { RoutesExtra.ROUTES_UPDATE_REASON_CLEAN_UP } diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/routealternatives/RouteAlternativesController.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/routealternatives/RouteAlternativesController.kt index a2434be1aa8..a4675156b33 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/routealternatives/RouteAlternativesController.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/routealternatives/RouteAlternativesController.kt @@ -4,9 +4,9 @@ import com.mapbox.api.directions.v5.models.DirectionsRoute import com.mapbox.navigation.base.route.RouteAlternativesOptions import com.mapbox.navigation.base.route.RouterOrigin import com.mapbox.navigation.core.directions.session.DirectionsSession -import com.mapbox.navigation.core.directions.session.RoutesExtra.ROUTES_UPDATE_REASON_ALTERNATIVE import com.mapbox.navigation.core.trip.session.TripSession import com.mapbox.navigation.navigator.internal.MapboxNativeNavigator +import java.util.concurrent.CopyOnWriteArrayList import java.util.concurrent.CopyOnWriteArraySet internal class RouteAlternativesController constructor( @@ -27,6 +27,7 @@ internal class RouteAlternativesController constructor( enableEmptyAlternativesRefresh(true) } + private val cachedAlternatives = CopyOnWriteArrayList() private val observers = CopyOnWriteArraySet() fun register(routeAlternativesObserver: RouteAlternativesObserver) { @@ -53,6 +54,10 @@ internal class RouteAlternativesController constructor( observers.clear() } + fun areAlternatives(routes: List): Boolean { + return routes.map { it in cachedAlternatives }.any { it } + } + private val nativeObserver = object : com.mapbox.navigator.RouteAlternativesObserver { override fun onRouteAlternativesChanged( routeAlternatives: List @@ -60,26 +65,16 @@ internal class RouteAlternativesController constructor( val routeProgress = tripSession.getRouteProgress() ?: return emptyList() - // Create a new list of routes, add the current route at index 0. - val changedRoutes = mutableListOf() - val currentRoute = routeProgress.route - changedRoutes.add(currentRoute) - // Map the alternatives from nav-native, add the existing RouteOptions. val alternatives = routeAlternatives.map { routeAlternative -> DirectionsRoute.fromJson( routeAlternative.route, - currentRoute.routeOptions(), + routeProgress.route.routeOptions(), null // We don't know the requestUuid at this point ) } - changedRoutes.addAll(alternatives) - - directionsSession.setRoutes( - routes = changedRoutes, - initialLegIndex = 0, - ROUTES_UPDATE_REASON_ALTERNATIVE - ) + cachedAlternatives.clear() + cachedAlternatives.addAll(alternatives) // Notify the listeners. // TODO https://github.com/mapbox/mapbox-navigation-native/issues/4409 diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt index d91f924a97b..6f475f99938 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt @@ -104,8 +104,7 @@ class MapboxNavigationTest { private val distanceFormatterOptions: DistanceFormatterOptions = mockk(relaxed = true) private val routingTilesOptions: RoutingTilesOptions = mockk(relaxed = true) private val routeRefreshController: RouteRefreshController = mockk(relaxUnitFun = true) - private val routeAlternativesController: RouteAlternativesController = - mockk(relaxUnitFun = true) + private val routeAlternativesController: RouteAlternativesController = mockk(relaxed = true) private val routeProgress: RouteProgress = mockk(relaxed = true) private val navigationSession: NavigationSession = mockk(relaxed = true) private val billingController: BillingController = mockk(relaxUnitFun = true) @@ -1073,6 +1072,19 @@ class MapboxNavigationTest { } } + @Test + fun `alternative route creates alternative reason`() { + createMapboxNavigation() + val routes = listOf(mockk()) + every { routeAlternativesController.areAlternatives(routes) } returns true + + mapboxNavigation.setRoutes(routes) + + verify { + directionsSession.setRoutes(routes, any(), RoutesExtra.ROUTES_UPDATE_REASON_ALTERNATIVE) + } + } + @Test fun `verify that billing controller is notified of instance destruction`() { createMapboxNavigation() diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/routealternatives/RouteAlternativesControllerTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/routealternatives/RouteAlternativesControllerTest.kt index 270f862ac9f..d3b59e308ee 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/routealternatives/RouteAlternativesControllerTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/routealternatives/RouteAlternativesControllerTest.kt @@ -5,7 +5,6 @@ import com.mapbox.navigation.base.route.RouteAlternativesOptions import com.mapbox.navigation.base.route.RouterOrigin import com.mapbox.navigation.base.trip.model.RouteProgress import com.mapbox.navigation.core.directions.session.DirectionsSession -import com.mapbox.navigation.core.directions.session.RoutesExtra import com.mapbox.navigation.core.trip.session.TripSession import com.mapbox.navigation.navigator.internal.MapboxNativeNavigator import com.mapbox.navigation.testing.FileUtils @@ -19,6 +18,8 @@ import io.mockk.slot import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test @@ -173,12 +174,17 @@ class RouteAlternativesControllerTest { } @Test - fun `should set route with the new alternatives`() { + fun `should set alternative RouteOptions to primary RouteOptions`() { + val originalCoordinates = "-122.270375,37.801429;-122.271496, 37.799063" val routeAlternativesController = routeAlternativesController() val nativeObserver = slot() every { controllerInterface.addObserver(capture(nativeObserver)) } just runs every { tripSession.getRouteProgress() } returns mockk { - every { route } returns mockk(relaxed = true) + every { route } returns mockk { + every { routeOptions() } returns mockk { + every { coordinates() } returns originalCoordinates + } + } } val firstObserver: RouteAlternativesObserver = mockk(relaxed = true) @@ -194,33 +200,35 @@ class RouteAlternativesControllerTest { ) ) - val routesSetCapture = slot>() + val routeProgressSlot = slot() + val alternativesSlot = slot>() + val routerOriginSlot = slot() verify(exactly = 1) { - directionsSession.setRoutes( - capture(routesSetCapture), - 0, - RoutesExtra.ROUTES_UPDATE_REASON_ALTERNATIVE + firstObserver.onRouteAlternatives( + capture(routeProgressSlot), + capture(alternativesSlot), + capture(routerOriginSlot) ) } - assertEquals(2, routesSetCapture.captured.size) - assertEquals(221.796, routesSetCapture.captured[1].duration(), 0.001) + + assertEquals( + originalCoordinates, routeProgressSlot.captured.route.routeOptions()?.coordinates() + ) + assertEquals( + originalCoordinates, alternativesSlot.captured[0].routeOptions()?.coordinates() + ) } @Test - fun `should set alternative RouteOptions to primary RouteOptions`() { - val originalCoordinates = "-122.270375,37.801429;-122.271496, 37.799063" + fun `areAlternatives is true when routes contains previously observed routes`() { val routeAlternativesController = routeAlternativesController() val nativeObserver = slot() every { controllerInterface.addObserver(capture(nativeObserver)) } just runs every { tripSession.getRouteProgress() } returns mockk { - every { route } returns mockk { - every { routeOptions() } returns mockk { - every { coordinates() } returns originalCoordinates - } + every { route } returns mockk(relaxed = true) { + every { duration() } returns 200.0 } } - val routesSetCapture = slot>() - every { directionsSession.setRoutes(capture(routesSetCapture), any(), any()) } just runs val firstObserver: RouteAlternativesObserver = mockk(relaxed = true) routeAlternativesController.register(firstObserver) @@ -235,11 +243,57 @@ class RouteAlternativesControllerTest { ) ) - assertEquals( - originalCoordinates, routesSetCapture.captured[0].routeOptions()?.coordinates() + val routeProgressSlot = slot() + val alternativesSlot = slot>() + verify(exactly = 1) { + firstObserver.onRouteAlternatives( + capture(routeProgressSlot), + capture(alternativesSlot), + any() + ) + } + + val updatedRoutes = mutableListOf() + updatedRoutes.add(routeProgressSlot.captured.route) + updatedRoutes.addAll(alternativesSlot.captured) + assertTrue(routeAlternativesController.areAlternatives(updatedRoutes)) + } + + @Test + fun `areAlternatives is false when routes does not contain observed alternatives`() { + val routeAlternativesController = routeAlternativesController() + val nativeObserver = slot() + every { controllerInterface.addObserver(capture(nativeObserver)) } just runs + every { tripSession.getRouteProgress() } returns mockk { + every { route } returns mockk(relaxed = true) { + every { duration() } returns 200.0 + } + } + + val firstObserver: RouteAlternativesObserver = mockk(relaxed = true) + routeAlternativesController.register(firstObserver) + val alternativeRouteJson = FileUtils.loadJsonFixture( + "route_alternative_from_native.txt" ) - assertEquals( - originalCoordinates, routesSetCapture.captured[1].routeOptions()?.coordinates() + nativeObserver.captured.onRouteAlternativesChanged( + listOf( + mockk { + every { route } returns alternativeRouteJson + } + ) ) + + val routeProgressSlot = slot() + verify(exactly = 1) { + firstObserver.onRouteAlternatives( + capture(routeProgressSlot), + any(), + any() + ) + } + + val updatedRoutes = mutableListOf() + updatedRoutes.add(routeProgressSlot.captured.route) + assertFalse(routeAlternativesController.areAlternatives(updatedRoutes)) } } diff --git a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/AlternativeRouteActivity.kt b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/AlternativeRouteActivity.kt index 5bbed67cc57..95e315a2376 100644 --- a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/AlternativeRouteActivity.kt +++ b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/AlternativeRouteActivity.kt @@ -115,7 +115,7 @@ class AlternativeRouteActivity : AppCompatActivity(), OnMapLongClickListener { mapboxNavigation.unregisterRouteProgressObserver(replayProgressObserver) mapboxNavigation.unregisterLocationObserver(locationObserver) mapboxNavigation.unregisterRoutesObserver(routesObserver) - mapboxNavigation.unregisterRouteAlternativesObserver(requestAlternativesObserver) + mapboxNavigation.unregisterRouteAlternativesObserver(routeAlternativesObserver) } override fun onDestroy() { @@ -134,7 +134,7 @@ class AlternativeRouteActivity : AppCompatActivity(), OnMapLongClickListener { mapboxNavigation.registerLocationObserver(locationObserver) mapboxNavigation.registerRouteProgressObserver(replayProgressObserver) mapboxNavigation.registerRoutesObserver(routesObserver) - mapboxNavigation.registerRouteAlternativesObserver(requestAlternativesObserver) + mapboxNavigation.registerRouteAlternativesObserver(routeAlternativesObserver) mapboxReplayer.pushRealLocation(this, 0.0) mapboxReplayer.playbackSpeed(1.5) mapboxReplayer.play() @@ -201,10 +201,17 @@ class AlternativeRouteActivity : AppCompatActivity(), OnMapLongClickListener { } /** - * Whenever alternatives have changed, request a new set of alternatives. + * Example of how to handle route alternatives during navigation. */ - private val requestAlternativesObserver = - RouteAlternativesObserver { _, _, _ -> + private val routeAlternativesObserver = + RouteAlternativesObserver { routeProgress, alternatives, _ -> + // Set the alternatives suggested + val updatedRoutes = mutableListOf() + updatedRoutes.add(routeProgress.route) + updatedRoutes.addAll(alternatives) + mapboxNavigation.setRoutes(updatedRoutes) + + // Request new alternatives instead of waiting for the interval. mapboxNavigation.requestAlternativeRoutes() }