Geofence: Add mission-planning geofence editor

[ArturoManzoli]

• Add a New Mission / GeoFence switch to the Mission Planning sidebar.
• Draw polygon fences with the same interactive flow as surveys; add circle fences by clicking the center then dragging to set the radius.
• Show an INCLUSION (blue) or EXCLUSION (orange) tag on each fence card; click the tag or the card switch to flip the type.
• Add a breach return point and its altitude.
• Edit autopilot fence parameters from a panel (ArduPilot FENCE_ACTION, FENCE_ALT_*, FENCE_MARGIN, FENCE_AUTOENABLE; PX4 GF_ACTION, GF_MAX_HOR_DIST, GF_MAX_VER_DIST, GF_PREDICT).
• Send the fence to the vehicle, pull the live fence back, or clear it on the vehicle.
• Export/import Cockpit's native .cfp, or the MAVLink-ecosystem .plan (mission + fence) so the same plan opens in other ground stations.
• Check planned waypoints against the active fence before upload; if any waypoint breaches, a dialog asks whether to go back to planning or upload anyway.
• Group the bottom-right mission-planning toolset buttons into a flex row to fit the new controls.

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

Screenshare.-.2026-05-07.11_23_36.AM.mp4

Second of three sequential PRs splitting #2671:

To be merged after #2807

[github-actions]

Automated PR Review (Claude)

0. Summary

Verdict: MINOR SUGGESTIONS

Critical items to address: 6.1

This PR adds a full geofence planning editor to the Mission Planning view: polygon and circle fences (inclusion/exclusion), breach return point, autopilot parameter editing (ArduPilot & PX4), fence upload/download/clear via the MAVLink mission micro-service, .cfp and .plan file import/export, and a mission-vs-fence breach pre-flight check. The implementation is split across well-organized new files (types, conversion library, Pinia store, three Vue components) and integrates cleanly into the existing MissionPlanningView.

1. Correctness & Implementation Bugs

1.1 (minor) — GeoFenceMapLayer.vue: the <style> block at line 1297 is unscoped (no scoped attribute). All rules (.fence-breach-return-icon, .fence-drag-handle, .fence-add-handle, .fence-center-handle, etc.) leak into the global scope. While some of them need to be unscoped because they target Leaflet-injected elements outside the component's template, consider wrapping only those in a separate unscoped <style> block and keeping the rest scoped, or at minimum adding a unique prefix/namespace to avoid collisions in the future.

1.2 (minor) — GeoFenceParametersPanel.vue: the MAVLink listener for PARAM_VALUE is registered on onMounted using the vehicle reference at mount time, but if the vehicle instance changes while the panel is mounted (reconnection, vehicle switch), the listener stays on the old instance and the new one is never subscribed. The companion onBeforeUnmount cleans up the stale reference, but the panel won't receive updates from the new vehicle. A watch on vehicleStore.mainVehicle (similar to the pattern in geoFence.ts at line ~2425) would fix this.

1.3 (minor) — GeoFenceMapLayer.vue: breachReturnMarker is created with draggable: !props.readonly, but the dragend listener is only attached on first creation. If the same breach-return point is removed and re-added, a new marker is created and the dragend handler is re-attached — this is fine. However, if the icon changes (e.g. altitude update changes the tooltip), the existing marker only gets a new tooltip but keeps the original icon div. The icon passed to L.marker(...) in the first creation branch is never updated; calling breachReturnMarker.setIcon(icon) on subsequent renderBreachReturn calls (when the marker already exists) would keep the tooltip + icon in sync.

1.4 (nit) — geofence-conversion.ts:buildFenceItem: the current field is hardcoded to 0 and autocontinue to 1. These are correct for fence items, but a brief inline comment explaining the reasoning would help future maintainers.

2. AGENTS.md Adherence — ✅

3. Security — ✅

4. Performance

4.1 (minor) — geoFence.ts: the FENCE_ENABLE polling interval (setInterval(refreshFenceEnabled, 3000)) fires every 3 seconds indefinitely while a fence is loaded and the tab is visible. This generates continuous MAVLink traffic. Consider a longer default (e.g. 5–10 s) or switching to a visibility-gated exponential backoff, since the parameter rarely changes outside of the takeoff moment.

