Skip to content

Adding support for sharing multiple instances of MapboxNavigationObserver#5829

Merged
kmadsen merged 1 commit intomainfrom
km-do-not-allow-duplicate-subscriptions
May 19, 2022
Merged

Adding support for sharing multiple instances of MapboxNavigationObserver#5829
kmadsen merged 1 commit intomainfrom
km-do-not-allow-duplicate-subscriptions

Conversation

@kmadsen
Copy link
Copy Markdown
Contributor

@kmadsen kmadsen commented May 19, 2022

Description

We're going to use MapboxNavigationApp.registerObserver to share subscriptions between different architectures. For example, android auto will register a MapboxNavigationObserver that may also be registered by drop in ui.

This pull request is adding safe ways to get and register MapboxNavigationObserver instances. Note that thread safety is not yet being address in this pull request.

Code samples

Register from multiple paths

Android Auto Session onCreate
and a Fragment onCreate

object AudioGuidanceObserver : MapboxNavigationObserver
if (MapboxNavigationApp.getObservers(MyObserver::class).isEmpty()) {
  MapboxNavigationApp.registerObserver(MyObserver)
}

This will ensure that only one MyObserver is registered. The unregister is tied to the MapboxNavigationApp.lifecycleOwner; meaning that when there are no lifecycles attached the MyObserver instance will be detached but still registered.

Sharing multiple subscriptions

The current MapboxNavigationApp.getObserver will crash when the observer is not found. There are implementations that we should support in case there are multiple/optional observers registered. For example, you may have registered multiple observers that are updating status on a map (nearby places while navigating for example).

MyFragment

// Periodically update the the waiting line size of ev charging stations
class EvStationObserver : MapboxNavigationObserver
val evStations = MapboxNavigationApp.getObservers(EvStationObserver::class)
evStations.forEach {
  // update map marker
}

// Clear all the instances of a specific observer type

class NearbyGasStationObserver : MapboxNavigationObserver
val evStations = MapboxNavigationApp.getObservers(NearbyGasStationObserver::class)
evStations.forEach {
  MapboxNavigationApp.unregister(evStations)
}

@kmadsen kmadsen requested a review from a team as a code owner May 19, 2022 01:50
@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2022

Codecov Report

Merging #5829 (9b5f7d3) into main (8472f8a) will increase coverage by 0.00%.
The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #5829   +/-   ##
=========================================
  Coverage     67.66%   67.67%           
- Complexity     3829     3835    +6     
=========================================
  Files           586      586           
  Lines         23572    23577    +5     
  Branches       2714     2716    +2     
=========================================
+ Hits          15951    15955    +4     
- Misses         6603     6604    +1     
  Partials       1018     1018           
Impacted Files Coverage Δ
...x/navigation/core/lifecycle/MapboxNavigationApp.kt 0.00% <0.00%> (ø)
...tion/core/lifecycle/MapboxNavigationAppDelegate.kt 91.89% <100.00%> (+0.46%) ⬆️
...navigation/core/lifecycle/MapboxNavigationOwner.kt 89.74% <100.00%> (+0.55%) ⬆️

@kmadsen kmadsen force-pushed the km-do-not-allow-duplicate-subscriptions branch 2 times, most recently from 28f13fa to e943866 Compare May 19, 2022 01:53
@kmadsen kmadsen force-pushed the km-do-not-allow-duplicate-subscriptions branch 2 times, most recently from 169a9db to a11fe52 Compare May 19, 2022 16:06
@kmadsen
Copy link
Copy Markdown
Contributor Author

kmadsen commented May 19, 2022

Another API i've considered is registering observers with a lifecycle. This would ensure 1 observer exists while a lifecycle is in an active state.

val observer1 = MapboxNavigationApp.register(fragmentLifecycle, State.STARTED, MyObserver::class)
val observer2 = MapboxNavigationApp.register(carSessionLifecycle, State.CREATED, MyObserver::class)
// observer1 == observer2

This is slightly complex to keep track of a multiple maps of Map<Class<extends MapboxNavigationObserver>, Set<Lifecycle>>, where the maps themselves are maintained lifecycle observers. Can be done, takes a bit of testing.

@kmadsen kmadsen force-pushed the km-do-not-allow-duplicate-subscriptions branch from a11fe52 to be27e88 Compare May 19, 2022 17:18
Copy link
Copy Markdown
Contributor

@tomaszrybakiewicz tomaszrybakiewicz left a comment

Choose a reason for hiding this comment

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

LGTM

@kmadsen kmadsen added UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. Core Work related to core navigation and integrations. Android Auto Bugs, improvements and feature requests on Android Auto. labels May 19, 2022
@kmadsen kmadsen force-pushed the km-do-not-allow-duplicate-subscriptions branch from be27e88 to 9b5f7d3 Compare May 19, 2022 19:39
@kmadsen kmadsen enabled auto-merge (squash) May 19, 2022 20:09
@kmadsen kmadsen merged commit 25da980 into main May 19, 2022
@kmadsen kmadsen deleted the km-do-not-allow-duplicate-subscriptions branch May 19, 2022 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Android Auto Bugs, improvements and feature requests on Android Auto. Core Work related to core navigation and integrations. UI Work related to visual components, Android Auto, Camera, 3D, voice, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants