Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
736ef94
Change default color for lines in maps
seadowg Jan 8, 2026
36fad66
Simplify MapIconCreator interface
seadowg Jan 9, 2026
ecd7772
Add convenience extension for create marker icon bitmaps
seadowg Jan 9, 2026
c825cc2
Add new specific icon for line points
seadowg Jan 9, 2026
12b6cd8
Fill center of shapes
seadowg Jan 9, 2026
b05cc7a
Make first point in polygon bigger
seadowg Jan 9, 2026
98c9d9b
Fix constructor use in tests
seadowg Jan 9, 2026
71dc983
Update default color tests
seadowg Jan 9, 2026
cdc6503
Fix assertions on MarkerIconDescription
seadowg Jan 9, 2026
6a76557
Correct method name
seadowg Jan 13, 2026
dff3c30
Update implementations
seadowg Jan 13, 2026
2b6491d
Store line points in GeoPolyFragment instead of in MapFragment
seadowg Jan 13, 2026
be829c4
Fix clear behaviour
seadowg Jan 14, 2026
b6c1276
Remove unneeded formatting for shape
seadowg Jan 14, 2026
29b7bb3
Switch to using polygons with map for shapes
seadowg Jan 14, 2026
8415fc2
Don't use new API for Google and Mapbox
seadowg Jan 14, 2026
7e585a2
Highlight last point rather than first
seadowg Jan 14, 2026
d5f30e7
Remove add/remove methods for poly points
seadowg Jan 15, 2026
3d90579
Store LocationTracker location in Flow
seadowg Jan 15, 2026
b45e310
Expose location as StateFlow
seadowg Jan 15, 2026
7d09ee0
Remove unused field
seadowg Jan 15, 2026
0f8d2f2
Use ViewModel for points state
seadowg Jan 15, 2026
e576fa6
Use LocationTracker to schedule location recording
seadowg Jan 15, 2026
91b0bea
Fix whitespace issue
seadowg Jan 15, 2026
6ae1adb
Add updatePolyLine method
seadowg Jan 15, 2026
d0281d4
Add updatePolygon method
seadowg Jan 15, 2026
2774e36
Simplify feature description constructor calls
seadowg Jan 15, 2026
055f227
Highlight last point with different color
seadowg Jan 15, 2026
f3147e3
Update fake
seadowg Jan 15, 2026
3e07038
Clean up unneeded code
seadowg Jan 15, 2026
d508feb
Break up single line
seadowg Jan 19, 2026
974898f
Rename method
seadowg Jan 19, 2026
a63222a
Rename class
seadowg Jan 19, 2026
f3c1845
Remove double save to cache
seadowg Jan 19, 2026
46f0155
Correct whitespace
seadowg Jan 19, 2026
16eabc9
Fix formatting
seadowg Jan 19, 2026
2164907
Correct typo
seadowg Jan 19, 2026
72a978f
Use isNotEmpty
seadowg Jan 19, 2026
f6ded2d
Use last()
seadowg Jan 19, 2026
35ae10e
Remove redundant let
seadowg Jan 19, 2026
e6d579d
Remove redundant alpha setting
seadowg Jan 19, 2026
0b63d63
Only update map when points updated
seadowg Jan 19, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Use LocationTracker to schedule location recording
  • Loading branch information
seadowg committed Jan 19, 2026
commit e576fa6b20e4bb4a3111423f69d2db7e08027069
52 changes: 6 additions & 46 deletions geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,6 @@ import org.odk.collect.maps.layers.OfflineMapLayersPickerBottomSheetDialogFragme
import org.odk.collect.maps.layers.ReferenceLayerRepository
import org.odk.collect.settings.SettingsProvider
import org.odk.collect.webpage.WebPageService
import java.util.concurrent.Executors
import java.util.concurrent.ScheduledExecutorService
import java.util.concurrent.ScheduledFuture
import java.util.concurrent.TimeUnit
import javax.inject.Inject

