feat(a750): initial A-750 robot support and blueprint#1911
Conversation
Greptile SummaryThis PR adds initial support for the A-750 robotic arm: a new hardware adapter, robot catalog entry, and
Confidence Score: 4/5Safe 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
Sequence DiagramsequenceDiagram
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
|
| # 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), |
There was a problem hiding this comment.
What is the reason for the change here? It seems unrelated to A-750.
There was a problem hiding this comment.
This happened due to the break in dev. All Path objects needed to be converted into str.
this is now resolved, so not needed.
There was a problem hiding this comment.
How do you suggest I change this? Remove the str(...)?
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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
|
I have now added adapter code for the A-750 to this PR |
| assert len(positions) == self._dof | ||
|
|
||
| if not self._enabled: | ||
| return False |
There was a problem hiding this comment.
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.
| 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 |
| 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 |
There was a problem hiding this comment.
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.
| 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 |
Co-authored-by: Copilot <copilot@github.com>
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
Implement initial support the A-750 robotic arm.
The following additional changes were made:
Bug fix:
JogState.from_fkwas 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 optionalq_homeparameter toJogState.from_fkand threadinghome_jointsfromRobotConfigthroughKeyboardTeleopConfig→KeyboardTeleopModule.The following additional changes are contemplated but not yet implemented:
Gripper control: Added
O/Ckey bindings toKeyboardTeleopModulefor open/close. The module gains agripper_command: Out[float]stream, andKeyboardTeleopConfiggainsgripper_joint,gripper_open_pos,gripper_closed_posfields. The a750 blueprint wires these fromRobotConfig.gripperand routesgripper_commandover LCM to the coordinator.CartesianIKTaskandControlCoordinatorwere 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-a750Contributor License Agreement