mapbox-java icon indicating copy to clipboard operation
mapbox-java copied to clipboard
verified publisher icon mapbox

Directions API. Url provider

Open RingerJK opened this issue 5 years ago • 6 comments

Description

In terms of Navigation for Android exists 2 types of Navigation(Router), it's Offline and Online. Both of them needs Route-URL provider.(Offline and Online are different modules , have own dependency graph and might be used independent).

Online Router's Route URL is provided by MapboxDirections that is a part of services-core library(under the hood is Retrofit(+OkHttp) library)

Problem

Offline Router cannot consume url from services-core because it brings "network" dependencies like Retrofit and OkHttp(they shouldn't be in Offline navigation). Offline Router has own route-url provider RouteUrl The main issue here: two classes where the same piece of logic exist.

Solution

  • keep service-core library interfaces as it is
  • create additional library (like router-url-provider) that depends on service-directions-model and (if java doesn't have clever approach for creating url) additional java library

How dependency tree looks now Screen Shot 2020-03-12 at 16 38 40

Suggestion Screen Shot 2020-03-12 at 16 47 59

Pros

router-url-provider might be used independently on service-core 👍

cc @mapbox/navigation-android @mapbox/maps-android @mapbox/navigation-api

RingerJK avatar Mar 12 '20 13:03 RingerJK

Tagging @langsmith for initial review - you probably are the most familiar with the current set up.

zugaldia avatar Mar 15 '20 01:03 zugaldia

Offline Router cannot consume url from services-core because it brings "network" dependencies like Retrofit and OkHttp(they shouldn't be in Offline navigation). Offline Router has own route-url provider RouteUrl The main issue here: two classes where the same piece of logic exist.

@RingerJK, is this still an issue? I ask because maybe things have changed in https://github.com/mapbox/mapbox-navigation-android since you opened this ticket. If so, is it an absolute blocker for navigation work?

langsmith avatar Apr 21 '20 19:04 langsmith

@langsmith yes, it's still actual issue. It's not a blocker, because we have opportunity to get url for Offline routing via internal implementation(RouteUrl), but would be great to resolve current issue to avoid problems with url in future

RingerJK avatar Apr 22 '20 10:04 RingerJK

@langsmith @zugaldia I can take it if you're never mind

RingerJK avatar May 26 '20 11:05 RingerJK

@RingerJK should we address this issue for 1.0, or can I wait until after?

zugaldia avatar May 27 '20 02:05 zugaldia

@zugaldia It's not a blocker for 1.0, event for 1.1. This Issue regarding resolve duplicating code in Navigation SDK and Directions API. Of course we might wait till 1.0 👍

RingerJK avatar May 27 '20 10:05 RingerJK