Conversation
Greptile SummaryThis PR successfully splits the monolithic Key changes:
The refactoring maintains backward compatibility—existing blueprints continue using the base module, while only perception-dependent blueprints ( Issue found: The Confidence Score: 4/5
Important Files Changed
Class Diagram%%{init: {'theme': 'neutral'}}%%
classDiagram
class ManipulationModule {
+In~JointState~ joint_state
-_world_monitor: WorldMonitor
-_planner: Planner
-_kinematics: Kinematics
+start()
+reset() str
+move_to_pose(x, y, z, roll, pitch, yaw) str
+move_to_joints(joints) str
+go_home() str
+go_init() str
+get_robot_state() str
+open_gripper() str
+close_gripper() str
+plan_to_pose(pose) bool
+plan_to_joints(joints) bool
+execute() bool
+add_obstacle(obstacle) bool
+remove_obstacle(id) bool
}
class PickAndPlaceModule {
+In~list~DetObject~~ objects
-_graspgen: DockerRunner
-_detection_snapshot: list~DetObject~
-_last_pick_position: Vector3
+start()
+stop()
+scan_objects(min_duration) str
+get_scene_info() str
+pick(object_name, object_id) str
+place(x, y, z) str
+place_back() str
+pick_and_place(object_name, x, y, z) str
+clear_perception_obstacles() str
+generate_grasps(pointcloud) PoseArray
+refresh_obstacles(min_duration) list
-_on_objects(objects)
}
class ManipulationModuleConfig {
+robots: list~RobotModelConfig~
+planning_timeout: float
+enable_viz: bool
+planner_name: str
+kinematics_name: str
}
class PickAndPlaceModuleConfig {
+graspgen_docker_image: str
+graspgen_gripper_type: str
+graspgen_num_grasps: int
+graspgen_topk_num_grasps: int
+graspgen_grasp_threshold: float
}
PickAndPlaceModule --|> ManipulationModule : extends
PickAndPlaceModuleConfig --|> ManipulationModuleConfig : extends
ManipulationModule ..> ManipulationModuleConfig : uses
PickAndPlaceModule ..> PickAndPlaceModuleConfig : uses
note for ManipulationModule "Base class: Core planning,\nmotion execution,\ngripper control"
note for PickAndPlaceModule "Subclass: Adds perception\nintegration, GraspGen,\nand pick/place skills"
Last reviewed commit: 0a87c29 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Resolve annotations using namespaces from the full MRO chain so that | ||
| # In/Out behind TYPE_CHECKING + `from __future__ import annotations` work. | ||
| # Iterate reversed MRO so the most specific class's namespace wins when | ||
| # parent modules shadow names (e.g. spec.perception.Image vs sensor_msgs.Image). | ||
| globalns: dict[str, Any] = {} | ||
| for c in reversed(module.__mro__): | ||
| if c.__module__ in sys.modules: | ||
| globalns.update(sys.modules[c.__module__].__dict__) |
There was a problem hiding this comment.
This looks quite hack-y. What is the issue? I thought I fixed from __future__ import annotations. Can you show an example where it fails?
There was a problem hiding this comment.
PR #1309 from Feb 20 added from future import annotations + TYPE_CHECKING guard for Out to the RealSenseCamera — to lazy-load pyrealsense2. That broke _BlueprintAtom.create because it couldn't resolve the camera's string annotations.
The fix I just applied makes _BlueprintAtom.create use MRO-based namespace resolution (with reversed order so the defining class wins).
The object scene registration module does not capture the camera image streams. Above quote is from Claude, after workaround was implemented.
It agree with it being hacky >.<
…or missing angles
2 participants
Problem
ManipulationModule is a 1600-line monolith mixing core planning infrastructure with pick/place behaviors, GraspGen integration, and perception subscriptions. This makes it impossible to use the planner without pulling in perception and grasp generation dependencies.
Additionally, _BlueprintAtom.create in blueprints.py fails to resolve In/Out port annotations for modules using from future import annotations with TYPE_CHECKING guards (e.g. RealSenseCamera), silently breaking autoconnect transport wiring.
Issue: DIM-451
Solution
Split ManipulationModule into a base class and a PickAndPlaceModule subclass:
PickAndPlaceModuleConfig extends ManipulationModuleConfig with GraspGen fields. Only xarm_perception and xarm_perception_agent blueprints switch to pick_and_place_module; all other blueprints use the base manipulation_module unchanged.
New blueprints: xarm7_planner_coordinator_agent for testing base module with an LLM agent.
Bug fixes:
Breaking Changes
None. ManipulationModule retains all base skills and RPCs. PickAndPlaceModule is a drop-in replacement where perception/pick-place is needed. Both are exported from dimos.manipulation.
How to Test
closes DIM-451