Replace OSMDroid with Mapbox by seadowg · Pull Request #7245 · getodk/collect

seadowg · 2026-05-28T17:11:09Z

Why is this the best possible solution? Were any other approaches considered?

The one big (potentially unexpected) change here is to simplify the MapConfigurator interface so that MapFragment implementations don't have to deal with them at all. This abstraction mostly worked, but the implementations are by definition implementation specific, so I'd argue it was only confusing to try and abstract details for settings away from them.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

This is a pretty massive change: all the OSM maps (OSM, USGS and Carto) are now implemented using Mapbox, and so we'll need to verify that they behave as before. It's hard to put a boundary around how the "tile set" changes here can affect behaviour. I've manually verified that reference layers work as expected, but I have limited experience in that area so it'd be good to double-check.

Before submitting this PR, please make sure you have:

added or modified tests for any new or changed behavior
run ./gradlew connectedAndroidTest (or ./gradlew testLab) and confirmed all checks still pass
added a comment above any new strings describing it for translators
added any new strings with date formatting to DateFormatsTest
verified that any code or assets from external sources are properly credited in comments and/or in the about file.
verified that any new UI elements use theme colors. UI Components Style guidelines

seadowg · 2026-06-01T09:41:09Z

    }

-    private static class SourceOption {
+    public static class SourceOption {


I think longer term, we really don't want MapConfiguratorProvider to be statically accessed, but that's a rabbit hole for another time.

grzesiek2010 · 2026-06-03T12:37:06Z

 object MapFragmentReferenceLayerUtils {

    @JvmStatic
    fun getReferenceLayerFile(


This is only used in tests. We can remove it.

grzesiek2010 · 2026-06-03T13:32:56Z

-            basemap == BASEMAP_SOURCE_USGS ||
-            basemap == BASEMAP_SOURCE_CARTO
+    private fun isMapbox(source: String?): Boolean {
+        return when (source) {


This could be simplified to:

when (source) { ProjectKeys.BASEMAP_SOURCE_MAPBOX, ProjectKeys.BASEMAP_SOURCE_OSM, ProjectKeys.BASEMAP_SOURCE_USGS, ProjectKeys.BASEMAP_SOURCE_CARTO -> true else -> false }

grzesiek2010 · 2026-06-03T13:35:53Z

+        val basemapSource = settings.getString(KEY_BASEMAP_SOURCE)
+        return if (isMapbox(basemapSource)) {
+            MapboxClassInstanceCreator.createMapboxMapFragment(
+                basemapSource ?: ProjectKeys.BASEMAP_SOURCE_MAPBOX


basemapSource can't be null here - isMapbox(null) returns false, so the ?: BASEMAP_SOURCE_MAPBOX fallback is unreachable.

grzesiek2010 · 2026-06-03T14:52:52Z

    }
    implementation project(':external-app')
    implementation project(':maps')
-    implementation project(':osmdroid')


You can remove osmdroid from libs.versions.toml.
Osmdroid is still mentioned in a couple of places, like: STATE.md, MapFragment (in the comment) etc.

grzesiek2010 · 2026-06-03T15:31:53Z

There is a crash when I try to change maps in settings:

FATAL EXCEPTION: main
Process: org.odk.collect.android, PID: 6809

java.lang.ArrayIndexOutOfBoundsException: length=0; index=0
    at org.odk.collect.android.geo.MapConfiguratorProvider.getOption(MapConfiguratorProvider.java:123)
    at org.odk.collect.android.geo.MapConfiguratorProvider.getConfigurator(MapConfiguratorProvider.java:76)
    at org.odk.collect.android.preferences.screens.MapsPreferencesFragment.initBasemapSourcePref(MapsPreferencesFragment.kt:118)
    at org.odk.collect.android.preferences.screens.MapsPreferencesFragment.onCreatePreferences(MapsPreferencesFragment.kt:84)
    at androidx.preference.PreferenceFragmentCompat.onCreate(PreferenceFragmentCompat.java:161)
    at org.odk.collect.android.preferences.screens.MapsPreferencesFragment.onCreate(MapsPreferencesFragment.kt:64)
    at androidx.fragment.app.Fragment.performCreate(Fragment.java:3099)
    at androidx.fragment.app.FragmentStateManager.create(FragmentStateManager.java:524)
    at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:282)
    at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:2214)
    at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:2109)
    at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:2052)
    at androidx.fragment.app.FragmentManager$5.run(FragmentManager.java:703)
    at android.os.Handler.handleCallback(Handler.java:958)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loopOnce(Looper.java:205)

It happened on a device without Google Play Services, this case seems to be handled when we open a map (there is an error message), but in settings, it crashes the app.

grzesiek2010 · 2026-06-03T15:50:38Z

+        val uri = if (configuration.uri != null) {
+            configuration.uri
+        } else if (configuration.styleSetting != null) {
+            configuration.styleOptions.getValue(settings.getString(configuration.styleSetting)!!)


getValue() returns a StyleOption, not a BasemapUri, so neither when branch matches and the style is never loaded.

grzesiek2010 · 2026-06-03T15:51:34Z

+    val styleOptions: Map<String, StyleOption> = emptyMap()
+)
+
+class StyleOption(val name: Int, uri: BasemapUri)


uri here is never used.

grzesiek2010 · 2026-06-03T15:59:01Z

+import java.io.File
+import kotlin.collections.toIntArray
+
+class MapboxMapConfigurator(private val configuration: Configuration) : MapConfigurator {


There are a few unused imports here.

grzesiek2010 · 2026-06-03T16:07:45Z

+
+    override fun getDisplayName(file: File): String {
+        val name = MbtilesFile.readName(file)
+        return if (name != null) name else file.getName()


This could be just: return name ?: file.name

grzesiek2010 · 2026-06-03T16:10:57Z

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8"?>
+<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"


Do we need this wrapper layout?

grzesiek2010 · 2026-06-03T16:23:19Z

                // This has to happen on the main thread but we might call `initialize` from tests
                MapView(context).onCreate(null)
            }
-            OsmDroidInitializer.initialize(userAgentProvider.userAgent)


You can remove userAgentProvider from the constructor.

grzesiek2010 · 2026-06-03T16:24:56Z

+            styleOptions = mapOf(
+                "positron" to StyleOption(
+                    name = R.string.carto_map_style_positron,
+                    BasemapUri.Raster("http://1.basemaps.cartocdn.com/light_all/{z}/{x}/{y}.png")


Doesn't it work with https?

I'm guessing they're HTTP because they were added 15+ years ago! Let's use the currently-recommended ones with https: https://github.com/cartodb/basemap-styles#1-web-raster-basemaps

grzesiek2010 · 2026-06-03T16:25:01Z

+                ),
+                "dark_matter" to StyleOption(
+                    name = R.string.carto_map_style_dark_matter,
+                    BasemapUri.Raster("http://1.basemaps.cartocdn.com/dark_all/{z}/{x}/{y}.png")


Doesn't it work with https?

grzesiek2010 · 2026-06-03T17:10:05Z

-        getMapViewModel().getSettings(mapConfigurator.prefKeys).observe(viewLifecycleOwner) {
-            val newConfig = mapConfigurator.buildConfig(it)
-            onConfigChanged(newConfig)
+        getMapViewModel().getSettings(setOf(KEY_MAPBOX_MAP_STYLE)).observe(viewLifecycleOwner) {


CollectionUtils from com.google.android.gms.common.util is GMS-internal (@KeepForSdk), so it can change or disappear in any Play Services release. Collections.singleton() would be safer here.

seadowg added 21 commits June 1, 2026 10:03

Deprecate MapFragment methods

1aa55cb

Remove reference layer key from GoogleMapConfigurator

e18fe42

Remove reference layer key from Mapbox

fccb958

Remove deprecated methods from MapboxMapConfigurator

b2e4882

Remove deprecated methods from GoogleMapConfigurator

af9ed2e

Use Mapbox for all-non Google Maps

056696b

Add attribution to different Mapbox configurations

b1b37e3

Fix reference layer loading

d8996a9

Add support for OSM basemaps in Mapbox

0b3c703

Add layer at bottom if there is already layers

7d6605f

Add support for carto and USGS maps back in

741e2b7

Delete OSMDroid

ba6dd33

Clean up strings

28e1498

Fix case where app runs with no available maps

b408252

Move fragment configurations to shared object

5fce5e4

Share configurations between fragment and configurator

c9e1640

Remove deprecated methods

1e9988a

Fix strings that don't appear when Mapbox not part of the build

b50be62

Fix tests

f5c47c6

Use default values for tileset

9062d7a

Configure fake MapConfigurator for Robolectric tests

3e57fef

seadowg force-pushed the osm branch from 0f98b79 to 3e57fef Compare June 1, 2026 09:11

seadowg commented Jun 1, 2026

View reviewed changes

seadowg marked this pull request as ready for review June 1, 2026 09:41

seadowg requested a review from grzesiek2010 June 1, 2026 09:41

grzesiek2010 requested changes Jun 3, 2026

View reviewed changes

grzesiek2010 reviewed Jun 3, 2026

View reviewed changes

		@@ -0,0 +1,11 @@
		<?xml version="1.0" encoding="utf-8"?>
		<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"

Conversation

seadowg commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Before submitting this PR, please make sure you have:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grzesiek2010 commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grzesiek2010 Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grzesiek2010 Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lognaturel Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

seadowg commented May 28, 2026 •

edited

Loading

grzesiek2010 commented Jun 3, 2026 •

edited

Loading

grzesiek2010 Jun 3, 2026 •

edited

Loading

grzesiek2010 Jun 3, 2026 •

edited

Loading

lognaturel Jun 3, 2026 •

edited

Loading