Async native methods#4881
Merged
Merged
Conversation
LukasPaczos
reviewed
Sep 23, 2021
LukasPaczos
left a comment
LukasPaczos
left a comment
There was a problem hiding this comment.
With the proposal, we can run into a situation like this:
2021-09-23 13:34:36.241 30154-30154/com.mapbox.navigation.examples E/AndroidRuntime: FATAL EXCEPTION: main
Process: com.mapbox.navigation.examples, PID: 30154
java.lang.IllegalStateException: Next route leg started while an active guidance session is not running.
Actual active SKU: Nav2SesFDTrip
at com.mapbox.navigation.core.accounts.BillingController$arrivalObserver$1.onNextRouteLegStart(BillingController.kt:237)
at com.mapbox.navigation.core.arrival.ArrivalProgressObserver$navigateNextRouteLeg$1.invokeSuspend(ArrivalProgressObserver.kt:65)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
at android.os.Handler.handleCallback(Handler.java:938)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loop(Looper.java:223)
at android.app.ActivityThread.main(ActivityThread.java:7656)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)
I can reproduce this by doing below once in active guidance:
mapboxNavigation.navigateNextRouteLeg {}
mapboxNavigation.setRoutes(emptyList())
This is because setRoutes synchronously resets the trip session state to free drive, and only then the async leg update returns.
It looks like we have 2 options:
- Do not observe arrival observer when session is not active guidance.
- Do not deliver arrival callbacks when the state is not valid anymore.
I feel like we need to do the latter - after completing the communication with NN, we should verify that the state is still valid and only then notify developers. Otherwise, there could be many more bugs with this approach, for example we could be delivering leg switch calls after a session is stopped, etc.
LukasPaczos
reviewed
Sep 23, 2021
korshaknn
force-pushed
the
knn-async-set-route
branch
from
September 27, 2021 10:52
67437f3 to
9150513
Compare
Contributor
Author
|
@LukasPaczos I've rebased and force-pushed the branch. |
korshaknn
force-pushed
the
knn-async-set-route
branch
from
September 28, 2021 10:49
9150513 to
88be8e1
Compare
korshaknn
marked this pull request as ready for review
September 28, 2021 10:49
korshaknn
force-pushed
the
knn-async-set-route
branch
from
September 28, 2021 10:51
88be8e1 to
785360c
Compare
Zayankovsky
reviewed
Sep 29, 2021
korshaknn
force-pushed
the
knn-async-set-route
branch
from
September 29, 2021 13:55
785360c to
3e02d17
Compare
Contributor
Author
|
@Guardiola31337 @Zayankovsky @LukasPaczos ready to the next round |
korshaknn
force-pushed
the
knn-async-set-route
branch
from
September 29, 2021 14:41
3e02d17 to
fb4acc7
Compare
korshaknn
added
backwards incompatible
Contributor
|
This is probably the same crash we were seeing in #4726 which will be solved by #4919 |
korshaknn
force-pushed
the
knn-async-set-route
branch
from
September 30, 2021 09:00
fb4acc7 to
d112202
Compare
Zayankovsky
reviewed
Sep 30, 2021
Zayankovsky
approved these changes
Sep 30, 2021
korshaknn
force-pushed
the
knn-async-set-route
branch
from
September 30, 2021 15:37
36d4480 to
778cea6
Compare
Guardiola31337
added
the
needs backporting
korshaknn
force-pushed
the
knn-async-set-route
branch
from
October 1, 2021 13:30
778cea6 to
d444f72
Compare
Guardiola31337
removed
the
needs backporting
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
NavNative async methods integration.
Changelog
Screenshots or Gifs