[Question] Disabling / intercepting off route recalculation

[ianthetechie]

I don't see discussions enabled on this repo for asking questions, so I'm opening an issue.

Android API: 33
Maplibre Navigation SDK version: 2.0.0

I am building a basic navigation app and am currently using the NavigationLauncher (as in part of the sample app) because it's easy and doing anything else looked like an inordinate amount of work to fit into a Jetpack Compose app. This cleanly launches everything in another activity... anyways...

My question is exactly how I'd go about either disabling or intercepting the off route detection -> forced recalculation. I am not using Mapbox, so the default behavior seems to just get the app into a broken state of "recalculating" but it will never actually succeed.

I believe it's possible to attach listeners and set more options on the MapboxNavigation object itself, but I can't quite figure out how this is supposed to be set up. It seems unsupported if using the launcher. Any pointers would be appreciated as to alternate approaches that would let us set up the listeners.

[boldtrn]

I enabled discussion for this repo 👍.

Regarding the version, I would recommend to use the latest master, once we merge #86, we will probably release a new major version.

You need to create a custom RouteFetcher. Essentially, create a custom version of:

maplibre-navigation-android/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/MapboxRouteFetcher.java

Line 32 in 0f8cf44

public class MapboxRouteFetcher extends RouteFetcher {

We are open for PRs, so feel free to contribute a fetcher for a different provider like Stadia maybe ;) - that said, we should probably still refactor a bit to have a clear interface all providers can reuse.

[boldtrn]

I don't think this will be easier with 3.0. I'd still stay it is relatively straight forward, essentially, you just need to disable rerouting. Since we use a custom UI, I am not 100% sure how this would behave here, a quick&dirty approach would be just not calculate a new route with the RouteFetcher, ideally you would catch this in the NavigationViewModel.

[ianthetechie]

Gotcha. Sorry for the late reply as I'm just getting back to this after an absence.

After taking some time to understand the "dependency graph" I think I understand what needs to be done internally, but want to get a sanity check before going too much deeper with code changes.

Basically, the NavigationViewModel is in control of several important downstream pieces. Crucially, it creates a MapboxRouteFetcher:

maplibre-navigation-android/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationViewModel.java

Line 238 in 019810e

