Skip to content

Commit

Permalink
[Rx -> Coroutine] Convert Flowable to Flow in MapController (#1837)
Browse files Browse the repository at this point in the history
* Convert flowable to flow in MapController for streaming camera update requests

* Update unit tests [currently all of them with non-empty stream are failing]

* Change method param for panAndZoomCamera to coordinates

* Simplify code using withIndex()

* Rename to _cameraPosition

* Move to BaseMapViewModel

* Simplify the camera update flow by directly emitting positions from the coroutines

* Remove MapController.kt and tests

* Move logic for getting last saved position to a method
  • Loading branch information
shobhitagarwal1612 authored Sep 6, 2023
1 parent 7101fc7 commit 63f7ca4
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 265 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ abstract class AbstractMapContainerFragment : AbstractFragment() {
getMapViewModel().locationLock.collect { onLocationLockStateChange(it, map) }
}
getMapViewModel().mapType.observe(viewLifecycleOwner) { map.mapType = it }
getMapViewModel().cameraUpdateRequests.observe(viewLifecycleOwner) { update ->
update.ifUnhandled { data -> onCameraUpdateRequest(data, map) }
lifecycleScope.launch {
getMapViewModel().getCameraUpdates().collect { onCameraUpdateRequest(it, map) }
}

// Enable map controls
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,16 @@ import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.asLiveData
import androidx.lifecycle.toLiveData
import androidx.lifecycle.viewModelScope
import com.google.android.ground.Config.DEFAULT_LOI_ZOOM_LEVEL
import com.google.android.ground.R
import com.google.android.ground.coroutines.IoDispatcher
import com.google.android.ground.model.Survey
import com.google.android.ground.model.geometry.Coordinates
import com.google.android.ground.model.imagery.TileSource
import com.google.android.ground.repository.LocationOfInterestRepository
import com.google.android.ground.repository.MapStateRepository
import com.google.android.ground.repository.OfflineAreaRepository
import com.google.android.ground.repository.SurveyRepository
import com.google.android.ground.rx.Event
import com.google.android.ground.rx.annotations.Hot
import com.google.android.ground.system.FINE_LOCATION_UPDATES_REQUEST
import com.google.android.ground.system.LocationManager
Expand All @@ -35,21 +39,26 @@ import com.google.android.ground.system.PermissionsManager
import com.google.android.ground.system.SettingsManager
import com.google.android.ground.ui.map.Bounds
import com.google.android.ground.ui.map.CameraPosition
import com.google.android.ground.ui.map.MapController
import com.google.android.ground.ui.map.MapType
import com.google.android.ground.ui.map.gms.GmsExt.toBounds
import com.google.android.ground.ui.map.gms.toCoordinates
import io.reactivex.BackpressureStrategy
import io.reactivex.Flowable
import io.reactivex.subjects.PublishSubject
import io.reactivex.subjects.Subject
import javax.inject.Inject
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.mapNotNull
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.flow.transform
import kotlinx.coroutines.flow.withIndex
import kotlinx.coroutines.launch
import timber.log.Timber

Expand All @@ -61,10 +70,12 @@ constructor(
private val settingsManager: SettingsManager,
private val offlineAreaRepository: OfflineAreaRepository,
private val permissionsManager: PermissionsManager,
mapController: MapController,
surveyRepository: SurveyRepository,
private val surveyRepository: SurveyRepository,
private val locationOfInterestRepository: LocationOfInterestRepository,
@IoDispatcher private val ioDispatcher: CoroutineDispatcher,
) : AbstractViewModel() {

private val _cameraPosition = MutableStateFlow<CameraPosition?>(null)
private val cameraZoomSubject: @Hot Subject<Float> = PublishSubject.create()
val cameraZoomUpdates: Flowable<Float> = cameraZoomSubject.toFlowable(BackpressureStrategy.LATEST)

Expand All @@ -91,7 +102,6 @@ constructor(
else LOCATION_LOCK_ICON_DISABLED
}
.stateIn(viewModelScope, SharingStarted.Lazily, LOCATION_LOCK_ICON_DISABLED)
val cameraUpdateRequests: LiveData<Event<CameraPosition>>

val locationAccuracy: StateFlow<Float?> =
locationLock
Expand All @@ -108,12 +118,14 @@ constructor(
val offlineImageryEnabled: Flow<Boolean> = mapStateRepository.offlineImageryFlow

init {
cameraUpdateRequests = mapController.getCameraUpdates().map { Event.create(it) }.toLiveData()
mapType = mapStateRepository.mapTypeFlowable.toLiveData()
tileOverlays =
surveyRepository.activeSurveyFlow
.mapNotNull { it?.tileSources?.mapNotNull(this::toLocalTileSource) ?: listOf() }
.asLiveData()

viewModelScope.launch(ioDispatcher) { updateCameraPositionOnLocationChange() }
viewModelScope.launch(ioDispatcher) { updateCameraPositionOnSurveyChange() }
}

// TODO(#1790): Maybe create a new data class object which is not of type TileSource.
Expand Down Expand Up @@ -175,6 +187,60 @@ constructor(
}
}

/** Emits a stream of camera update requests. */
fun getCameraUpdates(): Flow<CameraPosition> = _cameraPosition.filterNotNull()

/**
* Updates map camera when location changes. The first update pans and zooms the camera to the
* appropriate zoom level and subsequent ones only pan the map.
*/
private suspend fun updateCameraPositionOnLocationChange() {
locationManager.locationUpdates
.map { it.toCoordinates() }
.withIndex()
.collect { (index, coordinates) ->
if (index == 0) {
panAndZoomCamera(coordinates)
} else {
panCamera(coordinates)
}
}
}

/** Updates map camera when active survey changes. */
private suspend fun updateCameraPositionOnSurveyChange() {
surveyRepository.activeSurveyFlow
.filterNotNull()
.transform { getLastSavedPositionOrDefaultBounds(it)?.apply { emit(this) } }
.collect { updatePosition(it) }
}

private suspend fun getLastSavedPositionOrDefaultBounds(survey: Survey): CameraPosition? {
// Attempt to fetch last saved position from local storage.
val savedPosition = mapStateRepository.getCameraPosition(survey.id)
if (savedPosition != null) {
return savedPosition.copy(isAllowZoomOut = true)
}

// Compute the default viewport which includes all LOIs in the given survey.
val geometries = locationOfInterestRepository.getAllGeometries(survey)
return geometries.toBounds()?.let { CameraPosition(bounds = it) }
}

/** Requests moving the map camera to [coordinates]. */
private fun panCamera(coordinates: Coordinates) {
updatePosition(CameraPosition(coordinates))
}

/** Requests moving the map camera to [coordinates] with zoom level [DEFAULT_LOI_ZOOM_LEVEL]. */
fun panAndZoomCamera(coordinates: Coordinates) {
updatePosition(CameraPosition(coordinates, DEFAULT_LOI_ZOOM_LEVEL))
}

private fun updatePosition(cameraPosition: CameraPosition) {
_cameraPosition.value = cameraPosition
}

/** Called when the map camera is moved. */
open fun onMapCameraMoved(newCameraPosition: CameraPosition) {
newCameraPosition.zoomLevel?.let { cameraZoomSubject.onNext(it) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ class HomeScreenMapContainerFragment : Hilt_HomeScreenMapContainerFragment() {
state.locationOfInterest
?.geometry
?.takeIf { it is Point }
?.let { mapContainerViewModel.panAndZoomCamera(Point(it.center())) }
?.let { mapContainerViewModel.panAndZoomCamera(it.center()) }
}
BottomSheetState.Visibility.HIDDEN -> {
map.enableGestures()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import androidx.lifecycle.toLiveData
import androidx.lifecycle.viewModelScope
import com.google.android.ground.Config.CLUSTERING_ZOOM_THRESHOLD
import com.google.android.ground.Config.ZOOM_LEVEL_THRESHOLD
import com.google.android.ground.coroutines.IoDispatcher
import com.google.android.ground.model.geometry.Point
import com.google.android.ground.model.job.Job
import com.google.android.ground.model.locationofinterest.LocationOfInterest
Expand All @@ -36,12 +37,12 @@ import com.google.android.ground.ui.common.BaseMapViewModel
import com.google.android.ground.ui.common.SharedViewModel
import com.google.android.ground.ui.map.CameraPosition
import com.google.android.ground.ui.map.Feature
import com.google.android.ground.ui.map.MapController
import io.reactivex.Flowable
import io.reactivex.Observable
import io.reactivex.subjects.PublishSubject
import io.reactivex.subjects.Subject
import javax.inject.Inject
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
Expand All @@ -58,22 +59,23 @@ class HomeScreenMapContainerViewModel
@Inject
internal constructor(
private val locationOfInterestRepository: LocationOfInterestRepository,
private val mapController: MapController,
private val mapStateRepository: MapStateRepository,
locationManager: LocationManager,
settingsManager: SettingsManager,
offlineAreaRepository: OfflineAreaRepository,
permissionsManager: PermissionsManager,
surveyRepository: SurveyRepository,
@IoDispatcher private val ioDispatcher: CoroutineDispatcher
) :
BaseMapViewModel(
locationManager,
mapStateRepository,
settingsManager,
offlineAreaRepository,
permissionsManager,
mapController,
surveyRepository
surveyRepository,
locationOfInterestRepository,
ioDispatcher
) {

val mapLocationOfInterestFeatures: StateFlow<Set<Feature>>
Expand Down Expand Up @@ -163,13 +165,9 @@ internal constructor(
val geometry = features[0].geometry

if (geometry is Point) {
mapController.panAndZoomCamera(geometry.coordinates)
panAndZoomCamera(geometry.coordinates)
}
}

fun panAndZoomCamera(position: Point) {
mapController.panAndZoomCamera(position.coordinates)
}

fun getZoomThresholdCrossed(): Observable<Nil> = zoomThresholdCrossed
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import com.google.android.ground.coroutines.IoDispatcher
import com.google.android.ground.model.imagery.TileSource
import com.google.android.ground.repository.LocationOfInterestRepository
import com.google.android.ground.repository.MapStateRepository
import com.google.android.ground.repository.OfflineAreaRepository
import com.google.android.ground.repository.SurveyRepository
Expand All @@ -30,7 +31,6 @@ import com.google.android.ground.ui.common.Navigator
import com.google.android.ground.ui.common.SharedViewModel
import com.google.android.ground.ui.map.Bounds
import com.google.android.ground.ui.map.Map
import com.google.android.ground.ui.map.MapController
import com.google.android.ground.ui.map.MapType
import javax.inject.Inject
import kotlinx.coroutines.CoroutineDispatcher
Expand All @@ -49,16 +49,17 @@ internal constructor(
mapStateRepository: MapStateRepository,
settingsManager: SettingsManager,
permissionsManager: PermissionsManager,
mapController: MapController,
locationOfInterestRepository: LocationOfInterestRepository
) :
BaseMapViewModel(
locationManager,
mapStateRepository,
settingsManager,
offlineAreaRepository,
permissionsManager,
mapController,
surveyRepository
surveyRepository,
locationOfInterestRepository,
ioDispatcher
) {
enum class DownloadMessage {
STARTED,
Expand Down
Loading

0 comments on commit 63f7ca4

Please sign in to comment.