fix calculating remaining waypoints + add tests

Author: RingerJK

libnavigation-core/src/main/java/com/mapbox/navigation/core/trip/session/MapboxTripSession.kt

@@ -10,7 +10,6 @@ import com.mapbox.navigation.base.ExperimentalMapboxNavigationAPI
 import com.mapbox.navigation.base.internal.factory.RoadFactory
 import com.mapbox.navigation.base.internal.factory.TripNotificationStateFactory.buildTripNotificationState
 import com.mapbox.navigation.base.internal.route.refreshNativePeer
-import com.mapbox.navigation.base.internal.utils.internalWaypoints
 import com.mapbox.navigation.base.route.NavigationRoute
 import com.mapbox.navigation.base.trip.model.RouteLegProgress
 import com.mapbox.navigation.base.trip.model.RouteProgress
@@ -31,7 +30,7 @@ import com.mapbox.navigation.core.trip.session.eh.EHorizonObserver
 import com.mapbox.navigation.core.trip.session.eh.EHorizonSubscriptionManager
 import com.mapbox.navigation.navigator.internal.MapboxNativeNavigator
 import com.mapbox.navigation.navigator.internal.MapboxNativeNavigatorImpl
-import com.mapbox.navigation.navigator.internal.TripStatus
+import com.mapbox.navigation.navigator.internal.utils.calculateRemainingWaypoints
 import com.mapbox.navigation.utils.internal.JobControl
 import com.mapbox.navigation.utils.internal.ThreadController
 import com.mapbox.navigation.utils.internal.ifNonNull
@@ -48,7 +47,6 @@ import kotlinx.coroutines.Job
 import kotlinx.coroutines.cancelChildren
 import kotlinx.coroutines.launch
 import java.util.concurrent.CopyOnWriteArraySet
-import kotlin.math.max
 
 /**
  * Default implementation of [TripSession]
@@ -75,7 +73,6 @@ internal class MapboxTripSession(
 
     private companion object {
         private const val LOG_CATEGORY = "MapboxTripSession"
-        private const val INDEX_OF_INITIAL_LEG_TARGET = 1
     }
 
     override suspend fun setRoutes(
@@ -356,7 +353,7 @@ internal class MapboxTripSession(
                     nativeBannerInstruction?.index
                 )
             }
-            val remainingWaypoints = calculateRemainingWaypoints(tripStatus)
+            val remainingWaypoints = tripStatus.calculateRemainingWaypoints()
             val routeProgress = getRouteProgressFrom(
                 tripStatus.route,
                 tripStatus.navigationStatus,
@@ -378,33 +375,6 @@ internal class MapboxTripSession(
             triggerVoiceInstructionEvent(routeProgress, status)
             isOffRoute = tripStatus.navigationStatus.routeState == RouteState.OFF_ROUTE
         }
-
-        private fun calculateRemainingWaypoints(tripStatus: TripStatus): Int {
-            val routeCoordinates = tripStatus.route?.internalWaypoints()
-            return if (routeCoordinates != null) {
-                val waypointsCount = routeCoordinates.size
-                val nextWaypointIndex = normalizeNextWaypointIndex(
-                    tripStatus.navigationStatus.nextWaypointIndex
-                )
-                return waypointsCount - nextWaypointIndex
-            } else {
-                0
-            }
-        }
-
-        /**
-         * On the Android side, we always start navigation from the current position.
-         * So we expect that the next waypoint index will not be less than 1.
-         * But the native part considers the origin as a usual waypoint.
-         * It can return the next waypoint index 0. Be careful, this case isn't easy to reproduce.
-         *
-         * For example, nextWaypointIndex=0 leads to an incorrect rerouting.
-         * We don't want to get to an initial position even it hasn't been reached yet.
-         */
-        private fun normalizeNextWaypointIndex(nextWaypointIndex: Int) = max(
-            INDEX_OF_INITIAL_LEG_TARGET,
-            nextWaypointIndex
-        )
     }
 
     /**

libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/utils/TripStatusEx.kt

@@ -0,0 +1,34 @@
+package com.mapbox.navigation.navigator.internal.utils
+
+import com.mapbox.navigation.base.internal.utils.internalWaypoints
+import com.mapbox.navigation.navigator.internal.TripStatus
+import kotlin.math.max
+
+private const val INDEX_OF_INITIAL_LEG_TARGET = 1
+
+fun TripStatus.calculateRemainingWaypoints(): Int {
+    val routeWaypoints = this.route?.internalWaypoints()
+    return if (routeWaypoints != null) {
+        val waypointsCount = routeWaypoints.size
+        val nextWaypointIndex = normalizeNextWaypointIndex(
+            this.navigationStatus.nextWaypointIndex
+        )
+        return waypointsCount - nextWaypointIndex
+    } else {
+        0
+    }
+}
+
+/**
+ * On the Android side, we always start navigation from the current position.
+ * So we expect that the next waypoint index will not be less than 1.
+ * But the native part considers the origin as a usual waypoint.
+ * It can return the next waypoint index 0. Be careful, this case isn't easy to reproduce.
+ *
+ * For example, nextWaypointIndex=0 leads to an incorrect rerouting.
+ * We don't want to get to an initial position even it hasn't been reached yet.
+ */
+private fun normalizeNextWaypointIndex(nextWaypointIndex: Int) = max(
+    INDEX_OF_INITIAL_LEG_TARGET,
+    nextWaypointIndex
+)