4.2 (nit) — GeoFenceMapLayer.vue watcher (line ~1273): the deep watch serializes every polygon vertex into a string on every tick to detect changes. With many vertices this could become expensive. Since the store already calls markDirty() on every mutation, the watcher could instead key off fenceStore.dirty or a version counter to avoid the serialization cost.

5. UI / UX — ✅

6. Code Quality & Style

6.1 (major) — File size: MissionPlanningView.vue grows from ~4,838 lines to ~5,465 lines with this PR — almost 3× the ~2,000-line guideline. The fence drawing infrastructure added here (~280 lines of addFencePolygonPoint, createFenceVertexMarker, updateFencePolygonLayer, clearFencePolygonDrawingArtifacts, setPendingFenceCircleCenter, updatePendingFenceCircleLayer, clearPendingFenceCircleArtifacts, etc.) closely mirrors the existing survey-polygon drawing code. Per AGENTS.md ("shared-logic architecture"), this duplicated drawing logic is a prime candidate for extraction into a composable (e.g. usePolygonDrawing) that both the survey and fence flows can consume. Even without a full refactor now, moving the ~280 lines of fence-specific Leaflet drawing helpers into a useFenceDrawing composable would reduce the view's line count and improve testability.

6.2 (minor) — GeoFenceEditor.vue (line 398): the eslint-disable-next-line jsdoc/require-jsdoc for props is acceptable but GeoFenceMapLayer.vue (line 738) also has one. Both components define props with inline /** */ doc comments on individual props, which is good — but the disable comments are inconsistent with how other components in the codebase handle it. Consider using the @inheritdoc or interface-extract pattern seen elsewhere to avoid the disable entirely.

6.3 (nit) — geoFence.ts:clonePlan uses JSON.parse(JSON.stringify(plan)). This is fine for data-only objects but is slower than structuredClone (available in all targeted browsers). Not urgent, just a modernization opportunity.

6.4 (nit) — Several arrow functions in the store (addPolygon, addCircle, updatePolygon, updateCircle, deletePolygon, deleteCircle, togglePolygonInclusion, toggleCircleInclusion, setBreachReturn, clearAll, setInteractive, loadFromPlan, exportPlan) lack JSDoc. While AGENTS.md says "avoid JSDocs on private helpers, consts, and types, when the name + signature are self-describing", these are all exported from the store's return object and drive the public API. Adding at least one-liner JSDocs to the mutation helpers (addPolygon, addCircle, deletePolygon, deleteCircle, clearAll) would improve discoverability.

7. Commit Hygiene

