Skip to content

Make PickAnyObject retry and recenter missed grasps - #757

Draft
axelpey wants to merge 22 commits into
codex/sim-town-trafficfrom
codex/pick-any-object-retry
Draft

Make PickAnyObject retry and recenter missed grasps#757
axelpey wants to merge 22 commits into
codex/sim-town-trafficfrom
codex/pick-any-object-retry

Conversation

@axelpey

@axelpey axelpey commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • detect empty closes and retry a bounded two times before the robot backs away
  • reacquire and tightly recenter the target with the wrist camera before every retry
  • keep the hardware wrist-camera calibration while correcting the simulator aim point
  • accept a clearly lifted, non-empty grasp before the verification backup can knock it loose

Verification

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ... python3 -m pytest -q tests/test_pick_any_object_retry.py
  • ruff check workspace/innate_skills/pick_any_object.py tests/test_pick_any_object_retry.py
  • git diff --check
  • live simulator trace confirmed the wrist-centering correction reduced target offset from about 3.3 cm to about 1.5 cm and lifted the LEGO brick; the remaining drop was isolated to the now-bypassed verification backup

@greptile-apps

greptile-apps Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds bounded grasp retries with wrist-camera reacquisition, simulator-specific wrist calibration, tighter final centering, and an early verification path intended to avoid knocking loose a lifted object.

  • Retries empty closes up to two times and descends slightly lower on retries.
  • Re-centers the target with the wrist camera before each retry.
  • Selects separate simulator and hardware wrist-image target points.
  • Adds isolated state-machine tests for missed, held, and post-lift-empty grasps.

Confidence Score: 4/5

The grasp retry should be corrected before merging because it can reopen the gripper and drop a successfully lifted thin or compressible object.

The new post-lift retry decision equates a fully closed encoder position with an empty grasp, then immediately opens the gripper, despite the skill explicitly supporting materials that can remain held at that encoder position.

Files Needing Attention: workspace/innate_skills/pick_any_object.py, tests/test_pick_any_object_retry.py

Important Files Changed

Filename Overview
workspace/innate_skills/pick_any_object.py Adds simulator calibration, tighter wrist centering, bounded grasp retries, and lifted-grasp verification, but the post-lift empty classification can reopen on a successfully held thin object.
tests/test_pick_any_object_retry.py Adds focused retry-state tests, though its fake open-gripper constant differs from the canonical hardware convention.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Center and descend] --> B[Close gripper]
    B --> C{Encoder says empty?}
    C -- Yes --> D{Retries remain?}
    C -- No --> E[Lift grasp]
    E --> F{Encoder says empty after lift?}
    F -- No --> G[Accept lifted grasp]
    F -- Yes --> D
    D -- Yes --> H[Open and reacquire with wrist camera]
    H --> A
    D -- No --> I[Lift to safe posture and run verification]
Loading

Reviews (1): Last reviewed commit: "test(skills): make grasp retry test host..." | Re-trigger Greptile

Comment on lines +691 to +693
empty = self._gripper_closed_on_air()
if not empty:
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Retry drops thin objects

When a thin or compressible object remains held while j6 reaches the closed-on-air threshold, the post-lift check classifies the grasp as empty and the retry path opens the gripper, causing the successfully lifted object to be dropped before reacquisition.

