Add coverage on map #470

sim6 · 2025-01-30T22:44:23Z

No description provided.

mjaakko

Great, thanks! This should improve UX for BeaconDB contributors :)

We might have to rethink in the future where the map layers are configured (in the settings or on the map screen), but this is good enough for now

mjaakko · 2025-01-31T19:10:42Z

app/src/main/java/xyz/malkki/neostumbler/ui/screens/MapScreen.kt

+                                mapStyle.value!!.styleJson != null && map.style!!.json != mapStyle.value!!.styleJson
+                            )
+                        ) {
+                            map.setStyle(styleBuilder(mapStyle, getCoverageParams()))


I think you need to also check if the coverage params have changed here. Otherwise if the coverage params have changed, the coverage won't be shown unless the map style has also changed

Yeah, it's kind of an edge case and doesn't really happen in practice, but it's better to handle it now

When I change the settings and I back to the map the map also changed.

That's because it's very likely that the map screen will recompose after switching tabs and the factory method will run again. But it's not guaranteed that it will recompose

Done via https://github.com/mjaakko/NeoStumbler/compare/e833de128188e3d29cbcf9d0b0a81384b0873c55..ba63cd755621c804af990f292df3d7f8e11598cb

mjaakko · 2025-01-31T19:11:52Z

app/src/main/java/xyz/malkki/neostumbler/ui/screens/MapScreen.kt

@@ -279,6 +288,36 @@ fun MapScreen(mapViewModel: MapViewModel = viewModel()) {
    }
 }

