Skip to content

Fix: Validate hangar_sim surface path against the aircraft and abort stuck goals#775

Closed
griswaldbrooks wants to merge 1 commit into
mainfrom
fix/20427-hangar-surface-collision
Closed

Fix: Validate hangar_sim surface path against the aircraft and abort stuck goals#775
griswaldbrooks wants to merge 1 commit into
mainfrom
fix/20427-hangar-surface-collision

Conversation

@griswaldbrooks

Copy link
Copy Markdown

[written by AI]

Fixes the Plan Path Along Surface CI hang tracked in PickNikRobotics/moveit_pro#20427 (~70% of hangar_sim integration runs).

Problem

The objective plans a whole-body (arm + mecanum base) Cartesian path along the perceived fuselage contour with no trajectory validation, and the untuned contour window routes the path's tail into the aircraft. The aircraft's 13 convex collision meshes have always existed as URDF links fixed to world, but auto-generated SRDF disable_collisions entries (from the original Add hangar_sim commit) exempted the tail pieces from checking against every robot link — and constraints.goal_time: 0.0 makes the upstream JTC's goal-tolerance abort branch unreachable, so when MuJoCo contact stops the base short of the commanded terminal, the controller waits forever. The test fixture reports that silent hang as "did not return within 90.0s", and the abandoned goal pollutes the next test (rosbag forensics, phase timelines, and live reproduction on the moveit_pro issue).

Approach

Three minimal changes, each independently defensible:

  1. plan_path_along_surface.xml: retune the path so it is actually feasible — tip_offset 0.1→0.2 and contour crop window (start 2.4, span 2.25) so the arc clears the wing box and the coarse nose hull — and add GetCurrentPlanningScene + ValidateTrajectory before ExecuteTrajectory (the exact pattern plan_path_along_surface_3_passes.xml and cartesian_path_with_collision_checking.xml already use). The bare abort on validation failure (no WaitForMTCSolutionApproval fallback) is deliberate: in headless CI an approval prompt blocks forever, and the intent of this fix is a fast, diagnosable failure.
  2. picknik_ur.srdf: remove the 86 auto-generated disable_collisions entries between collision_Plane* and robot links. They made collision checking blind to the aircraft's tail pieces for every group and objective. The re-enabled pieces sit at y≈29–33, far outside any current objective's workspace, so nothing legitimate is newly rejected (verified: the stricter validation still passes 3 Passes).
  3. picknik_ur.ros2_control.yaml: constraints.goal_time 0.0→20.0, converting any future terminal non-convergence into a GOAL_TOLERANCE_VIOLATED abort that names the offending joint instead of an unbounded silent hang. Worst-case surface-path abort lands ~T+84s, inside the CI fixture's 90s cap.

Validation (live, main-based stack, moveit_pro overlay @ post-#20258 main)

  • Tuned objective: 3/3 SUCCEEDED (~40s each, 9–11 waypoints planned).
  • Untuned probe (original crop/tip values): aborts at plan time in ~30s with ValidateTrajectory Error: Trajectory collides at 82.7% feasible. Colliding objects: collision_Plane_009 - collision_vacuum_suction_cups — proving validation now covers the aircraft via its URDF links, no scene setup needed.
  • Plan Path Along Surface 3 Passes: SUCCEEDED under the stricter SRDF (goal_time + SRDF regression probe).

Follow-ups (not in this PR)

  • moveit_pro docs: src/docs/docs/tutorials/motion_planning.mdx needs its single-pass Behavior list updated to mention validation, and the 3-passes "extends" bullet reworded (validation is no longer unique to it).
  • objective_test_fixture.py (moveit_pro): cancel the objective on timeout so an abandoned goal can't pollute subsequent tests.
  • Optional: a regression test asserting the validation-abort failure mode itself, guarding against the SRDF exemptions creeping back.
  • Contour-crop angle tuning remains brittle by design (window is anchored to the perceived hull's angular start); documented in moveit_pro#20427.

Claude agent checks

  • code-reviewer (v1 + v2 re-review — no P0/P1; P2 naming fix applied in v1, P2 notes addressed in this description)
  • roboticist-bot (Required Change resolved by the SRDF root fix; TF/ACM/goal_time semantics verified)
  • documentation-bot (2 stale-corrections identified → moveit_pro docs follow-up above)
  • test-runner — SKIPPED: no buildable code in diff (XML/YAML/SRDF config); validated live per the matrix above; CI integration suite exercises the changed objective automatically

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cc7a18cb-0bde-4734-8fd2-6adbd7170628

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@marioprats

Copy link
Copy Markdown

@griswaldbrooks this flake was blocking me, so I implemented a minimal change in #776, which keeps the Objective as-is, and also keeps the path very similar to what it is today, just a bit shorter to avoid the wraparound at the edge of the perceived point cloud

@griswaldbrooks

Copy link
Copy Markdown
Author

[written by AI]

Superseded by #776, which fixed the wrap-around contour tail (the actual collision) with a one-line span trim. The hardening pieces here (ValidateTrajectory in the single-pass objective, goal_time 20.0, SRDF airplane-exemption cleanup) are documented in moveit_pro#20427 if the failure class ever resurfaces.

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.

2 participants