Viser Manipulation Module#2413
Conversation
Greptile SummaryThis PR introduces
Confidence Score: 4/5Safe to merge after addressing the concurrent planning defect in module.py. The Plan button never transitions action_status away from IDLE, so rapid double-clicks can dispatch two concurrent plan_to_joints RPC calls that race to write session.plan_state. The rest of the module is well-structured. dimos/manipulation/viser_panel/module.py — specifically the _run_operation action-status map and _can_plan_for_operation guard. Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant ViserServer
participant PanelModule
participant PreviewWorker
participant PollThread
participant ManipulationModule
PollThread->>ManipulationModule: list_robots() / get_robot_info()
ManipulationModule-->>PollThread: robot list + info
PollThread->>PanelModule: update session state
Browser->>ViserServer: drag ee_control handle
ViserServer->>PanelModule: _target_pose_changed(target)
PanelModule->>PreviewWorker: submit(PreviewRequest)
PreviewWorker->>ManipulationModule: evaluate_pose_target(pose)
ManipulationModule-->>PreviewWorker: result
PreviewWorker->>PanelModule: _apply_preview_result()
PanelModule->>ViserServer: update ghost robot + feasibility color
Browser->>ViserServer: click Plan
ViserServer->>PanelModule: _run_operation [new thread]
PanelModule->>ManipulationModule: plan_to_joints(target)
ManipulationModule-->>PanelModule: ok
PanelModule->>ViserServer: render_plan_path
Browser->>ViserServer: click Execute
ViserServer->>PanelModule: _run_operation [new thread]
PanelModule->>ManipulationModule: execute(robot)
ManipulationModule-->>PanelModule: ok
Reviews (7): Last reviewed commit: "fix: skip failed fk poses in viser path ..." | Re-trigger Greptile |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| def render_plan_path(self, path: Sequence[JointState], poses: Sequence[Pose]) -> None: | ||
| if self.server is None: | ||
| return | ||
| positions = [ | ||
| [float(pose.position.x), float(pose.position.y), float(pose.position.z)] | ||
| for pose in poses | ||
| ] | ||
| if "plan_path" in self.handles: | ||
| self.handles["plan_path"].remove() | ||
| self.handles.pop("plan_path", None) | ||
| if len(positions) >= 2: | ||
| self.handles["plan_path"] = self.server.scene.add_line_segments( | ||
| "/plans/path", | ||
| points=[[start, end] for start, end in itertools.pairwise(positions)], | ||
| colors=(80, 180, 255), | ||
| ) |
There was a problem hiding this comment.
None poses crash render_plan_path
get_planned_path_poses in manipulation_module.py returns [world_monitor.get_ee_pose(robot_id, joint_state=waypoint) for waypoint in path], where get_ee_pose is typed as -> Pose | None. When FK fails for any waypoint (e.g., malformed joint count or world-monitor state change), that slot is None. The list comprehension on line 106 then raises AttributeError: 'NoneType' object has no attribute 'position' at float(pose.position.x). Both _plan_impl and _preview_impl hit this path and pass the unchecked list directly to render_plan_path.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Problem
Going to replace the drake meshcat and provide a moveit style console.
Closes #2412
Solution
How to Test
Contributor License Agreement