Skip to content

feat(a750): initial A-750 robot support and blueprint#1911

Open
adob wants to merge 14 commits into
dimensionalOS:legacy-dev-dont-mergefrom
adob:dev
Open

feat(a750): initial A-750 robot support and blueprint#1911
adob wants to merge 14 commits into
dimensionalOS:legacy-dev-dont-mergefrom
adob:dev

Conversation

@adob
Copy link
Copy Markdown

@adob adob commented Apr 24, 2026

Implement initial support the A-750 robotic arm.

The following additional changes were made:

Bug fix: JogState.from_fk was computing the initial EE pose at the all-zeros joint configuration (q = [0,0,0,0,0,0]), while the A-750 home configuration is [0, 0, -90°, 0, 0, 0]. The immediately-published target pose was unreachable from home within the 15°/tick safety limit. Fixed by adding an optional q_home parameter to JogState.from_fk and threading home_joints from RobotConfig through KeyboardTeleopConfigKeyboardTeleopModule.

The following additional changes are contemplated but not yet implemented:

Gripper control: Added O/C key bindings to KeyboardTeleopModule for open/close. The module gains a gripper_command: Out[float] stream, and KeyboardTeleopConfig gains gripper_joint, gripper_open_pos, gripper_closed_pos fields. The a750 blueprint wires these from RobotConfig.gripper and routes gripper_command over LCM to the coordinator. CartesianIKTask and ControlCoordinator were extended to accept and forward gripper position commands alongside arm joint commands.

Breaking Changes

None. All new fields are optional with safe defaults; existing blueprints that don't configure a gripper are unaffected.

How to Test

# Start the keyboard teleop blueprint
dimos run keyboard-teleop-a750

Contributor License Agreement

  • I have read and approved the CLA.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 24, 2026

Greptile Summary

This PR adds initial support for the A-750 robotic arm: a new hardware adapter, robot catalog entry, and keyboard-teleop-a750 blueprint. It also fixes a bug where JogState.from_fk computed the initial EE pose at the all-zeros joint configuration rather than the robot's actual home, by threading an optional q_home parameter through KeyboardTeleopConfig.

  • New A-750 adapter (dimos/hardware/manipulators/a750/adapter.py): connects via serial, reads joint state and gripper position, commands joint positions and gripper. Several guard-related issues from the previous review round remain open (noted in prior threads).
  • New catalog + blueprint (dimos/robot/catalog/a750.py, dimos/robot/manipulators/a750/blueprints.py): configures the 6-DOF arm with correct home joints [0, 0, -90°, 0, 0, 0] and wires it into the autoconnect blueprint.
  • Bug fix propagated to existing blueprints: Piper gets the str() LfsPath fix; XArm6 gets the same fix, but XArm7 was missed — keyboard-teleop-xarm7 will still crash at startup.

Confidence Score: 4/5

Safe to merge for A-750 support, but XArm7 keyboard teleop remains broken.

The XArm6 blueprint received the str(LfsPath) fix but XArm7 did not. keyboard-teleop-xarm7 will throw AttributeError at startup — the same crash that originally motivated this PR's fix. The A-750 path itself is correct and the adapter guard issues flagged earlier are pre-existing rather than regressions introduced here.

dimos/robot/manipulators/xarm/blueprints.py — the XArm7 blueprint still passes a bare LfsPath to model_path and will crash at launch.

Important Files Changed

Filename Overview
dimos/hardware/manipulators/a750/adapter.py New A-750 hardware adapter with connection guards on read methods; write_joint_positions uses assert (stripped by -O) and lacks a _connected check; write_gripper_position lacks a _connected guard matching read_gripper_position.
dimos/robot/catalog/a750.py New A-750 robot config; device_path now correctly mapped to address; GripperConfig.joints=["finger"] may not match URDF joint names (joint7/joint8), but impact depends on downstream consumers.
dimos/robot/manipulators/a750/blueprints.py New keyboard-teleop-a750 blueprint; correctly uses str(A750_FK_MODEL) and threads home_joints; gripper transport removed from this version; wiring looks correct.
dimos/robot/manipulators/xarm/blueprints.py XArm6 blueprint fixed with str() conversion, but XArm7 blueprint still passes bare LfsPath to model_path, causing AttributeError at startup for keyboard-teleop-xarm7.
dimos/control/examples/cartesian_ik_jogger.py Adds optional q_home parameter to JogState.from_fk to fix initial pose at zero config; stale debug message still prints "q=0" regardless of actual q_init.
dimos/teleop/keyboard/keyboard_teleop_module.py Adds optional home_joints field to KeyboardTeleopConfig and threads it through to JogState.from_fk; clean, minimal change.
dimos/robot/manipulators/piper/blueprints.py Applies same str() fix to Piper FK model path; correct one-line change.

