Conversation
449a2b4 to
9340ff7
Compare
Greptile SummaryThis PR refactors the manipulation and control stack across three axes: (1) joint name separator changed from Key issues found:
Confidence Score: 2/5Not safe to merge as-is: the inverted Piper adapter condition means real hardware silently runs as mock, and velocity data is dropped from every joint-state update. Two P1 regressions (velocity data loss, ValueError in display helpers) and one P0 logic inversion (Piper always mock) block reliable use on real hardware. The Drake cache issue could corrupt multi-robot joint state. The core joint-separator migration and per-robot routing architecture are sound, but these implementation bugs need fixes before merging. dimos/robot/manipulators/piper/blueprints.py (inverted adapter logic), dimos/manipulation/manipulation_module.py (velocity drop + silent skip), dimos/manipulation/planning/world/drake_world.py (model-instance cache safety), dimos/manipulation/control/coordinator_client.py (ValueError in preview helpers) Important Files Changed
Sequence DiagramsequenceDiagram
participant Driver as Hardware Driver
participant MM as ManipulationModule
participant WM as WorldMonitor
participant RSM1 as RobotStateMonitor(robot_1)
participant RSM2 as RobotStateMonitor(robot_2)
Driver->>MM: JointState(all robots, positions+velocities)
Note over MM: Build name→index map
MM->>MM: For each robot: extract coord_names indices
alt All joints present
MM->>MM: sub_msg = JointState(coord_names, positions_only ⚠️)
MM->>WM: on_joint_state(sub_msg, robot_id=robot_1)
WM->>RSM1: on_joint_state(sub_msg)
RSM1->>RSM1: _extract_positions ✓
RSM1->>RSM1: _extract_velocities → None ⚠️
else Any joint missing
MM-->>MM: continue (silent skip ⚠️)
end
MM->>MM: For robot_2: extract coord_names indices
alt All joints present
MM->>MM: sub_msg2 = JointState(coord_names, positions_only)
MM->>WM: on_joint_state(sub_msg2, robot_id=robot_2)
WM->>RSM2: on_joint_state(sub_msg2)
end
Note over MM: Capture _init_joints[robot_name] on first receipt
|
…tion for all modules
…t link edge cases
…es our LFS packages natively. No monkey-patching.
b8e837a to
081d38d
Compare
Problem
The manipulation module broadcasts joint state to all robot monitors inefficiently (N robots = N redundant extraction attempts per tick), stores init joints as a single global variable (breaking multi-robot go_init), and uses ambiguous underscore-based joint naming (left_arm_joint1 — is the robot left_arm or left?).
Solution
Slash joint naming — Joint names now use / as separator: arm/joint1, base/vx instead of arm_joint1, base_vx. make_twist_base_joints, RobotConfig.joint_prefix, and all string-parsing code in coordinator and coordinator_client.
Per-robot joint state routing — _on_joint_state now splits the aggregated JointState by robot using each robot's coordinator joint names from config, then routes to the specific RobotStateMonitor with an explicit robot_id instead of broadcasting to all monitors.
Per-robot init joints — _init_joints changed from JointState | None to dict[RobotName, JointState]. All methods (go_init, get_init_joints, set_init_joints, set_init_joints_to_current, get_robot_info) are now per-robot aware.
Rename — WorldStateMonitor → RobotStateMonitor (file: robot_state_monitor.py). It monitors one robot's state, not the world.
Breaking Changes
Joint names use / separator: "arm/joint1" instead of "arm_joint1". Any code referencing old-style prefixed joint names must update.
get_init_joints() and set_init_joints() now accept an optional robot_name parameter.
WorldStateMonitor renamed to RobotStateMonitor (import path changed).
How to Test