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 25, 2021
1 parent 5d5117e commit 6f81113
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -27,6 +27,7 @@ internal class RouteAlternativesController constructor(
enableEmptyAlternativesRefresh(true)
}

private val cachedAlternatives = CopyOnWriteArrayList<DirectionsRoute>()
private val observers = CopyOnWriteArraySet<RouteAlternativesObserver>()

fun register(routeAlternativesObserver: RouteAlternativesObserver) {
Expand All @@ -53,33 +54,27 @@ internal class RouteAlternativesController constructor(
observers.clear()
}

fun areAlternatives(routes: List<DirectionsRoute>): Boolean {
return routes.map { it in cachedAlternatives }.any { it }
}

private val nativeObserver = object : com.mapbox.navigator.RouteAlternativesObserver {
override fun onRouteAlternativesChanged(
routeAlternatives: List<com.mapbox.navigator.RouteAlternative>
): List<Int> {
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.
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
Expand Down
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 @@ -1073,6 +1072,19 @@ class MapboxNavigationTest {
}
}

@Test
fun `alternative route creates alternative reason`() {
createMapboxNavigation()
val routes = listOf(mockk<DirectionsRoute>())
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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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<com.mapbox.navigator.RouteAlternativesObserver>()
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)
Expand All @@ -194,33 +200,35 @@ class RouteAlternativesControllerTest {
)
)

val routesSetCapture = slot<List<DirectionsRoute>>()
val routeProgressSlot = slot<RouteProgress>()
val alternativesSlot = slot<List<DirectionsRoute>>()
val routerOriginSlot = slot<RouterOrigin>()
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<com.mapbox.navigator.RouteAlternativesObserver>()
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<List<DirectionsRoute>>()
every { directionsSession.setRoutes(capture(routesSetCapture), any(), any()) } just runs

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

assertEquals(
originalCoordinates, routesSetCapture.captured[0].routeOptions()?.coordinates()
val routeProgressSlot = slot<RouteProgress>()
val alternativesSlot = slot<List<DirectionsRoute>>()
verify(exactly = 1) {
firstObserver.onRouteAlternatives(
capture(routeProgressSlot),
capture(alternativesSlot),
any()
)
}

val updatedRoutes = mutableListOf<DirectionsRoute>()
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<com.mapbox.navigator.RouteAlternativesObserver>()
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<RouteProgress>()
verify(exactly = 1) {
firstObserver.onRouteAlternatives(
capture(routeProgressSlot),
any(),
any()
)
}

val updatedRoutes = mutableListOf<DirectionsRoute>()
updatedRoutes.add(routeProgressSlot.captured.route)
assertFalse(routeAlternativesController.areAlternatives(updatedRoutes))
}
}
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 Down

0 comments on commit 6f81113

Please sign in to comment.