Claude/meshtastic offline map brtxh5

FORK.md

@@ -0,0 +1,100 @@
+# FORK.md
+
+Divergences of this fork from upstream [`meshtastic/Meshtastic-Android`](https://github.com/meshtastic/Meshtastic-Android).
+
+Purpose of the fork: a phone-based offline map for hiking in French Guiana. A ~400 MB SCAN 25 / Plan IGN `.mbtiles`
+file, no network in the field, and a non-technical second user who must be able to open the app and see the map with
+no setup.
+
+Licence: GPL-3.0, same as upstream. If an APK is distributed, the sources must be published with it.
+
+Trademark: "Meshtastic" is a registered trademark. The app name and `applicationId` are unchanged so far, which is
+fine for personal use. They must be changed before any distribution beyond that.
+
+## Divergences
+
+| # | Area | Change | Upstreamable |
+|---|------|--------|--------------|
+| 1 | Offline map persistence | Imported custom tile providers and the active layer now survive an app restart. | **Yes** — pure bug fix, no fork-specific behaviour. |
+| 2 | Offline map priority | On start-up, an imported local (MBTiles) provider is selected automatically when no valid saved selection applies. | No — deliberate product choice for this fork. |
+| 3 | Bluetooth-only connections | The transport selector is hidden and the Connections pane is pinned to BLE. | No — upstream deliberately supports three transports. |
+| 4 | Compass shortcut on the map | A map toolbar button opens the existing compass straight onto the favourite node. | Possibly — the `openCompass` route flag is generic; the map button is opinionated. |
+| 5 | Map control legibility | Map buttons pin an explicit 44dp touch target and a 26dp glyph. | **Yes** — it brings the controls up to the project's own documented minimum. |
+
+### 4. Compass shortcut on the map
+
+The compass itself is upstream's, and it already does the hard part: it reads the phone's magnetometer, so the arrow
+points where to walk rather than showing a bearing relative to north, and it reports distance, alignment, and the
+degraded cases (no magnetometer, no location permission, location off, no fix). Nothing here reimplements it.
+
+What the fork adds is reach. Upstream's path is: node list → find the right node → open it → tap the compass. This
+adds a toolbar button on the main map that lands on the compass in one tap:
+
+- `NodesRoute.NodeDetail` gains `openCompass: Boolean = false`, and `NodeDetailScreen` opens the compass overlay once
+  the node has loaded when it is set.
+- `MapViewProvider.MapView` and `LocalMapMainScreenProvider` gain a `navigateToNodeCompass` callback, defaulting to
+  plain node details so nothing else has to change. The osmdroid (fdroid) provider accepts it and ignores it.
+- The button appears only when a favourite node has a known position, so it is never a control that does nothing.
+
+Targeting is by **favourite node**, not a hard-coded node number: mark the companion's device as a favourite once,
+and it stays changeable in the field without a rebuild. If several nodes are favourited, the first with a position
+wins — fine for a two-person trip, worth revisiting for a larger group.
+
+### 5. Map control legibility
+
+`MapButton` sets its touch target and glyph size explicitly instead of inheriting Material's defaults, which left the
+map controls below the 44dp minimum in `.skills/design-standards`. Buttons are now 44dp with a 26dp glyph.
+
+Sized to 44dp and not the roomier 48dp on purpose: `HorizontalFloatingToolbar` does not scroll, and a fully-populated
+map toolbar (compass, find-favourite, filter, map type, layers, site planner, location) at 48dp overflows a 360dp-wide
+screen and clips its last buttons. 44dp meets the standard and still fits. If the toolbar ever gains horizontal
+scrolling, 48dp becomes the better value.
+
+Deliberately **not** changed: the icon glyphs themselves. Which symbols read as unclear is a judgement that needs eyes
+on a real screen, and swapping artwork blind would trade a known set of icons for an unverified one.
+
+### 1. Offline map persistence (bug fix)
+
+Symptom: after every app close, the imported `.mbtiles` had to be re-imported and its layer re-enabled by hand.
+
+Two independent root causes, each sufficient on its own:
+
+- **The provider list was lost on every cold start.** `CustomTileProviderRepositoryImpl` read
+  `MapTileProviderPrefs.customTileProviders.value` in its constructor. That flow was a `StateFlow` seeded with `null`
+  while the real value arrived asynchronously from DataStore, so the constructor always read the placeholder and cached
+  an empty list, permanently. The next edit then persisted a list built on that empty baseline, **destroying** the
+  stored providers rather than merely failing to display them — and orphaning their ~400 MB files in internal storage.
+- **A local provider's selection could never be restored.** `MapViewModel.loadPersistedMapType()` matched the saved
+  selection with `it.urlTemplate == savedCustomUrl && isValidTileUrlTemplate(savedCustomUrl)`. An imported MBTiles
+  provider has an empty `urlTemplate` and is persisted by its `file://` URI, which contains no `{z}/{x}/{y}`, so both
+  conditions were always false. The fallback branch then actively cleared the stored preference. The renderer
+  (`MapView.kt`) resolved the same selection correctly, so the two paths disagreed.
+
+Fix:
+
+- `MapTileProviderPrefs.customTileProviders` and `GoogleMapsPrefs.selectedCustomTileUrl` /
+  `selectedGoogleMapType` are plain `Flow`s instead of `StateFlow`s. A `StateFlow` has to invent an initial value, and
+  callers cannot tell that placeholder apart from a genuine "nothing saved".
+- `CustomTileProviderRepositoryImpl` keeps a nullable cache — `null` meaning "not read yet" — publishes only once the
+  store has answered, and serializes read-modify-write cycles behind a `Mutex`.
+- Selection resolution lives in `CustomTileProviderConfig.selectionKey` / `matchesSelection`, used by both the restore
+  path and the renderer so they cannot drift apart again.
+- `MapViewModel.restoreMapSelection()` awaits both stores before deciding, and only clears a stored selection once the
+  provider list is known to be loaded.
+- Fixed along the way: editing a provider left the persisted selection pointing at a stale key, so the layer was
+  dropped on the following start.
+
+Also removed: `feature/map/src/androidUnitTestGoogle/` (`MapViewModelTest`, `MBTilesProviderTest`). Those files sat in
+a source set no Gradle task builds, targeted an `org.meshtastic.feature.map` package that does not exist, and used a
+`MapViewModel` constructor signature several parameters out of date. They never compiled or ran — which is how this
+bug shipped. Replacement tests live in `androidApp/src/testGoogle/`, the source set that is actually wired up.
+
+### 2. Offline map priority (fork behaviour)
+
+`MapViewModel.restoreMapSelection()` selects the first local provider when there is no usable saved selection, so the
+app opens on the offline map with no network and no user action.
+
+Trade-off, stated plainly: while an MBTiles provider exists, a deliberate switch to a Google base map does not survive
+a restart — the offline layer wins again on the next launch. That is intended here (the app must always come up usable
+in the forest). To soften it, drop the `offlineProvider` branch in `restoreMapSelection()`; restoring an explicit saved
+selection keeps working without it.

androidApp/src/fdroid/kotlin/org/meshtastic/app/map/FdroidMapViewProvider.kt

@@ -27,7 +27,13 @@ import org.meshtastic.core.ui.util.MapViewProvider
 @Single
 class FdroidMapViewProvider : MapViewProvider {
     @Composable
-    override fun MapView(modifier: Modifier, navigateToNodeDetails: (Int) -> Unit, waypointId: Int?) {
+    // navigateToNodeCompass is accepted for interface parity; the osmdroid map has no compass shortcut button yet.
+    override fun MapView(
+        modifier: Modifier,
+        navigateToNodeDetails: (Int) -> Unit,
+        waypointId: Int?,
+        navigateToNodeCompass: (Int) -> Unit,
+    ) {
         val mapViewModel: MapViewModel = koinViewModel()
         LaunchedEffect(waypointId) { mapViewModel.setWaypointId(waypointId) }
         org.meshtastic.app.map.MapView(

androidApp/src/google/kotlin/org/meshtastic/app/map/GoogleMapViewProvider.kt

@@ -27,13 +27,19 @@ import org.meshtastic.core.ui.util.MapViewProvider
 @Single
 class GoogleMapViewProvider : MapViewProvider {
     @Composable
-    override fun MapView(modifier: Modifier, navigateToNodeDetails: (Int) -> Unit, waypointId: Int?) {
+    override fun MapView(
+        modifier: Modifier,
+        navigateToNodeDetails: (Int) -> Unit,
+        waypointId: Int?,
+        navigateToNodeCompass: (Int) -> Unit,
+    ) {
         val mapViewModel: MapViewModel = koinViewModel()
         LaunchedEffect(waypointId) { mapViewModel.setWaypointId(waypointId) }
         org.meshtastic.app.map.MapView(
             modifier = modifier,
             mapViewModel = mapViewModel,
             navigateToNodeDetails = navigateToNodeDetails,
+            navigateToNodeCompass = navigateToNodeCompass,
         )
     }
 }

androidApp/src/google/kotlin/org/meshtastic/app/map/MapView.kt

@@ -239,6 +239,7 @@ fun MapView(
     mapViewModel: MapViewModel = koinViewModel(),
     navigateToNodeDetails: (Int) -> Unit = {},
     mode: GoogleMapMode = GoogleMapMode.Main,
+    navigateToNodeCompass: (Int) -> Unit = navigateToNodeDetails,
 ) {
     val context = LocalContext.current
     val coroutineScope = rememberCoroutineScope()
@@ -656,7 +657,7 @@ fun MapView(
                 currentCustomTileProviderUrl?.let { url ->
                     val config =
                         mapViewModel.customTileProviderConfigs.collectAsStateWithLifecycle().value.find {
-                            it.urlTemplate == url || it.localUri == url
+                            it.matchesSelection(url)
                         }
                     mapViewModel.getTileProvider(config)?.let { tileProvider ->
                         TileOverlay(tileProvider = tileProvider, fadeIn = true, transparency = 0f, zIndex = -1f)
@@ -876,6 +877,16 @@ fun MapView(
         val showRefresh = visibleNetworkLayers.isNotEmpty()
         val isRefreshingLayers = visibleNetworkLayers.any { it.isRefreshing }
 
+        // Favourite node to point the compass at. Marking the companion's node as a favourite is the whole setup — no
+        // hard-coded node number, and it stays changeable in the field. Requires a known position, so the shortcut only
+        // appears when it can actually give a bearing. Main map only; the track and traceroute modes have one subject
+        // already. See FORK.md.
+        val nodesWithPosition by mapViewModel.nodesWithPosition.collectAsStateWithLifecycle(listOf())
+        val favoriteNodeToFind =
+            remember(nodesWithPosition, mode) {
+                nodesWithPosition.firstOrNull { node -> node.isFavorite }.takeIf { mode is GoogleMapMode.Main }
+            }
+
         MapControlsOverlay(
             modifier = Modifier.align(Alignment.TopCenter).padding(top = 8.dp),
             onToggleFilterMenu = { mapFilterMenuExpanded = true },
@@ -926,6 +937,7 @@ fun MapView(
             } else {
                 null
             },
+            onFindFavoriteClick = favoriteNodeToFind?.let { node -> { navigateToNodeCompass(node.num) } },
             isLocationTrackingEnabled = isLocationTrackingEnabled,
             onToggleLocationTracking = {
                 when {

androidApp/src/google/kotlin/org/meshtastic/app/map/MapViewModel.kt

@@ -156,7 +156,11 @@ class MapViewModel(
                 _errorFlow.emit("Invalid name, URL template, or local URI for custom tile provider.")
                 return@launch
             }
-            if (customTileProviderConfigs.value.any { it.name.equals(name, ignoreCase = true) }) {
+            if (
+                customTileProviderRepository.getCustomTileProviders().first().any {
+                    it.name.equals(name, ignoreCase = true)
+                }
+            ) {
                 _errorFlow.emit("Custom tile provider with name '$name' already exists.")
                 return@launch
             }
@@ -198,7 +202,7 @@ class MapViewModel(
                 _errorFlow.emit("Invalid name, URL template, or local URI for updating custom tile provider.")
                 return@launch
             }
-            val existingConfigs = customTileProviderConfigs.value
+            val existingConfigs = customTileProviderRepository.getCustomTileProviders().first()
             if (
                 existingConfigs.any {
                     it.id != configToUpdate.id && it.name.equals(configToUpdate.name, ignoreCase = true)
@@ -208,20 +212,15 @@ class MapViewModel(
                 return@launch
             }
 
+            // Read before writing: afterwards the store holds the new config, so the old selection key is gone.
+            val previous = customTileProviderRepository.getCustomTileProviderById(configToUpdate.id)
             customTileProviderRepository.updateCustomTileProvider(configToUpdate)
 
-            val originalConfig = customTileProviderRepository.getCustomTileProviderById(configToUpdate.id)
-            if (
-                _selectedCustomTileProviderUrl.value != null &&
-                originalConfig?.urlTemplate == _selectedCustomTileProviderUrl.value
-            ) {
-                // No change needed if URL didn't change, or handle if it did
-            } else if (originalConfig != null && _selectedCustomTileProviderUrl.value != originalConfig.urlTemplate) {
-                val currentlySelectedConfig =
-                    customTileProviderConfigs.value.find { it.urlTemplate == _selectedCustomTileProviderUrl.value }
-                if (currentlySelectedConfig?.id == configToUpdate.id) {
-                    _selectedCustomTileProviderUrl.value = configToUpdate.urlTemplate
-                }
+            // Follow the edit when the provider being displayed is the one that just changed, otherwise the persisted
+            // selection points at a key nothing matches any more and the layer is dropped on the next start.
+            if (previous != null && _selectedCustomTileProviderUrl.value == previous.selectionKey) {
+                _selectedCustomTileProviderUrl.value = configToUpdate.selectionKey
+                googleMapsPrefs.setSelectedCustomTileUrl(configToUpdate.selectionKey)
             }
         }
     }
@@ -232,10 +231,7 @@ class MapViewModel(
             customTileProviderRepository.deleteCustomTileProvider(configId)
 
             if (configToRemove != null) {
-                if (
-                    _selectedCustomTileProviderUrl.value == configToRemove.urlTemplate ||
-                    _selectedCustomTileProviderUrl.value == configToRemove.localUri
-                ) {
+                if (_selectedCustomTileProviderUrl.value?.let(configToRemove::matchesSelection) == true) {
                     _selectedCustomTileProviderUrl.value = null
                     // Also clear from prefs
                     googleMapsPrefs.setSelectedCustomTileUrl(null)
@@ -257,8 +253,7 @@ class MapViewModel(
                 googleMapsPrefs.setSelectedCustomTileUrl(null)
                 return
             }
-            // Use localUri if present, otherwise urlTemplate
-            val selectedUrl = config.localUri ?: config.urlTemplate
+            val selectedUrl = config.selectionKey
             _selectedCustomTileProviderUrl.value = selectedUrl
             _selectedGoogleMapType.value = MapType.NONE
             googleMapsPrefs.setSelectedCustomTileUrl(selectedUrl)
@@ -287,7 +282,7 @@ class MapViewModel(
             return null
         }
 
-        val selectedUrl = config.localUri ?: config.urlTemplate
+        val selectedUrl = config.selectionKey
         if (currentTileProvider != null && _selectedCustomTileProviderUrl.value == selectedUrl) {
             return currentTileProvider
         }
@@ -360,10 +355,7 @@ class MapViewModel(
             }
         }
 
-        viewModelScope.launch {
-            customTileProviderRepository.getCustomTileProviders().first()
-            loadPersistedMapType()
-        }
+        viewModelScope.launch { restoreMapSelection() }
 
         selectedWaypointId.value?.let { wpId ->
             viewModelScope.launch {
@@ -387,32 +379,45 @@ class MapViewModel(
         saveCameraPosition(cameraPositionState.position)
     }
 
-    private fun loadPersistedMapType() {
-        val savedCustomUrl = googleMapsPrefs.selectedCustomTileUrl.value
-        if (savedCustomUrl != null) {
-            // Check if this custom provider still exists
-            if (
-                customTileProviderConfigs.value.any { it.urlTemplate == savedCustomUrl } &&
-                isValidTileUrlTemplate(savedCustomUrl)
-            ) {
-                _selectedCustomTileProviderUrl.value = savedCustomUrl
-                _selectedGoogleMapType.value =
-                    MapType.NONE // MapType.NONE to hide google basemap when using custom provider
-            } else {
-                // The saved custom URL is no longer valid or doesn't exist, remove preference
-                googleMapsPrefs.setSelectedCustomTileUrl(null)
-                // Fallback to default Google Map type
-                _selectedGoogleMapType.value = MapType.NORMAL
-            }
-        } else {
-            val savedGoogleMapTypeName = googleMapsPrefs.selectedGoogleMapType.value
-            try {
-                _selectedGoogleMapType.value = MapType.valueOf(savedGoogleMapTypeName ?: MapType.NORMAL.name)
-            } catch (e: IllegalArgumentException) {
-                Logger.e(e) { "Invalid saved Google Map type: $savedGoogleMapTypeName" }
-                _selectedGoogleMapType.value = MapType.NORMAL // Fallback in case of invalid stored name
-                googleMapsPrefs.setSelectedGoogleMapType(null)
-            }
+    /**
+     * Restores the map layer chosen on a previous run, awaiting both stores before deciding.
+     *
+     * Both reads have to be awaited rather than sampled: the providers and the saved selection live in separate
+     * DataStores, and acting on a not-yet-loaded value silently discards the user's offline map.
+     */
+    private suspend fun restoreMapSelection() {
+        val configs = customTileProviderRepository.getCustomTileProviders().first()
+        val savedSelection = googleMapsPrefs.selectedCustomTileUrl.first()
+
+        val savedProvider = savedSelection?.let { selection -> configs.firstOrNull { it.matchesSelection(selection) } }
+        if (savedProvider != null) {
+            selectCustomTileProvider(savedProvider)
+            return
+        }
+        if (savedSelection != null) {
+            // configs is loaded here, so an unmatched selection really is a provider that no longer exists.
+            googleMapsPrefs.setSelectedCustomTileUrl(null)
+        }
+
+        val offlineProvider = configs.firstOrNull { it.isLocal }
+        if (offlineProvider != null) {
+            // Fork behaviour: an imported offline map always wins on start-up, so the app opens on usable tiles with no
+            // network and no user action. See FORK.md.
+            selectCustomTileProvider(offlineProvider)
+            return
+        }
+
+        restoreGoogleMapType()
+    }
+
+    private suspend fun restoreGoogleMapType() {
+        val savedGoogleMapTypeName = googleMapsPrefs.selectedGoogleMapType.first()
+        try {
+            _selectedGoogleMapType.value = MapType.valueOf(savedGoogleMapTypeName ?: MapType.NORMAL.name)
+        } catch (e: IllegalArgumentException) {
+            Logger.e(e) { "Invalid saved Google Map type: $savedGoogleMapTypeName" }
+            _selectedGoogleMapType.value = MapType.NORMAL // Fallback in case of invalid stored name
+            googleMapsPrefs.setSelectedGoogleMapType(null)
         }
     }