From 1263ce6e4cb8b81dd38f33ab62109c691442b575 Mon Sep 17 00:00:00 2001 From: Kyle Madsen Date: Tue, 30 Jun 2020 16:10:34 -0700 Subject: [PATCH] Compare routes using step names --- .../examples/core/ReplayActivity.kt | 12 +- .../examples/core/ReplayHistoryActivity.kt | 10 +- .../navigation/core/MapboxNavigation.kt | 3 + .../core/fasterroute/FasterRouteController.kt | 14 ++- .../core/fasterroute/FasterRouteDetector.kt | 17 ++- .../core/fasterroute/RouteComparator.kt | 50 ++++++++ .../fasterroute/FasterRouteControllerTest.kt | 23 ++-- .../fasterroute/FasterRouteDetectorTest.kt | 29 ++++- .../core/fasterroute/RouteComparatorTest.kt | 114 ++++++++++++++++++ 9 files changed, 243 insertions(+), 29 deletions(-) create mode 100644 libnavigation-core/src/main/java/com/mapbox/navigation/core/fasterroute/RouteComparator.kt create mode 100644 libnavigation-core/src/test/java/com/mapbox/navigation/core/fasterroute/RouteComparatorTest.kt diff --git a/examples/src/main/java/com/mapbox/navigation/examples/core/ReplayActivity.kt b/examples/src/main/java/com/mapbox/navigation/examples/core/ReplayActivity.kt index f4df742b18a..bcc42150d58 100644 --- a/examples/src/main/java/com/mapbox/navigation/examples/core/ReplayActivity.kt +++ b/examples/src/main/java/com/mapbox/navigation/examples/core/ReplayActivity.kt @@ -23,6 +23,7 @@ import com.mapbox.navigation.base.internal.extensions.applyDefaultParams import com.mapbox.navigation.base.internal.extensions.coordinates import com.mapbox.navigation.core.MapboxNavigation import com.mapbox.navigation.core.directions.session.RoutesRequestCallback +import com.mapbox.navigation.core.fasterroute.FasterRouteObserver import com.mapbox.navigation.core.replay.MapboxReplayer import com.mapbox.navigation.core.replay.ReplayLocationEngine import com.mapbox.navigation.core.replay.route.ReplayRouteMapper @@ -39,7 +40,7 @@ import kotlinx.android.synthetic.main.activity_replay_route_layout.* /** * This activity shows how to use the MapboxNavigation - * class with the Navigation SDK's [ReplayHistoryLocationEngine]. + * class with the Navigation SDK's [MapboxReplayer] and [ReplayLocationEngine]. */ class ReplayActivity : AppCompatActivity(), OnMapReadyCallback { @@ -82,6 +83,13 @@ class ReplayActivity : AppCompatActivity(), OnMapReadyCallback { navigationMapboxMap?.restoreFrom(state) } initializeFirstLocation() + + mapboxNavigation?.attachFasterRouteObserver(object : FasterRouteObserver { + override fun onFasterRoute(currentRoute: DirectionsRoute, alternatives: List, isAlternativeFaster: Boolean) { + navigationMapboxMap?.drawRoutes(alternatives) + mapboxNavigation?.setRoutes(alternatives) + } + }) } mapboxMap.addOnMapLongClickListener { latLng -> mapboxMap.locationComponent.lastKnownLocation?.let { originLocation -> @@ -110,7 +118,7 @@ class ReplayActivity : AppCompatActivity(), OnMapReadyCallback { override fun onRoutesReady(routes: List) { MapboxLogger.d(Message("route request success $routes")) if (routes.isNotEmpty()) { - navigationMapboxMap?.drawRoute(routes[0]) + navigationMapboxMap?.drawRoutes(routes) val replayEvents = replayRouteMapper.mapGeometry(routes[0].geometry()!!) mapboxReplayer.pushEvents(replayEvents) diff --git a/examples/src/main/java/com/mapbox/navigation/examples/core/ReplayHistoryActivity.kt b/examples/src/main/java/com/mapbox/navigation/examples/core/ReplayHistoryActivity.kt index 7e0aac93761..9d982eb07f7 100644 --- a/examples/src/main/java/com/mapbox/navigation/examples/core/ReplayHistoryActivity.kt +++ b/examples/src/main/java/com/mapbox/navigation/examples/core/ReplayHistoryActivity.kt @@ -23,6 +23,7 @@ import com.mapbox.navigation.base.internal.extensions.applyDefaultParams import com.mapbox.navigation.base.internal.extensions.coordinates import com.mapbox.navigation.core.MapboxNavigation import com.mapbox.navigation.core.directions.session.RoutesRequestCallback +import com.mapbox.navigation.core.fasterroute.FasterRouteObserver import com.mapbox.navigation.core.replay.MapboxReplayer import com.mapbox.navigation.core.replay.ReplayLocationEngine import com.mapbox.navigation.core.replay.history.CustomEventMapper @@ -171,6 +172,13 @@ class ReplayHistoryActivity : AppCompatActivity() { } }) + mapboxNavigation.attachFasterRouteObserver(object : FasterRouteObserver { + override fun onFasterRoute(currentRoute: DirectionsRoute, alternatives: List, isAlternativeFaster: Boolean) { + navigationContext?.navigationMapboxMap?.drawRoutes(alternatives) + navigationContext?.mapboxNavigation?.setRoutes(alternatives) + } + }) + playReplay.setOnClickListener { mapboxReplayer.play() mapboxNavigation.startTripSession() @@ -235,7 +243,7 @@ class ReplayHistoryActivity : AppCompatActivity() { override fun onRoutesReady(routes: List) { MapboxLogger.d(Message("route request success $routes")) if (routes.isNotEmpty()) { - navigationContext?.navigationMapboxMap?.drawRoute(routes[0]) + navigationContext?.navigationMapboxMap?.drawRoutes(routes) navigationContext?.startNavigation() } } 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 54f6fdecaab..b1e41d2a058 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 @@ -32,7 +32,9 @@ import com.mapbox.navigation.core.directions.session.DirectionsSession import com.mapbox.navigation.core.directions.session.RoutesObserver import com.mapbox.navigation.core.directions.session.RoutesRequestCallback import com.mapbox.navigation.core.fasterroute.FasterRouteController +import com.mapbox.navigation.core.fasterroute.FasterRouteDetector import com.mapbox.navigation.core.fasterroute.FasterRouteObserver +import com.mapbox.navigation.core.fasterroute.RouteComparator import com.mapbox.navigation.core.internal.MapboxDistanceFormatter import com.mapbox.navigation.core.internal.accounts.MapboxNavigationAccounts import com.mapbox.navigation.core.internal.trip.service.TripService @@ -207,6 +209,7 @@ class MapboxNavigation( directionsSession, tripSession, routeOptionsProvider, + FasterRouteDetector(RouteComparator()), logger ) routeRefreshController = RouteRefreshController(directionsSession, tripSession, logger) diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/fasterroute/FasterRouteController.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/fasterroute/FasterRouteController.kt index 9dd4f662aa1..8d660728766 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/fasterroute/FasterRouteController.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/fasterroute/FasterRouteController.kt @@ -9,17 +9,22 @@ import com.mapbox.navigation.core.directions.session.RoutesRequestCallback import com.mapbox.navigation.core.routeoptions.RouteOptionsProvider import com.mapbox.navigation.core.trip.session.TripSession import com.mapbox.navigation.utils.internal.MapboxTimer +import com.mapbox.navigation.utils.internal.ThreadController import java.util.concurrent.TimeUnit +import kotlinx.coroutines.cancelChildren +import kotlinx.coroutines.launch internal class FasterRouteController( private val directionsSession: DirectionsSession, private val tripSession: TripSession, private val routeOptionsProvider: RouteOptionsProvider, + private val fasterRouteDetector: FasterRouteDetector, private val logger: Logger ) { + private val jobControl = ThreadController.getMainScopeAndRootJob() + private val fasterRouteTimer = MapboxTimer() - private val fasterRouteDetector = FasterRouteDetector() private var fasterRouteObserver: FasterRouteObserver? = null fun attach(fasterRouteObserver: FasterRouteObserver) { @@ -40,6 +45,7 @@ internal class FasterRouteController( fun stop() { this.fasterRouteObserver = null fasterRouteTimer.stopJobs() + jobControl.job.cancelChildren() } private fun requestFasterRoute() { @@ -72,8 +78,10 @@ internal class FasterRouteController( override fun onRoutesReady(routes: List) { val currentRoute = directionsSession.routes.firstOrNull() ?: return - tripSession.getRouteProgress()?.let { progress -> - val isAlternativeFaster = fasterRouteDetector.isRouteFaster(routes[0], progress) + val routeProgress = tripSession.getRouteProgress() + ?: return + jobControl.scope.launch { + val isAlternativeFaster = fasterRouteDetector.isRouteFaster(routes[0], routeProgress) fasterRouteObserver?.onFasterRoute(currentRoute, routes, isAlternativeFaster) } } diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/fasterroute/FasterRouteDetector.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/fasterroute/FasterRouteDetector.kt index 238c79530b7..1eafde8bdf1 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/fasterroute/FasterRouteDetector.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/fasterroute/FasterRouteDetector.kt @@ -2,12 +2,21 @@ package com.mapbox.navigation.core.fasterroute import com.mapbox.api.directions.v5.models.DirectionsRoute import com.mapbox.navigation.base.trip.model.RouteProgress +import com.mapbox.navigation.utils.internal.ThreadController +import kotlinx.coroutines.withContext -internal class FasterRouteDetector { - fun isRouteFaster(newRoute: DirectionsRoute, routeProgress: RouteProgress): Boolean { - val newRouteDuration = newRoute.duration() ?: return false +internal class FasterRouteDetector( + private val routeComparator: RouteComparator +) { + + suspend fun isRouteFaster( + alternativeRoute: DirectionsRoute, + routeProgress: RouteProgress + ): Boolean = withContext(ThreadController.IODispatcher) { + val alternativeDuration = alternativeRoute.duration() ?: return@withContext false val weightedDuration = routeProgress.durationRemaining * PERCENTAGE_THRESHOLD - return newRouteDuration < weightedDuration + val isRouteFaster = alternativeDuration < weightedDuration + return@withContext isRouteFaster && routeComparator.isNewRoute(routeProgress, alternativeRoute) } companion object { diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/fasterroute/RouteComparator.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/fasterroute/RouteComparator.kt new file mode 100644 index 00000000000..3af0f1c1fde --- /dev/null +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/fasterroute/RouteComparator.kt @@ -0,0 +1,50 @@ +package com.mapbox.navigation.core.fasterroute + +import com.mapbox.api.directions.v5.models.DirectionsRoute +import com.mapbox.api.directions.v5.models.LegStep +import com.mapbox.navigation.base.trip.model.RouteProgress + +/** + * Compares if an alternative route is different from the current route progress. + */ +internal class RouteComparator { + + private val mapLegStepToName: (LegStep) -> String = { it.name() ?: "" } + + /** + * @param routeProgress current route progress + * @param alternativeRoute suggested new route + * + * @return true when the alternative route has different + * geometry from the current route progress + */ + fun isNewRoute(routeProgress: RouteProgress, alternativeRoute: DirectionsRoute): Boolean { + val currentDescription = routeDescription(routeProgress) + val alternativeDescription = alternativeDescription(alternativeRoute) + alternativeDescription.ifEmpty { + return false + } + + return isNewRoute(currentDescription, alternativeDescription) + } + + private fun routeDescription(routeProgress: RouteProgress): String { + val routeLeg = routeProgress.currentLegProgress?.routeLeg + val steps = routeLeg?.steps() + val stepIndex = routeProgress.currentLegProgress?.currentStepProgress?.stepIndex + ?: return "" + + return steps?.listIterator(stepIndex)?.asSequence() + ?.joinToString(transform = mapLegStepToName) ?: "" + } + + private fun alternativeDescription(directionsRoute: DirectionsRoute): String { + val steps = directionsRoute.legs()?.firstOrNull()?.steps() + return steps?.asSequence() + ?.joinToString(transform = mapLegStepToName) ?: "" + } + + private fun isNewRoute(currentGeometry: String, alternativeGeometry: String): Boolean { + return currentGeometry != alternativeGeometry + } +} diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/fasterroute/FasterRouteControllerTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/fasterroute/FasterRouteControllerTest.kt index 57b8dc23dd0..898889ff60d 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/fasterroute/FasterRouteControllerTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/fasterroute/FasterRouteControllerTest.kt @@ -8,6 +8,7 @@ import com.mapbox.navigation.core.directions.session.RoutesRequestCallback import com.mapbox.navigation.core.routeoptions.RouteOptionsProvider import com.mapbox.navigation.core.trip.session.TripSession import com.mapbox.navigation.testing.MainCoroutineRule +import io.mockk.coEvery import io.mockk.every import io.mockk.mockk import io.mockk.slot @@ -25,20 +26,22 @@ class FasterRouteControllerTest { val coroutineRule = MainCoroutineRule() private val directionsSession: DirectionsSession = mockk() - private val tripSession: TripSession = mockk() + private val tripSession: TripSession = mockk { + every { getRouteProgress() } returns mockk() + } private val fasterRouteObserver: FasterRouteObserver = mockk { every { restartAfterMillis() } returns FasterRouteObserver.DEFAULT_INTERVAL_MILLIS every { onFasterRoute(any(), any(), any()) } returns Unit } private val routesRequestCallbacks = slot() - private val logger: Logger = mockk() private val routeOptionsProvider: RouteOptionsProvider = mockk() - private val fasterRouteController = FasterRouteController(directionsSession, tripSession, routeOptionsProvider, logger) private val routeOptionsResultSuccess: RouteOptionsProvider.RouteOptionsResult.Success = mockk() private val routeOptionsResultSuccessRouteOptions: RouteOptions = mockk() - private val routeOptionsResultError: RouteOptionsProvider.RouteOptionsResult.Error = mockk() + private val fasterRouteDetector: FasterRouteDetector = mockk() + + private val fasterRouteController = FasterRouteController(directionsSession, tripSession, routeOptionsProvider, fasterRouteDetector, logger) @Before fun setup() { @@ -109,26 +112,22 @@ class FasterRouteControllerTest { @Test fun `should notify observer of a faster route`() = coroutineRule.runBlockingTest { + coEvery { fasterRouteDetector.isRouteFaster(any(), any()) } returns true mockRouteOptionsProvider(routeOptionsResultSuccess) val currentRoute: DirectionsRoute = mockk { every { routeIndex() } returns "0" - every { duration() } returns 801.332 } every { directionsSession.routes } returns listOf(currentRoute) every { tripSession.getEnhancedLocation() } returns mockk { every { latitude } returns -33.874308 every { longitude } returns 151.206087 } - every { tripSession.getRouteProgress() } returns mockk { - every { durationRemaining } returns 601.334 - } every { directionsSession.requestFasterRoute(any(), capture(routesRequestCallbacks)) } returns mockk() fasterRouteController.attach(fasterRouteObserver) coroutineRule.testDispatcher.advanceTimeBy(TimeUnit.MINUTES.toMillis(6)) val routes = listOf(mockk { every { routeIndex() } returns "0" - every { duration() } returns 351.013 }) routesRequestCallbacks.captured.onRoutesReady(routes) @@ -140,26 +139,22 @@ class FasterRouteControllerTest { @Test fun `should notify observer if current route is fastest`() = coroutineRule.runBlockingTest { + coEvery { fasterRouteDetector.isRouteFaster(any(), any()) } returns false mockRouteOptionsProvider(routeOptionsResultSuccess) val currentRoute: DirectionsRoute = mockk { every { routeIndex() } returns "0" - every { duration() } returns 801.332 } every { directionsSession.routes } returns listOf(currentRoute) every { tripSession.getEnhancedLocation() } returns mockk { every { latitude } returns -33.874308 every { longitude } returns 151.206087 } - every { tripSession.getRouteProgress() } returns mockk { - every { durationRemaining } returns 751.334 - } every { directionsSession.requestFasterRoute(any(), capture(routesRequestCallbacks)) } returns mockk() fasterRouteController.attach(fasterRouteObserver) coroutineRule.testDispatcher.advanceTimeBy(TimeUnit.MINUTES.toMillis(6)) val routes = listOf(mockk { every { routeIndex() } returns "0" - every { duration() } returns 951.013 }) routesRequestCallbacks.captured.onRoutesReady(routes) diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/fasterroute/FasterRouteDetectorTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/fasterroute/FasterRouteDetectorTest.kt index bf20a090ae7..bfaa7addbe5 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/fasterroute/FasterRouteDetectorTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/fasterroute/FasterRouteDetectorTest.kt @@ -4,16 +4,22 @@ import com.mapbox.api.directions.v5.models.DirectionsRoute import com.mapbox.navigation.base.trip.model.RouteProgress import io.mockk.every import io.mockk.mockk +import kotlinx.coroutines.runBlocking import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test class FasterRouteDetectorTest { - private val fasterRouteDetector = FasterRouteDetector() + private val routeComparator: RouteComparator = mockk { + every { isNewRoute(any(), any()) } returns true + } + + private val fasterRouteDetector = FasterRouteDetector(routeComparator) @Test - fun shouldDetectWhenRouteIsFaster() { + fun shouldDetectWhenRouteIsFaster() = runBlocking { + every { routeComparator.isNewRoute(any(), any()) } returns true val newRoute: DirectionsRoute = mockk() every { newRoute.duration() } returns 402.6 val routeProgress: RouteProgress = mockk() @@ -25,7 +31,20 @@ class FasterRouteDetectorTest { } @Test - fun shouldDetectWhenRouteIsSlower() { + fun shouldDetectWhenRouteIsFasterOnlyIfDifferent() = runBlocking { + every { routeComparator.isNewRoute(any(), any()) } returns false + val newRoute: DirectionsRoute = mockk() + every { newRoute.duration() } returns 402.6 + val routeProgress: RouteProgress = mockk() + every { routeProgress.durationRemaining } returns 797.447 + + val isFasterRoute = fasterRouteDetector.isRouteFaster(newRoute, routeProgress) + + assertFalse(isFasterRoute) + } + + @Test + fun shouldDetectWhenRouteIsSlower() = runBlocking { val newRoute: DirectionsRoute = mockk() every { newRoute.duration() } returns 512.2 val routeProgress: RouteProgress = mockk() @@ -37,7 +56,7 @@ class FasterRouteDetectorTest { } @Test - fun shouldDefaultToFalseWhenDurationIsNull() { + fun shouldDefaultToFalseWhenDurationIsNull() = runBlocking { val newRoute: DirectionsRoute = mockk() every { newRoute.duration() } returns null val routeProgress: RouteProgress = mockk() @@ -49,7 +68,7 @@ class FasterRouteDetectorTest { } @Test - fun shouldNotAllowSlightlyFasterRoutes() { + fun shouldNotAllowSlightlyFasterRoutes() = runBlocking { val newRoute: DirectionsRoute = mockk() every { newRoute.duration() } returns 634.7 val routeProgress: RouteProgress = mockk() diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/fasterroute/RouteComparatorTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/fasterroute/RouteComparatorTest.kt new file mode 100644 index 00000000000..cb9ad86d7a7 --- /dev/null +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/fasterroute/RouteComparatorTest.kt @@ -0,0 +1,114 @@ +package com.mapbox.navigation.core.fasterroute + +import com.mapbox.api.directions.v5.models.DirectionsRoute +import com.mapbox.navigation.base.trip.model.RouteProgress +import io.mockk.every +import io.mockk.mockk +import junit.framework.TestCase.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class RouteComparatorTest { + + private val routeComparator = RouteComparator() + + @Test + fun `route with different steps is new`() { + val routeProgress: RouteProgress = mockk { + every { currentLegProgress } returns mockk { + every { legIndex } returns 0 + every { currentStepProgress } returns mockk { + every { stepIndex } returns 0 + } + every { routeLeg } returns mockk { + every { steps() } returns listOf( + mockk { every { name() } returns "Pennsylvania Avenue" }, + mockk { every { name() } returns "19th Street" }, + mockk { every { name() } returns "Arkansas Street" } + ) + } + } + } + val directionsRoute: DirectionsRoute = mockk { + every { legs() } returns listOf(mockk { + every { steps() } returns listOf( + mockk { every { name() } returns "Pennsylvania Avenue" }, + mockk { every { name() } returns "20th Street" }, + mockk { every { name() } returns "Arkansas Street" } + ) + }) + } + + val isNewRoute = routeComparator.isNewRoute(routeProgress, directionsRoute) + + assertTrue(isNewRoute) + } + + @Test + fun `route with same steps is not new`() { + val routeProgress: RouteProgress = mockk { + every { currentLegProgress } returns mockk { + every { legIndex } returns 0 + every { currentStepProgress } returns mockk { + every { stepIndex } returns 0 + } + every { routeLeg } returns mockk { + every { steps() } returns listOf( + mockk { every { name() } returns "Pennsylvania Avenue" }, + mockk { every { name() } returns "20th Street" }, + mockk { every { name() } returns "De Haro Street" } + ) + } + } + } + val directionsRoute: DirectionsRoute = mockk { + every { legs() } returns listOf(mockk { + every { steps() } returns listOf( + mockk { every { name() } returns "Pennsylvania Avenue" }, + mockk { every { name() } returns "20th Street" }, + mockk { every { name() } returns "De Haro Street" } + ) + }) + } + + val isNewRoute = routeComparator.isNewRoute(routeProgress, directionsRoute) + + assertFalse(isNewRoute) + } + + @Test + fun `stepIndex should clip route progress`() { + val routeProgress: RouteProgress = mockk { + every { currentLegProgress } returns mockk { + every { legIndex } returns 0 + every { currentStepProgress } returns mockk { + every { stepIndex } returns 4 + } + every { routeLeg } returns mockk { + every { steps() } returns listOf( + mockk { every { name() } returns "Pennsylvania Avenue" }, + mockk { every { name() } returns "Cesar Chavez Street" }, + mockk { every { name() } returns "Bryant Street" }, + mockk { every { name() } returns "Precita Avenue" }, + mockk { every { name() } returns "Alabama Street" }, + mockk { every { name() } returns "Bradford Street" }, + mockk { every { name() } returns "Nevada Street" } + ) + } + } + } + val directionsRoute: DirectionsRoute = mockk { + every { legs() } returns listOf(mockk { + every { steps() } returns listOf( + mockk { every { name() } returns "Alabama Street" }, + mockk { every { name() } returns "Bradford Street" }, + mockk { every { name() } returns "Nevada Street" } + ) + }) + } + + val isNewRoute = routeComparator.isNewRoute(routeProgress, directionsRoute) + + assertFalse(isNewRoute) + } +}