Skip to content

Base Station: Add map marker and radio/tether coverage#2810

Draft
ArturoManzoli wants to merge 2 commits into
bluerobotics:masterfrom
ArturoManzoli:base-station-radio-coverage
Draft

Base Station: Add map marker and radio/tether coverage#2810
ArturoManzoli wants to merge 2 commits into
bluerobotics:masterfrom
ArturoManzoli:base-station-radio-coverage

Conversation

@ArturoManzoli

@ArturoManzoli ArturoManzoli commented Jun 24, 2026

Copy link
Copy Markdown
Contributor
  • Adds a base-station feature with a draggable map marker, a right-click context popup, and a side configuration panel.
  • Configurable comms type (tether or radio link) with antenna type (omni/sector/dish), gain, beamwidth, range, bearing, antenna height, and BlueBoat mast multiplier.
  • Coverage overlay drawn as omni rings or a sector arc, with a draggable bearing handle for radio links and "Aim at vehicle / mission" snap helpers.

First of three sequential PRs splitting #2739 — the follow-ups land on top of this one:

Screenshare.-.2026-05-27.5_59_03.PM.mp4
image image

@github-actions

Copy link
Copy Markdown

Automated PR Review (Claude)

0. Summary

Verdict: MINOR SUGGESTIONS

Items to consider: 1.1, 1.2, 6.1, 6.2, 6.3.

This PR adds a base-station feature to both the Map widget and the Mission Planning view. It introduces a draggable Leaflet marker, a right-click context popup, and a side configuration panel for radio/tether link parameters (antenna type, gain, beamwidth, range, bearing, antenna height, BlueBoat mast multiplier). Coverage overlays are rendered as concentric gradient rings (omni) or sector arcs (directional), with a draggable bearing handle. State is managed through a singleton composable (useBaseStation) backed by useBlueOsStorage, and all Leaflet rendering is isolated in a dedicated useBaseStationOverlay composable. New types and constants live in src/types/baseStation.ts.

1. Correctness & Implementation Bugs

1.1 (major) — BaseStationConfigPanel.vue ~line 612 in the diff: the vehicle-position guard uses truthiness, which fails at the equator or prime meridian (latitude or longitude = 0).

return lat && lng ? [lat, lng] : null

Should be:

return lat != null && lng != null ? [lat, lng] : null

1.2 (minor) — Map.vue at the 'remove-base-station' case (~diff line 1234) calls baseStationStore.remove() directly without the confirmation dialog that the same action uses in BaseStationConfigPanel.vue and BaseStationContextPopup.vue. This inconsistency means a right-click → "Remove base station" in the Map widget context menu will delete with no undo opportunity, while the other two entry points ask the user first.

1.3 (minor) — useBaseStation.ts: the initialize() function starts a navigator.geolocation.watchPosition that is torn down when trackByGps toggles off, but there is no onBeforeUnmount-style cleanup for the composable itself. Because useBaseStation is a singleton (never truly "unmounts"), this is unlikely to leak in practice, but the geoWatchId is module-scoped and survives hot-module-reload cycles in development, potentially stacking watchers. A minor defensive concern.

2. AGENTS.md Adherence

2.1 (minor) — Optional chaining preferred. BaseStationConfigPanel.vue ~diff line 610–612:

const lat = vehicleStore.coordinates?.latitude
const lng = vehicleStore.coordinates?.longitude
return lat && lng ? [lat, lng] : null

The && truthiness guard should be != null (see also 1.1), which is the AGENTS.md-preferred pattern.

2.2 (nit) — Scope discipline. The diff removes ref="map" from the Map.vue template div and ref="planningMap" from MissionPlanningView.vue, though neither removal is needed by the base-station feature (both refs are populated programmatically, making the template attribute a no-op). This is harmless but unrelated to the stated purpose of the PR.

3. Security — ✅

4. Performance

