Integrate current route geometry index

[dzinad]

Description

Integrates current_route_geometry_index parameter for route refresh. Note: NN version bump is required.

[Guardiola31337]

Note: NN version bump is required.

NN 111.0.0 is available now. I went ahead and bumped NN (along with Maps and Common) and fixed the listed TODOs in https://github.com/mapbox/mapbox-navigation-android/pull/6115/commits/89a25d980c935a6c65fc544a2e3718b777064bb1

This is ready for review.

cc @VysotskiVadim

[codecov[bot]]

Codecov Report

Merging #6115 (a61fffa) into main (a8a86f3) will increase coverage by 0.12%. The diff coverage is 76.98%.

@@             Coverage Diff              @@
##               main    #6115      +/-   ##
============================================
+ Coverage     68.83%   68.95%   +0.12%     
- Complexity     4262     4306      +44     
============================================
  Files           639      643       +4     
  Lines         25690    25871     +181     
  Branches       3008     3035      +27     
============================================
+ Hits          17684    17840     +156     
- Misses         6863     6879      +16     
- Partials       1143     1152       +9     

Impacted Files	Coverage Δ
...n/base/internal/factory/RouteLegProgressFactory.kt	0.00% <0.00%> (ø)
...tion/base/internal/factory/RouteProgressFactory.kt	0.00% <0.00%> (ø)
...box/navigation/base/trip/model/RouteLegProgress.kt	0.00% <0.00%> (ø)
...mapbox/navigation/base/trip/model/RouteProgress.kt	0.00% <0.00%> (ø)
...box/navigation/core/NavigationComponentProvider.kt	43.75% <0.00%> (-1.42%)	:arrow_down:
...box/navigation/core/internal/utils/ModulesUtils.kt	0.00% <0.00%> (ø)
...ore/routerefresh/RouteRefreshControllerProvider.kt	0.00% <0.00%> (ø)
...mapbox/navigation/core/trip/session/TripSession.kt	100.00% <ø> (ø)
...on/navigator/internal/MapboxNativeNavigatorImpl.kt	0.00% <0.00%> (ø)
...n/java/com/mapbox/navigation/core/SetRoutesInfo.kt	76.08% <76.08%> (ø)
... and 13 more

[Guardiola31337]

Noting that unit-tests-core is ❌

com.******.navigation.core.navigator.NavigatorMapperTest > route progress result sanity FAILED
    java.lang.AssertionError at NavigatorMapperTest.kt:112

906 tests completed, 1 failed, 2 skipped

[dzinad]

This is ready for the next round of review. I changed a lot, so please take a thorough look. @mapbox/navigation-android

[dzinad]

Capturing discussion from Slack. Navigator#refreshRoute method that accepts routeGeometryIndex expects routeRefreshResponse to contain only annotations that were returned by Directions API. While we don't and can't (because route could in theory be refreshed without using Directions API, for example if one uses custom Router) have this kind of information when invoking the method, we only have the result object. That's why I went back to using the old method for aligning java object with the native one while still using geometry index in the request. @LukasPaczos cc @DenisPryt @VysotskiVadim

[dzinad]

It will work as the old one by default, so it won't break API. But we can override it in our router and use geometry index.

I tried that, it won't work. It breaks when you implement the interface in Java.

TBH, I find synchronisation based on shared state in CurrentIndicesSnapshotProvider quite complex. Maybe we can achieve the same result easier?

Me too, but I didn't find a better solution.

[VysotskiVadim]

I tried that, it won't work. It breaks when you implement the interface in Java.

😮

[VysotskiVadim]

I tried that, it won't work. It breaks when you implement the interface in Java.

😮

have you tried @JvmDefault annotation?

[dzinad]

have you tried @JvmDefault annotation?

Yes. First, it's deprecated. Second, it requires some special compiler option which I wasn't able to integrate fast and decided that it would be too complex and might break something.

[VysotskiVadim]

Can you please try this patch? Does it work for you?

diff --git a/libnavigation-base/build.gradle b/libnavigation-base/build.gradle
index d8c93b9bb6..dbcd09208e 100644
--- a/libnavigation-base/build.gradle
+++ b/libnavigation-base/build.gradle
@@ -13,6 +13,10 @@ android {
         targetCompatibility = JavaVersion.VERSION_1_8
     }
 
