Native Router

[korshaknn]

Description

This PR removes platform side hybrid/onboard/offboard routers and adds RouterWrapper object. RouterWrapper translates router requests and route refresh requests to NavNative using RouterInterface.

refresh tests are blocked with RoutingProfile issue

Screenshots or Gifs

[LukasPaczos]

@vanmaxim just double-checking, does Nav Native respect the Common SDK OfflineSwitch and the connectivity listener it's tied to?

[mskurydin]

@LukasPaczos The hybrid router doesn't use the connectivity manager right now - it makes 2 requests in parallel, and if the online one fails, it uses the results from the onboard router.

[LukasPaczos]

The hybrid router doesn't use the connectivity manager right now - it makes 2 requests in parallel, and if the online one fails, it uses the results from the onboard router.

But since the HTTP client from Common SDK does respect the offline switch, then the resulting behavior will be correct after all 👍 We might just possibly log unnecessary errors or warnings for that failed HTTP request. Should we adjust Nav Native to respect the offline switch?

Also, should we make the fallback requests sequential instead of parallel? We are trading the speed of the fallback response for some CPU usage, do you think it's worth it when most of the fallback requests will never be used? Not saying it's bad behavior, just wanted to get your opinion.

[LukasPaczos]

The implementation LGTM to me in general but there seems to be one bug where onboard requests cannot be canceled:

request_not_canceled.mp4

This uses the IndependentRouteGenerationActivity which does integrate cancellation of previous route requests whenever a new one is scheduled. We should see only the second route but we see both because the first request couldn't be canceled.

This works correctly when the connection is available and offboard router is used but doesn't when the onboard router is used.

[korshaknn]

This uses the IndependentRouteGenerationActivity which does integrate cancellation of previous route requests whenever a new one is scheduled. We should see only the second route but we see both because the first request couldn't be canceled.
This works correctly when the connection is available and offboard router is used but doesn't when the onboard router is used.

@LukasPaczos it's NN bug, right?
@mskurydin

[LukasPaczos]

it's NN bug, right?

Yes, looks like it.

[mskurydin]

@LukasPaczos @korshaknn could you file a ticket for on-board router with more details?

[Guardiola31337]

Took this for a quick spin and for me when disabling connectivity I always get Route request failed. It seems Onboard is not currently working 🤔

cc @mskurydin @LukasPaczos

[mskurydin]

Route request failed.

Could you check what response the on-board router returns and file an issue? Wondering if the tiles are in place (the output of tilestore contents with find will help too).

[korshaknn]

@Guardiola31337 checked with Turn by turn activity from examples. Work both onboard and offboard

[Guardiola31337]

@korshaknn

checked with Turn by turn activity from examples. Work both onboard and offboard

Setup wise, what's different between Turn by turn and IndependentRouteGenerationActivity examples? I was testing IndependentRouteGenerationActivity based on @LukasPaczos's comment #4791 (review)

[Guardiola31337]

Noting that I re-tested IndependentRouteGenerationActivity and I'm still able to reproduce #4791 (review)

[korshaknn]

Noting that I re-tested IndependentRouteGenerationActivity and I'm still able to reproduce #4791 (review)

there is a bug on NN side. Onboard router requests can not be cancelled.
Other than that NN router works fine.

[Guardiola31337]

ui-robo-tests are ❌

Caused by: MapboxInvalidModuleException(type=NavigationRouter)
	at com.mapbox.common.module.provider.MapboxModuleProvider.createModule(Unknown Source:260)
	at f.c.f.b.d.<init>(Unknown Source:415)
	at f.c.f.b.e.a(Unknown Source:15)
	at com.mapbox.navigation.examples.core.MapboxVoiceActivity.k0(Unknown Source:26)
	at com.mapbox.navigation.examples.core.MapboxVoiceActivity.u(Unknown Source:0)
	at com.mapbox.navigation.examples.core.MapboxVoiceActivity.onCreate(Unknown Source:68)
	at android.app.Activity.performCreate(Activity.java:8000)
	at android.app.Activity.performCreate(Activity.java:7984)
	at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1309)
	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3422)
	... 11 more

This is Proguard (minifyEnabled true). Adding RouteWrapper here

mapbox-navigation-android/libnavigation-router/proguard-rules.pro

Line 2 in fa0ca58

-keep class com.mapbox.navigation.route.internal.hybrid.MapboxHybridRouter {*;}

does the trick 💃

[korshaknn]

@Guardiola31337 @LukasPaczos @kmadsen
RouteRefreshTest works again, used RoutingTilesOptions to mock NN requests.

[Guardiola31337]

Thanks for addressing the feedback @korshaknn

[LukasPaczos]

LGTM

[LukasPaczos]

Will we need to keep updating this file? Do we need to mock it in general? We are not mocking routing tiles request (yet), so why do we need to mock the routing tiles version requests?

[korshaknn]

we don't need to update it, we mock it to prevent mockWebServer exception

throw Error(
                    """Request url:
                      |${request.path}
                      |is not handled.
                      |
                      |Available handlers:
                      |$formattedHandlersBuilder
                    """.trimMargin()
                )

We are not mocking routing tiles request (yet), so why do we need to mock the routing tiles version requests?

not 100% sure, looks like because version request made by NN and tiles request by common (Tilestore).
I mocked it because of the mockWebServer exception.

[LukasPaczos]

I see, since we redirect everything localhost, the onboard router will probably always fail because it has no tiles. This shouldn't impact the test as long as the device doesn't go offline.

[Guardiola31337]

instrumentation-tests are failing now 😬

[korshaknn]

@Guardiola31337 fixed

[Guardiola31337]

There are still some random test failures 👀

10-26 11:54:18.191: I/nav-native(27906): Waypoints were not passed to setRoute. Trying to restore it from route itself.
10-26 11:54:18.191: W/nav-native(27906): Cannot find via_waypoints in passed route. On-board waypoints matching will be used.
10-26 11:54:18.191: W/nav-native(27906): Cannot find via_waypoints in passed route. On-board waypoints matching will be used.
10-26 11:54:18.192: I/nav-native(27906): Successful route response. Metadata: 
10-26 11:54:18.215: D/MbxNavigationTelemetry(27906): populateNavigationEvent
10-26 11:54:18.216: D/MbxNavigationTelemetry(27906): class com.mapbox.navigation.core.telemetry.events.NavigationDepartEvent event sent
10-26 11:54:18.217: W/System.err(27906): java.lang.NullPointerException: Null distance
10-26 11:54:18.225: W/System.err(27906): 	at com.mapbox.api.directions.v5.models.$AutoValue_DirectionsRoute.<init>($AutoValue_DirectionsRoute.java:40)
10-26 11:54:18.225: W/System.err(27906): 	at com.mapbox.api.directions.v5.models.AutoValue_DirectionsRoute.<init>(AutoValue_DirectionsRoute.java:28)
10-26 11:54:18.225: W/System.err(27906): 	at com.mapbox.api.directions.v5.models.AutoValue_DirectionsRoute$GsonTypeAdapter.read(AutoValue_DirectionsRoute.java:280)
10-26 11:54:18.225: W/System.err(27906): 	at com.mapbox.api.directions.v5.models.AutoValue_DirectionsRoute$GsonTypeAdapter.read(AutoValue_DirectionsRoute.java:31)
10-26 11:54:18.225: W/System.err(27906): 	at com.google.gson.Gson.fromJson(Gson.java:932)
10-26 11:54:18.225: W/System.err(27906): 	at com.google.gson.Gson.fromJson(Gson.java:1003)
10-26 11:54:18.225: W/System.err(27906): 	at com.google.gson.Gson.fromJson(Gson.java:975)
10-26 11:54:18.225: W/System.err(27906): 	at com.mapbox.api.directions.v5.models.DirectionsRoute.fromJson(DirectionsRoute.java:182)
10-26 11:54:18.225: W/System.err(27906): 	at com.mapbox.api.directions.v5.models.DirectionsRoute.fromJson(DirectionsRoute.java:204)
10-26 11:54:18.225: W/System.err(27906): 	at com.mapbox.navigation.core.routealternatives.RouteAlternativesController$nativeObserver$1.onRouteAlternativesChanged(RouteAlternativesController.kt:70)
10-26 11:54:18.225: E/nav-native(27906): RouteAlternativesControllerWorker unhandled observer unknown exception

Same as #5036 (comment) cc @mskurydin

10-26 11:54:26.608: E/AndroidRuntime(28442): FATAL EXCEPTION: MockWebServer TaskRunner
10-26 11:54:26.608: E/AndroidRuntime(28442): Process: com.mapbox.navigation.instrumentation_tests, PID: 28442
10-26 11:54:26.608: E/AndroidRuntime(28442): java.lang.Error: Request url:
10-26 11:54:26.608: E/AndroidRuntime(28442): /route-tiles/v2/mapbox/driving-traffic/versions?access_token=pk.eyJ1IjoiYWNjb3VudHMtbW9iaWxlbmF2LW1hcGJveCIsImEiOiJja3Q0dXZ6NXowMmVnMnducnJoNzVtNGduIn0.4am14Nd7WFl4uvSWD2XmPQ
10-26 11:54:26.608: E/AndroidRuntime(28442): is not handled.
10-26 11:54:26.608: E/AndroidRuntime(28442): Available handlers:
10-26 11:54:26.608: E/AndroidRuntime(28442): 	at com.mapbox.navigation.testing.ui.http.MockWebServerRule$1.dispatch(MockWebServerRule.kt:46)
10-26 11:54:26.608: E/AndroidRuntime(28442): 	at okhttp3.mockwebserver.MockWebServer$SocketHandler.processOneRequest(MockWebServer.kt:600)
10-26 11:54:26.608: E/AndroidRuntime(28442): 	at okhttp3.mockwebserver.MockWebServer$SocketHandler.handle(MockWebServer.kt:551)
10-26 11:54:26.608: E/AndroidRuntime(28442): 	at okhttp3.mockwebserver.MockWebServer$serveConnection$$inlined$execute$1.runOnce(TaskQueue.kt:220)
10-26 11:54:26.608: E/AndroidRuntime(28442): 	at okhttp3.internal.concurrent.TaskRunner.runTask(TaskRunner.kt:116)
10-26 11:54:26.608: E/AndroidRuntime(28442): 	at okhttp3.internal.concurrent.TaskRunner.access$runTask(TaskRunner.kt:42)
10-26 11:54:26.608: E/AndroidRuntime(28442): 	at okhttp3.internal.concurrent.TaskRunner$runnable$1.run(TaskRunner.kt:65)
10-26 11:54:26.608: E/AndroidRuntime(28442): 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1113)
10-26 11:54:26.608: E/AndroidRuntime(28442): 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:588)
10-26 11:54:26.608: E/AndroidRuntime(28442): 	at java.lang.Thread.run(Thread.java:818)
10-26 11:54:26.611: W/ActivityManager(860): Error in app com.mapbox.navigation.instrumentation_tests running instrumentation ComponentInfo{com.mapbox.navigation.instrumentation_tests.test/androidx.test.runner.AndroidJUnitRunner}:
10-26 11:54:26.611: W/ActivityManager(860):   java.lang.Error
10-26 11:54:26.611: W/ActivityManager(860):   java.lang.Error: Request url:
10-26 11:54:26.611: W/ActivityManager(860): /route-tiles/v2/mapbox/driving-traffic/versions?access_token=pk.eyJ1IjoiYWNjb3VudHMtbW9iaWxlbmF2LW1hcGJveCIsImEiOiJja3Q0dXZ6NXowMmVnMnducnJoNzVtNGduIn0.4am14Nd7WFl4uvSWD2XmPQ
10-26 11:54:26.611: W/ActivityManager(860): is not handled.
10-26 11:54:26.611: W/ActivityManager(860): Available handlers:
10-26 11:54:26.612: D/CrashJobIntentService(28442): onHandleWork
10-26 11:54:26.612: W/CrashReporter(28442): Root directory doesn't exist
10-26 11:54:26.613: D/AndroidRuntime(28404): Shutting down VM
10-26 11:54:26.614: I/ActivityManager(860): Force stopping com.mapbox.navigation.instrumentation_tests appid=10098 user=0: finished inst
10-26 11:54:26.614: I/ActivityManager(860): Killing 28442:com.mapbox.navigation.instrumentation_tests/u0a98 (adj 0): stop com.mapbox.navigation.instrumentation_tests

Refs. #4791 (comment)

[korshaknn]

I've fixed alternatives parsing (routeRequest instead of a route) and moved util fun to base module.
Check the last commit
cc @LukasPaczos @kmadsen @Guardiola31337

[Guardiola31337]

Thanks for addressing the feedback @korshaknn

[Guardiola31337]

Going ahead and merging here in preparation for the upcoming release 🚀

Hope you don't mind @korshaknn