Faster route: part 1 #6434
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,9 @@ import com.mapbox.navigation.core.directions.LegacyRouterAdapter | |
| import com.mapbox.navigation.core.directions.session.DirectionsSession | ||
| import com.mapbox.navigation.core.directions.session.RoutesExtra | ||
| import com.mapbox.navigation.core.directions.session.RoutesObserver | ||
| import com.mapbox.navigation.core.fasterroute.FasterRouteOptions | ||
| import com.mapbox.navigation.core.fasterroute.ComparisonFasterRouteTrackerCore | ||
| import com.mapbox.navigation.core.fasterroute.FasterRoutesTracker | ||
| import com.mapbox.navigation.core.history.MapboxHistoryReader | ||
| import com.mapbox.navigation.core.history.MapboxHistoryRecorder | ||
| import com.mapbox.navigation.core.internal.ReachabilityService | ||
|
|
@@ -272,6 +275,9 @@ class MapboxNavigation @VisibleForTesting internal constructor( | |
| ) | ||
| ) | ||
|
|
||
| @ExperimentalPreviewMapboxNavigationAPI | ||
| private var fasterRoutesInstance: FasterRoutesTracker? = null | ||
|
There was a problem hiding this comment. This should be destroyed in
Contributor
Author
There was a problem hiding this comment.
|
||
|
|
||
| private val routeUpdateMutex = Mutex() | ||
|
|
||
| // native Router Interface | ||
|
|
@@ -1635,6 +1641,32 @@ class MapboxNavigation @VisibleForTesting internal constructor( | |
| routeRefreshController.unregisterRouteRefreshStateObserver(routeRefreshStatesObserver) | ||
| } | ||
|
|
||
| /*** | ||
| * @return existing instance of [FasterRoutesTracker]. | ||
| * New instance is created for the first call or after calling [FasterRoutesTracker.destroy] on existing | ||
| */ | ||
| @ExperimentalPreviewMapboxNavigationAPI | ||
| @UiThread | ||
|
There was a problem hiding this comment. nit: This is redundant, the whole class is tagged as |
||
| fun getFasterRoutesTracker( | ||
| fasterRouteOptions: FasterRouteOptions = FasterRouteOptions.Builder().build() | ||
| ): FasterRoutesTracker { | ||
| val currentInstance = fasterRoutesInstance | ||
| return if (currentInstance == null || currentInstance.isDestroyed) { | ||
| if (directionsSession.routes.isNotEmpty()) { | ||
| error("FasterRoutes should be created before setting the routes") | ||
| } | ||
| val newInstance = FasterRoutesTracker( | ||
| this, | ||
| ComparisonFasterRouteTrackerCore(fasterRouteOptions), | ||
| threadController.getMainScopeAndRootJob().scope | ||
| ) | ||
| fasterRoutesInstance = newInstance | ||
| newInstance | ||
| } else { | ||
| currentInstance | ||
| } | ||
| } | ||
|
|
||
| private fun startSession(withTripService: Boolean, withReplayEnabled: Boolean) { | ||
| runIfNotDestroyed { | ||
| tripSession.start( | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| package com.mapbox.navigation.core.fasterroute | ||
|
|
||
| import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI | ||
| import com.mapbox.navigation.base.route.NavigationRoute | ||
| import com.mapbox.navigation.core.directions.session.RoutesExtra | ||
| import com.mapbox.navigation.core.directions.session.RoutesUpdatedResult | ||
| import com.mapbox.navigation.core.fasterroute.Log.Companion.FASTER_ROUTE_LOG_CATEGORY | ||
| import com.mapbox.navigation.core.routealternatives.AlternativeRouteMetadata | ||
| import com.mapbox.navigation.utils.internal.logD | ||
|
|
||
| @OptIn(ExperimentalPreviewMapboxNavigationAPI::class) | ||
| internal class ComparisonFasterRouteTrackerCore constructor( | ||
| options: FasterRouteOptions | ||
| ) : FasterRouteTrackerCore { | ||
|
|
||
| private val rejectedRoutesTracker = RejectedRoutesTracker( | ||
| maximumGeometrySimilarity = options.maxAcceptableGeometrySimilarityToRejectedAlternatives | ||
| ) | ||
|
|
||
| override fun fasterRouteDeclined(alternativeId: Int, route: NavigationRoute) { | ||
| rejectedRoutesTracker.addRejectedRoutes(mapOf(alternativeId to route)) | ||
| } | ||
|
|
||
| override suspend fun findFasterRouteInUpdate( | ||
| update: RoutesUpdatedResult, | ||
| alternativeRoutesMetadata: List<AlternativeRouteMetadata> | ||
| ): FasterRouteResult { | ||
| val metadataByRouteId = mutableMapOf<String, AlternativeRouteMetadata>().apply { | ||
| alternativeRoutesMetadata.forEach { this[it.navigationRoute.id] = it } | ||
| } | ||
| return when (update.reason) { | ||
| RoutesExtra.ROUTES_UPDATE_REASON_NEW, RoutesExtra.ROUTES_UPDATE_REASON_REROUTE -> { | ||
| onNewRoutes(alternativeRoutesMetadata) | ||
| FasterRouteResult.NoFasterRoute | ||
| } | ||
| RoutesExtra.ROUTES_UPDATE_REASON_ALTERNATIVE -> | ||
| onAlternativesChanged(alternativeRoutesMetadata, update, metadataByRouteId) | ||
| else -> FasterRouteResult.NoFasterRoute | ||
| } | ||
| } | ||
|
|
||
| private suspend fun onAlternativesChanged( | ||
| alternativeRoutesMetadata: List<AlternativeRouteMetadata>, | ||
| update: RoutesUpdatedResult, | ||
| metadataByRouteId: Map<String, AlternativeRouteMetadata> | ||
| ): FasterRouteResult { | ||
| val fasterAlternatives: Map<Int, NavigationRoute> = findFasterAlternatives( | ||
| alternativeRoutesMetadata, | ||
| update | ||
| ) | ||
| if (fasterAlternatives.isEmpty()) { | ||
| logD(FASTER_ROUTE_LOG_CATEGORY) { | ||
| "no alternatives which are faster then primary route found" | ||
| } | ||
| return FasterRouteResult.NoFasterRoute | ||
| } | ||
| logPotentialFasterRoutes(fasterAlternatives) | ||
|
|
||
| val untrackedAlternatives = rejectedRoutesTracker.findUntrackedAlternatives( | ||
| fasterAlternatives | ||
| ) | ||
| if (untrackedAlternatives.isEmpty()) { | ||
| logD(FASTER_ROUTE_LOG_CATEGORY) { | ||
| "all faster alternatives are similar to already rejected" | ||
| } | ||
| return FasterRouteResult.NoFasterRoute | ||
| } | ||
| logUntrackedFasterRoutes(untrackedAlternatives, metadataByRouteId) | ||
|
|
||
| return findTheFastestAlternative(untrackedAlternatives, metadataByRouteId, update) | ||
| } | ||
|
|
||
| private fun findTheFastestAlternative( | ||
| untracked: List<NavigationRoute>, | ||
| metadataByRouteId: Map<String, AlternativeRouteMetadata>, | ||
| update: RoutesUpdatedResult | ||
| ): FasterRouteResult { | ||
| val fasterAlternative = untracked.minByOrNull { | ||
| metadataByRouteId[it.id]!!.infoFromStartOfPrimary.duration | ||
| } ?: return FasterRouteResult.NoFasterRoute | ||
| val primaryRouteDuration = update.navigationRoutes.first().directionsRoute.duration() | ||
| val fasterAlternativeRouteDuration = metadataByRouteId[fasterAlternative.id]!! | ||
| .infoFromStartOfPrimary.duration | ||
| val fasterThanPrimary = primaryRouteDuration - fasterAlternativeRouteDuration | ||
| logD( | ||
| "route ${fasterAlternative.id} is faster than primary by $fasterThanPrimary", | ||
| FASTER_ROUTE_LOG_CATEGORY | ||
| ) | ||
| return FasterRouteResult.NewFasterRouteFound( | ||
| fasterAlternative, | ||
| fasterThanPrimaryBy = fasterThanPrimary, | ||
| alternativeId = metadataByRouteId[fasterAlternative.id]!!.alternativeId | ||
| ) | ||
| } | ||
|
|
||
| private fun findFasterAlternatives( | ||
| alternativeRoutesMetadata: List<AlternativeRouteMetadata>, | ||
| update: RoutesUpdatedResult | ||
| ): MutableMap<Int, NavigationRoute> { | ||
| return mutableMapOf<Int, NavigationRoute>() | ||
| .apply { | ||
| alternativeRoutesMetadata | ||
| .filter { | ||
| it.infoFromStartOfPrimary.duration < update.navigationRoutes.first() | ||
| .directionsRoute.duration() | ||
| } | ||
| .forEach { this[it.alternativeId] = it.navigationRoute } | ||
| } | ||
| } | ||
|
|
||
| private fun onNewRoutes(alternativeRoutesMetadata: List<AlternativeRouteMetadata>) { | ||
| val routeByAlternativeId: Map<Int, NavigationRoute> = mutableMapOf<Int, NavigationRoute>() | ||
| .apply { | ||
| alternativeRoutesMetadata.forEach { | ||
| this[it.alternativeId] = it.navigationRoute | ||
| } | ||
| } | ||
| logD( | ||
| "New routes were set, resetting tracker state", | ||
| FASTER_ROUTE_LOG_CATEGORY | ||
| ) | ||
| rejectedRoutesTracker.clean() | ||
| rejectedRoutesTracker.addRejectedRoutes(routeByAlternativeId) | ||
| } | ||
|
|
||
| private fun logUntrackedFasterRoutes( | ||
| untracked: List<NavigationRoute>, | ||
| metadataMap: Map<String, AlternativeRouteMetadata> | ||
| ) { | ||
| logD(FASTER_ROUTE_LOG_CATEGORY) { | ||
| "following routes are not similar to already rejected: " + | ||
| untracked.joinToString(separator = ", ") { | ||
| "${it.id}(${metadataMap[it.id]!!.alternativeId})" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun logPotentialFasterRoutes(fasterAlternatives: Map<Int, NavigationRoute>) { | ||
| val fasterAlternativesIdsLog = fasterAlternatives.entries | ||
| .joinToString(separator = ", ") { "${it.value.id}(${it.key})" } | ||
| logD(FASTER_ROUTE_LOG_CATEGORY) { | ||
| "considering following routes as a faster alternative: $fasterAlternativesIdsLog" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| internal sealed class FasterRouteResult { | ||
| object NoFasterRoute : FasterRouteResult() | ||
| data class NewFasterRouteFound( | ||
| val route: NavigationRoute, | ||
| val fasterThanPrimaryBy: Double, | ||
| val alternativeId: Int | ||
| ) : FasterRouteResult() | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,21 @@ | ||||||
| package com.mapbox.navigation.core.fasterroute | ||||||
|
|
||||||
| import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI | ||||||
|
|
||||||
| @ExperimentalPreviewMapboxNavigationAPI | ||||||
| data class FasterRouteOptions internal constructor( | ||||||
|
There was a problem hiding this comment.
Suggested change
Contributor
Author
There was a problem hiding this comment. I think it's okay to use data classes while feature is in experimental stage. I don't want to rewrite |
||||||
| val maxAcceptableGeometrySimilarityToRejectedAlternatives: Double | ||||||
| ) { | ||||||
| class Builder { | ||||||
|
|
||||||
| private var maxGeometrySimilarityToRejectedAlternatives = 0.5 | ||||||
|
There was a problem hiding this comment. This ratio might work for intercity routes (or longer) but might make us unable to suggest faster alternatives in city centers that often rely on taking a turn earlier to later rejoin the original route and avoid congested intersection. These types of routes could have both a significant overlap while still having a significant time delta. I'm just speculating but just wanted to mention this as a discussion point, is there a more complex metric that we could use to account for these situations? Maybe road classes could be taken into account.
Contributor
Author
There was a problem hiding this comment.
We do not compare alternative with the primary route. But if there is a rejected alternative which is very similar to the primary route, it could be a problem. What if we consider initial alternatives as rejected only if user picks slower route? It will decrease chance of this happening.
Contributor
Author
There was a problem hiding this comment.
I will try to add comparison of steps names
Contributor
Author
There was a problem hiding this comment. As an experiment, I switched from comparing geometries to
|
||||||
|
|
||||||
| fun maxAcceptableGeometrySimilarityToRejectedAlternatives(value: Double): Builder { | ||||||
| assert(value in 0.0..1.0) { "similarity should be a value between 0 and 1" } | ||||||
VysotskiVadim marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| maxGeometrySimilarityToRejectedAlternatives = value | ||||||
| return this | ||||||
| } | ||||||
|
|
||||||
| fun build() = FasterRouteOptions(maxGeometrySimilarityToRejectedAlternatives) | ||||||
| } | ||||||
| } | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package com.mapbox.navigation.core.fasterroute | ||
|
|
||
| import com.mapbox.navigation.base.route.NavigationRoute | ||
| import com.mapbox.navigation.core.directions.session.RoutesUpdatedResult | ||
| import com.mapbox.navigation.core.routealternatives.AlternativeRouteMetadata | ||
|
|
||
| internal interface FasterRouteTrackerCore { | ||
| fun fasterRouteDeclined(alternativeId: Int, route: NavigationRoute) | ||
|
|
||
| suspend fun findFasterRouteInUpdate( | ||
| update: RoutesUpdatedResult, | ||
| alternativeRoutesMetadata: List<AlternativeRouteMetadata> | ||
| ): FasterRouteResult | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
who knows how to get rid of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm not mistaken it's a metalava issue and we leave it as is.