mapbox-navigation-android
mapbox-navigation-android copied to clipboard
Add route selection by index
Mapbox Navigation SDK version: 2.1.0
EDIT: grammar
Follow up work from https://github.com/mapbox/mapbox-navigation-android/pull/4892. Anticipating this issue will require refactoring and testing in order to get it all just right. It is considered out of scope of the initial integration of continuous alternatives because we may not want to do this. Re-ordering the list and assuming route index zero also works.
Here is the existing way to change the route leg in MapboxNavigation
fun navigateNextRouteLeg(callback: LegIndexUpdatedCallback) {
arrivalProgressObserver.navigateNextRouteLeg(callback)
}
So here we're proposing adding and/or changing the functions in MapboxNavigation
// Adding routeIndex may be semver, since it changes the position of the setRoute arguments
fun setRoutes(routes: List<DirectionsRoute>, routeIndex: Int = 0, initialLegIndex: Int = 0) {
// After routes have been set, be able to change the route index with something like this.
fun setPrimaryRoute(routeIndex: Int)
fun setPrimaryRoute(route: DirectionsRoute)
Ui code to select a route
private val mapClickListener = OnMapClickListener { point ->
CoroutineScope(Dispatchers.Main).launch {
val result = routeLineApi.findClosestRoute(
point,
binding.mapView.getMapboxMap(),
routeClickPadding
)
val routeFound = result.value?.route
if (routeFound != null && routeFound != routeLineApi.getPrimaryRoute()) {
val reOrderedRoutes = routeLineApi.getRoutes()
.filter { it != routeFound }
.toMutableList()
.also {
it.add(0, routeFound)
}
mapboxNavigation.setRoutes(reOrderedRoutes)
}
}
false
}
After
private val mapClickListener = OnMapClickListener { point ->
CoroutineScope(Dispatchers.Main).launch {
val result = routeLineApi.findClosestRoute(
point,
binding.mapView.getMapboxMap(),
routeClickPadding
)
result.value?.route?.let { mapboxNavigation.setPrimaryRoute(it)
}
false
}
To avoid semver, why don't we introduce
fun setRoutes(routes: List<DirectionsRoute>, routeIndex: Int = 0, initialLegIndex: Int = 0)
fun setPrimaryRoute(routeIndex: Int)
fun setPrimaryRoute(route: DirectionsRoute)
as extensions?
We discussed this in the past and there is no additional power/flexibility that you gain by specifying the index versus reordering the list, it's just semantics.
We discussed this in the past and there is no additional power/flexibility that you gain by specifying the index versus reordering the list, it's just semantics.
This is not entirely true. Where this has come up in android auto is when selecting and viewing the route alternatives in a list. For example, the list that displays and allows you to select a route requires an index to indicate selection. When selection is done by re-ordering, the display list will automatically re-order. Re-ordering the list after every selection may not be desirable.
Further on this, reasonable to duplicate the view model from the data model for this reason (also a typical reason to duplicate data)
Interesting use case! We've never previously had an out-of-the-box route management/preview UI and with these use cases becoming more integrated into the SDK iteslf, an index-based selection could be preferable :+1:
Something to consider for the next major version of the SDK.
cc @VysotskiVadim for visibility
Something to consider for the next major version of the SDK.
@LukasPaczos , why can't we support index based selection in 2.x? Can't we duplicate methods which accept/return List<NavigationRoute>
by methods which accepts NavigationRoutes
everywhere
class NavigationRoutes(
val routes: List<NavigationRoute>,
val primaryRouteIndex: Int
) {
val primaryRoute get() = routes[primaryRouteIndex]
val alternativeRoutes get() = routes.filterIndexed { _, index -> index != primaryRouteIndex }
}
We already did similar change moving from DirectionRoute
to NavigationRoute
🤔
Yes, we definitely can if needed! I just wanted to add a semver label to make sure we revisit this topic before v3.
Fixed with a new label :upside_down_face: