Base Station: Add mobile-data coverage overlays

[ArturoManzoli]

• Adds mobile-data coverage overlays via OpenCellID and OSM Overpass, with provider/operator filters, per-bbox caching, a drag-to-fetch target tool, and rate-limited request queueing.
• Two display modes for mobile coverage: heatmap (density-weighted) and coverage rings (per-tower range).
• Bridges OpenCellID through the standalone Electron build, since browser CORS blocks the direct call from the Lite build.

Diff excluding the borrowed [drop] commits: $\color{green}\pmb{+2{,}380}$ $\color{red}\pmb{-41}$

Second of three sequential PRs splitting #2739 — stacked on #2810; merging it collapses the borrowed [drop] commits at the bottom of this branch:

• base-station-radio-coverage (Base Station: Add map marker and radio/tether coverage #2810) - base-station map marker, context popup, side config panel, and the radio/omni coverage overlay.
• base-station-path-signal (Base Station: Add offscreen indicators and mission-path signal strength coloring #2812) - offscreen edge arrow and CompassHUD bearing tick, mission/survey path coloring by expected comms quality, and the on-map signal toggles.

[ArturoManzoli]

/review

[github-actions]

Automated PR Review (Claude)

0. Summary

Verdict: MINOR SUGGESTIONS

Critical items to address: 1.1

This PR adds mobile-data coverage overlays to the Cockpit base-station feature, integrating OpenCellID and OSM Overpass data sources. It includes a config panel, context popup, per-bbox caching, a drag-to-fetch target tool, rate-limited request queueing, two display modes (heatmap and coverage rings), and an Electron IPC bridge to bypass browser CORS for OpenCellID. Approximately 4,200 lines added across 14 files, with new composables (useBaseStation, useBaseStationOverlay), Vue components, types, and an Electron service.

---

1. Correctness & Implementation Bugs

1.1 (major) — Electron OpenCellID bridge always requires an API key, contradicting the "key-free" claim.
src/electron/services/openCellId.ts:3960: fetchOpenCellIdCellsQueued throws 'OpenCellID: missing API key.' when bbox.apiKey is falsy. However, the renderer-side flow in useBaseStationOverlay.ts (around line 3581 of the diff) deliberately falls through the if (!apiKey) guard when isElectron() is true, passing an empty key to the Electron bridge — which will always throw. The README addition claims "Works key-free through the built-in bridge" but there is no anonymous/key-free endpoint implemented in the Electron service. Either the Electron service needs an anonymous path (e.g. using the OpenCellID ajax endpoint) or the no-key code path should be blocked on Electron too and the README updated.

1.2 (minor) — @remove-base-station in MissionPlanningView bypasses the confirmation dialog.
src/views/MissionPlanningView.vue line 4445 of the diff: the handler is baseStationStore.remove() — this skips the confirmRemoveBaseStation dialog that the Map widget and the context popup use. The ContextMenu properly emits the event, but the parent connects it directly to the destructive action. Should use confirmRemoveBaseStation(showDialog, closeDialog) for consistency.

1.3 (minor) — mobileCoverageDebounce timeout is not cleared in onBeforeUnmount.
src/composables/baseStation/useBaseStationOverlay.ts: The onBeforeUnmount calls teardownMobileCoverageData() which does clear the debounce, so this is actually covered. However, the pattern where mobileCoverageDebounce is set to setTimeout(...) without checking/clearing the previous timer on each call (lines 3716, 3725 of the diff) means consecutive rapid changes will stack timers instead of debouncing. Each setTimeout call should clearTimeout(mobileCoverageDebounce) first (the watch at line 3714 calls teardownMobileCoverageData() which clears it, but the reload watcher at line 3722 does not clear the previous timer before setting a new one).

1.4 (nit) — showSignalOnMap field referenced but not defined in this PR's BaseStationConfig type.
src/composables/baseStation/useBaseStation.ts references config.value.showSignalOnMap, which is not in src/types/baseStation.ts. This is presumably defined in the parent [drop] commits from #2810 and will resolve when the stack merges, but it will cause a TypeScript error if this PR is reviewed or built in isolation.

---

2. AGENTS.md Adherence

2.1 (minor) — Missing JSDoc on several exported functions.
AGENTS.md rule: "Always create docs for the @returns, unless the function has no specified return value." Multiple exported functions in src/types/baseStation.ts are covered, but several module-scope functions in useBaseStationOverlay.ts (e.g. sectorPolygonLatLngs, bearingHandlePosition, aimingArcLatLngs, bearingFromCenter, etc.) are top-level non-arrow function-declarations that lack JSDoc. The eslint-disable jsdoc/require-jsdoc blocks are used extensively to suppress this — which is reasonable for inline DTOs but covers many functions that are not simple DTOs (e.g. fetchOverpassTowers, fetchOpenCellIdTile, mapWithConcurrency). Consider narrowing the disable ranges.

2.2 (nit) — useBaseStation initialize() function suppresses both jsdoc/require-jsdoc and @typescript-eslint/explicit-function-return-type on a single eslint-disable line. The comment says "type inferred for the reactive() output" which is fair, but per AGENTS.md the final implementation should contain zero lint warnings — confirm these suppressions don't mask issues.

---

3. Security

3.1 (minor) — API key transmitted in URL query string (OpenCellID).
useBaseStationOverlay.ts (line 2218 of diff): the API key is placed directly in the URL query string for fetch. This is standard for OpenCellID's API contract and not avoidable, but the key is also persisted in useBlueOsStorage (cockpit-base-station-config), meaning it sits in local storage in plain text. This is acceptable for a user-owned key, but worth noting.

3.2 (minor) — Outbound network calls to two new external services.
The PR adds fetch calls to opencellid.org and overpass-api.de. Both are well-known, legitimate open-data APIs used for their documented purpose (cell tower lookups). No exfiltration concern.

---

4. Performance

4.1 (minor) — Full-canvas getImageData / putImageData on every map pan/zoom.
useBaseStationOverlay.ts CellIdHeatLayer._redraw: On every moveend/resize/viewreset/zoomend, the heatmap reads and writes the entire canvas pixel buffer (getImageData + per-pixel loop + putImageData). At 1920×1080, that's ~8 M pixel operations. This is a known trade-off for a custom heatmap without WebGL, and the early continue for zero-alpha pixels helps, but on large screens or many cells it could cause a visible hitch. Consider debouncing or using requestAnimationFrame for the redraw.

4.2 (nit) — watch(() => store.config, refreshAll, { deep: true }) re-renders on any config property change.
useBaseStationOverlay.ts line 3702 of diff: a deep watch on the entire config object will fire refreshAll on every keystroke in the name field, every slider drag tick, etc. Since refreshAll touches Leaflet layers, this may cause jank during rapid input. Consider watching only the properties that affect the overlay geometry.

---

5. UI / UX

5.1 (nit) — Config panel z-index of 600 may conflict with other overlays.
BaseStationConfigPanel.vue sets zIndex: 600 in getMarginsFromBarsHeight. Verify this stacks correctly with the mission planning side panel, video overlays, and other Cockpit panels.

---

6. Code Quality & Style

6.1 (minor) — useBaseStationOverlay.ts is 1,803 lines.
While under the ~2,000-line guideline threshold, this file is dense. The mobile coverage fetch logic, the heatmap renderer, the SVG label overlay, and the marker/bearing management are all in one composable. Consider extracting the CellIdHeatLayer custom Leaflet layer and the fetch/cache logic into separate .ts modules under src/composables/baseStation/ or src/libs/.

6.2 (nit) — Duplicate site-deduplication logic.
The key-generation pattern ${site.lat.toFixed(6)}:${site.lon.toFixed(6)}:${Math.round(site.rangeMeters)} appears in fetchOpenCellIdSites (line 2295) and again in mergeOpenCellIdSites (line 2909). Extract a shared openCellIdSiteKey(site) helper.

6.3 (nit) — Multiple eslint-disable blocks for jsdoc/require-jsdoc.
There are 6 pairs of eslint-disable/eslint-enable for jsdoc/require-jsdoc in useBaseStationOverlay.ts and baseStation.ts. Some of these cover non-trivial functions, not just DTOs. Narrowing these blocks or adding minimal JSDoc would improve maintainability.

---

7. Commit Hygiene

7.1 (minor) — [drop] commits included in the PR diff.
The first two commits (base-station: add draggable map marker and context menus [drop], base-station: add config panel for tether and radio links [drop]) are borrowed from the parent PR #2810. The PR description explains this is a stacked PR pattern, but these commits inflate the diff (+1,855 lines that belong to #2810). After #2810 merges, these will collapse — this is acceptable for the stacking workflow but reviewers should focus on commits 3–5 for this PR's actual delta.

---

8. Tests — ✅

9. Documentation

9.1 (nit) — README claim may be inaccurate.
README.md states the standalone build "Works key-free through the built-in bridge" but the Electron service currently requires a key (see finding 1.1). The README entry should match the actual behavior.

---

10. Nitpicks / Optional

10.1 (nit) — BaseStationConfigPanel.vue uses raw <input> and <select> elements instead of Vuetify form components. The rest of Cockpit's config panels use Vuetify's v-text-field, v-select, etc. The raw elements work but break Vuetify theming consistency. This is likely a deliberate choice for compactness, but worth noting.

10.2 (nit) — @keyframes reload-spin in scoped CSS could use Vuetify's built-in mdi-loading spin. The mdi-loading icon class already rotates by default — the custom @keyframes may be redundant.

10.3 (nit) — Module-scope mutable state in BaseStationContextPopup.vue. activeInstanceCount, documentClickHandler, and documentKeydownHandler are module-scope let variables. This ref-counting pattern works but is fragile with HMR; a shallowRef or a Map keyed by component UID would be more robust.

Generated by Claude. This is advisory; a human reviewer must still approve.

[ArturoManzoli]

/review

[github-actions]

Automated PR Re-review 2 (Claude)

Comparing 20c34410f62320be353bcb3d10e34a5223f9260e → f9d2395f03cee05e27fc8d5a3b3b5fcec0860feb

Note: The incremental diff (4,476 lines) is nearly identical in size to the full PR diff (4,525 lines), suggesting a rebase or force-push rewrote the branch history. Re-check is based on the full pr.diff.

Previous findings status

#	Finding	Severity	Status
1.1	Electron OpenCellID bridge always requires an API key, contradicting the "key-free" claim	major	✅ Addressed
1.2	@remove-base-station in MissionPlanningView bypasses the confirmation dialog	minor	✅ Addressed
1.3	mobileCoverageDebounce timeout not cleared before setting a new one in the reload watcher	minor	✅ Addressed
1.4	showSignalOnMap field referenced but not defined in this PR's BaseStationConfig type	nit	⚪ No longer applicable
2.1	Missing JSDoc on several exported functions, wide eslint-disable ranges	minor	:large_yellow_circle: Partially addressed
2.2	useBaseStation initialize() suppresses both jsdoc/require-jsdoc and @typescript-eslint/explicit-function-return-type	nit	❌ Not addressed
3.1	API key transmitted in URL query string (OpenCellID)	minor	⚪ No longer applicable
3.2	Outbound network calls to two new external services	minor	⚪ No longer applicable
4.1	Full-canvas getImageData/putImageData on every map pan/zoom	minor	:large_yellow_circle: Partially addressed
4.2	watch(() => store.config, refreshAll, { deep: true }) fires on any config property change	nit	❌ Not addressed
5.1	Config panel z-index of 600 may conflict with other overlays	nit	❌ Not addressed
6.1	useBaseStationOverlay.ts is 1,803 lines	minor	❌ Not addressed
6.2	Duplicate site-deduplication key logic	nit	❌ Not addressed
6.3	Multiple eslint-disable blocks for jsdoc/require-jsdoc	nit	❌ Not addressed
7.1	[drop] commits included in the PR diff	minor	⚪ No longer applicable
9.1	README claim may be inaccurate (key-free)	nit	✅ Addressed
10.1	Raw <input> and <select> instead of Vuetify form components	nit	❌ Not addressed
10.2	@keyframes reload-spin possibly redundant with mdi-loading	nit	❌ Not addressed
10.3	Module-scope mutable state in BaseStationContextPopup.vue	nit	❌ Not addressed

Summary of status changes:

• 1.1 (major): The README now correctly states "still needs a personal OpenCellID API key" and directs users to OpenStreetMap for key-free coverage. The Electron service enforces the key requirement consistently.
• 1.2: MissionPlanningView.vue now uses confirmRemoveBaseStation(showDialog, closeDialog) at line 4416 of the diff.
• 1.3: Both the fetch watcher (line 3706) and the reload watcher (line 3715) call teardownMobileCoverageData() first, which clears the debounce timer before setting a new one.
• 1.4: No reference to showSignalOnMap remains in this PR's diff; presumably resolved when the stacked PR base was rebased.
• 4.1: A _scheduleRedraw method was added that coalesces redraws into a single requestAnimationFrame, preventing multiple pixel sweeps per frame. The per-pixel loop itself remains, but the worst case is now one pass per animation frame rather than per event.

New findings

0. Summary

Verdict: MINOR SUGGESTIONS

This PR adds mobile-data coverage overlays (OpenCellID and OSM Overpass) to the base-station feature. The major finding from the first review (1.1) has been addressed — the README now accurately describes the API-key requirement. Remaining items are code quality and style nits that do not block merging.

---

1. Correctness & Implementation Bugs

1.5 (minor) — fetchMobileCoverageData does not abort the previous mobileCoverageController before creating a new one.
useBaseStationOverlay.ts line 3573 of the diff: a new AbortController is assigned to mobileCoverageController without first calling .abort() on the previous one. If two fetches overlap (e.g. position changes rapidly), the first request keeps running in the background until it finishes or times out, and its results are silently discarded because mobileCoverageController no longer matches. This is mostly harmless but wastes bandwidth/API quota. By contrast, fetchAndAppendMobileCoverage (line 3488) correctly aborts the previous controller. The same pattern should be applied here.

1.6 (minor) — Electron fetchOpenCellIdTileInElectron ignores the signal / is un-cancellable.
useBaseStationOverlay.ts line 2218: The Electron IPC bridge (fetchNearbyOpenCellIdCells) does not accept an AbortSignal, so in-flight requests cannot be cancelled. The code comment at line 2227 of cosmos.ts even notes this limitation. However, since fetchOpenCellIdSites (line 2260) passes the Electron fetcher through mapWithConcurrency, multiple tiles may be in flight concurrently when the user navigates away — they will all complete even after the component unmounts. Consider at minimum checking signal.aborted between tile fetches inside mapWithConcurrency.

---

2. AGENTS.md Adherence — ✅

---

3. Security — ✅

---

4. Performance — ✅

---

5. UI / UX — ✅

---

6. Code Quality & Style

6.4 (minor) — useBaseStationOverlay.ts is 1,825 lines in the current diff.
This file has grown past the ~1,803 lines noted in the first review. The CellIdHeatLayer custom Leaflet layer class (~110 lines), the fetch/cache logic (~200 lines for OpenCellID, ~60 for Overpass), and the SVG label overlay renderer (~70 lines) are all self-contained and could be extracted into separate modules under src/composables/baseStation/ or src/libs/ without changing any API surface. This would ease future maintenance, especially since this PR is one of three sequential feature branches.

---

7. Commit Hygiene — ✅

8. Tests — ✅

9. Documentation — ✅

10. Nitpicks / Optional — ✅

Generated by Claude. This is advisory; a human reviewer must still approve.