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

Fix route alternatives parsing #5043

Closed
wants to merge 3 commits into from

Conversation

korshaknn
Copy link
Contributor

@korshaknn korshaknn commented Oct 27, 2021

Description

depends on #4791

Fixed:
RouteAlternative.route coming from NN is not a route, it's routeResponse, so it should be parsed as DirectionsResponse

Follow up questions

  • alterantives min update interval on SDK is 10sec, in NN 30sec - breaking change?
  • on route refresh NN returns route, in alternatives - route response. it's confusing and lead to errors
  • invalid alternatives request intervals. see logs
interval = 30sec

2021-10-27 15:33:59.412 19654-27831/com.mapbox.navigation.qa_test_app D/nav-native: SDK onRouteAlternativesChanged size = 0, routeProgress = null
2021-10-27 15:34:08.112 19654-27831/com.mapbox.navigation.qa_test_app D/nav-native: SDK onRouteAlternativesChanged size = 1, routeProgress = null
2021-10-27 15:34:32.244 19654-27829/com.mapbox.navigation.qa_test_app D/nav-native: SDK onRouteAlternativesChanged size = 0, routeProgress = RouteProgress(route=DirectionsRoute{routeIndex=0, 
2021-10-27 15:35:02.662 19654-27831/com.mapbox.navigation.qa_test_app D/nav-native: SDK onRouteAlternativesChanged size = 1, routeProgress = RouteProgress(route=DirectionsRoute{routeIndex=0, 
2021-10-27 15:35:20.256 19654-27832/com.mapbox.navigation.qa_test_app D/nav-native: SDK onRouteAlternativesChanged size = 0, routeProgress = RouteProgress(route=DirectionsRoute{routeIndex=0, 
2021-10-27 15:35:50.566 19654-27829/com.mapbox.navigation.qa_test_app D/nav-native: SDK onRouteAlternativesChanged size = 1, routeProgress = RouteProgress(route=DirectionsRoute{routeIndex=0, 
2021-10-27 15:36:04.882 19654-27832/com.mapbox.navigation.qa_test_app D/nav-native: SDK onRouteAlternativesChanged size = 0, routeProgress = RouteProgress(route=DirectionsRoute{routeIndex=0, 
2021-10-27 15:36:05.200 19654-27831/com.mapbox.navigation.qa_test_app D/nav-native: SDK onRouteAlternativesChanged size = 1, routeProgress = RouteProgress(route=DirectionsRoute{routeIndex=0, 
2021-10-27 15:36:46.907 19654-27831/com.mapbox.navigation.qa_test_app D/nav-native: SDK onRouteAlternativesChanged size = 0, routeProgress = RouteProgress(route=DirectionsRoute{routeIndex=0, 
  • there are issues with request canceling. If we comment code in onRouteAlternativesChanged
directionsSession.setRoutes(
                routes = changedRoutes,
                initialLegIndex = 0,
                ROUTES_UPDATE_REASON_ALTERNATIVE
            )

requests go as expected, but with this code, the half of requests is canceled. Is it SDK or NN issue? Logs

2021-10-27 15:40:45.254 19655-28581/com.mapbox.navigation.qa_test_app I/nav-native: Directions Service: Get route request error (type 'RequestCancelled'): Request cancelled
2021-10-27 15:40:45.254 19655-28542/com.mapbox.navigation.qa_test_app W/nav-native: RouteAlternativesControllerWorker::onRoutesRetrieved: Cancelled, type: 3, reqId: 4

@kmadsen have you had such issues? might they be related to NN router?

Changelog

<changelog>Fixed alternatives parsing</changelog>

Screenshots or Gifs

@korshaknn korshaknn requested a review from a team as a code owner October 27, 2021 12:49
@korshaknn korshaknn self-assigned this Oct 27, 2021
@korshaknn korshaknn added the ⚠️ DO NOT MERGE PR should not be merged! label Oct 27, 2021
@korshaknn korshaknn force-pushed the knn-fix-route-alternatives-parsing branch from 523cd47 to d41f713 Compare October 27, 2021 13:15
@korshaknn
Copy link
Contributor Author

korshaknn commented Oct 27, 2021

@kmadsen ah, I've missed #5037
looks like NN router adds extra issues )

@korshaknn
Copy link
Contributor Author

closing the PR, I'll cherry-pick needed changes to NN router PR #4791

@korshaknn korshaknn closed this Oct 27, 2021
@LukasPaczos LukasPaczos deleted the knn-fix-route-alternatives-parsing branch November 1, 2021 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ DO NOT MERGE PR should not be merged!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant