Skip to content

Conversation

@LukasPaczos
Copy link

@LukasPaczos LukasPaczos commented Aug 26, 2020

This PR consolidates all router modules into one. This removes the granular modularity of onboard and offboard router modules and leaves only one routing engine implementation that encapsulates all of the hybrid logic. This opens the door for Mapbox to improve this logic in the future by centralizing it in a native codebase available for all platforms which wouldn't be possible with onboard and offboard routers being modular as well.

The default, hybrid router is still modular by excluding its dependency:

implementation("com.mapbox.navigation:core:1.0.0") {
    exclude group: "com.mapbox.navigation", module: "router"
}

and providing a custom implementation annotated with:

@MapboxModule(MapboxModuleType.NavigationRouter)

This PR also updates the routers package:

com.mapbox.navigation.route -> com.mapbox.navigation.router

and the module name:

libdirections-hybrid -> libnavigation-router

@LukasPaczos LukasPaczos added backwards incompatible Requires a SEMVER major version change. Core Work related to core navigation and integrations. labels Aug 26, 2020
@LukasPaczos LukasPaczos added this to the v1.0.0 milestone Aug 26, 2020
@LukasPaczos LukasPaczos requested a review from a team August 26, 2020 13:17
@LukasPaczos
Copy link
Author

This PR also updates the routers package

After looking at the diffs, this seems unnecessary and doesn't match the base interface anymore so I'm gonna go ahead and revert this change.

@RingerJK
Copy link
Contributor

RingerJK commented Aug 26, 2020

Do I get right, that from now we don't have separate offline and online routing? And if user want to replace the offline router only(as sample) he doesn't have that option?

@LukasPaczos
Copy link
Author

That's right @RingerJK. Users will still be able to replace the routing logic as a whole, but not only the offline routing or only the online routing. Ultimately, we haven't heard any specific request for this much control, so we might always reconsider in the future if those come.

@RingerJK
Copy link
Contributor

@LukasPaczos get it. From this view perspective we should reinvestigate RouteUrl class. One was created because Offline navigation(onboard module) cannot consume network-dependency libraries, but seems that now it might 😄

@LukasPaczos
Copy link
Author

One was created because Offline navigation(onboard module) cannot consume network-dependency libraries, but seems that now it might

Yup, good point 👍 I'm going to ticket that as tail work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backwards incompatible Requires a SEMVER major version change. Core Work related to core navigation and integrations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants