-
Notifications
You must be signed in to change notification settings - Fork 319
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
Create a delegate for MapboxNavigation
called requireMapboxNavigation
#6233
Conversation
Codecov Report
@@ Coverage Diff @@
## main #6233 +/- ##
============================================
+ Coverage 68.81% 68.84% +0.02%
- Complexity 4346 4348 +2
============================================
Files 649 650 +1
Lines 26183 26223 +40
Branches 3067 3075 +8
============================================
+ Hits 18019 18052 +33
- Misses 6998 7001 +3
- Partials 1166 1170 +4
|
7d11345
to
0f6a748
Compare
|
||
override fun onStop() { | ||
super.onStop() | ||
mapboxNavigation.unregisterRouteProgressObserver(replayProgressObserver) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bug. If you background the app it will unsubscribe. The subscribe happens in onCreate so the subscription is lost.
e92feb7
to
e8c6b00
Compare
...n-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationDelegate.kt
Outdated
Show resolved
Hide resolved
...re/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationDelegateTest.kt
Outdated
Show resolved
Hide resolved
qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/FeedbackActivity.kt
Outdated
Show resolved
Hide resolved
20250d4
to
254b936
Compare
libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/MapboxNavigationOwner.kt
Outdated
Show resolved
Hide resolved
027af78
to
1d63ae9
Compare
...n-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationDelegate.kt
Outdated
Show resolved
Hide resolved
64e6ff6
to
13785bf
Compare
mapboxNavigation.unregisterLocationObserver(locationObserver) | ||
mapboxNavigation.stopTripSession() | ||
} | ||
private val mapboxNavigation by requireMapboxNavigation( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This never loads unless you reference it. The lazy load can be confusing.
Also, the onCreate
callback from activity happens before the Lifecycle
onCreate. That causes a crash if you try to reference this delegate without using Lifecycle callbacks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it but was finding another race condition along the way.
mapboxNavigation.setRoutes(listOf(route))
binding.startNavigation.visibility = View.GONE
startSimulation(mapboxNavigation.getRoutes()[0]) <=== crashes because routes are not set yet
...avigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt
Outdated
Show resolved
Hide resolved
8a8286f
to
aa87a7d
Compare
...avigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt
Show resolved
Hide resolved
aa87a7d
to
6234305
Compare
6234305
to
b145c53
Compare
0b3b832
to
8dbc73a
Compare
…ion` (#6233) * Create a delegate for MapboxNavigation * Make requireMapboxNavigation attach during initialization
Description
There is some consistent boilerplate required to setup the
MapboxNavigationApp
. I've just spent a bit of time migrating:libnavui-androidauto:
to use theMapboxNavigationApp
#6141 and I've explored a few approaches with the examples repository mapbox/mapbox-navigation-android-examples#129. We also discussed some ambiguous scenarios where we want to accessMapboxNavigation
directly and it requires an unsafe callMapboxNavigationApp.current()!!
. #6226 (comment). This delegate removes the need for!!
with specific guidelines.This is my favorite approach so far. Create a delegate that hooks up
MapboxNavigation
to yourLifecycleOwner
. I migrated a random example from theqa-test-app
to show what it looks like, take a look at theFeedbackActivity
. EDIT migrated another exampleInactiveRouteStylingActivity
.Screenshots or Gifs
Inside any
LifecycleOwner
you can use therequireMapboxNavigation
delegate like this. And then you have access tomapboxNavigation
as if you constructed it inside.Default can be used when [MapboxNavigationApp] is setup elsewhere.
val mapboxNavigation by requireMapboxNavigation()
Initialize the [MapboxNavigationApp] when you are ready to use it
Initialize and setup subscriptions
Another example