class GeoPolyFragment @JvmOverloads constructor(
Expand Down Expand Up @@ -73,9 +69,6 @@ class GeoPolyFragment @JvmOverloads constructor(
lateinit var webPageService: WebPageService

private var previousState: Bundle? = null
private val executorServiceScheduler: ScheduledExecutorService =
Executors.newSingleThreadScheduledExecutor()
private var schedulerHandler: ScheduledFuture<*>? = null

private var map: MapFragment? = null
private var featureId = -1 // will be a positive featureId once map is ready
Expand Down Expand Up @@ -105,7 +98,7 @@ class GeoPolyFragment @JvmOverloads constructor(
private val viewModel: GeoPolyViewModel by viewModels {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We can probably move a bunch more of the Fragment's state in to this (or another) ViewModel, but I think this is a good start for now: we can continue improving this as part of #6970.

viewModelFactory {
addInitializer(GeoPolyViewModel::class) {
GeoPolyViewModel(outputMode, inputPolygon)
GeoPolyViewModel(outputMode, inputPolygon, locationTracker)
}
}
}
Expand Down Expand Up @@ -188,28 +181,13 @@ class GeoPolyFragment @JvmOverloads constructor(
state.putInt(ACCURACY_THRESHOLD_INDEX_KEY, accuracyThresholdIndex)
}

override fun onDestroy() {
schedulerHandler?.let {
if (!it.isCancelled) {
it.cancel(true)
}
}

locationTracker.stop()
super.onDestroy()
}

fun initMap(newMapFragment: MapFragment?, binding: GeopolyLayoutBinding) {
map = newMapFragment

binding.clear.setOnClickListener { showClearDialog() }
binding.pause.setOnClickListener {
viewModel.stopRecording()
inputActive = false
try {
schedulerHandler?.cancel(true)
} catch (_: Exception) {
// Do nothing
}
updateUi()
}

Expand Down Expand Up @@ -336,28 +314,10 @@ class GeoPolyFragment @JvmOverloads constructor(
override fun startInput() {
inputActive = true
if (recordingEnabled && recordingAutomatic) {
locationTracker.start(retainMockAccuracy)

recordPoint(map!!.getGpsLocation())
schedulerHandler = executorServiceScheduler.scheduleAtFixedRate(
{
requireActivity().runOnUiThread {
val currentLocation = locationTracker.getCurrentLocation()
if (currentLocation != null) {
val currentMapPoint = MapPoint(
currentLocation.latitude,
currentLocation.longitude,
currentLocation.altitude,
currentLocation.accuracy.toDouble()
)

recordPoint(currentMapPoint)
}
}
},
INTERVAL_OPTIONS[intervalIndex].toLong(),
INTERVAL_OPTIONS[intervalIndex].toLong(),
TimeUnit.SECONDS
viewModel.startRecording(
retainMockAccuracy,
ACCURACY_THRESHOLD_OPTIONS[accuracyThresholdIndex],
INTERVAL_OPTIONS[intervalIndex].toLong() * 1000
)
}
updateUi()
Expand Down
46 changes: 44 additions & 2 deletions geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyViewModel.kt
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
package org.odk.collect.geo.geopoly

import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.launch
import org.odk.collect.geo.geopoly.GeoPolyFragment.OutputMode
import org.odk.collect.location.tracker.LocationTracker
import org.odk.collect.maps.MapPoint
import kotlin.collections.plus

class GeoPolyViewModel(outputMode: OutputMode, points: List<MapPoint>) :
class GeoPolyViewModel(
outputMode: OutputMode,
points: List<MapPoint>,
private val locationTracker: LocationTracker
) :
ViewModel() {

private val _points = MutableStateFlow(
Expand All @@ -22,6 +28,29 @@ class GeoPolyViewModel(outputMode: OutputMode, points: List<MapPoint>) :
}
)
val points: StateFlow<List<MapPoint>> = _points
private var accuracyThreshold: Int = 0

init {
viewModelScope.launch {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We've talked off and on about whether to use coroutines directly like this (as opposed to through our Scheduler interface) - especially with ViewModels as it gives us viewModelScope. I can't immediately think of a benefit to using Scheduler here as this work is dispatched on the UI thread, so there's no need to integrate with IdlingResource for Espresso. That said, as far as I can tell, there are no tests for the automatic location recording. I'll backfill those as a follow up and see how this fits in.

locationTracker.getLocation().collect {
if (it != null) {
accuracyThreshold.let { threshold ->
if (threshold == 0 || it.accuracy <= threshold) {
add(
MapPoint(
it.latitude,
it.longitude,
it.altitude,
it.accuracy.toDouble()
)
)
}
}

}
}
}
}

fun add(point: MapPoint) {
val points = _points.value
Expand All @@ -37,4 +66,17 @@ class GeoPolyViewModel(outputMode: OutputMode, points: List<MapPoint>) :
fun update(points: List<MapPoint>) {
_points.value = points
}

fun startRecording(retainMockAccuracy: Boolean, accuracyThreshold: Int, interval: Long) {
this.accuracyThreshold = accuracyThreshold
locationTracker.start(retainMockAccuracy, interval)
}

fun stopRecording() {
locationTracker.stop()
}

override fun onCleared() {
locationTracker.stop()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito
import org.mockito.kotlin.any
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
import org.odk.collect.androidshared.ui.FragmentFactoryBuilder
Expand Down Expand Up @@ -358,7 +360,7 @@ class GeoPolyFragmentTest {
}

startInput(R.id.automatic_mode)
verify(locationTracker).start(true)
verify(locationTracker).start(eq(true), any())
}

@Test
Expand All @@ -374,7 +376,7 @@ class GeoPolyFragmentTest {
}

startInput(R.id.automatic_mode)
verify(locationTracker).start(false)
verify(locationTracker).start(eq(false), any())
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class LocationTrackerService : Service(), LocationClient.LocationClientListener
val interval = intent.getLongExtra(EXTRA_UPDATE_INTERVAL, -1)
locationClient.setUpdateIntervals(
interval,
interval / 2
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we simplify #setUpdateIntervals as now we alwasy pass the same values?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I can follow up with that as I'd like to investigate white it was the way it was.

interval
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class ForegroundServiceLocationTrackerTest : LocationTrackerTest() {
locationTracker.start(updateInterval = 1000)
runBackground()

assertThat(locationClient.getUpdateIntervals(), equalTo(Pair(1000L, 500L)))
assertThat(locationClient.getUpdateIntervals(), equalTo(Pair(1000L, 1000L)))
}

@Test
Expand All @@ -90,7 +90,7 @@ class ForegroundServiceLocationTrackerTest : LocationTrackerTest() {
runBackground()

assertThat(locationClient.getRetainMockAccuracy(), equalTo(true))
assertThat(locationClient.getUpdateIntervals(), equalTo(Pair(2000L, 1000L)))
assertThat(locationClient.getUpdateIntervals(), equalTo(Pair(2000L, 2000L)))
}
}

Expand Down