Skip to content

fix(manip): sync the pick/place gripper on feedback and verify the grasp - #3701

Merged
mustafab0 merged 1 commit into
mainfrom
mustafa/fix/pick-grasp-feedback
Aug 27, 2026
Merged

fix(manip): sync the pick/place gripper on feedback and verify the grasp#3701
mustafab0 merged 1 commit into
mainfrom
mustafa/fix/pick-grasp-feedback

Conversation

@mustafab0

@mustafab0 mustafab0 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

The defect

dimos/manipulation/pick_and_place_module.py synchronized the gripper with wall-clock sleeps, and nothing anywhere verified a grasp:

line sleep what it was waiting for
:543 time.sleep(0.5) pre-grasp open
:561 time.sleep(1.5) # Wait for gripper to close
:652 time.sleep(1.0) release at place

Open-loop timing means a slow close gets truncated and a fast one wastes time. Worse, a missed grasp sailed straight on to the lift.

The mechanism

Gripper readback is measured, not an echo of the command

Each sleep is now a deadline poll that settles the readback, matching the house style of PlanExecutionManager.wait (dimos/manipulation/execution_manager.py:212)

Tests

  • test_grasp_verification.py (13) — the poll helper against a mocked readback with an injected clock: settle, stall band, timeout, never-travelled, late readback, band edges, config validation.

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.97166% with 47 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/manipulation/pick_and_place_module.py 11.76% 45 Missing ⚠️
dimos/manipulation/grasp_verification.py 97.61% 1 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##             main    #3701      +/-   ##
==========================================
+ Coverage   77.98%   78.00%   +0.01%     
==========================================
  Files        1296     1298       +2     
  Lines      124870   125110     +240     
  Branches    10901    10930      +29     
==========================================
+ Hits        97383    97586     +203     
- Misses      24315    24353      +38     
+ Partials     3172     3171       -1     
Flag Coverage Δ
OS-ubuntu-24.04-arm 73.06% <80.97%> (+0.01%) ⬆️
OS-ubuntu-latest 74.88% <80.97%> (+0.01%) ⬆️
Py-3.10 74.87% <80.97%> (+0.01%) ⬆️
Py-3.11 74.87% <80.97%> (+0.01%) ⬆️
Py-3.12 74.87% <80.97%> (+0.01%) ⬆️
Py-3.13 74.87% <80.97%> (+0.01%) ⬆️
Py-3.14 74.87% <80.97%> (+0.01%) ⬆️
Py-3.14t 74.87% <80.97%> (+0.01%) ⬆️
SelfHosted-Large 29.91% <28.34%> (+<0.01%) ⬆️
SelfHosted-Linux 35.05% <28.34%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/manipulation/planning/spec/config.py 96.22% <100.00%> (+0.14%) ⬆️
dimos/manipulation/skill_errors.py 100.00% <ø> (ø)
dimos/manipulation/test_grasp_verification.py 100.00% <100.00%> (ø)
dimos/manipulation/grasp_verification.py 97.61% <97.61%> (ø)
dimos/manipulation/pick_and_place_module.py 29.05% <11.76%> (-2.31%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change replaces fixed gripper delays with feedback-based settlement, adds grasp verification, and reports gripper command failures more clearly.

A settled gripper readback is currently accepted as a completed opening without confirming that the jaws reached the requested open position. If the gripper stalls partway open, pick and place can continue with partially closed jaws, potentially interfering with approach, release, or retraction.

T-Rex validation blocked

The deterministic partial-opening reproduction could not execute the gripper method because the Python package filelock was missing. The script failed during module import before reaching the simulated accepted command and settled partial-open feedback path.

Confidence Score: 4/5

Do not merge until opening completion verifies that the settled gripper position is sufficiently close to the requested open position.

The affected success path accepts any available settled readback without comparing it with the configured open target. The dedicated runtime reproduction was blocked before the method could run, but the control flow establishes the incomplete-opening path.

Files Needing Attention: dimos/manipulation/pick_and_place_module.py, especially the settled-feedback success branch in _open_gripper.

T-Rex T-Rex Logs

What T-Rex did

  • An automated deterministic reproduction was prepared to issue an open command to 1.000 with a feedback sequence of 0.700, 0.800, 0.800, 0.800, but the run failed during module import due to ModuleNotFoundError: No module named 'filelock', so the open path could not be exercised and no runtime result was observed.
  • T-Rex produced a proof for a posted P1 finding, and the details are documented in the corresponding review comment.

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "fix(manip): sync the pick/place gripper ..." | Re-trigger Greptile

Comment thread dimos/manipulation/pick_and_place_module.py Outdated
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 26, 2026
@mustafab0
mustafab0 force-pushed the mustafa/fix/pick-grasp-feedback branch from 83efbd1 to 4ad9909 Compare August 26, 2026 21:21
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 26, 2026
@mustafab0
mustafab0 force-pushed the mustafa/fix/pick-grasp-feedback branch 3 times, most recently from d249e9d to a4244f5 Compare August 26, 2026 21:40
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 26, 2026
Comment thread dimos/manipulation/grasp_verification.py Outdated
paul-nechifor
paul-nechifor previously approved these changes Aug 26, 2026
The pick/place flow synchronized with wall-clock sleeps: 0.5s after the
pre-grasp open, 1.5s after the close, 1.0s after the release. Nothing ever
checked the object was held, so a slow close got truncated, a fast one wasted
time, and a missed grasp sailed on to the lift.

Each sleep becomes a deadline poll on the measured gripper readback, matching
the PlanExecutionManager.wait style. A close now also classifies where the
jaws stalled: readback settling above closed+empty_epsilon and below
open-open_margin means an object is in the way, at or below the low edge
means an empty grasp, and expiry fails loudly.

Thresholds live on RobotModelConfig.grasp_verification alongside the other
per-robot pick tunables, and verification is on by default.
@mustafab0
mustafab0 force-pushed the mustafa/fix/pick-grasp-feedback branch from a4244f5 to 0c535a6 Compare August 26, 2026 23:55
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 26, 2026
@mustafab0
mustafab0 enabled auto-merge August 27, 2026 00:02
@mustafab0
mustafab0 added this pull request to the merge queue Aug 27, 2026
Merged via the queue into main with commit 5daec90 Aug 27, 2026
41 of 57 checks passed
@mustafab0
mustafab0 deleted the mustafa/fix/pick-grasp-feedback branch August 27, 2026 00:39
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.

2 participants