4.1 (minor) — useBaseStationOverlay.ts watches store.config with { deep: true } and calls refreshAll(), which tears down and recreates every coverage layer (up to 12 polygons/circles in the gradient loop) on every single reactive change — including during a bearing-handle drag (high frequency). Consider diffing only the fields that actually require layer recreation (e.g. skip recreation when only bearing changes and instead just update the existing polygon coordinates in place, as is already done for the bearing handle/line/arc).

5. UI / UX — ✅

6. Code Quality & Style

6.1 (minor) — useBaseStationOverlay.ts directly imports L from 'leaflet'. While this is acceptable for a composable acting as the Leaflet abstraction layer, note that the AGENTS.md section on shared-logic architecture says "keep third-party map specifics (leaflet) behind composables/abstractions." The composable meets this requirement for now, but the CSS in Map.vue (global, unscoped block starting ~diff line 1248) leaks .base-station-marker-* classes into the global namespace. If another widget ever mounts the overlay, these will conflict. Consider moving them into the composable as inline styles or a scoped approach.

6.2 (minor) — BaseStationConfigPanel.vue at 810 lines and useBaseStationOverlay.ts at 337 lines are individually fine, but the Map.vue file is already at ~2305 lines (base) and grows by another ~134 lines in this PR. It is approaching the ~2000-line guidance threshold. The reactive menuItems array management (base-station entries spliced via tagged watchers) adds complexity; extracting the right-click menu logic into a composable or child component would improve maintainability.

6.3 (nit) — BaseStationConfigPanel.vue has extensive scoped CSS (~150 lines) for layout classes like .config-row, .config-label, .config-input. Some of these duplicate Tailwind utilities already used elsewhere in the same component (e.g. display: flex, align-items: center, justify-content: space-between on .config-row vs the flex items-center justify-between utilities on other elements). Consider consolidating on Tailwind for consistency.

6.4 (nit) — The BaseStationMenuTags type in Map.vue uses eslint-disable jsdoc/require-jsdoc around two optional boolean fields. Since the type is internal and the field names are self-describing, this is fine, but a single inline /* eslint-disable-next-line */ on the type declaration would be tidier than the enable/disable block.

7. Commit Hygiene — ✅

Two clean, logically separated commits with meaningful messages. Good structure.

8. Tests — ✅

9. Documentation — ✅

10. Nitpicks / Optional

10.1 (nit) — BaseStationContextPopup.vue uses module-scoped let variables (activeInstanceCount, documentClickHandler, documentKeydownHandler) to reference-count mounted instances. This works but is fragile under HMR; consider using a shallowRef or a shared reactive from the store to survive hot reloads cleanly.

10.2 (nit) — The POPUP_HEIGHT constant (110) in BaseStationContextPopup.vue is a magic number that must be manually kept in sync with the rendered height. Consider computing it or documenting the assumption.

10.3 (nit) — The bearingTo function in BaseStationConfigPanel.vue uses a non-null assertion config.value.position! that is safe because canSnapToVehicle / canSnapToMission guards it, but an early return or optional-chain would be more defensive.

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

@ArturoManzoli ArturoManzoli force-pushed the base-station-radio-coverage branch from 510b6d3 to 51832ce Compare June 24, 2026 15:42
@ArturoManzoli

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown

Automated PR Re-review 2 (Claude)

Comparing 510b6d32ed316fe1740e7040748c1797fc915d2851832ce14089f14b99b7993b5bde75123f2a12c1

Note: The incremental diff contains the full PR content (likely due to a rebase/force-push), so this re-check is based on the full pr.diff.

Previous findings status