+    kotlinOptions {
+        freeCompilerArgs += '-Xjvm-default=all'
+    }
+
     defaultConfig {
         minSdkVersion androidVersions.minSdkVersion
         targetSdkVersion androidVersions.targetSdkVersion
diff --git a/libnavigation-base/src/main/java/com/mapbox/navigation/base/TestJavaRouter.java b/libnavigation-base/src/main/java/com/mapbox/navigation/base/TestJavaRouter.java
new file mode 100644
index 0000000000..ff21f50b39
--- /dev/null
+++ b/libnavigation-base/src/main/java/com/mapbox/navigation/base/TestJavaRouter.java
@@ -0,0 +1,47 @@
+package com.mapbox.navigation.base;
+
+import androidx.annotation.NonNull;
+import com.mapbox.api.directions.v5.models.DirectionsRoute;
+import com.mapbox.api.directions.v5.models.RouteOptions;
+import com.mapbox.navigation.base.route.NavigationRoute;
+import com.mapbox.navigation.base.route.NavigationRouter;
+import com.mapbox.navigation.base.route.NavigationRouterCallback;
+import com.mapbox.navigation.base.route.NavigationRouterRefreshCallback;
+import com.mapbox.navigation.base.route.RouteRefreshCallback;
+import com.mapbox.navigation.base.route.RouterCallback;
+
+public class TestJavaRouter implements NavigationRouter {
+  @Override public long getRoute(@NonNull RouteOptions routeOptions, @NonNull NavigationRouterCallback callback) {
+    return 0;
+  }
+
+  @Override public long getRouteRefresh(@NonNull NavigationRoute route, int legIndex,
+    @NonNull NavigationRouterRefreshCallback callback) {
+    return 0;
+  }
+
+  @Override public long getRoute(@NonNull RouteOptions routeOptions, @NonNull RouterCallback callback) {
+    return 0;
+  }
+
+  @Override public void cancelRouteRequest(long requestId) {
+
+  }
+
+  @Override public void cancelAll() {
+
+  }
+
+  @Override
+  public long getRouteRefresh(@NonNull DirectionsRoute route, int legIndex, @NonNull RouteRefreshCallback callback) {
+    return 0;
+  }
+
+  @Override public void cancelRouteRefreshRequest(long requestId) {
+
+  }
+
+  @Override public void shutdown() {
+
+  }
+}
diff --git a/libnavigation-base/src/main/java/com/mapbox/navigation/base/route/NavigationRouter.kt b/libnavigation-base/src/main/java/com/mapbox/navigation/base/route/NavigationRouter.kt
index 0843a78ec0..6ff9dcecde 100644
--- a/libnavigation-base/src/main/java/com/mapbox/navigation/base/route/NavigationRouter.kt
+++ b/libnavigation-base/src/main/java/com/mapbox/navigation/base/route/NavigationRouter.kt
@@ -8,6 +8,13 @@ import com.mapbox.api.directions.v5.models.RouteOptions
  */
 interface NavigationRouter : Router {
 
+    fun getRouteRefresh(
+        route: NavigationRoute,
+        legIndex: Int,
+        currentGeometryIndex: Int,
+        callback: NavigationRouterRefreshCallback
+    ): Long = getRouteRefresh(route, legIndex, callback)
+
     /**
      * Fetch routes based on [RouteOptions].
      *
 

[dzinad]

@VysotskiVadim the solution with compiler options requires these options to be in the module that implements the router. So we still can't resolve the problem on our side only.

[dzinad]

I'm merging it because I need a somewhat working solution in alpha, will address all the other comments asap in #6182.

[VysotskiVadim]

@VysotskiVadim the solution with compiler options requires these options to be in the module that implements the router. So we still can't resolve the problem on our side only.

That's very strange. The compiler is supposed to generate an interface with default implementation, which should not require the same flag in the consumer module. But it doesn't really matter as the PR is already merged 🙂

[dzinad]

Have a look at https://github.com/mapbox/mapbox-navigation-android/pull/6190.