Uh oh!
There was an error while loading. Please reload this page.
add open door action - #554
Conversation
Greptile SummaryThe PR adds an OpenDoor atomic action that derives a limit-aware opening arc from observed hinge state and adds UR10/Robotiq support to the atomic-action tutorials.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| embodichain/lab/sim/atomic_actions/primitives/open_door.py | Adds the staged OpenDoor planner and now constrains its opening delta using observed hinge position, opening direction, and legal joint limits. |
| embodichain/lab/sim/atomic_actions/affordance.py | Adds the OpenDoor affordance contract and articulation-based hinge topology resolution. |
| embodichain/lab/sim/objects/articulation.py | Adds a public, copied articulation-joint ancestry representation used to resolve door hinges without exposing backend internals. |
| scripts/tutorials/atomic_action/tutorial_utils.py | Adds UR10/Robotiq tutorial configuration and multi-joint gripper command support. |
| tests/sim/atomic_actions/test_actions.py | Exercises OpenDoor planning across batched targets, hinge observations, opening direction, invalid inputs, and reached targets. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
G[OpenDoor goal] --> A[Resolve door affordance]
A --> H[Read observed hinge position]
H --> L[Map opening fraction to legal joint limits]
L --> R[Compute remaining hinge rotation]
R --> P[Plan approach and grasp]
P --> O[Generate limit-aware opening arc]
O --> X[Release and retract]
Reviews (7): Last reviewed commit: "try fix" | Re-trigger Greptile
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Adds a new OpenDoor atomic action to EmbodiChain’s typed planning/execution framework, including an affordance factory that resolves a door hinge from a handle link, plus docs, tutorial, and tests to integrate the new primitive alongside existing interaction actions.
Changes:
- Introduces
OpenDoorprimitive (OpenDoorGoal,OpenDoorOptions) and exports it through the atomic-actions public API. - Adds
OpenDoorAffordance(includingfrom_articulation(...)) for handle-mesh + parent-hinge axis/origin resolution. - Adds tutorial + documentation pages/entries and expands the unit test suite to cover affordance resolution and planning behavior.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/sim/atomic_actions/test_affordance.py | Adds unit tests for OpenDoorAffordance.from_articulation() hinge resolution and error handling. |
| tests/sim/atomic_actions/test_actions.py | Adds integration-style planning/contract tests for OpenDoor (skill id, descriptors, planning segments, validation paths). |
| scripts/tutorials/atomic_action/open_door.py | New tutorial script demonstrating OpenDoor on a microwave door handle with auto hinge resolution. |
| embodichain/lab/sim/atomic_actions/primitives/open_door.py | New OpenDoor primitive implementation (goal/options/planning and arc interpolation helpers). |
| embodichain/lab/sim/atomic_actions/primitives/init.py | Exposes OpenDoor from primitives package and updates __all__. |
| embodichain/lab/sim/atomic_actions/affordance.py | Adds OpenDoorAffordance and updates module exports. |
| embodichain/lab/sim/atomic_actions/init.py | Exposes OpenDoor* types in the top-level atomic-actions package. |
| docs/source/overview/sim/atomic_actions/builtin_actions.md | Documents the new built-in OpenDoor action (card + detailed section + table row). |
| docs/source/api_reference/embodichain/embodichain.lab.sim.atomic_actions.rst | Adds OpenDoor* to atomic-actions API reference. |
| docs/source/api_reference/embodichain/embodichain.lab.sim.atomic_actions.primitives.rst | Adds open_door primitive module to the primitives API reference. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| opened_link_poses, opened_grasp_xpos = self._opened_link_and_eef_poses( | ||
| link_pose, | ||
| grasp_xpos, | ||
| affordance.rotation_axis, | ||
| affordance.axis_origin, | ||
| options.open_angle, | ||
| options.door_waypoint_count, | ||
| ) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
embodichain/lab/sim/atomic_actions/affordance.py:525
OpenDoorAffordance.from_articulation()always passesjoint_limits=(float(hinge.lower_limit), float(hinge.upper_limit)), but_validate_joint_metadata()requires finite ordered limits. For assets with missing/continuous limits (None/±inf), this will raise even thoughjoint_limitsis optional. Consider only settingjoint_limitswhen both limits are present and finite; otherwise passNone.
vertices, triangles = articulation.get_link_vert_face(link_name)
return cls(
mesh_vertices=vertices,
mesh_triangles=triangles,
rotation_axis=rotation_axis_local,
axis_origin=tuple(float(value) for value in axis_origin_local),
joint_name=hinge.name,
joint_limits=(float(hinge.lower_limit), float(hinge.upper_limit)),
)
embodichain/lab/sim/atomic_actions/primitives/open_door.py:472
_opened_link_and_eef_poses()generates arc waypoints starting atopen_angle / waypoint_count, which excludes the unrotated grasp pose (angle 0). This makes the firstopenwaypoint discontinuous with the end of thereachsegment (which targetsgrasp_xpos), and it doesn’t align with_motion_segment_lengths()reservingdoor_waypoint_count + 1samples (start + arc). Consider including the grasp pose as the first waypoint (angle 0) and returningwaypoint_count + 1poses; update the corresponding tests/assertions to match.
angles = torch.linspace(
open_angle / waypoint_count,
open_angle,
waypoint_count,
dtype=torch.float32,
device=self.device,
)
yuecideng
left a comment
There was a problem hiding this comment.
Focused review of the OpenDoor atomic-action contract and public API. I left inline comments on semantic naming, hinge resolution, backend coupling, the failing test configuration, and documentation/context coverage.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| mesh_vertices=vertices, | ||
| mesh_triangles=triangles, | ||
| rotation_axis=rotation_axis_local, | ||
| axis_origin=tuple(float(value) for value in axis_origin_local), | ||
| joint_name=hinge.name, |
| The hand control part is intentionally expressed as one regular expression. | ||
| Robot materialization expands it over the six active finger/knuckle joints | ||
| while excluding the gripper's fixed joints. The solver TCP includes the | ||
| Robotiq mounting rotation and its 0.23 m tool offset. | ||
| Franka always uses its PyTorch kinematics solver; ``solver="ur"`` selects | ||
| the analytical solver only when ``robot_type="ur5"``. The mounting layout, | ||
| control-part names, gripper component, and downstream action bindings stay | ||
| identical across both robot choices. | ||
| the analytical solver for UR5 and UR10. UR5 and Franka use the shared PGI | ||
| hand, while ``ur10`` uses the six-DOF Robotiq 2F-140 and its | ||
| rotated 0.23 m TCP. The mounting layout, control-part names, and downstream | ||
| action bindings stay identical across robot choices. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
scripts/tutorials/atomic_action/tutorial_utils.py:1061
- Docstring says the Robotiq TCP has a “0.23 m tool offset”, but
ROBOTIQ_2F_140_TCPis defined with a Z offset of0.21(see constant in this module) and tests assert against that value. Please make the docstring match the actual TCP being used (or update the TCP + tests together).
"""Build a UR10 arm with a six-DOF Robotiq 2F-140 gripper.
The hand control part is intentionally expressed as one regular expression.
Robot materialization expands it over the six active finger/knuckle joints
while excluding the gripper's fixed joints. The solver TCP includes the
Robotiq mounting rotation and its 0.23 m tool offset.
scripts/tutorials/atomic_action/scenario_utils.py:242
- Docstring references a “rotated 0.23 m TCP” for the UR10/Robotiq variant, but the UR10 TCP constant used (
ROBOTIQ_2F_140_TCP) has Z offset0.21. This mismatch can confuse tutorial users; update the text to match the constant (or update the constant + tests if 0.23 is intended).
"""Build a dual tutorial robot from the selected arm and hand.
Franka always uses its PyTorch kinematics solver; ``solver="ur"`` selects
the analytical solver for UR5 and UR10. UR5 and Franka use the shared PGI
hand, while ``ur10`` uses the six-DOF Robotiq 2F-140 and its
rotated 0.23 m TCP. The mounting layout, control-part names, and downstream
action bindings stay identical across robot choices.
| HAND_OVER_POSE_PROVIDER = ConfiguredHandOverPoseProvider( | ||
| middle_position=(0.0, 0.0, 0.7), | ||
| middle_quaternion_wxyz=(0.7071067812, 0.7071067812, 0.0, 0.0), | ||
| final_position=(0.0, -0.2, 0.7), | ||
| final_position=(0.0, -0.2, 0.6), | ||
| final_quaternion_wxyz=(0.7071067812, 0.7071067812, 0.0, 0.0), |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/tutorials/atomic_action/tutorial_utils.py:1060
- The docstring claims the Robotiq TCP has a 0.23 m tool offset, but the configured ROBOTIQ_2F_140_TCP uses 0.21 m (see ROBOTIQ_2F_140_TCP[2][3] == 0.21). This inconsistency can mislead users tuning offsets.
The hand control part is intentionally expressed as one regular expression.
Robot materialization expands it over the six active finger/knuckle joints
while excluding the gripper's fixed joints. The solver TCP includes the
Robotiq mounting rotation and its 0.23 m tool offset.
scripts/tutorials/atomic_action/scenario_utils.py:242
- This docstring says the UR10/Robotiq variant uses a rotated 0.23 m TCP, but ROBOTIQ_2F_140_TCP in tutorial_utils encodes a 0.21 m Z offset. Please align the docstring with the actual constant (or adjust the constant if 0.23 m is intended).
Franka always uses its PyTorch kinematics solver; ``solver="ur"`` selects
the analytical solver for UR5 and UR10. UR5 and Franka use the shared PGI
hand, while ``ur10`` uses the six-DOF Robotiq 2F-140 and its
rotated 0.23 m TCP. The mounting layout, control-part names, and downstream
action bindings stay identical across robot choices.
Description
python scripts/tutorials/atomic_action/coordinated_pickment.py --robot ur10 --auto_playType of change
Checklist
black .command to format the code base.python docs/scripts/check_api_docs.py), if applicable