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

NAVAND-1636: do not set routes to NN when session is already started #7661

Merged
merged 2 commits into from
Dec 12, 2023

Conversation

dzinad
Copy link
Contributor

@dzinad dzinad commented Dec 8, 2023

@dzinad dzinad requested a review from a team as a code owner December 8, 2023 14:01
@dzinad dzinad added the needs backporting Requires cherry-picking to a currently running release branch label Dec 8, 2023
Copy link

github-actions bot commented Dec 8, 2023

Changelog

Features

Bug fixes and improvements

  • Fixed an issue where the first voice instruction might have been played thrice when switching between regular session and replay with route being set. [#dd](https://github.com/mapbox/mapbox-navigation-android/pull/dd)

Known issues ⚠️

Other changes

Android Auto Changelog

Features

Bug fixes and improvements

  • The app is now considered as the one in active navigation only when an active route is set to MapboxNavigation. Previously it was always considered active. [#7366](https://github.com/mapbox/mapbox-navigation-android/pull/7366)
  • When Android Auto host tells the app to stop active navigation because another app starts navigating, now SDK will enter FreeDrive mode instead of stopping trip session completely. [#7366](https://github.com/mapbox/mapbox-navigation-android/pull/7366)

Comment on lines 1156 to 1164
private fun resetTripSessionRoutes() {
threadController.getMainScopeAndRootJob().scope.launch(Dispatchers.Main.immediate) {
routeUpdateMutex.withLock {
val routes = directionsSession.routes
val legIndex = latestLegIndex ?: directionsSession.initialLegIndex
setRoutesToTripSession(routes, SetRoutes.NewRoutes(legIndex))
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was it needed? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there were cases when the routes might have been present on the NavSDK side, but not on NN side. Or it was added just in case, because we didn't know exactly how NN works with routes.

Copy link

codecov bot commented Dec 8, 2023

Codecov Report

Merging #7661 (4d70f4f) into release-v2.17 (a5491d0) will increase coverage by 0.00%.
Report is 8 commits behind head on release-v2.17.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff                @@
##             release-v2.17    #7661   +/-   ##
================================================
  Coverage            74.08%   74.08%           
  Complexity            6152     6152           
================================================
  Files                  833      833           
  Lines                33128    33130    +2     
  Branches              3957     3958    +1     
================================================
+ Hits                 24544    24546    +2     
  Misses                7050     7050           
  Partials              1534     1534           
Files Coverage Δ
...ava/com/mapbox/navigation/core/MapboxNavigation.kt 69.55% <100.00%> (+0.08%) ⬆️

@dzinad dzinad changed the title NAVAND-1636: do not set routes to NN when starting a session NAVAND-1636: do not set routes to NN when session is already started Dec 11, 2023
@VysotskiVadim VysotskiVadim self-requested a review December 12, 2023 10:41
@dzinad dzinad enabled auto-merge (rebase) December 12, 2023 12:29
@dzinad dzinad merged commit 21d7467 into release-v2.17 Dec 12, 2023
31 of 35 checks passed
@dzinad dzinad deleted the NAVAND-1636-dd branch December 12, 2023 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs backporting Requires cherry-picking to a currently running release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants