Skip to content

Mission Planning: Fix crosshatch survey path detouring to polygon corner - #2835

Merged
ArturoManzoli merged 1 commit into
bluerobotics:masterfrom
ArturoManzoli:2834-crosshatch-inefficient-path
Jul 16, 2026
Merged

Mission Planning: Fix crosshatch survey path detouring to polygon corner#2835
ArturoManzoli merged 1 commit into
bluerobotics:masterfrom
ArturoManzoli:2834-crosshatch-inefficient-path

Conversation

@ArturoManzoli

Copy link
Copy Markdown
Contributor

Problem:

  • With no turnaround distance, consecutive survey transects were joined by moveAlongEdge walking the polygon ring in fixed vertex order, which always inserted the next polygon corner between lines.
  • On a rectangular north/south survey the vehicle ran a line to the edge, turned off to a far corner, did a 180, then came back to the next line — poor for photogrammetry coverage and risky for the towfish.

Fix:

  • moveAlongEdge now finds the ring edge each transect endpoint lies on and returns only the vertices between them along the shorter perimeter direction (empty when both share an edge, giving a direct connector).
  • Transect connectors are now direct hops that stay inside the survey area; the far-corner detour is gone.
  • Made moveAlongEdge module-private, since it is only used within utils-map.ts.

Crosshatch survey grid

Closes #2834

@github-actions

Copy link
Copy Markdown

Automated PR Review (Claude)

0. Summary

Verdict: MINOR SUGGESTIONS

This PR fixes an inefficient survey-transect connector path in the crosshatch survey generator. Previously, moveAlongEdge walked the polygon ring in a fixed vertex order and always injected the next corner between consecutive transect lines, causing the vehicle to detour to a far polygon corner. The new implementation finds which ring edge each transect endpoint lies on, collects the ring vertices between them in both directions, and picks the shorter perimeter path (or returns an empty path when both endpoints share the same edge). The moveAlongEdge function is also made module-private, and two clean helpers (findRingEdgeIndex, pathLengthThroughVertices) are extracted.

Minor suggestion to address: 1.1.

1. Correctness & Implementation Bugs

1.1 (minor) — booleanPointOnLine may fail on floating-point-imprecise points. findRingEdgeIndex uses turf.booleanPointOnLine without an explicit epsilon option. Transect endpoints are generated by intersecting scan lines with the polygon boundary, so they should lie very close to an edge, but floating-point drift can cause booleanPointOnLine (default tolerance ≈ 0) to return false for all edges, making findRingEdgeIndex return -1. When both return -1, moveAlongEdge returns [] and the connector degrades to a direct line (functionally safe), but this silently skips the edge-hugging logic. Consider passing a small epsilon (e.g. { epsilon: 1e-6 }) to booleanPointOnLine for robustness.

src/libs/map/utils-map.ts (new findRingEdgeIndex function, approx. line 434 in the PR):

if (turf.booleanPointOnLine(point, turf.lineString([coords[i], coords[i + 1]]), { epsilon: 1e-6 })) return i

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

10.1 (nit) — The pathLengthThroughVertices helper builds a new turf.point for every pair inside the loop. Since it is only called twice per connector (forward vs. backward), this is not a performance concern, but if you ever reuse it on longer paths you could accumulate with a single turf.length(turf.lineString(points)) call instead of pointwise turf.distance.

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

@ArturoManzoli
ArturoManzoli force-pushed the 2834-crosshatch-inefficient-path branch from 0117bff to 83ffeec Compare July 13, 2026 18:45
@ArturoManzoli

Copy link
Copy Markdown
Contributor Author

Done:

  • (1.1) passed { epsilon: 1e-6 } to booleanPointOnLine in findRingEdgeIndex so float drift on the clipped boundary points still matches their edge instead of silently falling back to a direct connector.
  • (10.1) simplified pathLengthThroughVertices to a single turf.length(turf.lineString(...)) call.

@ArturoManzoli

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown

Automated PR Re-review 2 (Claude)

Comparing 0117bff01bd2bdffffb222cb0a83e1e7581baed083ffeec06a81bcc6a1f7cfbc090b593dcde9f3a1

Previous findings status

# Finding Severity Status
1.1 booleanPointOnLine missing epsilon in findRingEdgeIndex minor ✅ Addressed
10.1 pathLengthThroughVertices building pointwise turf.point pairs instead of single turf.lineString nit ✅ Addressed

Discussion since last review

@ArturoManzoli confirmed both findings were addressed: "(1.1) passed { epsilon: 1e-6 } to booleanPointOnLine" and "(10.1) simplified pathLengthThroughVertices to a single turf.length(turf.lineString(...)) call". The current diff confirms both changes are in place.

New findings

0. Summary

Verdict: READY TO MERGE

Both previous findings have been addressed. The findRingEdgeIndex helper now passes { epsilon: 1e-6 } to booleanPointOnLine, and pathLengthThroughVertices now uses a single turf.length(turf.lineString(...)) call. No new issues found in the incremental changes. The single commit is clean and well-scoped.

1. Correctness & Implementation Bugs — ✅

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

Copy link
Copy Markdown
Contributor Author

Ready to go.

@rafaellehmkuhl

Copy link
Copy Markdown
Member

@ArturoManzoli how should I test it to confirm the bug is fixed?

@ArturoManzoli

ArturoManzoli commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

@ArturoManzoli how should I test it to confirm the bug is fixed?

The bug placed extra waypoints on the far left edge of the crosshatch mission, so you can run a SITL vehicle through the mission, or just visually confirm by checking the WP numbers on the mission.

@rafaellehmkuhl

rafaellehmkuhl commented Jul 14, 2026

Copy link
Copy Markdown
Member

@ArturoManzoli just to confirm is this the expected result?

Notice there's a 180 degree turn on waypoint 15.

image

@ArturoManzoli

Copy link
Copy Markdown
Contributor Author

@ArturoManzoli just to confirm is this the expected result?

Notice there's a 180 degree turn on waypoint 15.

Not at all. This was pretty much the bug itself. I'll check what went wrong

With no turnaround distance, consecutive transects were joined by walking
the polygon ring in fixed vertex order, which always inserted the next
corner between lines and sent the vehicle out to a far corner and back
before resuming the correct line. moveAlongEdge now returns only the ring
vertices between the two boundary points along the shorter direction, and
none when they share an edge, so connectors become direct hops that stay
inside the survey area. findRingEdgeIndex resolves those boundary points to
the metrically closest edge rather than booleanPointOnLine, whose
degree-based epsilon still read points as lying on a near-axis-aligned edge
tens of meters away and reintroduced the detour. The crosshatch second pass
also enters at whichever of its four boustrophedon corners sits nearest the
first pass exit, so the inter-pass transit stays short instead of flying to
a far endpoint and doubling straight back.
@ArturoManzoli
ArturoManzoli force-pushed the 2834-crosshatch-inefficient-path branch from 83ffeec to cfd573e Compare July 14, 2026 21:22
@ArturoManzoli

Copy link
Copy Markdown
Contributor Author

@ArturoManzoli just to confirm is this the expected result?

Notice there's a 180 degree turn on waypoint 15.

Fixed

@rafaellehmkuhl rafaellehmkuhl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better! Nice work!

Image

@ArturoManzoli
ArturoManzoli merged commit 25c525b into bluerobotics:master Jul 16, 2026
13 checks passed
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.

Crosshatch surveys calculates an inefficient vehicle path

2 participants