Add haptic feedback support (Manus + OpenXR controllers)#550
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds comprehensive haptic feedback support to IsaacTeleop, enabling vibration control for OpenXR motion controllers and Manus haptic gloves. The implementation includes new C++ haptic APIs in the controller tracker, Python device adapters following a vendor-agnostic interface pattern, tactile tensor type schemas, retargeting nodes for sim-to-device mapping, and two end-to-end example scripts with full test coverage. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The PR spans multiple interdependent subsystems (C++ APIs, Python adapters, retargeters, examples, tests) across diverse file types and requires careful review of the haptic parameter semantics, error handling guarantees (non-throwing on transient hardware failures), per-side logging gating, and tensor type round-tripping contracts. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@examples/haptic_feedback/python/hand_pinch_haptic_example.py`:
- Around line 436-460: The three CLI float validators (_positive_float,
_non_negative_float, _unit_float) currently accept non-finite values like nan
and inf; update each to import and use math.isfinite to reject non-finite inputs
by raising argparse.ArgumentTypeError (e.g., "expected a finite number, got
...") after parsing the float but before range checks in _positive_float and
_non_negative_float, and ensure _unit_float still calls _non_negative_float then
additionally enforces n <= 1.0 while also rejecting non-finite values via the
same check.
In `@examples/haptic_feedback/python/openxr_controller_haptic_example.py`:
- Around line 296-320: The CLI float validators (_positive_float,
_non_negative_float, _unit_float) currently only check numeric ranges and
therefore allow non-finite values (nan/inf); after parsing the float in each
validator call float(value), validate finiteness using math.isfinite and raise
argparse.ArgumentTypeError (with a message mentioning non-finite values like
NaN/Inf) if not finite, then continue the existing range checks (for
_positive_float: ensure >0, for _non_negative_float: ensure >=0, for
_unit_float: ensure <=1).
In `@src/core/live_trackers/cpp/live_controller_tracker_impl.cpp`:
- Around line 480-518: Guard the haptic inputs before constructing
XrHapticVibration: validate amplitude, duration_s, and frequency_hz with
std::isfinite and sanitize NaNs/inf by mapping them to safe defaults (e.g.,
amplitude -> 0.0f or clamped [0,1], frequency_hz -> XR_FREQUENCY_UNSPECIFIED if
non-finite) and treat non-finite or non-positive duration_s as
XR_MIN_HAPTIC_DURATION; when converting duration_s to nanoseconds compute as
double then clamp the result into the representable XrDuration range (avoid
direct float->integer cast of NaN/Inf) before the static_cast<XrDuration>, and
apply std::clamp for amplitude (0.0f..1.0f) after validating finiteness so NaN
cannot slip through; update the code around XrHapticVibration construction
(references: XrHapticVibration, amplitude, duration_s, frequency_hz) to perform
these checks and clamping.
🪄 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: 9b9430a6-2ab4-4f91-a7c4-9778574ceb7e
📒 Files selected for processing (35)
CMakeLists.txtexamples/haptic_feedback/CMakeLists.txtexamples/haptic_feedback/python/README.mdexamples/haptic_feedback/python/hand_pinch_haptic_example.pyexamples/haptic_feedback/python/openxr_controller_haptic_example.pyexamples/haptic_feedback/python/pyproject.tomlsrc/core/AGENTS.mdsrc/core/deviceio_base/cpp/inc/deviceio_base/controller_tracker_base.hppsrc/core/deviceio_trackers/cpp/controller_tracker.cppsrc/core/deviceio_trackers/cpp/inc/deviceio_trackers/controller_tracker.hppsrc/core/deviceio_trackers/python/tracker_bindings.cppsrc/core/live_trackers/cpp/live_controller_tracker_impl.cppsrc/core/live_trackers/cpp/live_controller_tracker_impl.hppsrc/core/oxr_utils/cpp/inc/oxr_utils/oxr_funcs.hppsrc/core/python/pyproject.toml.insrc/core/replay_trackers/cpp/replay_controller_tracker_impl.hppsrc/core/retargeting_engine/python/deviceio_source_nodes/__init__.pysrc/core/retargeting_engine/python/deviceio_source_nodes/haptic_sink.pysrc/core/retargeting_engine/python/tensor_types/__init__.pysrc/core/retargeting_engine/python/tensor_types/indices.pysrc/core/retargeting_engine/python/tensor_types/tactile_types.pysrc/core/retargeting_engine_tests/python/test_haptic_devices.pysrc/core/retargeting_engine_tests/python/test_haptic_sink.pysrc/core/retargeting_engine_tests/python/test_tactile_retargeters.pysrc/haptic_devices/CMakeLists.txtsrc/haptic_devices/__init__.pysrc/haptic_devices/interface.pysrc/haptic_devices/manus.pysrc/haptic_devices/openxr_controller.pysrc/plugins/manus/CMakeLists.txtsrc/plugins/manus/core/manus_hand_tracking_plugin.cppsrc/plugins/manus/inc/core/manus_hand_tracking_plugin.hppsrc/plugins/manus/python/CMakeLists.txtsrc/plugins/manus/python/manus_haptic_bindings.cppsrc/retargeters/tactile_retargeters.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/live_trackers/cpp/live_controller_tracker_impl.cpp (1)
463-553:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCI blocked: run clang-format on this file.
The pipeline is failing with clang-format violations across lines 463-548 (function signature, conditional expressions, std::cerr statements). The logic itself is sound—non-finite sanitization and duration clamping are correctly implemented—but the build won't pass until formatting is fixed.
clang-format -i src/core/live_trackers/cpp/live_controller_tracker_impl.cpp🤖 Prompt for 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. In `@src/core/live_trackers/cpp/live_controller_tracker_impl.cpp` around lines 463 - 553, The file fails CI due to clang-format style violations in LiveControllerTrackerImpl::apply_haptic_feedback (signature, conditional expressions, and std::cerr lines); fix by running clang-format on the file and committing the result (e.g., run `clang-format -i src/core/live_trackers/cpp/live_controller_tracker_impl.cpp`), ensure the formatted changes cover the function body (references: apply_haptic_feedback, safe_amplitude/safe_duration_s/safe_frequency_hz sanitization, vibration.duration/frequency clamping, and the std::cerr error logs) and re-run CI to verify the style errors are resolved.
🤖 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.
Outside diff comments:
In `@src/core/live_trackers/cpp/live_controller_tracker_impl.cpp`:
- Around line 463-553: The file fails CI due to clang-format style violations in
LiveControllerTrackerImpl::apply_haptic_feedback (signature, conditional
expressions, and std::cerr lines); fix by running clang-format on the file and
committing the result (e.g., run `clang-format -i
src/core/live_trackers/cpp/live_controller_tracker_impl.cpp`), ensure the
formatted changes cover the function body (references: apply_haptic_feedback,
safe_amplitude/safe_duration_s/safe_frequency_hz sanitization,
vibration.duration/frequency clamping, and the std::cerr error logs) and re-run
CI to verify the style errors are resolved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2d0f31bf-b679-4e9d-afeb-0a6e9bb835dc
📒 Files selected for processing (5)
examples/haptic_feedback/python/hand_pinch_haptic_example.pyexamples/haptic_feedback/python/openxr_controller_haptic_example.pysrc/core/live_trackers/cpp/live_controller_tracker_impl.cppsrc/core/retargeting_engine_tests/python/test_tactile_retargeters.pysrc/retargeters/tactile_retargeters.py
Adds a backwards data path through the existing Isaac Teleop retargeting
pipeline so simulators can drive haptic devices. Vendor-neutral by design:
a
HapticSinkretargeter dispatches to anyIHapticDeviceadapter, withtwo v1 integrations:
plugin singleton.
tracker extension on
LiveControllerTrackerImpl, not a new plugin.Haply Inverse3 force feedback is architecturally pre-approved as the next
integration; no interface changes needed.
Design doc:
IsaacLab/docs/tactile_haptic_feedback_design.md.Summary by CodeRabbit
New Features
Documentation
Tests