libnavigator/src/test/java/com/mapbox/navigation/navigator/internal/router/utils/TripStatusExTest.kt

@@ -0,0 +1,102 @@
+package com.mapbox.navigation.navigator.internal.router.utils
+
+import com.mapbox.geojson.Point
+import com.mapbox.navigation.base.internal.route.Waypoint
+import com.mapbox.navigation.base.internal.utils.WaypointFactory
+import com.mapbox.navigation.base.internal.utils.internalWaypoints
+import com.mapbox.navigation.base.route.NavigationRoute
+import com.mapbox.navigation.navigator.internal.TripStatus
+import com.mapbox.navigation.navigator.internal.utils.calculateRemainingWaypoints
+import io.mockk.every
+import io.mockk.mockk
+import junit.framework.Assert.assertEquals
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+
+class TripStatusExTest {
+
+    @RunWith(Parameterized::class)
+    class CalculateRemainingWaypointsTest(
+        val description: String,
+        val routeWithWaypoints: NavigationRoute?,
+        val nextWaypointIndex: Int,
+        val expectedResult: Int,
+    ) {
+        companion object {
+            @JvmStatic
+            @Parameterized.Parameters(name = "{0}")
+            fun data() = listOf(
+                arrayOf(
+                    "null route returns 0",
+                    null,
+                    Int.MIN_VALUE,
+                    0,
+                ),
+                arrayOf(
+                    "next waypoint index normalized is normalized to default initial leg target",
+                    mockk<NavigationRoute> {
+                        every {
+                            internalWaypoints()
+                        } returns provideMockListOfWaypoints(
+                            Waypoint.REGULAR,
+                            Waypoint.REGULAR
+                        )
+                    },
+                    0,
+                    1,
+                ),
+                arrayOf(
+                    "all waypoints are taken into account to get remaining waypoints",
+                    mockk<NavigationRoute> {
+                        every {
+                            internalWaypoints()
+                        } returns provideMockListOfWaypoints(
+                            Waypoint.REGULAR,
+                            Waypoint.SILENT,
+                            Waypoint.EV_CHARGING,
+                            Waypoint.SILENT,
+                            Waypoint.REGULAR,
+                            Waypoint.REGULAR,
+                            Waypoint.SILENT,
+                            Waypoint.EV_CHARGING,
+                            Waypoint.EV_CHARGING,
+                            Waypoint.REGULAR,
+                            Waypoint.REGULAR,
+                        )
+                    },
+                    8,
+                    3,
+                ),
+            )
+
+            private fun provideMockListOfWaypoints(
+                @Waypoint.Type vararg types: Int
+            ): List<Waypoint> =
+                types.map { type ->
+                    WaypointFactory.provideWaypoint(
+                        Point.fromLngLat(0.0, 0.0),
+                        "",
+                        null,
+                        type
+                    )
+                }
+        }
+
+        @Test
+        fun testCases() {
+            val tripStatus = TripStatus(
+                routeWithWaypoints,
+                mockk {
+                    every {
+                        nextWaypointIndex
+                    } returns this@CalculateRemainingWaypointsTest.nextWaypointIndex
+                }
+            )
+
+            val result = tripStatus.calculateRemainingWaypoints()
+
+            assertEquals(description, expectedResult, result)
+        }
+    }
+}