mapbox-navigation-android icon indicating copy to clipboard operation
mapbox-navigation-android copied to clipboard

navigation specific route options builder

Open VysotskiVadim opened this issue 3 years ago • 5 comments

Problem

Developers use RouteObject from the mapbox-java directly. They can easily make a mistake requesting a route: just forget to add any required parameter and Android Navigation SDK may crash.

Imagine a team that is trying out different Nav SDK to compare and select for their application. They may didn't notice applyDefaultNavigationOptions and make a request with coordinates and profile only, which leads to the crash. It's hard to understand that you're getting an IndexOutOfBounds error because you forgot to include steps=true in the request. Would you pick Mapbox Navigation SDK if had such problems during the first use?

Solution

Create such an API for requesting routes so that it isn't possible to make an error.

Changelog

<changelog>Added `MapboxNavigation:buildRouteOptions` that is an experimentation API to safely request a route.</changelog>

TODO:

  • [x] cancel request
  • [x] java support
  • [x] default profile?
  • [x] mix profiles: error or override ?
  • [x] reconsider API
  • [x] compare with swift solution
  • [x] add instrumentation test
  • [x] manual testing
  • [x] Can silent waypoint have bearing and zLevel? I think they should, but we need to test

Do in the context of the following tickets

  • [ ] approaches
  • [ ] round about exists?
  • [ ] driving specific params: depart at, arrive at, max height, max width, alley_bias
  • [ ] driving-traffic specific params depart at, max height, max width, snapping_include_closures
  • [ ] API for language\units
  • [ ] Compare API's capabilities with swift solution
  • [ ] How to safely share routes?
  • [ ] experiment with different API formats: compile safe, runtime safe, etc (see more )

VysotskiVadim avatar Jan 31 '22 11:01 VysotskiVadim

is the PR ready for review?

RingerJK avatar Feb 09 '22 11:02 RingerJK

is the PR ready for review?

yes.

VysotskiVadim avatar Feb 10 '22 08:02 VysotskiVadim

@tomaszrybakiewicz , @LukasPaczos , thank you for feedback. I fixed or replied to your suggestions. In general I think that PR is good enough to be merged, but I need a few more iterations before new API can become not experimental. If you don't have strong opinion about some of the new changes and approve it, I will create tickets for the next iterations based on list in the PR description, see "Do in the context of the following tickets".

VysotskiVadim avatar Mar 29 '22 14:03 VysotskiVadim

where are we here? @VysotskiVadim

RingerJK avatar Jul 22 '22 10:07 RingerJK

@RingerJK , I don't work on it anymore because there is always something more important than this. Do you think I should reconsider priority?

VysotskiVadim avatar Jul 26 '22 14:07 VysotskiVadim