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 cbfb7cd4381..52f60647822 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 @@ -426,7 +426,6 @@ class MapboxNavigation @VisibleForTesting internal constructor( routeAlternativesController = RouteAlternativesControllerProvider.create( navigationOptions.routeAlternativesOptions, navigator, - directionsSession, tripSession ) routeRefreshController = RouteRefreshControllerProvider.createRouteRefreshController( @@ -621,10 +620,14 @@ class MapboxNavigation @VisibleForTesting internal constructor( } rerouteController?.interrupt() + // Telemetry uses this field to determine what type of event should be triggered. @RoutesExtra.RoutesUpdateReason val reason = when { routes.isEmpty() -> { RoutesExtra.ROUTES_UPDATE_REASON_CLEAN_UP } + routes.first() == directionsSession.routes.firstOrNull() -> { + RoutesExtra.ROUTES_UPDATE_REASON_ALTERNATIVE + } else -> { RoutesExtra.ROUTES_UPDATE_REASON_NEW } 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 27ac8a71148..6b96f8a93d2 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 @@ -6,8 +6,6 @@ import com.mapbox.base.common.logger.model.Tag import com.mapbox.navigation.base.internal.utils.parseDirectionsResponse 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 com.mapbox.navigation.utils.internal.logI @@ -17,9 +15,9 @@ import java.util.concurrent.CopyOnWriteArraySet internal class RouteAlternativesController constructor( private val options: RouteAlternativesOptions, private val navigator: MapboxNativeNavigator, - private val directionsSession: DirectionsSession, private val tripSession: TripSession ) { + private val nativeRouteAlternativesController = navigator.createRouteAlternativesController() .apply { setRouteAlternativesOptions( @@ -65,33 +63,20 @@ 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. - var alternatives: List + // TODO make async - runBlocking { - alternatives = routeAlternatives.map { routeAlternative -> + val alternatives: List = runBlocking { + routeAlternatives.map { routeAlternative -> parseDirectionsResponse( routeAlternative.route, - currentRoute.routeOptions() + routeProgress.route.routeOptions() ) { logI(TAG, Message("Response metadata: $it")) }.first() } } - changedRoutes.addAll(alternatives) - - directionsSession.setRoutes( - routes = changedRoutes, - initialLegIndex = 0, - ROUTES_UPDATE_REASON_ALTERNATIVE - ) - // Notify the listeners. // TODO https://github.com/mapbox/mapbox-navigation-native/issues/4409 // There is no way to determine if the route was Onboard or Offboard diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/routealternatives/RouteAlternativesControllerProvider.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/routealternatives/RouteAlternativesControllerProvider.kt index bc96fef23aa..8ec105da2bc 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/routealternatives/RouteAlternativesControllerProvider.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/routealternatives/RouteAlternativesControllerProvider.kt @@ -1,7 +1,6 @@ package com.mapbox.navigation.core.routealternatives import com.mapbox.navigation.base.route.RouteAlternativesOptions -import com.mapbox.navigation.core.directions.session.DirectionsSession import com.mapbox.navigation.core.trip.session.TripSession import com.mapbox.navigation.navigator.internal.MapboxNativeNavigator @@ -10,12 +9,10 @@ internal object RouteAlternativesControllerProvider { fun create( options: RouteAlternativesOptions, navigator: MapboxNativeNavigator, - directionsSession: DirectionsSession, tripSession: TripSession ) = RouteAlternativesController( options, navigator, - directionsSession, tripSession ) } 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 87984ae05b2..34137cfd095 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) @@ -181,7 +180,7 @@ class MapboxNavigationTest { } returns routeRefreshController mockkObject(RouteAlternativesControllerProvider) every { - RouteAlternativesControllerProvider.create(any(), any(), any(), any()) + RouteAlternativesControllerProvider.create(any(), any(), any()) } returns routeAlternativesController every { applicationContext.applicationContext } returns applicationContext @@ -1074,6 +1073,22 @@ class MapboxNavigationTest { } } + @Test + fun `adding or removing alternative routes creates alternative reason`() { + createMapboxNavigation() + val primaryRoute = mockk() + val alternativeRoute = mockk() + every { directionsSession.routes } returns listOf(primaryRoute) + + mapboxNavigation.setRoutes(listOf(primaryRoute, alternativeRoute)) + mapboxNavigation.setRoutes(listOf(primaryRoute)) + + verifyOrder { + directionsSession.setRoutes(any(), any(), RoutesExtra.ROUTES_UPDATE_REASON_ALTERNATIVE) + directionsSession.setRoutes(any(), 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..f55bb2823a4 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 @@ -4,8 +4,6 @@ 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.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 @@ -32,7 +30,6 @@ class RouteAlternativesControllerTest { private val navigator: MapboxNativeNavigator = mockk { every { createRouteAlternativesController() } returns controllerInterface } - private val directionsSession: DirectionsSession = mockk(relaxed = true) private val tripSession: TripSession = mockk(relaxed = true) private fun routeAlternativesController( @@ -40,7 +37,6 @@ class RouteAlternativesControllerTest { ) = RouteAlternativesController( options, navigator, - directionsSession, tripSession, ) @@ -172,40 +168,6 @@ class RouteAlternativesControllerTest { assertEquals(RouterOrigin.Onboard, routerOriginSlot.captured) } - @Test - fun `should set route with the new 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) - } - - val firstObserver: RouteAlternativesObserver = mockk(relaxed = true) - routeAlternativesController.register(firstObserver) - val alternativeRouteJson = FileUtils.loadJsonFixture( - "route_alternative_from_native.txt" - ) - nativeObserver.captured.onRouteAlternativesChanged( - listOf( - mockk { - every { route } returns alternativeRouteJson - } - ) - ) - - val routesSetCapture = slot>() - verify(exactly = 1) { - directionsSession.setRoutes( - capture(routesSetCapture), - 0, - RoutesExtra.ROUTES_UPDATE_REASON_ALTERNATIVE - ) - } - assertEquals(2, routesSetCapture.captured.size) - assertEquals(221.796, routesSetCapture.captured[1].duration(), 0.001) - } - @Test fun `should set alternative RouteOptions to primary RouteOptions`() { val originalCoordinates = "-122.270375,37.801429;-122.271496, 37.799063" @@ -219,8 +181,6 @@ class RouteAlternativesControllerTest { } } } - val routesSetCapture = slot>() - every { directionsSession.setRoutes(capture(routesSetCapture), any(), any()) } just runs val firstObserver: RouteAlternativesObserver = mockk(relaxed = true) routeAlternativesController.register(firstObserver) @@ -235,11 +195,22 @@ class RouteAlternativesControllerTest { ) ) + val routeProgressSlot = slot() + val alternativesSlot = slot>() + val routerOriginSlot = slot() + verify(exactly = 1) { + firstObserver.onRouteAlternatives( + capture(routeProgressSlot), + capture(alternativesSlot), + capture(routerOriginSlot) + ) + } + assertEquals( - originalCoordinates, routesSetCapture.captured[0].routeOptions()?.coordinates() + originalCoordinates, routeProgressSlot.captured.route.routeOptions()?.coordinates() ) assertEquals( - originalCoordinates, routesSetCapture.captured[1].routeOptions()?.coordinates() + originalCoordinates, alternativesSlot.captured[0].routeOptions()?.coordinates() ) } } 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..8df2c7c8899 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() } @@ -222,7 +229,7 @@ class AlternativeRouteActivity : AppCompatActivity(), OnMapLongClickListener { routes: List, routerOrigin: RouterOrigin ) { - mapboxNavigation.setRoutes(routes) + mapboxNavigation.setRoutes(routes.reversed()) } override fun onFailure(