Skip to content

Geofence: Add vehicle sync and plan types#2807

Draft
ArturoManzoli wants to merge 6 commits into
bluerobotics:masterfrom
ArturoManzoli:geofence-vehicle-sync
Draft

Geofence: Add vehicle sync and plan types#2807
ArturoManzoli wants to merge 6 commits into
bluerobotics:masterfrom
ArturoManzoli:geofence-vehicle-sync

Conversation

@ArturoManzoli

Copy link
Copy Markdown
Contributor
  • Add geofence plan and plan-file types (inclusion/exclusion polygons, circle fences, breach return point) plus the Cockpit .cfp and MAVLink .plan envelopes and their type guards.
  • Add MAVLink geofence conversion helpers between Cockpit's GeoFencePlan and the MissionItemInt array format.
  • Track the autopilot capability bitmask from AUTOPILOT_VERSION on the MAVLink vehicle.
  • Add generic parameter-request helpers to the MAVLink vehicle.
  • Add fence upload, fetch and clear over the geofence mission micro-service, including the ArduPilot polygon fence-type bit handling.
  • Expose the autopilot capabilities and the fence helpers (uploadFence, fetchFence, clearFence, requestParameter) on the main-vehicle store.
  • Add the geoFence Pinia store: editor draft state, persistence under cockpit- keys, vehicle sync, visibility-gated FENCE_ENABLE enforcement polling, and mission-vs-fence breach detection.

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

  • geofence-planning-editor — Mission/GeoFence sidebar switch, polygon and circle drawing, inclusion/exclusion tagging, breach return point, parameter panel, vehicle send/fetch/clear, .cfp/.plan import-export, and the pre-upload breach check.
  • geofence-map-overlay — live fence overlay and enforcement toggle on the Map widget.

Adds the runtime types backing the new geofencing flow: FenceLatLng,
FencePolygon (with inclusion flag and vertex list), FenceCircle,
BreachReturnPoint, the Cockpit-level GeoFencePlan, the on-disk
CockpitFencePlanFile (.cfp) envelope, and the MavlinkPlanFile envelope
that lets fence (and mission) plans round-trip with other ground stations
through the de-facto MAVLink-ecosystem .plan JSON format. Type guards
(instanceOfGeoFencePlan, instanceOfCockpitFencePlanFile,
instanceOfMavlinkPlanFile) only inspect the outer discriminator + version
so deeper validation stays at the call site that knows the schema.

Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
On vehicle bring-up, send MAV_CMD_REQUEST_AUTOPILOT_CAPABILITIES and
cache the bitmask received via the AUTOPILOT_VERSION message. Exposes
`capabilities()` / `hasCapability(bit)` accessors, and a new
`onCapabilities` signal so consumers can react when the bitmask first
arrives. Failures are logged at warn level and gracefully fall back to
"optimistic" capability reporting (consumers default to enabling
features when no AUTOPILOT_VERSION has been seen yet).

Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
@github-actions

Copy link
Copy Markdown

Automated PR Review (Claude)

0. Summary

Verdict: MINOR SUGGESTIONS

Minor items to consider: 1.1, 1.2, 1.3, 4.1, 6.1, 6.2

This PR adds the foundational types, MAVLink conversion helpers, vehicle-level fence upload/fetch/clear, autopilot capability tracking, parameter request helpers, and a comprehensive geoFence Pinia store (editor draft state, persistence, vehicle sync, FENCE_ENABLE polling, breach detection). It is the first of three PRs splitting #2671. The code is generally well-structured, well-documented, and follows existing patterns.

1. Correctness & Implementation Bugs

1.1 minorclearFence is fire-and-forget without ACK confirmation.
src/libs/vehicle/mavlink/vehicle.ts (new clearFence method): The method sends MISSION_CLEAR_ALL but does not wait for a MISSION_ACK from the vehicle. Both uploadFence and fetchFence go through the full micro-service handshake, but clearFence is asymmetric. If the vehicle NAKs or the message is lost, the caller has no way to know. Consider awaiting the ACK the same way the upload/download flows do, or at minimum documenting the limitation.

1.2 minor_fetchMissionItems shares the _messages cache across concurrent calls.
src/libs/vehicle/mavlink/vehicle.ts: The refactored _fetchMissionItems reads from this._messages.get(MAVLinkType.MISSION_COUNT) and this._messages.get(MAVLinkType.MISSION_ITEM_INT), which are shared across all mission types. If a regular-mission download and a fence download were ever initiated concurrently (or near-concurrently), MISSION_COUNT and MISSION_ITEM_INT replies from one could be consumed by the other. In practice this is unlikely today since callers are sequential, but worth noting as a design constraint; a comment would help future maintainers.

1.3 minorrequestParameterValue listener is never cleaned up on scope dispose.
src/libs/vehicle/mavlink/vehicle.ts (new method around line 918 in the diff): The slot callback is added to this.onParameter and is removed either on match or on timeout, which is correct for the happy path. However, if the Promise is never awaited (e.g. the caller is GC'd) and neither the match nor the timeout fires (extremely unlikely but theoretically possible with weird timer edge cases), the slot would leak. This is low-risk but worth a defensive comment.

2. AGENTS.md Adherence — ✅

3. Security — ✅

4. Performance

4.1 minorFENCE_ENABLE polling with setInterval at 3 s.
src/stores/geoFence.ts (~line 340 in the diff): Polling FENCE_ENABLE every 3 seconds generates continuous MAVLink traffic. The implementation correctly gates on isArduPilot, lastUploadedPlan, isVehicleOnline, and document visibility, which is good. However, once the fence is uploaded this runs indefinitely. Consider increasing the interval (e.g. 5–10 s) since the enforcement state rarely changes mid-flight, or pausing when the mission-planning view is not active.

5. UI / UX — ✅

6. Code Quality & Style

6.1 nit — Redundant console.warn paired with capability-request failure.
src/libs/vehicle/mavlink/vehicle.ts (diff lines 263–264): Two consecutive console.warn calls — one for the message and one for the error object. Per AGENTS.md, snackbar/log calls shouldn't be duplicated. While this isn't an openSnackbar case, the double-warn could be collapsed into a single console.warn('Failed to request autopilot capabilities. Capability gating will fall back to optimistic mode.', error).

6.2 nitsetFenceEnabled and setFenceAutoEnable are declared async but contain no await.
src/stores/geoFence.ts: Both functions are marked async and return Promise<void>, but their bodies are synchronous (v.setParameter(...) is void, not a promise). The async keyword is technically harmless but misleading — callers may think these await a vehicle ACK. Either remove async (returning void) or add a comment explaining the intent (e.g. future ACK waiting).

7. Commit Hygiene — ✅

8. Tests — ✅

9. Documentation — ✅

10. Nitpicks / Optional

10.1 nitEARTH_RADIUS_M is defined locally in geoFence.ts but @turf/turf (already imported) provides turf.earthRadius which equals the same value. Using turf's constant would avoid the local redefinition.

10.2 nithaversine-distance is listed in package.json dependencies and is used elsewhere in the codebase for distance calculations. The new distanceMeters helper in the geo-fence store uses turf.distance instead, which is fine (turf is also already a dependency), but it's worth noting the inconsistency for future consolidation.

10.3 nit — The re-export of CockpitMission at the bottom of src/types/geofence.ts (export type { CockpitMission }) is unused in this PR and may introduce a confusing dependency direction (geofence types re-exporting mission types). Consider removing it until actually needed.

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

@ArturoManzoli ArturoManzoli changed the title Mission Planning: Add geofence vehicle sync and plan types Add geofence vehicle sync and plan types Jun 23, 2026
Adds two helpers that turn the existing PARAM_REQUEST_READ / PARAM_VALUE
round-trip into something callers can actually \`await\`:

- \`requestParameter(name)\`: fires a single PARAM_REQUEST_READ for the
  given parameter id; the reply still bubbles up through \`onParameter\`
  for code that wants to keep listening passively.
- \`requestParameterValue(name, timeoutMs)\`: subscribes a one-shot listener
  that resolves with the value as soon as the matching PARAM_VALUE arrives
  (or undefined on timeout). Both the listener and the timeout handle are
  cleared as soon as the promise settles to avoid stray callbacks.

Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
@ArturoManzoli ArturoManzoli force-pushed the geofence-vehicle-sync branch from fba258a to 8ff9d6b Compare June 24, 2026 11:24
@ArturoManzoli

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown

Automated PR Re-review 2 (Claude)

Comparing fba258a4a9d8a4f66bcea4e5b89b8d63cf9534b18ff9d6b19f54de8d38498cb8bb9e9b282a3575ee

Previous findings status

# Finding Severity Status
1.1 clearFence fire-and-forget without ACK minor ✅ Addressed — clearFence now polls for MISSION_ACK with a timeout loop, throwing on NAK or timeout.
1.2 _fetchMissionItems shares _messages cache across concurrent calls minor ✅ Addressed — the JSDoc on _fetchMissionItems now documents the serialization constraint explicitly.
1.3 requestParameterValue listener cleanup edge case minor ✅ Addressed — a comment now explains the slot lifetime is bounded by the timeout.
4.1 FENCE_ENABLE polling at 3 s minor ✅ Addressed — interval increased to 10 s (FENCE_ENABLE_POLL_INTERVAL_MS = 10000), with an updated comment explaining the lazy cadence.
6.1 Redundant console.warn pair in capability-request catch nit ❌ Not addressed — two consecutive console.warn calls remain in the requestAutopilotCapabilities().catch() handler (src/libs/vehicle/mavlink/vehicle.ts).
6.2 setFenceEnabled/setFenceAutoEnable declared async but body is sync nit ❌ Not addressed — both functions in src/stores/geoFence.ts are still marked async returning Promise<void> without any await in their body.
10.1 Local EARTH_RADIUS_M instead of turf's constant nit ⚪ No longer applicable — the constant is used only by offsetCoordinates which does manual arithmetic, not turf calls, so a local constant is reasonable.
10.2 turf.distance vs haversine-distance inconsistency nit ⚪ No longer applicable — informational note; no action required.
10.3 Unused CockpitMission re-export in geofence types nit ❌ Not addressed — export type { CockpitMission } still present at the bottom of src/types/geofence.ts.

New findings

0. Summary

Verdict: MINOR SUGGESTIONS

Minor items to consider: 6.1, 6.2 (carried over nits)

This re-review covers the incremental changes since the first review. The author addressed all four substantive findings (1.1–1.3, 4.1) — clearFence now properly awaits an ACK, the shared-cache constraint is documented, the listener lifetime is commented, and the polling interval was relaxed to 10 s. Two nits (redundant console.warn, misleading async) and a minor type-hygiene point remain, but nothing blocking.

1. Correctness & Implementation Bugs — ✅

2. AGENTS.md Adherence — ✅

3. Security — ✅

4. Performance — ✅

5. UI / UX — ✅

6. Code Quality & Style — ✅

No new findings beyond the two carried-over nits (6.1 and 6.2) noted in the status table above.

7. Commit Hygiene — ✅

8. Tests — ✅

9. Documentation — ✅

10. Nitpicks / Optional — ✅

No new nits beyond 10.3 carried over above.

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

Wires geofence support into the existing MAVLink mission micro-service:

- geofence-conversion.ts: pure helpers that translate between a Cockpit
  GeoFencePlan and a flat list of MissionItemInt[]. Polygons are emitted
  vertex-per-item with a shared param1=vertex_count; circles use param1
  for the radius; the optional breach return point goes last as a
  MAV_FRAME_GLOBAL_RELATIVE_ALT item. The reverse path detects bad/incomplete
  polygon item streams from the autopilot and surfaces a descriptive error.
- vehicle.ts: extracts _fetchMissionItems / _uploadMissionItems from the
  mission code so fence and mission paths share the handshake, then layers
  uploadFence / fetchFence / clearFence on top using
  MAV_MISSION_TYPE_FENCE. uploadFence also calls
  _ensureArduPilotPolygonFenceTypeBit to set the Polygon bit (4) of
  FENCE_TYPE on ArduPilot — without it the autopilot silently ignores the
  uploaded polygons/circles. Other FENCE_TYPE bits are preserved.

Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
Surfaces the new MAVLink capability and fence APIs through the existing
main-vehicle Pinia store so consumers don't need to reach into the
underlying MAVLinkVehicle instance:

- \`capabilities\` ref + \`onCapabilities\` subscription, kept in sync as
  AUTOPILOT_VERSION arrives.
- \`uploadFence\` / \`fetchFence\` / \`clearFence\` proxy actions, mirroring
  the existing mission helpers (and emitting the same "Geofence deleted
  from vehicle" snackbar on clear).
- \`requestParameter\` proxy so other stores (notably the upcoming
  geo-fence store) can pull individual parameter values without
  duplicating the MAVLink boilerplate.

Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
Pinia store that owns all editor-side geofencing state:

- Reactive polygons, circles and breach-return point with
  \`cockpit-draft-fence\` (in-progress) and \`cockpit-vehicle-fence\`
  (last uploaded) persistence so the user doesn't lose work across
  reloads.
- Plan import/export between Cockpit's runtime shape, the .cfp envelope
  and the de-facto MAVLink-ecosystem .plan file.
- Vehicle sync: \`uploadToVehicle\` (auto-enables FENCE_ENABLE and
  FENCE_AUTOENABLE on ArduPilot once the upload completes),
  \`downloadFromVehicle\`, \`clearOnVehicle\`, plus \`setFenceEnabled\` /
  \`setFenceAutoEnable\` that talk to the new MAVLink helpers.
- \`FENCE_ENABLE\` polling watch (3 s) tied to ArduPilot + a loaded plan
  so the Map widget toggle reflects autopilot-side flips (e.g.
  takeoff-time activation) the autopilot doesn't broadcast.
- Capability gate via \`isFenceSupported\` (\`MISSION_FENCE\` bit) and a
  \`detectMissionBreaches\` helper that flags mission waypoints which
  fall outside an inclusion fence or inside an exclusion fence so the
  UI can warn before mission upload.

Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
@ArturoManzoli ArturoManzoli force-pushed the geofence-vehicle-sync branch from 8ff9d6b to eed1509 Compare June 24, 2026 12:54
@ArturoManzoli ArturoManzoli changed the title Add geofence vehicle sync and plan types Geofence: Add vehicle sync and plan types Jun 24, 2026
@ArturoManzoli ArturoManzoli marked this pull request as draft July 1, 2026 16:29
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