Manus plugin: HMD hand tracking, installer, docs, and visualizer#431
Open
jiwenc-nv wants to merge 2 commits intoNVIDIA:mainfrom
Open
Manus plugin: HMD hand tracking, installer, docs, and visualizer#431jiwenc-nv wants to merge 2 commits intoNVIDIA:mainfrom
jiwenc-nv wants to merge 2 commits intoNVIDIA:mainfrom
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 pull request adds comprehensive Manus Gloves device integration to the Isaac Teleop framework. It introduces XDev optical hand tracking support with OpenXR extension integration and automatic fallback to controller pose when optical tracking is unavailable. The changes include a new Vulkan-based hand skeleton visualizer rendered in real-time via X11, an automated installer script that downloads the SDK, configures system permissions, and builds/installs components, updated documentation covering installation and troubleshooting, new public APIs to query hand skeleton topology, and CMake component tagging for organized installation. Supporting changes include udev rules for device access permissions, a diagnostic CLI tool that prints joint data and displays the visualizer, and OpenXR function declarations for extension enumeration. Sequence DiagramsequenceDiagram
actor User
participant ManusTrkr as ManusTracker<br/>(Main Plugin)
participant OpenXR as OpenXR<br/>(Session)
participant XDev as XDev API<br/>(Optical Tracking)
participant CtrlFB as Controller<br/>(Fallback)
User->>ManusTrkr: update() per frame
ManusTrkr->>XDev: update_xdev_hand(Hand::LEFT)
alt XDev has valid wrist pose
XDev-->>ManusTrkr: return wrist_pose + TRACKED
else XDev unavailable or stale
XDev-->>ManusTrkr: return nullptr/stale
ManusTrkr->>CtrlFB: fetch left controller pose
CtrlFB-->>ManusTrkr: controller_pose
ManusTrkr->>ManusTrkr: use controller as fallback
end
ManusTrkr->>OpenXR: inject selected wrist pose<br/>+ set TRACKED bit
OpenXR-->>ManusTrkr: acknowledge
ManusTrkr-->>User: hand pose ready for OpenXR
User->>User: Visualizer thread polls update()
User->>User: Builds vertex data from skeleton nodes
User->>User: Renders via Vulkan on X11 surface
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 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 |
Collaborator
Author
|
Squashed and rebased: #235 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugins/manus/70-manus-hid.rules`:
- Around line 2-5: The udev rules currently grant world-writable device access
via MODE:="0666" on the rules matching SUBSYSTEMS=="usb",
ATTRS{idVendor}=="3325", the rule with
ATTRS{idVendor}=="1915",ATTRS{idProduct}=="83fd", and the KERNEL=="hidraw*",
ATTRS{idVendor}=="3325" rule; change these to least-privilege group-owned rules
by replacing MODE:="0666" with a group-based rule such as GROUP="plugdev"
MODE:="0660" (or GROUP="hidraw" MODE:="0660" if your distro provides a hidraw
group), apply the same change to all three matching rules, and ensure any users
who need access are added to that group on target systems.
In `@src/plugins/manus/install_manus.sh`:
- Line 145: The cmake build command uses an unquoted command substitution for
parallel jobs which Shellcheck flags (SC2046); update the cmake invocation (the
line invoking cmake --build build --target manus_hand_plugin
manus_hand_tracker_printer -j$(nproc)) to quote the substitution so the -j
argument uses "$(nproc)" instead of $(nproc), preserving the same targets
(manus_hand_plugin and manus_hand_tracker_printer) and behaviour while
preventing potential word-splitting.
In `@src/plugins/manus/tools/manus_hand_visualizer.hpp`:
- Around line 1262-1271: The swapchain recreation path leaks the old
VkCommandPool because createCommandBuffers() creates a new pool without
destroying the previous m_cmd_pool; fix by ensuring the existing command pool is
destroyed before creating a new one — either add cleanup of m_cmd_pool in
destroySwapchainResources() (destroy and set m_cmd_pool = VK_NULL_HANDLE) or
have createCommandBuffers() check m_cmd_pool and vkDestroyCommandPool(m_dev,
m_cmd_pool, nullptr) before allocating a new pool; update whichever function
(createCommandBuffers or destroySwapchainResources) to handle the destruction
and nulling of m_cmd_pool to prevent the leak.
🪄 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: 9f16a7bc-71a1-46a7-9dd7-68b348505122
📒 Files selected for processing (20)
docs/source/device/index.rstdocs/source/device/manus.rstexamples/native_openxr/xdev_list/main.cppexamples/oxr/cpp/oxr_session_sharing.cppsrc/core/deviceio_trackers/python/tracker_bindings.cppsrc/core/live_trackers/cpp/live_controller_tracker_impl.cppsrc/core/oxr_utils/cpp/inc/oxr_utils/oxr_funcs.hppsrc/core/schema/python/controller_bindings.hsrc/core/schema/python/pedals_bindings.hsrc/plugins/manus/70-manus-hid.rulessrc/plugins/manus/CMakeLists.txtsrc/plugins/manus/README.mdsrc/plugins/manus/app/CMakeLists.txtsrc/plugins/manus/core/CMakeLists.txtsrc/plugins/manus/core/manus_hand_tracking_plugin.cppsrc/plugins/manus/inc/core/manus_hand_tracking_plugin.hppsrc/plugins/manus/install_manus.shsrc/plugins/manus/tools/CMakeLists.txtsrc/plugins/manus/tools/manus_hand_tracker_printer.cppsrc/plugins/manus/tools/manus_hand_visualizer.hpp
Use HMD hand tracking when available to position the hands, with the existing tracker-based pose as a fallback. The hand-tracking plugin is substantially reworked: detection of an OpenXR hand-tracking extension at init time, conditional pose composition, and updated public types. Tooling: - Add an install_manus.sh helper and a 70-manus-hid.rules udev rule. After review, the full installer was scaled back so the rule file is the abstracted, distributable artifact and the script focuses on bootstrapping the supported manual-setup path. - Expand the tracker printer into a visualizer (manus_hand_visualizer.hpp) for inspecting tracker output, and clarify the CLI usage. Remove the obsolete Isaac Lab start script. Documentation: - Convert the Manus README to reStructuredText and add it to the device docs index (docs/source/device/manus.rst), with clarifications to the manual setup flow and updated log/installation guidance. Core fix: - Declare xrEnumerateInstanceExtensionProperties alongside xrGetInstanceProcAddr under XR_NO_PROTOTYPES in oxr_funcs, so the Manus plugin can enumerate supported OpenXR extensions at init time without a compile error when prototypes are suppressed. Quality: - clang-format and pre-commit linting passes across the touched files. - Address CodeRabbit and GitHub Copilot review feedback.
jiwenc-nv
force-pushed
the
manus/hmd-hand-tracking-rebased
branch
from
75429a0 to
5f62bfa
Compare
Collaborator
Author
|
/preview-docs |
Contributor
|
✅ Preview deployed: https://NVIDIA.github.io/IsaacTeleop/preview/pr-431/ |
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.
Use HMD hand tracking when available to position the hands, with the existing tracker-based pose as a fallback. The hand-tracking plugin is substantially reworked: detection of an OpenXR hand-tracking extension at init time, conditional pose composition, and updated public types.
Tooling:
Documentation:
Core fix:
Quality:
Summary by CodeRabbit
Release Notes
New Features
Documentation