From 353a8a70eba70635c010047912600acfcb7beaf8 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Mon, 27 Jun 2022 15:06:54 +0200 Subject: [PATCH] Using SharedFlow instead of callback for location updates to remove the need of synchronization --- .../location/LocationSharingService.kt | 17 ++-- .../location/LocationSharingViewModel.kt | 15 +++- .../app/features/location/LocationTracker.kt | 85 +++++++++---------- 3 files changed, 64 insertions(+), 53 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt index 07213ae992..9b3a1432de 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt @@ -68,9 +68,20 @@ class LocationSharingService : VectorService(), LocationTracker.Callback { super.onCreate() Timber.i("### LocationSharingService.onCreate") + initLocationTracking() + } + + private fun initLocationTracking() { // Start tracking location locationTracker.addCallback(this) locationTracker.start() + + launchWithActiveSession { session -> + val job = locationTracker.locations + .onEach(this@LocationSharingService::onLocationUpdate) + .launchIn(session.coroutineScope) + jobs.add(job) + } } override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { @@ -124,8 +135,7 @@ class LocationSharingService : VectorService(), LocationTracker.Callback { tryToDestroyMe() } - @Synchronized - override fun onLocationUpdate(locationData: LocationData) { + private fun onLocationUpdate(locationData: LocationData) { Timber.i("### LocationSharingService.onLocationUpdate. Uncertainty: ${locationData.uncertainty}") // Emit location update to all rooms in which live location sharing is active @@ -156,7 +166,6 @@ class LocationSharingService : VectorService(), LocationTracker.Callback { stopSelf() } - @Synchronized private fun tryToDestroyMe() { if (roomArgsMap.isEmpty()) { Timber.i("### LocationSharingService. Destroying self, time is up for all rooms") @@ -177,12 +186,10 @@ class LocationSharingService : VectorService(), LocationTracker.Callback { destroyMe() } - @Synchronized private fun addRoomArgs(beaconEventId: String, roomArgs: RoomArgs) { roomArgsMap[beaconEventId] = roomArgs } - @Synchronized private fun removeRoomArgs(roomId: String) { roomArgsMap.toMap() .filter { it.value.roomId == roomId } diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt index 30476d064f..b9a2dc830c 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt @@ -39,6 +39,7 @@ import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.getRoom import org.matrix.android.sdk.api.session.getUser import org.matrix.android.sdk.api.util.toMatrixItem +import timber.log.Timber /** * Sampling period to compare target location and user location. @@ -65,13 +66,20 @@ class LocationSharingViewModel @AssistedInject constructor( companion object : MavericksViewModelFactory by hiltMavericksViewModelFactory() init { - locationTracker.addCallback(this) - locationTracker.start() + initLocationTracking() setUserItem() updatePin() compareTargetAndUserLocation() } + private fun initLocationTracking() { + locationTracker.addCallback(this) + locationTracker.locations + .onEach(::onLocationUpdate) + .launchIn(viewModelScope) + locationTracker.start() + } + private fun setUserItem() { setState { copy(userItem = session.getUser(session.myUserId)?.toMatrixItem()) } } @@ -172,7 +180,8 @@ class LocationSharingViewModel @AssistedInject constructor( ) } - override fun onLocationUpdate(locationData: LocationData) { + private fun onLocationUpdate(locationData: LocationData) { + Timber.d("onLocationUpdate()") setState { copy(lastKnownUserLocation = locationData) } diff --git a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt index 013014ca2a..3a37d40b56 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt @@ -25,28 +25,27 @@ import androidx.annotation.VisibleForTesting import androidx.core.content.getSystemService import androidx.core.location.LocationListenerCompat import im.vector.app.BuildConfig -import im.vector.app.core.utils.Debouncer -import im.vector.app.core.utils.createBackgroundHandler +import im.vector.app.core.di.ActiveSessionHolder +import im.vector.app.features.session.coroutineScope +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.asSharedFlow +import kotlinx.coroutines.flow.debounce +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch import timber.log.Timber import javax.inject.Inject import javax.inject.Singleton -private const val BKG_HANDLER_NAME = "LocationTracker.BKG_HANDLER_NAME" -private const val LOCATION_DEBOUNCE_ID = "LocationTracker.LOCATION_DEBOUNCE_ID" - @Singleton class LocationTracker @Inject constructor( - context: Context + context: Context, + private val activeSessionHolder: ActiveSessionHolder ) : LocationListenerCompat { private val locationManager = context.getSystemService() interface Callback { - /** - * Called on every location update. - */ - fun onLocationUpdate(locationData: LocationData) - /** * Called when no location provider is available to request location updates. */ @@ -62,9 +61,17 @@ class LocationTracker @Inject constructor( @VisibleForTesting var hasLocationFromGPSProvider = false - private var lastLocation: LocationData? = null + // TODO update unit tests + private val _locations = MutableSharedFlow(replay = 1) - private val debouncer = Debouncer(createBackgroundHandler(BKG_HANDLER_NAME)) + /** + * SharedFlow to collect location updates + */ + val locations = _locations.asSharedFlow() + .onEach { Timber.d("new location emitted") } + .debounce(MIN_TIME_TO_UPDATE_LOCATION_MILLIS) + .onEach { Timber.d("new location emitted after debounce") } + .map { it.toLocationData() } @RequiresPermission(anyOf = [Manifest.permission.ACCESS_COARSE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION]) fun start() { @@ -119,33 +126,35 @@ class LocationTracker @Inject constructor( } @RequiresPermission(anyOf = [Manifest.permission.ACCESS_COARSE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION]) + @VisibleForTesting fun stop() { Timber.d("stop()") locationManager?.removeUpdates(this) - synchronized(this) { - callbacks.clear() - } - debouncer.cancelAll() + callbacks.clear() hasLocationFromGPSProvider = false hasLocationFromFusedProvider = false } /** - * Request the last known location. It will be given async through Callback. - * Please ensure adding a callback to receive the value. + * Request the last known location. It will be given async through corresponding flow. + * Please ensure collecting the flow before calling this method. */ fun requestLastKnownLocation() { - lastLocation?.let { locationData -> onLocationUpdate(locationData) } + Timber.d("requestLastKnownLocation") + activeSessionHolder.getSafeActiveSession()?.coroutineScope?.launch { + _locations.replayCache.firstOrNull()?.let { + Timber.d("emitting last location from cache") + _locations.emit(it) + } + } } - @Synchronized fun addCallback(callback: Callback) { if (!callbacks.contains(callback)) { callbacks.add(callback) } } - @Synchronized fun removeCallback(callback: Callback) { callbacks.remove(callback) if (callbacks.size == 0) { @@ -183,21 +192,19 @@ class LocationTracker @Inject constructor( } } - debouncer.debounce(LOCATION_DEBOUNCE_ID, MIN_TIME_TO_UPDATE_LOCATION_MILLIS) { - notifyLocation(location) - } + notifyLocation(location) } private fun notifyLocation(location: Location) { - if (BuildConfig.LOW_PRIVACY_LOG_ENABLE) { - Timber.d("notify location: $location") - } else { - Timber.d("notify location: ${location.provider}") - } + activeSessionHolder.getSafeActiveSession()?.coroutineScope?.launch { + if (BuildConfig.LOW_PRIVACY_LOG_ENABLE) { + Timber.d("notify location: $location") + } else { + Timber.d("notify location: ${location.provider}") + } - val locationData = location.toLocationData() - lastLocation = locationData - onLocationUpdate(locationData) + _locations.emit(location) + } } override fun onProviderDisabled(provider: String) { @@ -215,7 +222,6 @@ class LocationTracker @Inject constructor( } } - @Synchronized private fun onNoLocationProviderAvailable() { callbacks.toList().forEach { try { @@ -226,17 +232,6 @@ class LocationTracker @Inject constructor( } } - @Synchronized - private fun onLocationUpdate(locationData: LocationData) { - callbacks.toList().forEach { - try { - it.onLocationUpdate(locationData) - } catch (error: Exception) { - Timber.e(error, "error in onLocationUpdate callback $it") - } - } - } - private fun Location.toLocationData(): LocationData { return LocationData(latitude, longitude, accuracy.toDouble()) }