Rule Used: j6 gripper convention: 0 = closed, 0.85 = open (Ma... (source)

Knowledge Base Used: Workspace skills library



class _Manipulation:
GRIPPER_OPEN = 0.8

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Fixture uses wrong open position

The test double defines GRIPPER_OPEN as 0.8 instead of the canonical 0.85, so it models a different gripper range from production and weakens coverage of boundary-sensitive grasp and teardown behavior.

Suggested change
GRIPPER_OPEN = 0.8
GRIPPER_OPEN = 0.85

Rule Used: j6 gripper convention: 0 = closed, 0.85 = open (Ma... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@axelpey
axelpey marked this pull request as draft September 4, 2026 06:49
@axelpey
axelpey changed the base branch from main to feat/town-traffic-on-hot-switch September 4, 2026 18:23
@axelpey
axelpey force-pushed the codex/pick-any-object-retry branch from 22aadb2 to 172624d Compare September 4, 2026 18:23
@axelpey
axelpey force-pushed the feat/town-traffic-on-hot-switch branch from 926efb4 to d391bae Compare September 4, 2026 22:01
@axelpey
axelpey force-pushed the codex/pick-any-object-retry branch from 172624d to b5448eb Compare September 4, 2026 22:01
DavidDobas and others added 13 commits September 4, 2026 15:08
An environment pack is a directory under sim/environments with a
manifest.json binding its MuJoCo collision/visual meshes, Nav2 map,
browser assets and spawn pose; the apartment becomes the first one.

`./innate-sim up --environment NAME` (or [simulation].environment in
sim/config.toml) picks the pack the world server loads. A running
simulator switches in place: the world server rebuilds VirtualMars for
the new pack while the old world keeps stepping, swaps it under the
physics lock, advances world_epoch so the driver and grid_localizer_sim
treat it as a reset, resends the roster frame, and asks Nav2 for the
pack's map over /nav/change_map. The launcher sends the same
`switch_environment` RPC to a server that is already up.

The viewer reads the pack from the roster: it unloads the rooms and
collision hulls, streams the new ones, and re-spawns the robot on the
first pose of the new world. The Scene setup panel gains an Environment
picker once the server offers more than one pack.
…iles

A pack's visual mesh can be a zero-volume plane; MuJoCo refuses to compute
volume inertia for it, and visuals carry no mass anyway.
rws answers a failed service call with result=false and no values.
rws answered the unknown name with result=false and no body, so Nav2
never followed an environment switch.
sim/tools/build_environment_pack.py derives a whole pack from one glTF
scene -- CoACD hulls, textured MuJoCo meshes, a lidar nav map, and the
viewer's floor-aligned glb plus hull soup -- in the asset image's two
stages, so the bake caches on its own layer under the apartment's. The
seeder pins the source glb (Backrooms VR by carlcapu9, CC BY 4.0) and
the launcher installs the new work units like the apartment's.
… for

Drops and goals are coordinates in one world, so a sidecar names its packs
in `environments`; a placement-free challenge (Victory Lap) leaves it out
and is offered everywhere. The roster only lists what the running pack
can host, and start refuses the rest. A bare VirtualMars() is the
apartment pack, so sandbox and notebook users see nothing change.
…kage

The asset image's CoACD stage carries no driver by design; only the nav
map compiles a world, so it imports the driver where it needs it.
- Nav2 changes map before the world is swapped, so the epoch bump reseeds
  AMCL on the right map; a map Nav2 cannot load fails the switch with the
  old world still running. A server with no Nav2 in sight does not wait.
- The old world's rendered frames are dropped at the swap, and a render
  that outlived the swap cannot put them back.
- Room and hull loads that finish after their pack was unloaded dispose
  their result instead of attaching it to the next pack.
- The pack tool keeps one part per placement, so instanced glTF geometry
  is no longer collapsed into one cumulatively transformed copy.
switch_to restores the previous target on timeout, so the bridge's
reconciliation cannot later move Nav2 onto the rejected pack's map. An
abandoned hull load disposes its geometries only; hullMaterial is shared.
VirtualMars now loads sim/environments/apartment/manifest.json, so the
demo image copies the tree (and its publish workflow watches it, along
with the other copied trees it was missing).
@axelpey
axelpey force-pushed the codex/pick-any-object-retry branch from b5448eb to 6d2a384 Compare September 4, 2026 22:17
@axelpey
axelpey force-pushed the feat/town-traffic-on-hot-switch branch from d391bae to 4857ff0 Compare September 4, 2026 22:17
DavidDobas and others added 8 commits September 4, 2026 15:25
… at boot

With two maps installed the mode manager booted on the alphabetically
first one and the map bridge changed it under a stack still coming up,
which left Nav2 half-activated for minutes. The launcher now seeds
data/.last_map from the pack before the ROS session starts; the bridge
waits out Nav2's boot before its first request and, while a switch is
waiting, retries 'already in progress' replies every few seconds.
@axelpey
axelpey force-pushed the codex/pick-any-object-retry branch from 6d2a384 to 0cad4eb Compare September 4, 2026 22:33
@axelpey
axelpey changed the base branch from feat/town-traffic-on-hot-switch to codex/sim-town-traffic September 4, 2026 22:33
@axelpey
axelpey force-pushed the codex/sim-town-traffic branch 2 times, most recently from 005102b to a68baee Compare September 5, 2026 05:09
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