Skip to content

Commit

Permalink
Remove route alternatives setRoute
Browse files Browse the repository at this point in the history
  • Loading branch information
kmadsen committed Oct 28, 2021
1 parent 82c65f4 commit 1f8fe8f
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,6 @@ class MapboxNavigation @VisibleForTesting internal constructor(
routeAlternativesController = RouteAlternativesControllerProvider.create(
navigationOptions.routeAlternativesOptions,
navigator,
directionsSession,
tripSession
)
routeRefreshController = RouteRefreshControllerProvider.createRouteRefreshController(
Expand Down Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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<DirectionsRoute>()
val currentRoute = routeProgress.route
changedRoutes.add(currentRoute)

// Map the alternatives from nav-native, add the existing RouteOptions.
var alternatives: List<DirectionsRoute>

// TODO make async
runBlocking {
alternatives = routeAlternatives.map { routeAlternative ->
val alternatives: List<DirectionsRoute> = 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
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -10,12 +9,10 @@ internal object RouteAlternativesControllerProvider {
fun create(
options: RouteAlternativesOptions,
navigator: MapboxNativeNavigator,
directionsSession: DirectionsSession,
tripSession: TripSession
) = RouteAlternativesController(
options,
navigator,
directionsSession,
tripSession
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1074,6 +1073,22 @@ class MapboxNavigationTest {
}
}

@Test
fun `adding or removing alternative routes creates alternative reason`() {
createMapboxNavigation()
val primaryRoute = mockk<DirectionsRoute>()
val alternativeRoute = mockk<DirectionsRoute>()
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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -32,15 +30,13 @@ 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(
options: RouteAlternativesOptions = RouteAlternativesOptions.Builder().build()
) = RouteAlternativesController(
options,
navigator,
directionsSession,
tripSession,
)

Expand Down Expand Up @@ -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<com.mapbox.navigator.RouteAlternativesObserver>()
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<List<DirectionsRoute>>()
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"
Expand All @@ -219,8 +181,6 @@ class RouteAlternativesControllerTest {
}
}
}
val routesSetCapture = slot<List<DirectionsRoute>>()
every { directionsSession.setRoutes(capture(routesSetCapture), any(), any()) } just runs

val firstObserver: RouteAlternativesObserver = mockk(relaxed = true)
routeAlternativesController.register(firstObserver)
Expand All @@ -235,11 +195,22 @@ class RouteAlternativesControllerTest {
)
)

val routeProgressSlot = slot<RouteProgress>()
val alternativesSlot = slot<List<DirectionsRoute>>()
val routerOriginSlot = slot<RouterOrigin>()
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()
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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()
Expand Down Expand Up @@ -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<DirectionsRoute>()
updatedRoutes.add(routeProgress.route)
updatedRoutes.addAll(alternatives)
mapboxNavigation.setRoutes(updatedRoutes)

// Request new alternatives instead of waiting for the interval.
mapboxNavigation.requestAlternativeRoutes()
}

Expand All @@ -222,7 +229,7 @@ class AlternativeRouteActivity : AppCompatActivity(), OnMapLongClickListener {
routes: List<DirectionsRoute>,
routerOrigin: RouterOrigin
) {
mapboxNavigation.setRoutes(routes)
mapboxNavigation.setRoutes(routes.reversed())
}

override fun onFailure(
Expand Down

0 comments on commit 1f8fe8f

Please sign in to comment.