Skip to content

feat: xarm manipulator adapter improvement#2353

Open
TomCC7 wants to merge 5 commits into
dimensionalOS:mainfrom
TomCC7:feat/xarm-manipulator-updates
Open

feat: xarm manipulator adapter improvement#2353
TomCC7 wants to merge 5 commits into
dimensionalOS:mainfrom
TomCC7:feat/xarm-manipulator-updates

Conversation

@TomCC7
Copy link
Copy Markdown

@TomCC7 TomCC7 commented Jun 4, 2026

Closes #1183

Solution

This PR resolves the following issues observed on teleoperating xarm:

KeyboardTeleopModule initial position sync

Wire robot joint states to the keyboard teleop module so that it initialize target to the robot startup position. A more proper fix in my mind should be to refactor keyboard teleop module to only output relative cartesian motion and wire that to some relative cartesian task so that the teleop module is decoupled from the actual robot state.

XArm graceful start/stop

Added two lifecycle methods in manipulator adapter activate/deactivate to execute functions required before robot starts/stop movement. The semantic is different from connect/disconnect in that sometime you might want to only pause robot commanding while still keeping the connection. Now the xarm will move to the default position on start/stop.

How to Test

# connects to a real robot for testing
dimos --xarm6-ip=192.168.1.210 run keyboard-teleop-xarm6

Contributor License Agreement

  • I have read and approved the CLA.

TomCC7 added 4 commits June 3, 2026 14:35
- Fix _XARM6/7_INITIAL_JOINTS to use degrees instead of radians
- Add motion_enable(False) before set_state(4) in stop()
- Update custom arm docs with activate/deactivate lifecycle methods
- Ignore .omo/ directory
Remove unnecessary getattr/callable/hasattr guards since
ManipulatorAdapter Protocol guarantees these methods exist.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 4, 2026

Greptile Summary

This PR improves the XArm teleoperation workflow by wiring live joint states into the keyboard teleop module for accurate initialization, and introducing activate/deactivate lifecycle hooks on manipulator adapters so the arm returns to a safe home pose on start and stop.

  • Lifecycle hooks: activate() and deactivate() are added to ManipulatorAdapter protocol, MockAdapter, ShmMujocoAdapter, and XArmAdapter; the coordinator calls them automatically in _setup_hardware and stop(), with a graceful fallback to write_enable(True) for adapters that don't implement them.
  • Keyboard teleop init: _pygame_loop now reads the robot's actual joint state (with a configurable timeout) before computing the initial cartesian pose, and the SPACE key syncs to the live pose rather than a fixed home configuration.
  • A/D key direction swap: The left/right keybinding was inverted to match the correct +Y/-Y convention.

Confidence Score: 4/5

Safe to merge for simulation/testing; a physical-robot safety concern in the deactivation path warrants review before deploying to a real arm.

The deactivate() method on XArmAdapter clears active error/warning codes and re-enables motors before commanding the home-pose move. If the arm stopped because of a collision or an operator E-stop, this silently resumes motion without requiring any acknowledgment, which is unexpected on a physical robot. Everything else — the coordinator lifecycle wiring, the joint-state init for keyboard teleop, and the test coverage — looks solid.

dimos/hardware/manipulators/xarm/adapter.py — specifically the _prepare_for_position_motion() call inside deactivate().

Important Files Changed

Filename Overview
dimos/hardware/manipulators/xarm/adapter.py Adds activate/deactivate lifecycle methods; _prepare_for_position_motion() ignores SDK return codes from motion_enable/set_mode/set_state, and clears active errors during deactivation which could resume motion after a safety stop.
dimos/control/coordinator.py Wires activate/deactivate into hardware setup and shutdown; gracefully skips adapters without the methods, and falls back to write_enable(True) for the old path.
dimos/teleop/keyboard/keyboard_teleop_module.py Reads live joint state on startup (configurable timeout) and syncs on SPACE; fixes A/D inversion; module-level docstring still says 'Reset to home pose' instead of 'Sync to current pose'.
dimos/hardware/manipulators/spec.py Adds activate/deactivate to ManipulatorAdapter Protocol; existing adapters that omit these methods won't satisfy the protocol statically, but the coordinator handles missing methods via getattr gracefully.
dimos/hardware/manipulators/mock/adapter.py Implements activate (write_enable True) and deactivate (write_stop + write_enable False) for the mock adapter.
dimos/hardware/manipulators/sim/adapter.py Mirrors mock adapter's lifecycle implementation for the shared-memory MuJoCo sim adapter.
dimos/control/test_control.py Adds two new lifecycle integration tests: one verifying connect→activate→deactivate→disconnect ordering, one confirming adapters without lifecycle methods still start/stop cleanly.
dimos/robot/manipulators/xarm/blueprints.py Passes coordinator_joint_names to KeyboardTeleopModule.blueprint for both xarm6 and xarm7 so the teleop module can filter joint states by name.

Sequence Diagram

sequenceDiagram
    participant C as ControlCoordinator
    participant A as XArmAdapter
    participant K as KeyboardTeleopModule

    Note over C,A: start() / _setup_hardware()
    C->>A: connect()
    A-->>C: True
    C->>A: activate()
    A->>A: _prepare_for_position_motion()
    A->>A: _move_to_initial_pose() [blocking]
    A->>A: set_control_mode(SERVO_POSITION)
    A-->>C: True
    C->>C: publish joint_state stream

    Note over K: start() / _pygame_loop thread
    K->>C: "joint_state.get_next(timeout=5s)"
    C-->>K: JointState (initial positions)
    K->>K: JogState.from_fk(initial_joints)

    Note over K: SPACE pressed
    K->>C: "joint_state.get_next(timeout=0.1s)"
    C-->>K: JointState (current)
    K->>K: sync current_pose to live FK

    Note over C,A: stop()
    C->>C: tick_loop.stop()
    C->>A: deactivate()
    A->>A: _prepare_for_position_motion()
    A->>A: _move_to_initial_pose() [blocking]
    A->>A: motion_enable(False), set_state(4)
    A-->>C: True
    C->>A: disconnect()
Loading

Reviews (2): Last reviewed commit: "fix: address lifecycle review feedback" | Re-trigger Greptile

Comment thread dimos/teleop/keyboard/keyboard_teleop_module.py
Comment thread dimos/teleop/keyboard/keyboard_teleop_module.py
Comment thread dimos/control/coordinator.py
- Guard activate()/deactivate() calls in the coordinator so adapters
  without lifecycle methods (twist bases, whole-body) no longer raise
  AttributeError; restore the write_enable(True) fallback on setup
- Implement activate()/deactivate() in MockAdapter and ShmMujocoAdapter
  to satisfy the extended ManipulatorAdapter protocol
- Log and set the stop event when keyboard teleop fails to read the
  initial joint state instead of exiting the thread silently
- Remove unused home_pose computation in keyboard teleop
- Add coordinator test covering adapters without lifecycle methods

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread dimos/hardware/manipulators/xarm/adapter.py
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.

integrate Xarm with dimos

1 participant