Repository navigation
fix(coverage): Mapterhorn credit on MapLibre and antimeridian regions - #7759
Conversation
A coverage estimate is a GeoJSON source, which carries no attribution, so the MapLibre map credited Mapterhorn only while hillshade drew. A coverage layer now carries the credit and the main map adds it to the attribution button, deduplicated against the hillshade credit.
A region whose west is east of its east crosses the 180th meridian, and the overlap check rejected ground on both sides of it, so the estimate went to the network for terrain the region holds.
Walkthrough
Priority: Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Tests Prove The Path, Not The End State |  |
Two added tests do not prove the antimeridian change. Against the base implementation (west < other.east && other.west < east), aWrappedRegionMissesGroundBetweenItsEnds is already false for longit… |
Remove the two redundant tests, or make each test include an assertion that fails with the base implementation. Retain the two-sided wrapped-overlap assertion as the regression check, and fold non-overlap and boundary assertions into a test… |
| Regression Coverage For Changed Behavior | |
The antimeridian path has regression coverage, but the Mapterhorn attribution path still has no test. RenderableLayers.kt:80-88 assigns MAPTERHORN_ATTRIBUTION_HTML only to coverage layers. `MapLib… |
Add regression tests in the MapLibre test source, or extract and test a pure attribution-assembly helper. Test that a URI-backed LayerType.COVERAGE produces MAPTERHORN_ATTRIBUTION_HTML and that a non-coverage layer does not. Test that s… |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly summarizes both main changes: Mapterhorn attribution on MapLibre and antimeridian support for coverage regions. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Sibling Call Sites And Presence Semantics PASS. The only changed absent-value field is CustomLayer.attributionHtml: String? = null. AllCustomLayerconstruction sites were checked: coverage layers set Mapterhorn attribution, other non-KML…Moved Code Diffed Against Its Original PASS — The PR does not move a type or function to another file or module. MeshMapOrnamentsbecomesmeshMapOrnamentsin the same file, andCollapsedAttributionButtonbecomesCreditButtonin the…Full details: Sibling Call Sites And Presence Semantics
-
Explanation
PASS. The only changed absent-value field is
CustomLayer.attributionHtml: String? = null. AllCustomLayerconstruction sites were checked: coverage layers set Mapterhorn attribution, other non-KML layers use null, and converted KML layers correctly have no attribution. The only consumer collects it withlayers.mapNotNull { it.attributionHtml }. No sibling call site is unfixed. The diff adds no new field with a default of0for RSSI, temperature, current, voltage, particulate concentration, or SNR.
Full details: Moved Code Diffed Against Its Original
-
Explanation
PASS — The PR does not move a type or function to another file or module.
MeshMapOrnamentsbecomesmeshMapOrnamentsin the same file, andCollapsedAttributionButtonbecomesCreditButtonin the same file. The diff adds intentional credit parameters and preserves the old collapsed call throughstartExpanded = false; no moved declaration loses validation, annotations, or required defaults. The other changes modify existing code or add tests.
-
Full details: Tests Prove The Path, Not The End State
-
Explanation
Two added tests do not prove the antimeridian change. Against the base implementation (
west < other.east && other.west < east),aWrappedRegionMissesGroundBetweenItsEndsis already false for longitudes 177..-178 versus 0..1, andboxesThatOnlyTouchDoNotIntersectis already false because the comparisons are strict. OnlyaWrappedRegionMeetsGroundOnBothSidesOfTheLinefails when the production change is reverted.Resolution
Remove the two redundant tests, or make each test include an assertion that fails with the base implementation. Retain the two-sided wrapped-overlap assertion as the regression check, and fold non-overlap and boundary assertions into a test that also exercises a positive wrapped overlap or replace them with cases that distinguish the antimeridian implementation.
Full details: Regression Coverage For Changed Behavior
-
Explanation
The antimeridian path has regression coverage, but the Mapterhorn attribution path still has no test.
RenderableLayers.kt:80-88assignsMAPTERHORN_ATTRIBUTION_HTMLonly to coverage layers.MapLibreMapViewProvider.kt:150-175forwards visible-layer credits toMeshMap, andMapOrnaments.kt:95-104,150-161merges them with style credits and handles basemaps with no style attribution. This shared Compose UI path runs in both the F-Droid provider and the desktop provider. The only changed test isRegionTerrainTest; the existing tileAttributionTestdoes not exercise this path.Resolution
Add regression tests in the MapLibre test source, or extract and test a pure attribution-assembly helper. Test that a URI-backed
LayerType.COVERAGEproducesMAPTERHORN_ATTRIBUTION_HTMLand that a non-coverage layer does not. Test that style and extra credits are both retained, duplicate Mapterhorn credits occur once, and an extra credit is shown when the style has no attribution. Also test the empty-extra-credits path still uses the normal attribution overlay. These tests must fail against the pre-change implementation.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
-
-
- Source mode: Auto
- Repository: meshtastic/Meshtastic-Android
- Branch: fix/coverage-followups
- File: .coderabbit.yaml
- Commit: a7d3107
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Advanced
- Run ID:
7ff964db-dbb6-4ad9-8b17-1636ffcbcbd8
-
feature/coverage/src/commonMain/kotlin/org/meshtastic/feature/coverage/RegionTerrain.ktfeature/coverage/src/commonTest/kotlin/org/meshtastic/feature/coverage/RegionTerrainTest.ktfeature/map-maplibre/src/commonMain/kotlin/org/meshtastic/feature/map/maplibre/MapLibreMapViewProvider.ktfeature/map-maplibre/src/commonMain/kotlin/org/meshtastic/feature/map/maplibre/MeshMap.ktfeature/map-maplibre/src/commonMain/kotlin/org/meshtastic/feature/map/maplibre/NodeTrackMap.ktfeature/map-maplibre/src/commonMain/kotlin/org/meshtastic/feature/map/maplibre/SecondaryMaps.ktfeature/map-maplibre/src/commonMain/kotlin/org/meshtastic/feature/map/maplibre/component/MapOrnaments.ktfeature/map-maplibre/src/commonMain/kotlin/org/meshtastic/feature/map/maplibre/component/ZoomControls.ktfeature/map-maplibre/src/commonMain/kotlin/org/meshtastic/feature/map/maplibre/layers/CustomLayer.ktfeature/map-maplibre/src/commonMain/kotlin/org/meshtastic/feature/map/maplibre/layers/RenderableLayers.ktfeature/map-maplibre/src/commonMain/kotlin/org/meshtastic/feature/map/maplibre/layers/TerrainLayers.kt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
-
The library button draws nothing for a style with no attribution, as a custom raster basemap may have, which dropped the Mapterhorn credit with it. The main map now uses the same held-state button as the small maps, fed the extra credit.
There was a problem hiding this comment.
Actionable comments posted: 1
-
-
- Source mode: Auto
- Repository: meshtastic/Meshtastic-Android
- Branch: fix/coverage-followups
- File: .coderabbit.yaml
- Commit: 18ffa4a
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Advanced
- Run ID:
cd7c77b7-0ddd-42aa-8e08-ea7d7e41c003
-
feature/map-maplibre/src/commonMain/kotlin/org/meshtastic/feature/map/maplibre/component/MapOrnaments.kt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
-
Two follow-ups from #7758. The MapLibre map didn't credit Mapterhorn under a coverage layer, because a GeoJSON source has no attribution field. And an offline region that crosses the antimeridian never matched the ground it holds.
🐛 Bug Fixes
GeoBounds.intersectssplits a region whose west is east of its east at ±180°, so the estimate reads that region's terrain on both sides of the line.Testing Performed
RegionTerrainTest: a wrapped region meets ground on both sides of the line, misses ground between its ends, and boxes that only touch do not intersect.Summary by CodeRabbit