# Finding Severity Status
1.1 vehiclePosition truthiness guard fails at lat/lng = 0 major ✅ Addressed
1.2 Map.vue remove-base-station case lacks confirmation dialog minor ✅ Addressed
1.3 useBaseStation.ts geoWatchId not cleaned up on HMR minor :large_yellow_circle: Partially addressed — the watch now correctly stops the watcher when enabled flips to false, but the module-scoped geoWatchId still survives HMR re-execution, which can stack watchers in dev. Unlikely to matter in prod.
2.1 Optional chaining / truthiness on vehicle position minor ✅ Addressed (same fix as 1.1)
2.2 Scope discipline — removed template refs nit ⚪ No longer applicable — the current diff no longer removes those template attributes
4.1 Deep watch + full layer recreation on bearing drag minor ✅ Addressed — canUpdateInPlace logic now reuses existing step layers when the antenna type hasn't changed
6.1 Global CSS leaking from Map.vue minor ✅ Addressed — styles moved to baseStationOverlay.css co-located with the composable
6.2 Map.vue growing past ~2000 lines minor ❌ Not addressed — Map.vue gains another ~79 lines of menu-item management in this PR
6.3 Scoped CSS in BaseStationConfigPanel duplicates Tailwind nit ❌ Not addressed
6.4 eslint-disable block style for BaseStationMenuTags nit ❌ Not addressed
10.1 Module-scoped let vars in BaseStationContextPopup fragile under HMR nit ❌ Not addressed
10.2 POPUP_HEIGHT magic number nit ❌ Not addressed
10.3 Non-null assertion on config.value.position! in bearingTo nit ❌ Not addressed

New findings

0. Summary

Verdict: MINOR SUGGESTIONS

Items to consider: 1.4.

This PR adds a base-station feature with a draggable Leaflet marker, right-click context popup, and side configuration panel. Radio/tether coverage overlays are rendered as gradient rings or sector arcs. The major correctness bug from the first review (truthiness guard at lat/lng = 0) and the performance concern (deep-watch full-rebuild during drag) have both been fixed. One new inconsistency was introduced in the MissionPlanningView.

1. Correctness & Implementation Bugs

1.4 (minor) — MissionPlanningView.vue at the @remove-base-station handler (~diff line 2037) calls baseStationStore.remove() directly without the confirmation dialog:

@remove-base-station="baseStationStore.remove()"

Meanwhile, the Map.vue equivalent correctly uses confirmRemoveBaseStation(showDialog, closeDialog). This is the same class of bug originally flagged in 1.2 (which was fixed in Map.vue) but now present in MissionPlanningView.vue instead. The ContextMenu.vue handleRemoveBaseStation emits the event, and the parent should show the confirmation before destroying the user's configuration.

2. AGENTS.md Adherence — ✅

3. Security — ✅

4. Performance — ✅

5. UI / UX — ✅

6. Code Quality & Style — ✅

7. Commit Hygiene — ✅

8. Tests — ✅

9. Documentation — ✅

10. Nitpicks / Optional — ✅

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

@ArturoManzoli ArturoManzoli force-pushed the base-station-radio-coverage branch from 51832ce to 614d7e3 Compare June 24, 2026 18:39
@ArturoManzoli ArturoManzoli force-pushed the base-station-radio-coverage branch from 614d7e3 to bdd1bff Compare June 24, 2026 20:24
Cockpit had no way to anchor the operator's physical base station on
the map, so any downstream feature that needed a station position —
range arcs, offscreen indicators, signal hints — had nothing to hang
off of.

Add the foundation pieces:

- a persistent `useBaseStationStore` holding position and enabled
  state plus reset/remove actions,
- a draggable Leaflet marker rendered through a new
  `useBaseStationOverlay` composable,
- a right-click `BaseStationContextPopup` for status and quick
  actions,
- "Set / Move base station here" entries in the mission-planning
  and Map widget context menus.
A bare position pin can't drive a meaningful coverage visualization —
operators using a tether or a radio link have to express the link's
characteristics (antenna type, gain, beamwidth, range, height, mast
multiplier) before any range overlay can be drawn.

- Add `BaseStationConfigPanel.vue` with tether-length and radio-link
  controls.
- Extend `useBaseStationOverlay` with omni/sector coverage polygons,
  gradient steps, and a draggable bearing handle plus aiming arc.
- Persist the new fields through `useBaseStationStore` and the
  `BaseStationConfig` type.
@ArturoManzoli ArturoManzoli force-pushed the base-station-radio-coverage branch from bdd1bff to 2d863b4 Compare June 29, 2026 14:05
@ArturoManzoli ArturoManzoli marked this pull request as draft July 1, 2026 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant