refactor(xarm): one grasping stack, hardware by default - #3873
Conversation
xarm-perception and xarm-room-sim were the same stack described twice, in two files, with the sim copy carrying the newer thinking and the hardware copy quietly rotting. blueprints/grasp.py replaces both with one composition whose arm-versus-sim split is decided at import time, the way mujoco_if_sim already does it. Hardware is the default; --simulation swaps the hardware adapter, the base pose, the camera and the engine behind it, the detector backends and the home pose, and nothing else. Three things that only existed on one side are now on both. The hardware path gains a coordinator, which xarm-perception never had -- it could plan and had nothing to drive the arm with. Both paths get the rerun bridge, which only xarm-perception-sim composed. And both get the viser viewer, which the room sim turned off. Two hardware-only defects die with the old file. xarm-perception pitched the planning base 45 degrees; the arm is mounted at identity, and a wrong base pose puts every planned pose wrong the same way the 12 cm pedestal did. The room sim then made the mirror-image mistake, inheriting data/xarm7's 12 cm pedestal base_pose against a scene that bolts link_base to the world origin, so the planner solved every pose 12 cm high; make_xarm7_sim_robot_config now takes the scene's base pose. The one hardcoded planner obstacle survives for now: floor_z=-0.02 stays on the hardware path, and goes when the voxel map replaces it later in this stack. xarm-perception-sim is untouched; it is the plain planning/teleop sim and not part of the grasping stack.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## mb/grasp-05-reset-and-cancel #3873 +/- ##
================================================================
- Coverage 77.72% 77.65% -0.07%
================================================================
Files 1324 1324
Lines 125488 125505 +17
Branches 10912 10915 +3
================================================================
- Hits 97537 97465 -72
- Misses 24865 24895 +30
- Partials 3086 3145 +59
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 6 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Greptile SummaryThis change consolidates the xArm grasping blueprints into a hardware-default stack that switches its model, hardware adapter, sensing, and scene registration components for MuJoCo. Two launch paths need attention before merging: the Python API applies the MuJoCo override after resolving the grasp blueprint, and the standalone grasp composition does not explicitly preserve the detector isolation used by the agent composition. T-Rex validation blockedThe Python API check could not load the project because the available environment was missing the Confidence Score: 3/5Not safe to merge until the simulation-resolution ordering and standalone worker isolation are addressed. The code paths indicate two independent launch reliability failures, but the focused runtime exercises could not complete because required runtime packages and a working project interpreter were unavailable. Files Needing Attention: dimos/robot/manipulators/xarm/blueprints/grasp.py needs attention at the import-time simulation selection and the standalone blueprint composition.
 T-Rex LogsWhat T-Rex did
Reviews (1): Last reviewed commit: "refactor(xarm): one grasping stack, hard..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| from dimos.utils.data import LfsPath | ||
| from dimos.visualization.rerun.bridge import RerunBridgeModule | ||
|
|
||
| SIMULATED = bool(global_config.simulation) |
There was a problem hiding this comment.
When Dimos(simulation="mujoco").run("xarm-grasp") resolves the blueprint, SIMULATED captures the default value before the Python API applies its configuration overrides. The resulting composition initializes the physical xArm and RealSense instead of MuJoCo, causing the simulation-requested run to fail without the hardware devices and configuration.
| ), | ||
| ) | ||
|
|
||
| xarm_grasp = autoconnect(*_XARM_GRASP_MODULES, HeuristicGraspModule.blueprint()) |
There was a problem hiding this comment.
When the standalone xarm-grasp blueprint runs, the dedicated Rerun bridge occupies one of the default two workers and every remaining module is packed into the other. This recreates the detector packing that the agent variant explicitly avoids because its lazy Transformers import fails, preventing the standalone grasp stack from initializing.
1 participant
This is the shape PR of the stack. Everything after it composes onto the file it introduces, so the thing worth deciding here is the shape, not the diff.
What was there.
xarm-perceptionandxarm-room-simwere the same stack described twice, in two files. They planned the same way, picked the same way, and registered the same kind of scene; what actually differed between them was the hardware adapter, the base pose, the camera, the detector backends and the home pose. Because the difference was expressed by writing the whole stack out twice, every improvement landed on one copy. The sim copy got the newer thinking and the hardware copy rotted:xarm-perceptionhad no coordinator at all, so it could plan and had nothing to drive the arm with; the sim copy had one but no rerun bridge and no viewer.What replaces it.
blueprints/grasp.py, one composition, with the arm-versus-sim split decided at import time the waymujoco_if_simalready does it — it has to be import time, because composition runs before module config is applied. Hardware is the default.--simulation mujocoswaps exactly the five things that genuinely differ, through two small functions (_sensing,_scene_registration) and oneifover the model and the hardware component. Everything else is written once, in a shared module tuple, so a later PR that adds the voxel chain or a grasp provider adds it to both paths or neither.Why a tuple and not more blueprints.
_XARM_GRASP_MODULESis everything except the grasp provider, and the next PR composes a second variant from it by adding one module. That is the only axis on which the grasping blueprints are allowed to differ, and it stays visible: exactly oneGraspGenSpecprovider may be composed in, becausePickAndPlaceModuleresolves its generator by spec and two would be ambiguous.Two geometry lies die here.
xarm-perceptionpitched the planning base 45 degrees; the arm is mounted at identity, and a wrong base pose puts every planned pose wrong in exactly the way the 12 cm pedestal does.xarm-room-simthen made the mirror-image mistake, inheritingdata/xarm7's 12 cm pedestalbase_poseagainst a scene that boltslink_baseto the world origin, so the planner solved every pose 12 cm high — the exact failure the comment onXARM7_SIM_BASE_POSEwarns about.make_xarm7_sim_robot_confignow takes the scene's base pose.One hardcoded obstacle survives, deliberately.
floor_z=-0.02installs a 0.6 x 1.2 x 0.2 m box at (0.7, 0, -0.12) — not a floor, one bench, pinned to one spot, leaving everything nearer than x=0.4 unguarded. It is a lie, and it is the only thing standing in for the table on hardware, so it stays until the voxel map replaces it three PRs later rather than leaving the hardware planner with no ground at all in between.xarm-perception-simis untouched: it is the plain planning and teleop sim, not part of the grasping stack.Sixth of nine in the xArm grasping re-landing stack.