refactor(so101)!: consolidate the clutch retargeter onto engage-relative full pose - #822
Open
jiwenc-nv wants to merge 2 commits into
Open
refactor(so101)!: consolidate the clutch retargeter onto engage-relative full pose#822jiwenc-nv wants to merge 2 commits into
jiwenc-nv wants to merge 2 commits into
Conversation
Contributor
|
📝 Docs preview is not auto-deployed for fork PRs. A maintainer with write access to |
Contributor
📝 WalkthroughWalkthroughThe pull request adds scikit-build-core source and editable-install packaging, Python-version CMake presets, SKBUILD-specific wheel and executable installation, and CI smoke-test coverage. It also introduces Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant uv
participant scikit_build_core
participant CMake
participant isaacteleop_wheel
uv->>scikit_build_core: start editable source installation
scikit_build_core->>CMake: configure and build with SKBUILD
CMake->>isaacteleop_wheel: install staged package and executables
isaacteleop_wheel-->>uv: expose installed isaacteleop package
sequenceDiagram
participant ControllerInput
participant SO101EngageRelativeClutchRetargeter
participant MeasuredBaseTEE
participant RetargetingEngine
ControllerInput->>SO101EngageRelativeClutchRetargeter: provide grip pose and squeeze state
MeasuredBaseTEE->>SO101EngageRelativeClutchRetargeter: provide optional measured transform
SO101EngageRelativeClutchRetargeter->>RetargetingEngine: emit 7D ee_pose
RetargetingEngine-->>SO101EngageRelativeClutchRetargeter: provide reset or execution state
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/getting_started/build_from_source/index.rst`:
- Around line 110-119: Update the documented CMake prerequisite to require
version 3.21 or newer, matching the schema and cmakeMinimumRequired values used
by CMakePresets.json and the documented preset workflow.
In `@src/retargeters/SO101/engage_relative_clutch_retargeter.py`:
- Around line 394-397: Update the measured pose handling near
MEASURED_BASE_T_EE_INPUT to validate base_T_ee[:3, 3] with a finiteness check
before copying it into _home_pos. Treat None or non-finite translation as
invalid and preserve the existing warn-once fallback behavior, ensuring invalid
measurements cannot propagate into commanded or latched pose state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 30370915-01de-4240-baed-9072b73658ba
📒 Files selected for processing (16)
.github/workflows/build-ubuntu.ymlCMakeLists.txtCMakePresets.jsonCMakePresets.json.licensecmake/SetupPython.cmakedocs/source/getting_started/build_from_source/index.rstdocs/source/references/build.rstdocs/source/references/retargeting/index.rstdocs/source/references/retargeting/so101.rstpyproject.tomlsrc/core/python/CMakeLists.txtsrc/core/retargeting_engine_tests/python/test_retargeter_reset.pysrc/core/retargeting_engine_tests/python/test_so101_retargeters.pysrc/retargeters/SO101/clutch_retargeter.pysrc/retargeters/SO101/engage_relative_clutch_retargeter.pysrc/retargeters/__init__.py
jiwenc-nv
force-pushed
the
jiwenc/clutch-v2
branch
2 times, most recently
from
July 28, 2026 06:04
4c038a7 to
b527706
Compare
jiwenc-nv
changed the title
An operator's comfortable arm travel (~0.7 m) is roughly twice the SO-101's reach (~0.35 m), so integrating controller deltas 1:1 ratchets the commanded EE target out of the workspace across clutch cycles. Add a validated position_scale constructor parameter (default 1.0, position only) that scales the clutch delta. Non-positive and non-finite values are rejected: zero freezes the EE, negative inverts the mapping, and inf * 0 = nan would reach the SE3 IK. Adds tests for the scaled compute path, validation, and the ratchet property. Refs NVIDIA#733 Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
…ive full pose
V1 clutched position only and composed a fixed orientation_offset. The LeRobot
XR path had grown a full-pose, squeeze-gated clutch that re-centers on every
engage -- the behaviour these docs already promise. Fold both into one
retargeter:
pos = home_pos + position_scale * (grip_pos - origin_pos)
rot = (R_ctrl . R_origin^-1) . R_home
On the engage frame this is exactly the home pose, so there is no teleport.
BREAKING CHANGE: home_base_T_ee is now required positionally and both of its
blocks are used; engagement also requires squeeze > squeeze_threshold;
orientation_offset and debug_log_engage are gone -- put the gripper's base-frame
orientation into home_base_T_ee instead.
Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
jiwenc-nv
force-pushed
the
jiwenc/clutch-v2
branch
from
July 29, 2026 06:44
b527706 to
4d0ee27
Compare
Collaborator
Author
|
/preview-docs |
Contributor
|
✅ Preview deployed: https://NVIDIA.github.io/IsaacTeleop/preview/pr-822/ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Consolidates the SO-101 clutch retargeter so the Isaac Lab and LeRobot paths share one
implementation, and adds the motion scale requested in #733.
V1 clutched position only with a fixed
orientation_offset; the LeRobot XR path hadmeanwhile grown a full-pose, squeeze-gated clutch that re-centers on every engage — the
behaviour this repository's own docs already promise. Both are now one
SO101ClutchRetargeter:On the engage frame this is exactly the home pose whatever the scale — no teleport.
position_scale(default1.0) maps an operator's ~0.7 m arm travel into the SO-101's~0.35 m reach; at 1:1, re-engage cycles ratchet the target out of the workspace.
Fixes #733
Breaking changes
home_base_T_ee— required 2nd positional, both blocks used (was optional, translation only)input_device,position_scale,squeeze_threshold— keyword-onlysqueeze > squeeze_thresholdas well asRUNNINGorientation_offset,debug_log_engage— removed; put the gripper's base-frame orientation inhome_base_T_eeType of change
Testing
82 unit tests pass; verified against the reference implementation (position bit-exact,
orientation 1.116e-15). Hardware validation on a physical SO-101 is still pending.
Checklist
SKIP=check-copyright-year pre-commit run --all-filesgit commit -s) per the DCO