7.1 (nit) — Several commits carry a [drop] suffix in their subject line (e.g. types: add geofence plan and plan-file envelope types [drop]). The PR body explains these are "borrowed" from an earlier PR (#2807) and presumably will be dropped during rebase. This is fine for a stacked-PR workflow, but worth confirming the [drop] commits will indeed be removed before merge so git history stays clean.

8. Tests — ✅

9. Documentation — ✅

10. Nitpicks / Optional

10.1 (nit) — GeoFenceEditor.vue:591: the exported .plan always writes mission.items: [] and mission.plannedHomePosition from mapCenter. If the user already has mission waypoints loaded, these are silently discarded in the export. Consider merging the existing missionStore.currentPlanningWaypoints into the mission section, or at least adding a tooltip/warning that the .plan export includes only the fence (not the current mission).

10.2 (nit) — MissionPlanningView.vue: the mapCenter computed/ref used by GeoFenceEditor isn't visible in the diff excerpt. If it's derived from planningMap.value.getCenter(), be aware that this can be stale if the user hasn't panned. A computed that reads planningMap.value?.getCenter() on demand (or a moveend listener) would ensure freshness.

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

[ArturoManzoli]

/review

[github-actions]

Automated PR Re-review 2 (Claude)

Comparing 2dadf8bdd231e41d30c92a6bac259f6dad357031 → a1f52a9e22e8b33d2ffe63154e5759c232aea2db

Previous findings status

#	Finding	Severity	Status
1.1	GeoFenceMapLayer.vue: unscoped <style> block	minor	✅ Addressed
1.2	GeoFenceParametersPanel.vue: PARAM_VALUE listener not re-bound on vehicle change	minor	✅ Addressed
1.3	GeoFenceMapLayer.vue: breach-return marker icon not refreshed on re-render	minor	✅ Addressed
1.4	geofence-conversion.ts: current/autocontinue hardcoded without comment	nit	❌ Not addressed
4.1	geoFence.ts: FENCE_ENABLE polling at 3 s without visibility gating	minor	✅ Addressed
4.2	GeoFenceMapLayer.vue: deep watch serializes vertices to strings on every tick	nit	❌ Not addressed
6.1	MissionPlanningView.vue file size — fence drawing logic should be extracted	major	✅ Addressed
6.2	eslint-disable-next-line jsdoc/require-jsdoc on props in two components	minor	❌ Not addressed
6.3	geoFence.ts: JSON.parse(JSON.stringify(plan)) vs structuredClone	nit	❌ Not addressed
6.4	Store mutation helpers lack JSDoc	nit	✅ Addressed
7.1	[drop] commits should be removed before merge	nit	:large_yellow_circle: Partially addressed
10.1	.plan export writes empty mission.items discarding loaded waypoints	nit	❌ Not addressed
10.2	mapCenter freshness concern	nit	❌ Not addressed

Notes on key resolutions:

• 1.1: The unscoped <style> block now has a clear HTML comment explaining it must stay global because Leaflet injects its DOM outside Vue's render tree, and all rules use the fence- namespace prefix. This is the correct pattern.
• 1.2: GeoFenceParametersPanel.vue now tracks attachedVehicle and has a watch on vehicleStore.mainVehicle that detaches the old listener and re-attaches to the new vehicle — exactly the pattern I suggested.
• 1.3: renderBreachReturn now calls breachReturnMarker.setIcon(icon) on the existing-marker branch.
• 4.1: Polling interval raised to 10 s (FENCE_ENABLE_POLL_INTERVAL_MS = 10000), gated by document visibility (useDocumentVisibility), and auto-cleared when the vehicle disconnects or the plan is emptied.
• 6.1: Fence drawing logic has been extracted into src/composables/map/useFenceDrawing.ts (~400 lines), which mirrors the survey-polygon UX but in the orange fence palette. MissionPlanningView.vue now delegates through the composable API. This is a substantial improvement.
• 6.4: Most store actions (drawing lifecycle, CRUD, upload/download, breach detection) now have proper JSDocs with @param/@returns.
• 7.1: The [drop] commits still appear in the commit history (confirmed via gh pr view). The PR description notes these are "borrowed" from Geofence: Add vehicle sync and plan types #2807 and will be dropped during rebase, so this is expected for the stacked-PR workflow but must be cleaned up before merge.

New findings

0. Summary

Verdict: MINOR SUGGESTIONS

This re-review covers the delta from the first review. The three substantive findings (1.2 vehicle-listener stale binding, 4.1 aggressive polling, 6.1 file-size / extraction) have all been properly addressed. The new useFenceDrawing composable is well-structured with thorough JSDocs, the GeoFenceParametersPanel correctly re-binds on vehicle changes, and polling is now visibility-gated at a sensible interval. The remaining open items are nits. One new minor finding below (1.5).

1. Correctness & Implementation Bugs

1.5 (minor) — geoFence.ts: the clearFence method on MAVLinkVehicle reads from the shared _messages cache for MISSION_ACK (line ~2447 of vehicle.ts). This cache is also written by uploadMission/fetchMission/uploadFence/fetchFence. If the user triggers a fence clear while another mission micro-service operation has just landed an ACK, clearFence could consume the wrong ACK (its only guard is ack.epoch < initTime). This is the same concurrency hazard documented in the _fetchMissionItems JSDoc ("concurrent downloads across mission types would consume each other's replies, so callers must serialize fetches"). clearFence should mention this constraint as well, and ideally the store-level callers (uploadToVehicle, downloadFromVehicle, clearOnVehicle) should serialize against each other — today they are individually guarded by syncInProgress but uploadMission is not aware of syncInProgress.

2. AGENTS.md Adherence — ✅

3. Security — ✅

4. Performance — ✅

5. UI / UX — ✅

6. Code Quality & Style

6.5 (nit) — useFenceDrawing.ts:1769: the non-null assertion el.querySelector('.measure-area-pill')!.textContent = label will throw if the marker's inner HTML structure ever changes. A guarded assignment (const pill = el.querySelector('.measure-area-pill'); if (pill) pill.textContent = label) is safer and consistent with the surrounding null checks.

7. Commit Hygiene — ✅

8. Tests — ✅

9. Documentation — ✅

10. Nitpicks / Optional — ✅

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