private void initializeRouter() {

1. I think the first thing is to figure out how to make this configurable. I think the most straightforward way is to add a public constructor that lets you pass in any router subclass. Thoughts?

A secondary question is where such code should live. It looks like the RouteFetcher is still in libnavigation-android, but MapboxRouteFetcher was moved to the UI library. Attempting to trace usage, it looks like NavigationViewModel and NavigationRouter are the only places where either are used. These classes are also part of the UI project.

Basically, it seems like there's a decision that UI == Mapbox, since all the Mapbox stuff only lives in the UI, and on the flip side, many of the core pieces of the UI are currently Mapbox-specific. While I assume most Mapbox users also use the default UI, I'm not sure I understand why this coupling is necessary. Two follow-up questions related to this:

2. Do you have any objection to adding other RouteFetchers to the main library?
3. Is there a reason we can't move the MapboxRouteFetcher back into the main library? The StadiaMapsRouteFetcher certainly feels like it should live outside the UI library, as I know we have some users with custom UIs similar to what I think you're doing via GraphHopper. But the responses from our (lightly modified) Valhalla-backed endpoints are drop-in compatible with Mapbox SDKs so it makes sense to subclass MapboxRouteFetcher as far as I can tell and it'd be a shame if that meant you HAD to import the whole UI lib.

[boldtrn]

I think the first thing is to figure out how to make this configurable. I think the most straightforward way is to add a public constructor that lets you pass in any router subclass. Thoughts?

Yes, or some other way to make this configureable, like a factory that picks the one given in some config or ...

While I assume most Mapbox users also use the default UI, I'm not sure I understand why this coupling is necessary.

We should try to remove all the Mapbox specifics out of the core. Ideally even out of the UI and move this to some 3rd module that handles Mapbox / OSRM specific stuff.

We could then add more modules for GraphHopper, Valhalla, etc.

Do you have any objection to adding other RouteFetchers to the main library?

IMHO the core should be provider agnostic, we need to define a good RouteFetcher Interface that works for all providers. And then add more fetchers for different providers, but probably each would live in their own module.

Is there a reason we can't move the MapboxRouteFetcher back into the main library?

Yes, the navigation core should not care about how a route is fetched. The navigation should do the core navigation, meaning, find the route progress, calculates the current instructions, detect off route, etc., routing is not part of that.

The navigation does not need to know how a new route is calculated, if this is done via http, rpc, or calling other libraries. More importantly, 3rd party apps should not need to import all these dependencies and bulk up their apps with unnecessary dependencies.

as far as I can tell and it'd be a shame if that meant you HAD to import the whole UI lib.

I agree, I would love to see some kind of plugin/module mechanism that has the implementation logic in a 3rd module.

Sidenote: the directions api package that we currently use for Mapbox is very outdated and the code is already deleted upstream. (see: #37) - building the StadiaRouteFetcher on this might not be a great solution.

[ianthetechie]

We should try to remove all the Mapbox specifics out of the core. Ideally even out of the UI and move this to some 3rd module that handles Mapbox / OSRM specific stuff.

That's actually a really good point. Ok, I'll give a bit of thought to that. That definitely needs to happen either way.

IMHO the core should be provider agnostic, we need to define a good RouteFetcher Interface that works for all providers. And then add more fetchers for different providers, but probably each would live in their own module.

Good call. So just to make sure I understand correctly, these would be separate gradle libs, right? Like libandroid-navigation-stadia for example?

Sidenote: the directions api package that we currently use for Mapbox is very outdated and the code is already deleted upstream. (see: #37) - building the StadiaRouteFetcher on this might not be a great solution.

heh ;) Interesting. So before I sign up to do a crapload of duplicate work I'll have to think this over. I'm actually working on an unrelated navigation SDK and the decision I took there was to split up what I call a "route adapter" into a "route request generator" and a "rote response parser" which all integrate with the core. This is important because OSRM, Mapbox, and Valhalla all have a common response format in my mind. yes, Mapbox has their own extensions, which are mostly adapted by Valhalla, but writing the same code 3x is insanity; it's basically an industry standard.

So a RouteFetcher is a bit simplistic of an abstraction in my opinion. It actually does several things: generating a request, executing it, and parsing the response. The request side of Valhalla, OSRM, and Mapbox are all wildly different. But the parser could be the same. I'm trying to think of the best way to reflect this insight in the existing codebase.

[boldtrn]

Good call. So just to make sure I understand correctly, these would be separate gradle libs, right? Like libandroid-navigation-stadia for example?

This would be my idea, yes. I am open to different solutions though, if you have a better idea :)

So a RouteFetcher is a bit simplistic of an abstraction in my opinion. It actually does several things: generating a request, executing it, and parsing the response. The request side of Valhalla, OSRM, and Mapbox are all wildly different. But the parser could be the same. I'm trying to think of the best way to reflect this insight in the existing codebase.

At least for now, we do have a very simple solution for that. Our own local models have a fromJson method. See for example these two lines here:

https://github.com/maplibre/maplibre-navigation-android/blob/main/app/src/main/java/com/mapbox/services/android/navigation/testapp/MockNavigationActivity.kt#L243

You could essentially get the Json from Stadia/Mapbox/OSRM and just use the fromJson. So this could be 10 lines of code sending a request with OkHttp and just dump the Json here. So probably you could even reuse the code you wrote already?

I am currently looking into further simplifying the models we use here, as they contain a lot of Mapbox specifics that are not relevant for navigation, but I plan on keeping an easy process to read OSRM like Json into these models, so that using these provider continues to be very simple. So the internal models will still contain most of the data that is here right now.

'm actually working on an unrelated navigation SDK

I know, I have checked out ferrostar and I think we are both trying to solve the same issue with different approaches :)