diff --git a/changelog/unreleased/bugfixes/6841.md b/changelog/unreleased/bugfixes/6841.md new file mode 100644 index 00000000000..4c56a280a22 --- /dev/null +++ b/changelog/unreleased/bugfixes/6841.md @@ -0,0 +1 @@ +- Removed `NavigationRoute#hasUnexpectedClosures` and added `RouteProgress#hasUnexpectedUpcomingClosures` instead that checks whether route has upcoming unexpected closures (the algorithm does not take into account closures that the puck has already been on) diff --git a/libnavigation-base/api/current.txt b/libnavigation-base/api/current.txt index b0576516e0c..c436e261a65 100644 --- a/libnavigation-base/api/current.txt +++ b/libnavigation-base/api/current.txt @@ -1745,7 +1745,7 @@ package com.mapbox.navigation.base.utils { package com.mapbox.navigation.base.utils.route { public final class NavigationRouteUtils { - method @com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI public static suspend Object? hasUnexpectedClosures(com.mapbox.navigation.base.route.NavigationRoute, kotlin.coroutines.Continuation); + method @com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI public static suspend Object? hasUnexpectedUpcomingClosures(com.mapbox.navigation.base.trip.model.RouteProgress, kotlin.coroutines.Continuation); } } diff --git a/libnavigation-base/src/main/java/com/mapbox/navigation/base/utils/route/NavigationRouteEx.kt b/libnavigation-base/src/main/java/com/mapbox/navigation/base/utils/route/RouteProgressEx.kt similarity index 67% rename from libnavigation-base/src/main/java/com/mapbox/navigation/base/utils/route/NavigationRouteEx.kt rename to libnavigation-base/src/main/java/com/mapbox/navigation/base/utils/route/RouteProgressEx.kt index 44883bd2a59..3bab649d033 100644 --- a/libnavigation-base/src/main/java/com/mapbox/navigation/base/utils/route/NavigationRouteEx.kt +++ b/libnavigation-base/src/main/java/com/mapbox/navigation/base/utils/route/RouteProgressEx.kt @@ -8,7 +8,9 @@ import com.mapbox.api.directions.v5.models.RouteOptions import com.mapbox.geojson.Point import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.base.route.NavigationRoute +import com.mapbox.navigation.base.trip.model.RouteProgress import com.mapbox.navigation.base.utils.DecodeUtils.stepGeometryToPoints +import com.mapbox.navigation.utils.internal.ifNonNull import com.mapbox.navigation.utils.internal.logD import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -16,27 +18,47 @@ import kotlinx.coroutines.withContext private const val LOG_CATEGORY = "NavigationRouteUtils" /** - * This function checks whether the [NavigationRoute] has unexpected closures, which could be a reason to re-route. + * This function checks whether the [NavigationRoute] has unexpected upcoming closures, which could be a reason to re-route. * - * `true` is returned whenever there's any [RouteLeg.closures] outside of a direct region around a coordinate from [RouteOptions.coordinatesList] + * The algorithm does not take to account current closures, where the puck is on. + * + * `true` is returned whenever there's any upcoming [RouteLeg.closures] outside of a direct region around a coordinate from [RouteOptions.coordinatesList] * that was allowed for snapping to closures with [RouteOptions.snappingIncludeClosuresList] or [RouteOptions.snappingIncludeStaticClosuresList]. * flags. Otherwise, `false` is returned. */ @ExperimentalPreviewMapboxNavigationAPI -suspend fun NavigationRoute.hasUnexpectedClosures(): Boolean = +suspend fun RouteProgress.hasUnexpectedUpcomingClosures(): Boolean = withContext(Dispatchers.Default) { - val snappingResultList = directionsRoute.getSnappingResultList() + val snappingResultList = navigationRoute.directionsRoute.getSnappingResultList() + + val routeProgressData = ifNonNull( + currentLegProgress, + ) { legProgress -> + RouteProgressData(legProgress.legIndex, legProgress.geometryIndex) + } var currentLegFirstWaypointIndex = 0 - directionsRoute.legs()?.forEachIndexed { routeLegIndex, routeLeg -> + navigationRoute.directionsRoute.legs()?.forEachIndexed { routeLegIndex, routeLeg -> val legLastGeometryIndex by lazy { - directionsRoute.stepsGeometryToPoints(routeLeg).lastIndex + navigationRoute.directionsRoute.stepsGeometryToPoints(routeLeg).lastIndex } val silentWaypoints = routeLeg.silentWaypoints() routeLeg.closures()?.forEach { closure -> + if (routeProgressData != null) { + if (routeProgressData.currentLegIndex > routeLegIndex) { + // skipping passed legs + return@forEach + } else if ( + routeProgressData.currentLegIndex == routeLegIndex && + routeProgressData.currentGeometryLegIndex >= closure.geometryIndexStart() + ) { + // skipping current and passed closures on the current leg + return@forEach + } + } val silentWaypointsInClosureRange = silentWaypoints.inGeometryRange( closure.geometryIndexStart(), closure.geometryIndexEnd() @@ -49,9 +71,10 @@ suspend fun NavigationRoute.hasUnexpectedClosures(): Boolean = snappingResultList.getOrNull(silentWaypointIndex) ?: false if (!isSnapAllowed) { logD( - "Route with id [${this@hasUnexpectedClosures.id}] has closure " + - "at leg index $routeLegIndex, that overlaps silent (via) " + - "waypoint", + "Route with id " + + "[${this@hasUnexpectedUpcomingClosures.navigationRoute.id}] " + + "has closure at leg index $routeLegIndex, that overlaps " + + "silent (via) waypoint", LOG_CATEGORY ) return@withContext true @@ -63,8 +86,9 @@ suspend fun NavigationRoute.hasUnexpectedClosures(): Boolean = silentWaypointsInClosureRange.isEmpty() ) { logD( - "Route with id [${this@hasUnexpectedClosures.id}] has closure at leg " + - "index $routeLegIndex", + "Route with id " + + "[${this@hasUnexpectedUpcomingClosures.navigationRoute.id}] has " + + "closure at leg index $routeLegIndex", ) return@withContext true } @@ -72,8 +96,9 @@ suspend fun NavigationRoute.hasUnexpectedClosures(): Boolean = snappingResultList.getOrNull(currentLegFirstWaypointIndex) != true ) { logD( - "Route with id [${this@hasUnexpectedClosures.id}] has closure at the " + - "start of the leg, leg index $routeLegIndex", + "Route with id " + + "[${this@hasUnexpectedUpcomingClosures.navigationRoute.id}] has " + + "closure at the start of the leg, leg index $routeLegIndex", LOG_CATEGORY ) return@withContext true @@ -84,8 +109,9 @@ suspend fun NavigationRoute.hasUnexpectedClosures(): Boolean = ) != true ) { logD( - "Route with id [${this@hasUnexpectedClosures.id}] has closure at " + - "the end of the leg, leg index $routeLegIndex", + "Route with id " + + "[${this@hasUnexpectedUpcomingClosures.navigationRoute.id}] has " + + "closure at the end of the leg, leg index $routeLegIndex", LOG_CATEGORY ) return@withContext true @@ -98,6 +124,11 @@ suspend fun NavigationRoute.hasUnexpectedClosures(): Boolean = return@withContext false } +private class RouteProgressData( + val currentLegIndex: Int, + val currentGeometryLegIndex: Int, +) + private fun DirectionsRoute.getSnappingResultList(): List { val snappingIncludeClosuresList = routeOptions()?.snappingIncludeClosuresList() val snappingIncludeStaticClosuresList = routeOptions()?.snappingIncludeStaticClosuresList() diff --git a/libnavigation-base/src/test/java/com/mapbox/navigation/base/internal/route/NavigationRouteExTest.kt b/libnavigation-base/src/test/java/com/mapbox/navigation/base/internal/route/RouteProgressExTest.kt similarity index 99% rename from libnavigation-base/src/test/java/com/mapbox/navigation/base/internal/route/NavigationRouteExTest.kt rename to libnavigation-base/src/test/java/com/mapbox/navigation/base/internal/route/RouteProgressExTest.kt index 4d6d64f5303..ce0cb2156db 100644 --- a/libnavigation-base/src/test/java/com/mapbox/navigation/base/internal/route/NavigationRouteExTest.kt +++ b/libnavigation-base/src/test/java/com/mapbox/navigation/base/internal/route/RouteProgressExTest.kt @@ -31,7 +31,7 @@ import org.junit.runner.RunWith import org.junit.runners.Parameterized import java.net.URL -class NavigationRouteExTest { +class RouteProgressExTest { @Test fun `update Navigation route`() { diff --git a/libnavigation-base/src/test/java/com/mapbox/navigation/base/utils/route/NavigationRouteExTest.kt b/libnavigation-base/src/test/java/com/mapbox/navigation/base/utils/route/RouteProgressExTest.kt similarity index 62% rename from libnavigation-base/src/test/java/com/mapbox/navigation/base/utils/route/NavigationRouteExTest.kt rename to libnavigation-base/src/test/java/com/mapbox/navigation/base/utils/route/RouteProgressExTest.kt index c6a7719f509..c777ac04324 100644 --- a/libnavigation-base/src/test/java/com/mapbox/navigation/base/utils/route/NavigationRouteExTest.kt +++ b/libnavigation-base/src/test/java/com/mapbox/navigation/base/utils/route/RouteProgressExTest.kt @@ -4,11 +4,12 @@ import com.mapbox.api.directions.v5.DirectionsCriteria import com.mapbox.api.directions.v5.models.DirectionsRoute import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.base.route.NavigationRoute +import com.mapbox.navigation.base.trip.model.RouteProgress import com.mapbox.navigation.testing.FileUtils import com.mapbox.navigation.testing.LoggingFrontendTestRule import io.mockk.every import io.mockk.mockk -import junit.framework.Assert +import junit.framework.Assert.assertEquals import kotlinx.coroutines.runBlocking import org.junit.Rule import org.junit.Test @@ -17,22 +18,32 @@ import org.junit.runners.Parameterized private typealias BooleansProvider = () -> List -class NavigationRouteExTest { +class RouteProgressExTest { @OptIn(ExperimentalPreviewMapboxNavigationAPI::class) @RunWith(Parameterized::class) - class RouteHasUnexpectedClosures( + class RouteHasUnexpectedUpcomingClosures( private val description: String, private val routeRaw: String, private val snappingIncludeClosures: BooleansProvider?, private val snappingIncludeStaticClosures: BooleansProvider?, - private val expectedHasUnexpectedClosures: Boolean, + private val currentLegIndex: Int, + private val currentGeometryLegIndex: Int, + private val expectedHasUnexpectedUpcomingClosures: Boolean, ) { @get:Rule val loggerRule = LoggingFrontendTestRule() companion object { + /** + * - `route_closure_start_coordinate`: closure[0, 8], leg [0]; + * - `route_closure_second_waypoint`: closure[5, 8], leg [0] and closure[0, 8], leg[1]; + * - `route_closure_second_silent_waypoint`: closure[5, 16], leg [0]; + * - `route_closure_last_coordinate`: closure[4, 7], leg [1]; + * - `route_closure_between_silent_and_regular_waypoints`: closure[12, 13], leg [0]; + * - `route_closure_between_two_regular_waypoints`: closure[2, 3], leg [1]; + */ @JvmStatic @Parameterized.Parameters(name = "{0}") fun data(): List> = @@ -42,20 +53,27 @@ class NavigationRouteExTest { "multileg_route.json", { null }, { null }, + 0, + 0, false, ), arrayOf( - "route closure at starting waypoint, include closures = false", + "route closure at starting waypoint, include closures = false; " + + "the puck is in the very beginning", "route_closure_start_coordinate.json", provideBooleansProvider(false, true, true), provideBooleansProvider(false, true, true), - true, + 0, + 0, + false, ), arrayOf( "route closure at starting waypoint, include closures = true", "route_closure_start_coordinate.json", provideBooleansProvider(true, false, false), provideBooleansProvider(true, false, false), + 0, + 0, false, ), arrayOf( @@ -63,6 +81,8 @@ class NavigationRouteExTest { "route_closure_start_coordinate.json", provideBooleansProvider(false, false, false), provideBooleansProvider(true, false, false), + 0, + 0, false, ), arrayOf( @@ -70,6 +90,8 @@ class NavigationRouteExTest { "route_closure_start_coordinate.json", provideBooleansProvider(true, false, false), provideBooleansProvider(false, false, false), + 0, + 0, false, ), arrayOf( @@ -77,27 +99,65 @@ class NavigationRouteExTest { "route_closure_second_waypoint.json", provideBooleansProvider(true, false, true), provideBooleansProvider(true, false, true), + 0, + 0, true, ), + arrayOf( + "route closure at second waypoint, include closures = false; " + + "the puck is on the first closure of 2", + "route_closure_second_waypoint.json", + provideBooleansProvider(true, false, true), + provideBooleansProvider(true, false, true), + 0, + 6, + true, + ), + arrayOf( + "route closure at second waypoint, include closures = false; " + + "the puck is on the second closure of 2", + "route_closure_second_waypoint.json", + provideBooleansProvider(true, false, true), + provideBooleansProvider(true, false, true), + 1, + 7, + false, + ), arrayOf( "route closure at second waypoint, include closures = true", "route_closure_second_waypoint.json", provideBooleansProvider(false, true, false), provideBooleansProvider(false, true, false), + 0, + 0, false, ), arrayOf( "route closure at second *silent* waypoint, include closures = false", - "route_closure_second_silent_waypoin.json", + "route_closure_second_silent_waypoint.json", provideBooleansProvider(true, false, true), provideBooleansProvider(true, false, true), + 0, + 5, + false, + ), + arrayOf( + "route closure at second *silent* waypoint, include closures = false;" + + "the puck has just stepped on the closure", + "route_closure_second_silent_waypoint.json", + provideBooleansProvider(true, false, true), + provideBooleansProvider(true, false, true), + 0, + 0, true, ), arrayOf( "route closure at second *silent* waypoint, include closures = true", - "route_closure_second_silent_waypoin.json", + "route_closure_second_silent_waypoint.json", provideBooleansProvider(false, true, false), provideBooleansProvider(false, true, false), + 0, + 0, false, ), arrayOf( @@ -105,6 +165,8 @@ class NavigationRouteExTest { "route_closure_last_coordinate.json", provideBooleansProvider(true, true, false), provideBooleansProvider(true, true, false), + 0, + 0, true, ), arrayOf( @@ -112,6 +174,8 @@ class NavigationRouteExTest { "route_closure_last_coordinate.json", provideBooleansProvider(false, false, true), provideBooleansProvider(false, false, true), + 0, + 0, false, ), arrayOf( @@ -119,6 +183,8 @@ class NavigationRouteExTest { "route_closure_between_silent_and_regular_waypoints.json", provideBooleansProvider(true, true, true), provideBooleansProvider(true, true, true), + 0, + 0, true, ), arrayOf( @@ -126,6 +192,8 @@ class NavigationRouteExTest { "route_closure_between_two_regular_waypoints.json", provideBooleansProvider(true, true, true), provideBooleansProvider(true, true, true), + 0, + 0, true, ), ) @@ -158,6 +226,18 @@ class NavigationRouteExTest { } } + private fun mockRouteProgress( + navigationRoute: NavigationRoute, + currentLegIndex: Int, + currentLegGeometryIndex: Int, + ): RouteProgress = mockk { + every { this@mockk.navigationRoute } returns navigationRoute + every { currentLegProgress } returns mockk { + every { legIndex } returns currentLegIndex + every { geometryIndex } returns currentLegGeometryIndex + } + } + private fun provideBooleansProvider( vararg bool: Boolean?, ): BooleansProvider = { bool.asList() } @@ -165,18 +245,22 @@ class NavigationRouteExTest { @Test fun testCases() = runBlocking { - val navRoute = mockNavigationRoute( - routeRaw, - snappingIncludeClosures, - snappingIncludeStaticClosures, + val routeProgress = mockRouteProgress( + mockNavigationRoute( + routeRaw, + snappingIncludeClosures, + snappingIncludeStaticClosures, + ), + currentLegIndex, + currentGeometryLegIndex, ) - val hasUnexpectedClosures = navRoute.hasUnexpectedClosures() + val hasUnexpectedUpcomingClosures = routeProgress.hasUnexpectedUpcomingClosures() - Assert.assertEquals( + assertEquals( description, - expectedHasUnexpectedClosures, - hasUnexpectedClosures + expectedHasUnexpectedUpcomingClosures, + hasUnexpectedUpcomingClosures ) } } diff --git a/libnavigation-base/src/test/resources/route_closure_second_silent_waypoin.json b/libnavigation-base/src/test/resources/route_closure_second_silent_waypoint.json similarity index 100% rename from libnavigation-base/src/test/resources/route_closure_second_silent_waypoin.json rename to libnavigation-base/src/test/resources/route_closure_second_silent_waypoint.json