+private fun styleBuilder(mapStyle: State<MapViewModel.MapStyle?>, coverageParams: CoverageParams): Style.Builder {


Also thinking about this, maybe it would be better to add the TileJSON layer independently from the map style?

I do not know how to add the layer without org.maplibre.android.maps.Style. Could you give me a hint?

I mean you need the style, but you don't have to set the base style (meaning the background map layer) and the TileJSON coverage layer at the same time. You could use getStyle() to get the base style and then add the coverage layer using that. Or do something similar like what is done with the hexagon layer

Done via https://github.com/mjaakko/NeoStumbler/compare/e833de128188e3d29cbcf9d0b0a81384b0873c55..ba63cd755621c804af990f292df3d7f8e11598cb

mjaakko · 2025-01-31T19:14:21Z

app/src/main/java/xyz/malkki/neostumbler/CoverageParams.kt

+
+data class CoverageParams(
+    val tileJson: String?,
+    val layerId: String?,


Is this layer ID necessary? From UX perspective it might be kind of confusing, especially for non-technical users. I think we can just use all layers specified in the TileJSON?

Layer ID should be used as source layer to render it. I will change the code to parse it from the TileJSON.

That would definitely make it more usable 👍 Maybe we can add all layers from the TileJSON? Then in the future if there are multiple layers (e.g. different ones for Wi-Fi coverage and cell tower coverage), they would work without changes

Done via https://github.com/mjaakko/NeoStumbler/compare/e833de128188e3d29cbcf9d0b0a81384b0873c55..ba63cd755621c804af990f292df3d7f8e11598cb

sim6 · 2025-02-07T12:41:02Z

I reworked it, please take another look.

mjaakko · 2025-02-07T14:18:25Z

app/src/main/java/xyz/malkki/neostumbler/ui/screens/MapScreen.kt

@@ -284,6 +300,43 @@ fun MapScreen(mapViewModel: MapViewModel = viewModel()) {
    }
 }

+private fun addCoverageLayer(style: Style, tileJson: String) {
+    getTileJsonLayerIds(tileJson) { layerIds ->
+        Handler(Looper.getMainLooper()).post {


This Handler feels a little bit awkward here. I think it would be better to load the layer IDs with coroutines in the view model and then just add them to the style here

mjaakko · 2025-02-07T14:19:27Z

app/src/main/java/xyz/malkki/neostumbler/ui/screens/settings/SettingsScreen.kt

@@ -50,6 +51,7 @@ fun SettingsScreen() {
            GeosubmitEndpointSettings()
            AutoUploadToggle()
            DbPruneSettings()
+            CoverageLayerSettings()


I feel like this should be placed directly below the endpoint configuration

I done it via https://github.com/mjaakko/NeoStumbler/compare/a6077a257e0d9492838691b6c82814f9a4629907..93d572b5d4ec354df5c1113b07c5300d79f21904

mjaakko · 2025-02-07T14:24:03Z

app/src/main/java/xyz/malkki/neostumbler/utils/tileJson.kt

+fun getTileJsonLayerIds(tileJson: String, callback: (List<String>) -> Unit) {
+    val layerIds = mutableListOf<String>()
+
+    OkHttpClient().newCall(Request.Builder().url(tileJson).build()).enqueue(object : Callback {


An existing OkHttpClient instance should be reused here so that we can benefit from caching and connection pooling. In MapViewModel it's available from httpClient.value.

Also coroutines could be used here

I reused it.

mjaakko · 2025-02-07T14:24:32Z

app/src/main/java/xyz/malkki/neostumbler/utils/tileJson.kt

+
+    OkHttpClient().newCall(Request.Builder().url(tileJson).build()).enqueue(object : Callback {
+        override fun onFailure(call: Call, e: IOException) {
+            Log.e("Network", "Request failed: ${e.message}")


Use Timber for logging

Done via https://github.com/mjaakko/NeoStumbler/compare/93d572b5d4ec354df5c1113b07c5300d79f21904..6a973c91edb0b80ec4b15eb26ec93b293984c403

mjaakko

I left a couple more comments. If it feels like too much work to refactor this to use coroutines, it's ok to keep it as it is - I'm going to do some refactoring anyway so I can fix it then

But if you could fix the other things I commented on, then we're ready to merge this 🙂

sim6 · 2025-02-07T15:53:04Z

I added a URL format check via https://github.com/mjaakko/NeoStumbler/compare/65e800d33861bfed07c79030f3cb79bce8fd7fc7..a6077a257e0d9492838691b6c82814f9a4629907

sim6 · 2025-02-07T18:30:23Z

I reworked it again, please take another look. Thanks for your support!

https://github.com/mjaakko/NeoStumbler/compare/a6077a257e0d9492838691b6c82814f9a4629907..9de8dabc6bd41c207316586ae08e627c01deea49

sim6 · 2025-02-07T19:09:45Z

I refactored names to be more clear via https://github.com/mjaakko/NeoStumbler/compare/9de8dabc6bd41c207316586ae08e627c01deea49..7b45e2bc070043e669ebebe36454752b04714949

Signed-off-by: Simó Albert i Beltran <sim6@probeta.net>

sim6 · 2025-02-07T19:13:50Z

I added a handler for JSON parsing errors via https://github.com/mjaakko/NeoStumbler/compare/7b45e2bc070043e669ebebe36454752b04714949..214cb90f7826c43ab6cd46ae09465e5067354884

sim6 force-pushed the add-coverage-map branch from b4f0f4d to aea9e61 Compare January 30, 2025 22:50

sim6 changed the title ~~Add coverage map~~ Add coverage on map Jan 30, 2025

sim6 mentioned this pull request Jan 30, 2025

Custom map layers #145

Open

sim6 marked this pull request as draft January 31, 2025 09:43

sim6 force-pushed the add-coverage-map branch from aea9e61 to d2db4c5 Compare January 31, 2025 10:12

sim6 marked this pull request as ready for review January 31, 2025 10:13

sim6 force-pushed the add-coverage-map branch from d2db4c5 to e833de1 Compare January 31, 2025 10:19

mjaakko reviewed Jan 31, 2025

View reviewed changes

sim6 force-pushed the add-coverage-map branch from e833de1 to ba63cd7 Compare February 7, 2025 12:39

sim6 requested a review from mjaakko February 7, 2025 12:41

sim6 force-pushed the add-coverage-map branch from ba63cd7 to 65e800d Compare February 7, 2025 13:14

mjaakko reviewed Feb 7, 2025

View reviewed changes

sim6 force-pushed the add-coverage-map branch from 65e800d to a6077a2 Compare February 7, 2025 15:52

sim6 force-pushed the add-coverage-map branch 4 times, most recently from 2e214c0 to 9de8dab Compare February 7, 2025 18:28

sim6 force-pushed the add-coverage-map branch from 9de8dab to 7b45e2b Compare February 7, 2025 19:09

Add coverage on map

214cb90

Signed-off-by: Simó Albert i Beltran <sim6@probeta.net>

sim6 force-pushed the add-coverage-map branch from 7b45e2b to 214cb90 Compare February 7, 2025 19:12

sim6 requested a review from mjaakko February 7, 2025 20:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add coverage on map #470

Add coverage on map #470

sim6 commented Jan 30, 2025 •

edited

Loading

mjaakko left a comment

mjaakko Jan 31, 2025

sim6 Feb 1, 2025

mjaakko Feb 1, 2025

sim6 Feb 7, 2025

mjaakko Jan 31, 2025

sim6 Feb 1, 2025

mjaakko Feb 1, 2025

sim6 Feb 7, 2025

mjaakko Jan 31, 2025

sim6 Feb 1, 2025 •

edited

Loading

mjaakko Feb 1, 2025

sim6 Feb 7, 2025

sim6 commented Feb 7, 2025

mjaakko Feb 7, 2025

sim6 Feb 7, 2025

mjaakko Feb 7, 2025

sim6 Feb 7, 2025

mjaakko Feb 7, 2025

sim6 Feb 7, 2025

mjaakko Feb 7, 2025

sim6 Feb 7, 2025

mjaakko left a comment

sim6 commented Feb 7, 2025

sim6 commented Feb 7, 2025 •

edited

Loading

sim6 commented Feb 7, 2025 •

edited

Loading

sim6 commented Feb 7, 2025

Add coverage on map #470

Are you sure you want to change the base?

Add coverage on map #470

Conversation

sim6 commented Jan 30, 2025 • edited Loading

mjaakko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sim6 Feb 1, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sim6 commented Feb 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjaakko left a comment

Choose a reason for hiding this comment

sim6 commented Feb 7, 2025

sim6 commented Feb 7, 2025 • edited Loading

sim6 commented Feb 7, 2025 • edited Loading

sim6 commented Feb 7, 2025

sim6 commented Jan 30, 2025 •

edited

Loading

sim6 Feb 1, 2025 •

edited

Loading

sim6 commented Feb 7, 2025 •

edited

Loading

sim6 commented Feb 7, 2025 •

edited

Loading