Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rerouting after driving off course does not allow clients to easily modify RouteOptions #4426

Closed
johnnewman opened this issue Apr 13, 2023 · 2 comments
Labels
bug Something isn’t working

Comments

@johnnewman
Copy link

johnnewman commented Apr 13, 2023

Mapbox Navigation SDK version

2.11.0 with Xcode 14.2

Steps to reproduce

Start a navigation session using a NavigationViewController. Once started, deviate from your route by changing your simulated lat/lon.

After deviating from the route, SDK clients are not provided access to the RouteOptions instance that is created for the Directions API request spawning from the reroute. The NavigationViewControllerDelegate function navigationViewController(_:modifiedOptionsForReroute:) is not executed in this case. I believe this is a bug. (The shouldRerouteFrom, willRerouteFrom, and didRerouteAlong delegates are all properly executed.)

 /**
     When reroute is happening, navigation view controller suggests to customize the `RouteOptions` used to calculate new route.
     
     This method is called after `navigationViewController(_:willRerouteFrom:)` is called, and before `navigationViewController(_:didRerouteAlong:)` is called. This method is not called on proactive rerouting.
     
     Default implementation does no modifications.
     
     - parameter navigationViewController: The navigation view controller that will calculate a new route.
     - parameter options: Original `RouteOptions`.
     - returns: Modified `RouteOptions`.
 */
func navigationViewController(_ navigationViewController: NavigationViewController, modifiedOptionsForReroute options: RouteOptions) -> RouteOptions

This method is not called on proactive rerouting.

I don't think this applies here; I have this disabled via navigationService.router.reroutesProactively = false. (My understanding of Router's implementation is to enable finding better routes at regular set intervals, which isn't the same as a reroute after driving off course.) In my use case, I do want rerouting to occur after steering off course, but I do not want to proactively search for better routes while driving on an existing route.

Under the hood, when I deviate from my route, this reroute(forUrl:callback:) function is executed on RerouteController. The URL gets turned into a RouteOptions instance, which is supplied to thecustomRoutingProvider to calculate a new route. I would like to be able to modify the RouteOptions at this point, before the Directions request is initiated.

I'm not 100% sure if the RerouteController's reroutesProactively has the same meaning as Router's reroutesProactively. Insight here would be appreciated! RerouteController's internal reroutesProactively flag is true after setting Router's to false.

Expected behavior

The navigationViewController(_:modifiedOptionsForReroute:) delegate function is executed before being supplied to the Directions API.

Calling this delegate function before supplying the RouteOptions to the RoutingProvider would allow me to customize the RouteOptions before hitting the Directions API. Because this delegate is not executing, the only solution I'm aware of is creating a custom RoutingProvider from scratch and supplying this to the NavigationService.

Actual behavior

The navigationViewController(_:modifiedOptionsForReroute:) delegate function is not executed. There is no straightforward way to modify RouteOptions before the Directions API call, other than creating a custom RoutingProvider.

Is this a one-time issue or a repeatable issue?

repeatable

@johnnewman johnnewman added the bug Something isn’t working label Apr 13, 2023
@Udumft
Copy link
Contributor

Udumft commented Apr 14, 2023

Hi, @johnnewman!
That is indeed strange. If other rerouting methods (will/did reroute) are called properly, this one should be called to. There is one condition when it is ignored: user specified customRoutingProvider to the NavigationService or NavigationViewController. In that case it is up to this custom implementation to prepare RouteOptions as needed. So, do you use your RoutingProvider implementation? (You've mentioned that 'The URL gets turned into a RouteOptions instance, which is supplied to thecustomRoutingProvider to calculate a new route.' but then said that 'the only solution I'm aware of is creating a custom RoutingProvider from scratch' so I'am a little bit lost if you've created custom implementation or not.)

I'm not 100% sure if the RerouteController's reroutesProactively has the same meaning as Router's reroutesProactively. Insight here would be appreciated! RerouteController's internal reroutesProactively flag is true after setting Router's to false.

Right, there is a bit of a mess with naming. RerouteController's 'proactive rerouting' is basically a 'usual' rerouting, and is not related to searching for alternative routes. So it is OK that it stays 'true' while Router.reroutesProactively is false.

@johnnewman
Copy link
Author

Thank you for taking a look @Udumft!

I was mistakenly initializing the NavigationService with a MapboxRoutingProvider as the customRoutingProvider. That was the root of my issue:

MapboxNavigationService(
    indexedRouteResponse: IndexedRouteResponse(
        routeResponse: routeResponse,
        routeIndex: routeIndex
    ),
    customRoutingProvider: MapboxRoutingProvider(),
    credentials: Credentials(accessToken: token, host: nil),
    locationSource: nil,
    eventsManagerType: nil,
    simulating: simOption,
    routerType: nil,
    customActivityType: nil
)

After switching back to a null customRoutingProvider, the navigationViewController(_:modifiedOptionsForReroute:) delegate callback now fires, allowing me to customize the options before the Directions API request.

Appreciate the details on the reroutesProactively flag, that clears things up for me. I think we're good to close this ticket out.

@Udumft Udumft closed this as completed Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working
Projects
None yet
Development

No branches or pull requests

2 participants