Skip to content

Add SAM3 perception and persistent placed-box obstacles to hangar_sim ML Move Boxes#746

Draft
griswaldbrooks wants to merge 3 commits into
mainfrom
feat/19112-sam3-collision-move-boxes
Draft

Add SAM3 perception and persistent placed-box obstacles to hangar_sim ML Move Boxes#746
griswaldbrooks wants to merge 3 commits into
mainfrom
feat/19112-sam3-collision-move-boxes

Conversation

@griswaldbrooks

@griswaldbrooks griswaldbrooks commented Jun 26, 2026

Copy link
Copy Markdown

[written by AI]

Migrates the hangar_sim ML Move Boxes to Loading Zone Objective to SAM3 perception and makes the base stop driving over boxes it has already placed. Two self-contained commits.

Problem

  • Perception used a CLIPSeg text-query + SAM2 point-refine chain — two models and a refinement hop where SAM3 does it in one.
  • The outer loop (RetryUntilSuccessful + ForceFailure) never terminated cleanly and could spin forever (#19112).
  • The mobile base drove over boxes it had already placed in the loading zone, because nothing in the planning scene represented them (#19973).

Approach

Commit 1 — SAM3 perception. Replace GetMasks2DFromTextQuery(CLIPSeg) + GetMasks2DFromPointQuery(SAM2) with GetMasks2DFromExemplar(SAM3), which produces high-quality per-detection masks directly from a text prompt — no point-refine step. Restructure the loop into a drain-aware Fallback { RepeatUnlessFailureEachTick + ScriptCondition } that exits SUCCESS when the scene is cleared and surfaces real pick errors as FAILURE (the #19112 fix). Adds a FilterMasks2DByArea false-positive filter, discards noisy box-yaw from grasps (max_ik_solutions=16), and ships a calibrate_sam3_mask_areas diagnostic Objective for tuning the area thresholds.

This commit reconstructs work that was reviewed and merged (PRs #649/#652) onto an integration branch that was never proposed to main. It re-applies the two main-side fixes that postdate that branch's base: the #732 grasp-cup offset (ee_to_tcp 0;0;00;0;0.045) and the SetupMTCUpdateGroupCollisionRuleSetupMTCSetCollisionRule migration (with the matching monitored_stage string).

Commit 2 — persistent placed-box obstacles. After each box is released at the loading zone, a persistent collision object is added at the world-frame place_pose (dropped to floor height) under a unique id (placed_box_N). The whole-body manipulator group includes the planar base joints, so the base plans around every already-placed box; the placed_box_* ids are not in the SRDF disable list. ResetPlanningSceneObjects at objective start clears boxes from a previous run; a placed_count counter (ported across perception passes) keeps the ids unique.

The obstacle is placed from place_pose, not the ICP target_pose: target_pose is base-relative but stamped world, so placing the mesh there (and carrying it via attach/detach) dropped the obstacles ~10 m off the robot. place_pose is built world-frame by the parent Objective, so the obstacle lands where the box was actually placed. This drops the carried-box (in-transit) avoidance the earlier attach approach intended — acceptable, since the base-drives-over-placed-boxes symptom is the #19973 concern and the carried box moves with the robot.

Commit 3 — post-grasp lift + pick tuning. Each grasped box is lifted ~10 cm straight up (PlanCartesianPath along grasp_link -Z) before transiting, so it clears the surface and does not drag along the ground. The pick is tuned for speed and consistency: num_orientation_samples = 8 and max_ik_solutions = 8 (fewer candidates — the pose IK is the bottleneck), and cost_joint_names/cost_joint_weights bias the redundant base yaw so MTC selects the lowest-base-yaw grasp IK and the omni-base stops pirouetting between picks.

🚫 BLOCKING dependency — do NOT merge before moveit_pro#20073. cost_joint_names/cost_joint_weights are not ports on SetupMTCBatchPoseIK in current main. BT.CPP 4 throws on an unknown XML port, so the entire Objective fails to load (it won't appear in the UI at all) on any core build without moveit_pro#20073. This is a hard load failure, not a degraded mode. The pirouette also needs the pose_ik seed-distance config from moveit_pro_example_ws#736, which this overlaps. Merge order: #20073 first, then reconcile with #736, then this. Alternative: drop those two attributes to make this PR independently mergeable — SAM3 + obstacles + lift carry no such dependency.

Carried-box-in-scene — investigated, deferred

Attaching the carried box so the planner accounts for it in-transit was attempted and deferred. Root cause of the difficulty: MTC builds collision checking from in-task SetupMTCSetCollisionRule, not the live ACM, and the transit is the shared Move to Pose No Preview SubTree, so a held box can't be whitelisted without forking that SubTree into an inline MTC task. Compounding it, the hangar boxes are baked into the URDF as 16 static collision_Cube_* links, so an attached box overlaps its own static duplicate. Left as a follow-up; the placed-box obstacles already deliver the #19973 base-avoidance.

Relationship to other work

Verification

Run live on the SAM3 backend: the combined objective picks boxes via SAM3, and the placed-box obstacles land on the floor in the loading-zone line exactly at the place poses ((-4.50, 8.40, 0.10), (-4.90, 8.40, 0.10), …), tracking where each box was placed; the tree loads with no port/behavior errors.

Release notes

None.

Replace the CLIPSeg text-query + SAM2 point-refine perception chain with
SAM3 (GetMasks2DFromExemplar), which produces high-quality per-detection
masks directly from a text prompt. Restructure the outer loop to a
drain-aware Fallback so it exits SUCCESS when the scene is cleared and
surfaces real pick errors as FAILURE (fixes the #19112 infinite loop).
Adds an area-based SAM3 mask false-positive filter, discards noisy box-yaw
from grasps with max_ik_solutions=16, and ships a calibrate_sam3_mask_areas
diagnostic Objective. Carries forward the #732 grasp-cup offset and the
SetupMTCSetCollisionRule migration from main.
@coderabbitai

coderabbitai Bot commented Jun 26, 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: 582acbdb-e9c9-4978-a8a4-365d4f5503c1

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

After each box is released at the loading zone, add a persistent collision
object at the world-frame place_pose (dropped to floor height) under a unique
id (placed_box_N), so the whole-body base+arm planner avoids every already-placed
box on later picks instead of driving over them (#19973). ResetPlanningSceneObjects
at objective start clears boxes from a previous run; a placed_count counter
(ported across perception passes) keeps the ids unique.

The obstacle is placed from place_pose, not the ICP target_pose: target_pose is
base-relative but stamped world, so using it (with attach/detach) dropped the
obstacles ~10 m off the robot. place_pose is built world-frame by the parent
Objective, so the obstacle lands where the box was actually placed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@griswaldbrooks griswaldbrooks force-pushed the feat/19112-sam3-collision-move-boxes branch from fad7d97 to 47b1269 Compare June 26, 2026 16:23
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@griswaldbrooks griswaldbrooks force-pushed the feat/19112-sam3-collision-move-boxes branch from bccd327 to 68e8c2a Compare June 28, 2026 19:28
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

Lift each grasped box ~10 cm (Cartesian, grasp_link -Z) before transiting so it
clears the surface and does not drag along the ground. Tune the pick for speed
and consistency: drop num_orientation_samples 20->8 and max_ik_solutions 16->8
(fewer candidates, the pose IK is the bottleneck), and bias the redundant base
yaw via cost_joint_names/cost_joint_weights so MTC selects the lowest-base-yaw
grasp IK and the omni-base stops pirouetting between picks.

The cost_joint_names/cost_joint_weights ports require the SetupMTCBatchPoseIK
support added in moveit_pro#20073 and the pose_ik seed-distance config from
moveit_pro_example_ws#736; this objective therefore overlaps #736's pirouette
work and is not standalone-mergeable to main until those land.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@griswaldbrooks griswaldbrooks force-pushed the feat/19112-sam3-collision-move-boxes branch from 68e8c2a to 0eecc95 Compare June 29, 2026 14:43
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

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