Sequence Diagram

sequenceDiagram
    participant User as Keyboard
    participant KT as KeyboardTeleopModule
    participant JF as JogState.from_fk
    participant CC as ControlCoordinator
    participant A7 as A750Adapter
    participant HW as A-750 Hardware

    KT->>JF: from_fk(model_path, ee_joint_id, q_home=[0,0,-pi/2,0,0,0])
    JF-->>KT: initial JogState (pose at home config)

    loop Teleop tick
        User->>KT: keypress (WASD/arrows)
        KT->>CC: cartesian_command (PoseStamped via LCM)
        CC->>CC: CartesianIK solve
        CC->>A7: write_joint_positions(q)
        A7->>HW: command_joint_positions (serial)
        HW-->>A7: get_current_state()
        A7-->>CC: read_joint_positions()
        CC->>KT: joint_state (JointState via LCM)
    end
Loading

Comments Outside Diff (1)

  1. dimos/robot/manipulators/xarm/blueprints.py, line 86 (link)

    P1 This PR applied str(XARM6_FK_MODEL) to the XArm6 blueprint but left XARM7_FK_MODEL as a bare LfsPath. JogState.from_fk calls model_path.endswith(".xml"), which will raise AttributeError: 'LfsPath' object has no attribute 'endswith' when keyboard-teleop-xarm7 is loaded, making it completely non-functional.

Reviews (11): Last reviewed commit: "Add real-time control loop details for A..." | Re-trigger Greptile

Comment thread dimos/robot/catalog/a750.py Outdated
Comment thread dimos/robot/manipulators/a750/blueprints.py Outdated
Comment thread dimos/robot/catalog/a750.py Outdated
# XArm6 mock sim + keyboard teleop + Drake visualization
keyboard_teleop_xarm6 = autoconnect(
KeyboardTeleopModule.blueprint(model_path=XARM6_FK_MODEL, ee_joint_id=_xarm6_cfg.dof),
KeyboardTeleopModule.blueprint(model_path=str(XARM6_FK_MODEL), ee_joint_id=_xarm6_cfg.dof),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for the change here? It seems unrelated to A-750.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happened due to the break in dev. All Path objects needed to be converted into str.

this is now resolved, so not needed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you suggest I change this? Remove the str(...)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adob I was asking why the change was made in the first place since it's seemingly in an unrelated file? Mustafa says it was needed at some point. Looks like it's not needed anymore, so yes, should be reverted.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still crashes in current dev without the str:

Failed to deploy module: ValidationError: 1 validation error for               │ │
│ │ KeyboardTeleopConfig                                                                         │ │
│ │ model_path                                                                                   │ │
│ │   Input should be a valid string 

Comment thread dimos/core/coordination/module_coordinator.py Outdated
Comment thread dimos/hardware/manipulators/a750/adapter.py
Comment thread dimos/hardware/manipulators/a750/adapter.py
@adob
Copy link
Copy Markdown
Author

adob commented May 1, 2026

I have now added adapter code for the A-750 to this PR

Comment thread dimos/hardware/manipulators/a750/adapter.py Outdated
Comment thread dimos/hardware/manipulators/a750/adapter.py Outdated
Comment on lines +196 to +199
assert len(positions) == self._dof

if not self._enabled:
return False
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 assert is stripped when Python runs with the -O flag, so this length check will silently vanish in optimised builds. For safety-critical robot control code, replace it with an explicit if guard that raises ValueError unconditionally.

Suggested change
assert len(positions) == self._dof
if not self._enabled:
return False
if len(positions) != self._dof:
raise ValueError(f"Expected {self._dof} joint positions, got {len(positions)}")
if not self._enabled:
return False

Comment on lines +264 to +270
def write_gripper_position(self, position: float) -> bool:
"""Command gripper position."""
if not self._enabled:
return False

self._robot.command_gripper_position(position)
return True
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 write_gripper_position only guards against _enabled being False. If is_connected() detects a hardware-level disconnect and sets _connected = False without also resetting _enabled, then _enabled remains True and this method proceeds to call self._robot.command_gripper_position(position) on a stale/disconnected robot object. Add a _connected guard to be consistent with read_gripper_position.

Suggested change
def write_gripper_position(self, position: float) -> bool:
"""Command gripper position."""
if not self._enabled:
return False
self._robot.command_gripper_position(position)
return True
def write_gripper_position(self, position: float) -> bool:
"""Command gripper position."""
if not self._connected:
return False
if not self._enabled:
return False
self._robot.command_gripper_position(position)
return True

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 6, 2026

Want your agent to iterate on Greptile's feedback? Try greploops.

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.

3 participants