HMD Optical Hand tracking and MANUS automatic installation#235
Open
timvw-manusvr wants to merge 16 commits intoNVIDIA:mainfrom
Open
HMD Optical Hand tracking and MANUS automatic installation#235timvw-manusvr wants to merge 16 commits intoNVIDIA:mainfrom
timvw-manusvr wants to merge 16 commits intoNVIDIA:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request integrates XDev hand tracking with optional OpenXR extension probing into the Manus hand tracking plugin, adds a Vulkan-based real-time hand skeleton visualizer, and introduces automated SDK installation and runtime environment setup workflows. Changes
Sequence DiagramsequenceDiagram
actor User
participant Printer as Printer CLI
participant Plugin as Manus Plugin
participant XR as OpenXR
participant XDev as XDev System
participant Manus as Manus SDK
participant Vis as Vulkan Visualizer
User->>Printer: Launch manus_hand_tracker_printer
Printer->>Plugin: Create tracker instance
Plugin->>XR: Probe extensions (XR_EXT_hand_tracking, XR_MNDX_xdev_space)
Plugin->>Manus: Connect to Manus SDK
Plugin->>XDev: Initialize native hand trackers (if advertised)
XDev->>Plugin: Return tracker handles
Plugin-->>Printer: Tracker ready
Printer->>Vis: Launch visualizer thread
Printer-->>User: Ready for input
loop Per-frame
Plugin->>Manus: Fetch glove skeleton data
Plugin->>XDev: Update hand poses (if available)
alt XDev pose valid
Plugin->>Plugin: Use XDev wrist pose
else
Plugin->>XR: Fallback to controller pose
end
Plugin->>XR: Inject hand joint data
Printer->>Plugin: Retrieve left/right node info
Printer-->>User: Print joint data
Vis->>Plugin: Fetch node topology & poses
Vis->>Vis: Project 3D skeleton to 2D (4-view layout)
Vis->>Vis: Render with Vulkan
Vis-->>User: Display hand visualization
end
User->>Printer: Press stop key
Printer->>Plugin: Shutdown
Plugin->>XDev: Cleanup hand trackers
Plugin->>XR: Cleanup session
Vis->>Vis: Destroy Vulkan resources
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Pull request overview
This PR adds support for using the HMD's built-in optical hand tracking (via the XR_MNDX_xdev_space extension) as the wrist position source when using Manus gloves, alongside two new automated installation scripts and expanded documentation.
Changes:
- Adds
initialize_xdev_hand_trackers()/cleanup_xdev_hand_trackers()/update_xdev_hand()/get_controller_wrist_pose()to the plugin, with automatic fallback from HMD optical tracking to controller-based wrist positioning. - Introduces two new shell scripts (
install_manus.sh,install-dependencies.sh) for end-to-end automated installation including SDK download, dependency setup (with optional gRPC), and plugin build. - Updates
README.mdwith automated installation instructions, CloudXR setup steps, a new section on the two wrist-tracking modes, and an expanded troubleshooting guide.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/plugins/manus/core/manus_hand_tracking_plugin.cpp |
Adds XDev/hand tracking initialization, wrist-source auto-selection, and simplifies injector lifecycle |
src/plugins/manus/inc/core/manus_hand_tracking_plugin.hpp |
Adds new private methods, XDev handle/function-pointer members, and HandTracker member |
src/plugins/manus/tools/manus_hand_tracker_printer.cpp |
Adds [Manus] prefix to all console output lines |
src/plugins/manus/install_manus.sh |
New all-in-one installation and build script |
src/plugins/manus/install-dependencies.sh |
New script for building gRPC v1.28.1 and system dependencies |
src/plugins/manus/README.md |
Expanded with automated install, CloudXR setup, tracking modes, and troubleshooting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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/core/manus_hand_tracking_plugin.cpp`:
- Around line 99-100: The printed diagnostic incorrectly says "Z-up" while the
coordinate system is actually configured with up = AxisPolarity_PositiveY;
update the log string printed before calling
CoreSdk_InitializeCoordinateSystemWithVUH(t_VUH, true) (the std::cout line) to
reflect "Y-up, right-handed, meters" so the diagnostic matches the SDK
configuration (referencing AxisPolarity_PositiveY and the call to
CoreSdk_InitializeCoordinateSystemWithVUH).
- Around line 507-514: The code currently treats a wrist pose as "valid" in
update_xdev_hand by checking XR_SPACE_LOCATION_POSITION_VALID_BIT and
XR_SPACE_LOCATION_ORIENTATION_VALID_BIT but does not ensure the pose is actively
tracked; change the logic in update_xdev_hand where wrist is read (using
joint_locations[XR_HAND_JOINT_WRIST_EXT] and wrist.locationFlags) to also check
XR_SPACE_LOCATION_POSITION_TRACKED_BIT and
XR_SPACE_LOCATION_ORIENTATION_TRACKED_BIT before returning success/setting
out_wrist_pose, or alternatively return/propagate both a valid and a tracked
boolean so inject_hand_data (which uses is_root_tracked) can add tracked bits
only when the wrist is actually tracked; update references to is_root_tracked
accordingly so last-known-but-untracked poses are not advertised as fully
tracked.
- Around line 116-129: The code currently unconditionally adds
XR_EXT_HAND_TRACKING_EXTENSION_NAME (via including core::HandTracker in
trackers) and XR_MNDX_XDEV_SPACE_EXTENSION_NAME to extensions before
constructing the OpenXR session, which causes xrCreateInstance() to fail if the
runtime lacks those optional extensions; modify the flow to query the runtime's
supported extensions (or probe via core::OpenXRSession/DeviceIOSession helper)
before pushing optional names: only include XR_EXT_HAND_TRACKING_EXTENSION_NAME
if the runtime reports it supported (or defer adding core::HandTracker to
trackers until after support is confirmed), and only push
XR_MNDX_XDEV_SPACE_EXTENSION_NAME if supported; construct m_session =
std::make_shared<core::OpenXRSession>(app_name, extensions) after gating
optional extensions and then call initialize_xdev_hand_trackers()/create hand
trackers as needed so controller fallback remains possible.
In `@src/plugins/manus/install_manus.sh`:
- Around line 53-64: The apt install block in install_manus.sh currently omits a
downloader and unzip and doesn't validate downloads; update the apt-get install
list (the sudo apt-get install -y ... block) to include a downloader (curl or
wget) and unzip, change the SDK fetch command (the curl/wget invocation in the
SDK download/extract step around lines referenced) to use a failing download
mode (curl -fL or wget --server-response -O) so HTTP errors fail, save to a
deterministic filename, then compute and verify the archive checksum (e.g.
sha256sum) against a known expected value before extracting, and only run
unzip/tar after the checksum matches; if verification fails, abort with a clear
error.
- Around line 181-187: The script currently builds only the manus_hand_plugin
target but README expects the manus_hand_tracker_printer binary; update the
build/install steps to include the printer binary: adjust the cmake --build
invocation (or add a second cmake --build call) to also build the
manus_hand_tracker_printer target and ensure the corresponding binary/component
is installed (e.g., include its name alongside manus or call cmake --install for
that component). Locate the build/install lines referencing targets
manus_hand_plugin and component manus and add the manus_hand_tracker_printer
target/component so the documented ./build/bin/manus_hand_tracker_printer exists
after the script runs.
- Around line 173-179: The script currently skips CMake configuration when
build/CMakeCache.txt exists, which leaves stale discovery results after copying
the ManusSDK; change the logic so CMake is forced to reconfigure after the SDK
copy by removing or invalidating build/CMakeCache.txt (or always running the
cmake -S . -B build -DCMAKE_BUILD_TYPE=Release command) instead of conditionally
skipping it; update the block that checks "build/CMakeCache.txt" and ensure the
cmake invocation is executed unconditionally (or after deleting the cache) so
the project is deterministically reconfigured.
In `@src/plugins/manus/install-dependencies.sh`:
- Around line 14-22: The script currently writes "deb
http://deb.debian.org/debian bookworm main" directly into
/etc/apt/sources.list.d/bookworm-gcc.list which can allow unrelated Bookworm
packages to be installed; instead, add an apt pin/priority file that pins only
the specific packages you need (e.g., gcc-11, g++-11, libgcc-11) with a high
priority for the target release, or use an isolated toolchain source (PPA or apt
repository that provides only GCC packages). Update the Debian branch of the
install step (the block that runs apt-get install -y debian-archive-keyring and
echoes the repo) to also create a /etc/apt/preferences.d/bookworm-gcc.pref entry
that pins the listed package names to release "n=bookworm" with a high
Pin-Priority, and ensure apt-get update is called before installing the pinned
packages so only those specific packages are allowed from Bookworm rather than
the whole main archive.
- Around line 38-44: The existing check that skips cloning when the grpc repo
directory exists can leave a stale or dirty checkout; change the else branch to
normalize the existing checkout: fetch all tags and branches, checkout the
pinned tag v1.28.1, hard-reset the worktree to that tag, remove untracked files,
and ensure submodules are synchronized and updated/initialized recursively so
the repo is deterministic before continuing with build steps that expect
v1.28.1.
In `@src/plugins/manus/README.md`:
- Around line 44-46: Update the MANUS version references in the README: change
both URLs and any text that point to 3.1.0 to 3.1.1 (specifically replace
https://docs.manus-meta.com/3.1.0/Resources/ and
https://docs.manus-meta.com/3.1.0/Plugins/SDK/Linux/ with the 3.1.1 equivalents)
so the manual-install instructions match the automated installer; ensure the
mention of the ManusSDK folder and MANUS_SDK_ROOT remains the same but reflects
the 3.1.1 archive name where applicable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c7bcbfd3-5e8e-4006-9d8b-2a78c775fabb
📒 Files selected for processing (6)
src/plugins/manus/README.mdsrc/plugins/manus/core/manus_hand_tracking_plugin.cppsrc/plugins/manus/inc/core/manus_hand_tracking_plugin.hppsrc/plugins/manus/install-dependencies.shsrc/plugins/manus/install_manus.shsrc/plugins/manus/tools/manus_hand_tracker_printer.cpp
jiwenc-nv
reviewed
Mar 11, 2026
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/plugins/manus/install_manus.sh (1)
53-64:⚠️ Potential issue | 🟡 MinorInstall
unzipin the minimal dependency path.The minimal dependency installation doesn't include
unzip, but the SDK extraction step (line 113) requires it and will fail with an error if it's not present.🔧 Proposed fix
sudo apt-get install -y \ build-essential \ cmake \ git \ libssl-dev \ zlib1g-dev \ libc-ares-dev \ libzmq3-dev \ libncurses-dev \ libudev-dev \ - libusb-1.0-0-dev + libusb-1.0-0-dev \ + unzip🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/manus/install_manus.sh` around lines 53 - 64, The minimal dependency apt-get install block in install_manus.sh is missing unzip, which the later SDK extraction step requires; update the apt-get install command (the sudo apt-get install -y ... block) to include unzip alongside the other packages so the SDK extraction step succeeds without error.
🤖 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/core/manus_hand_tracking_plugin.cpp`:
- Around line 471-522: The code currently hardcodes the expected serials "Head
Device (0)" and "Head Device (1)" when matching xdevs (see left_xdev_id,
right_xdev_id and the comparison against properties.serial obtained via
XrGetXDevInfoMNDX); make these strings configurable by reading environment
variables or a config (e.g. MANUS_LEFT_XDEV_SERIAL / MANUS_RIGHT_XDEV_SERIAL)
with fallbacks to the current defaults, use those configured values for the
comparisons instead of the literals, and update the diagnostic message that
prints seen_serials to include the actual expected strings being used so
operators know what was attempted.
In `@src/plugins/manus/install_manus.sh`:
- Line 27: Change the interactive prompt so the shell read invocation that fills
INSTALL_CHOICE uses the -r flag to avoid backslash mangling; in other words,
update the read command (the read that prompts "Enter your choice (1 or 2): "
and sets INSTALL_CHOICE) to include the -r option so input is read raw without
interpreting backslashes.
In `@src/plugins/manus/install-dependencies.sh`:
- Around line 130-139: Replace the multiple echo append lines that write udev
rules into /etc/udev/rules.d/70-manus-hid.rules with a single heredoc to improve
readability and atomicity: in the install-dependencies.sh script, locate the if
block that checks for /etc/udev/rules.d/70-manus-hid.rules and the series of
echo "..." >> /etc/udev/rules.d/70-manus-hid.rules lines, and change them to
create the directory if needed and write the full rules content with a
here-document (e.g., cat <<'EOF' > /etc/udev/rules.d/70-manus-hid.rules ... EOF)
so all rules for SUBSYSTEMS=="usb" and KERNEL=="hidraw*" are written in one
operation.
- Line 54: In the install-dependencies.sh script update the make invocations
that use unquoted command substitution to prevent word splitting: replace
instances of make -j$(nproc) (and the chained commands `make -j$(nproc) && make
-j$(nproc) check && make install && ldconfig`) with versions that quote the
substitution (i.e., use "$(nproc)"); apply the same change to the other
occurrences flagged on lines 85 and 109 so all make -j$(nproc) uses are changed
to make -j"$(nproc)" throughout the script.
In `@src/plugins/manus/README.md`:
- Line 65: Update the README.md markdown formatting: change the ordered list
prefix that begins with "Build from the TeleopCore root:" to use a `1.` prefix
instead of `4.`, add a blank line after all headings (for example after headings
near "Build from the TeleopCore root:" and the later section headings
mentioned), remove the trailing whitespace on the line containing the stray
space (the lone trailing-space line reported), and ensure fenced code blocks
(the blocks around the commands shown later in the README) have a blank line
before and after the ``` fences so they are separated from surrounding text.
---
Duplicate comments:
In `@src/plugins/manus/install_manus.sh`:
- Around line 53-64: The minimal dependency apt-get install block in
install_manus.sh is missing unzip, which the later SDK extraction step requires;
update the apt-get install command (the sudo apt-get install -y ... block) to
include unzip alongside the other packages so the SDK extraction step succeeds
without error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7681132b-821c-447c-8bd2-7ca6a732ef5c
📒 Files selected for processing (5)
scripts/run_isaac_lab.shsrc/plugins/manus/README.mdsrc/plugins/manus/core/manus_hand_tracking_plugin.cppsrc/plugins/manus/install-dependencies.shsrc/plugins/manus/install_manus.sh
Comment on lines
+471
to
+522
| // Find native hand tracking devices by matching against their serial strings. | ||
| // | ||
| // NOTE: The serial values "Head Device (0)" (left) and "Head Device (1)" (right) are | ||
| // NOT defined by the XR_MNDX_xdev_space specification. They are an observed runtime- | ||
| // specific naming convention (e.g. Monado). If a runtime changes these display names | ||
| // across firmware or software updates the match below will silently fail. | ||
| // See: https://registry.khronos.org/OpenXR/specs/1.0/html/xrspec.html (XR_MNDX_xdev_space) | ||
| XrXDevIdMNDX left_xdev_id = 0; | ||
| XrXDevIdMNDX right_xdev_id = 0; | ||
| std::vector<std::string> seen_serials; | ||
|
|
||
| for (const auto& xdev_id : xdev_ids) | ||
| { | ||
| XrGetXDevInfoMNDX get_info{ XR_TYPE_GET_XDEV_INFO_MNDX }; | ||
| get_info.id = xdev_id; | ||
|
|
||
| XrXDevPropertiesMNDX properties{ XR_TYPE_XDEV_PROPERTIES_MNDX }; | ||
| result = m_pfn_get_xdev_properties(m_xdev_list, &get_info, &properties); | ||
| if (XR_FAILED(result)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| std::string serial_str = properties.serial ? properties.serial : ""; | ||
| seen_serials.push_back(serial_str); | ||
|
|
||
| if (serial_str == "Head Device (0)") | ||
| { | ||
| left_xdev_id = xdev_id; | ||
| } | ||
| else if (serial_str == "Head Device (1)") | ||
| { | ||
| right_xdev_id = xdev_id; | ||
| } | ||
| } | ||
|
|
||
| if (left_xdev_id == 0 || right_xdev_id == 0) | ||
| { | ||
| std::string serials_list; | ||
| for (const auto& s : seen_serials) | ||
| { | ||
| if (!serials_list.empty()) serials_list += ", "; | ||
| serials_list += '"'; | ||
| serials_list += s; | ||
| serials_list += '"'; | ||
| } | ||
| std::cerr << "[Manus] Could not match optical hand-tracking XDevs by serial. " | ||
| << "Expected \"Head Device (0)\" (left) and \"Head Device (1)\" (right), " | ||
| << "but found: [" << serials_list << "]. " | ||
| << "These serial strings are runtime-specific and may have changed." | ||
| << std::endl; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider making XDev serial strings configurable.
The hardcoded serial strings "Head Device (0)" and "Head Device (1)" are documented as runtime-specific. While the diagnostic logging is helpful, consider making these configurable via environment variables or a config file for easier adaptation to different runtimes.
This is a forward-looking suggestion—the current implementation with good diagnostics is acceptable for an initial version.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/plugins/manus/core/manus_hand_tracking_plugin.cpp` around lines 471 -
522, The code currently hardcodes the expected serials "Head Device (0)" and
"Head Device (1)" when matching xdevs (see left_xdev_id, right_xdev_id and the
comparison against properties.serial obtained via XrGetXDevInfoMNDX); make these
strings configurable by reading environment variables or a config (e.g.
MANUS_LEFT_XDEV_SERIAL / MANUS_RIGHT_XDEV_SERIAL) with fallbacks to the current
defaults, use those configured values for the comparisons instead of the
literals, and update the diagnostic message that prints seen_serials to include
the actual expected strings being used so operators know what was attempted.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/plugins/manus/install_manus.sh (1)
188-188: 🧹 Nitpick | 🔵 TrivialQuote
$(nproc)to prevent word splitting.Static analysis flagged unquoted command substitution. While
nprocreturns a single number, quoting is best practice for defensive scripting.🔧 Proposed fix
-cmake --build build --target manus_hand_plugin manus_hand_tracker_printer -j$(nproc) +cmake --build build --target manus_hand_plugin manus_hand_tracker_printer -j"$(nproc)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/manus/install_manus.sh` at line 188, The cmake build invocation uses unquoted command substitution for nproc in the -j flag which can cause word splitting; update the cmake command in install_manus.sh (the line calling cmake --build build --target manus_hand_plugin manus_hand_tracker_printer -j$(nproc)) to quote the substitution so the -j argument uses "$(nproc)" to prevent potential word splitting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/source/device/manus.rst`:
- Around line 98-104: Update the "Running the Plugin" section to consistently
document CloudXR environment setup by mentioning both options: either source the
provided script (scripts/setup_cloudxr_env.sh) or set the variables manually;
specifically reference the environment variables NV_CXR_RUNTIME_DIR and
XR_RUNTIME_JSON and show the export commands as an alternative, and make the
troubleshooting note (which currently references scripts/setup_cloudxr_env.sh)
match this wording so readers know they can either run the script or export
NV_CXR_RUNTIME_DIR and XR_RUNTIME_JSON themselves.
---
Duplicate comments:
In `@src/plugins/manus/install_manus.sh`:
- Line 188: The cmake build invocation uses unquoted command substitution for
nproc in the -j flag which can cause word splitting; update the cmake command in
install_manus.sh (the line calling cmake --build build --target
manus_hand_plugin manus_hand_tracker_printer -j$(nproc)) to quote the
substitution so the -j argument uses "$(nproc)" to prevent potential word
splitting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e1fb9eab-1ae6-4238-8db8-2d4f16a11bdd
📒 Files selected for processing (7)
docs/source/device/index.rstdocs/source/device/manus.rstsrc/plugins/manus/README.mdsrc/plugins/manus/core/manus_hand_tracking_plugin.cppsrc/plugins/manus/inc/core/manus_hand_tracking_plugin.hppsrc/plugins/manus/install-dependencies.shsrc/plugins/manus/install_manus.sh
Comment on lines
+98
to
+104
| Source the CloudXR environment and start the runtime before running the plugin: | ||
|
|
||
| .. code-block:: bash | ||
|
|
||
| export NV_CXR_RUNTIME_DIR=~/.cloudxr/run | ||
| export XR_RUNTIME_JSON=~/.cloudxr/openxr_cloudxr.json | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor inconsistency in CloudXR environment setup references.
The "Running the Plugin" section (lines 98-104) shows manual export commands for CloudXR environment variables, but the troubleshooting section (line 161) references scripts/setup_cloudxr_env.sh. Consider clarifying whether users should source the script or set the variables manually, or document both approaches consistently.
Also applies to: 160-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/source/device/manus.rst` around lines 98 - 104, Update the "Running the
Plugin" section to consistently document CloudXR environment setup by mentioning
both options: either source the provided script (scripts/setup_cloudxr_env.sh)
or set the variables manually; specifically reference the environment variables
NV_CXR_RUNTIME_DIR and XR_RUNTIME_JSON and show the export commands as an
alternative, and make the troubleshooting note (which currently references
scripts/setup_cloudxr_env.sh) match this wording so readers know they can either
run the script or export NV_CXR_RUNTIME_DIR and XR_RUNTIME_JSON themselves.
nv-jakob
reviewed
Mar 31, 2026
jiwenc-nv
reviewed
Mar 31, 2026
added 9 commits
April 3, 2026 11:56
Added installation bash script HMD hand tracking is now used when available to position the hands
timvw-manusvr
force-pushed
the
manus/hmd-hand-tracking
branch
from
eca1051 to
b4c59d3
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/plugins/manus/tools/CMakeLists.txt (1)
23-39:⚠️ Potential issue | 🟡 MinorRemove duplicate
set_target_propertiescall.Lines 23-30 and 32-39 set identical properties on the same target. This appears to be a copy-paste error.
🔧 Proposed fix
set_target_properties(manus_hand_tracker_printer PROPERTIES CXX_VISIBILITY_PRESET hidden VISIBILITY_INLINES_HIDDEN YES BUILD_WITH_INSTALL_RPATH NO SKIP_BUILD_RPATH NO BUILD_RPATH "${_ISAAC_MANUS_EFFECTIVE_BUILD_RPATH_STR}" RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin" ) - -set_target_properties(manus_hand_tracker_printer PROPERTIES - CXX_VISIBILITY_PRESET hidden - VISIBILITY_INLINES_HIDDEN YES - BUILD_WITH_INSTALL_RPATH NO - SKIP_BUILD_RPATH NO - BUILD_RPATH "${_ISAAC_MANUS_EFFECTIVE_BUILD_RPATH_STR}" - RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin" -)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/manus/tools/CMakeLists.txt` around lines 23 - 39, The file contains two identical set_target_properties blocks for the target manus_hand_tracker_printer; remove the duplicate block so the properties are only set once. Locate the two set_target_properties(...) invocations that mention manus_hand_tracker_printer and delete one of them, ensuring the remaining block retains CXX_VISIBILITY_PRESET, VISIBILITY_INLINES_HIDDEN, BUILD_WITH_INSTALL_RPATH, SKIP_BUILD_RPATH, BUILD_RPATH and RUNTIME_OUTPUT_DIRECTORY settings.
♻️ Duplicate comments (3)
src/plugins/manus/install-dependencies.sh (1)
84-84: 🧹 Nitpick | 🔵 TrivialQuote
$(nproc)to follow shell best practices.Static analysis flagged unquoted command substitution. While unlikely to cause issues in practice, quoting is best practice.
🔧 Proposed fix
- make -j$(nproc) && make -j$(nproc) check && make install && ldconfig + make -j"$(nproc)" && make -j"$(nproc)" check && make install && ldconfigAlso apply to lines 115 and 139:
- make -j$(nproc) + make -j"$(nproc)"-make -j$(nproc) +make -j"$(nproc)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/manus/install-dependencies.sh` at line 84, The make invocation uses unquoted command substitution (make -j$(nproc)) which static analysis flagged; update every occurrence of the job-count invocation (instances of the literal string "make -j$(nproc)") to quote the substitution as make -j"$(nproc)" (apply the same change to all occurrences, e.g., the ones around the build/check/install commands that include ldconfig), ensuring you only change the quoting around $(nproc) and not other flags or commands.src/plugins/manus/install_manus.sh (1)
188-188: 🧹 Nitpick | 🔵 TrivialQuote
$(nproc)for consistency.🔧 Proposed fix
-cmake --build build --target manus_hand_plugin manus_hand_tracker_printer -j$(nproc) +cmake --build build --target manus_hand_plugin manus_hand_tracker_printer -j"$(nproc)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/manus/install_manus.sh` at line 188, The cmake build invocation uses an unquoted command substitution for the parallel jobs flag; update the cmake command that builds the manus targets (the line invoking cmake --build with targets manus_hand_plugin and manus_hand_tracker_printer) to quote the $(nproc) substitution (e.g. use -j"$(nproc)" or -j "$(nproc)") so the shell treats the expansion consistently and safely.docs/source/device/manus.rst (1)
100-105:⚠️ Potential issue | 🟡 MinorDocument the CloudXR setup path consistently.
Lines 100-105 only show manual
exports, while Lines 167-168 tell readers to sourcescripts/setup_cloudxr_env.sh. Either document both options in both places or standardize on one flow; right now the troubleshooting note contradicts the main run instructions.Also applies to: 167-168
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/source/device/manus.rst` around lines 100 - 105, The docs currently show manual export commands (NV_CXR_RUNTIME_DIR and XR_RUNTIME_JSON) in one place and instruct readers to source scripts/setup_cloudxr_env.sh elsewhere; make these consistent by choosing one canonical setup flow and updating both locations to match: either replace the manual export block with a note telling users to source scripts/setup_cloudxr_env.sh (and mention that the script sets NV_CXR_RUNTIME_DIR and XR_RUNTIME_JSON), or insert the explicit export commands into the later troubleshooting section; update any text that references “source the CloudXR environment” to use the same phrasing (e.g., “source scripts/setup_cloudxr_env.sh” or “export NV_CXR_RUNTIME_DIR and XR_RUNTIME_JSON”) so both the main run instructions and the troubleshooting note are identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/source/device/manus.rst`:
- Around line 138-140: Update the sentence to match the implementation: state
that when XR_MNDX_xdev_space is supported the plugin prefers the XDev wrist pose
and only falls back to the controller pose if XDev returns no valid wrist pose;
clarify that if XDev returns a valid-but-untracked wrist pose the plugin keeps
that pose but clears the tracked bits rather than falling back. Reference
XR_MNDX_xdev_space and the logic in manus_hand_tracking_plugin.cpp that
distinguishes "no valid wrist pose" from "valid-but-untracked" so the docs
reflect the actual behavior.
In `@src/plugins/manus/core/manus_hand_tracking_plugin.cpp`:
- Around line 415-423: The cached node topology (tracker.m_left_node_info and
tracker.m_right_node_info) isn’t cleared when CoreSdk_GetRawSkeletonNodeCount
fails or returns 0, leaving stale topology on reconnect; update the connect
handling in manus_hand_tracking_plugin.cpp around the
CoreSdk_GetRawSkeletonNodeCount calls so you explicitly clear
tracker.m_left_node_info (and symmetrically tracker.m_right_node_info in the
other block) before attempting to repopulate, and also clear them on any
non-success return from CoreSdk_GetRawSkeletonNodeInfoArray; use the existing
mutex (tracker.m_skeleton_mutex) while clearing/resizing to avoid races and
mirror this change in the other occurrence referenced (the block at lines
429-436).
In `@src/plugins/manus/README.md`:
- Line 76: Fix the Markdown formatting in README.md by ensuring each heading
(e.g., "Setup CloudXR Environment" and the other headings referenced around the
sections for lines ~86, ~95, and ~102-106) is followed by a blank line, and
remove the trailing space at the end of the line currently at line 106; update
the headings and the paragraph breaks so all top-level and sub-headings have a
single blank line after them and trim any trailing whitespace on the affected
lines.
- Around line 117-122: The fenced code block in README.md lacks blank lines
before and after which violates Markdown lint; edit the Manus README (around the
USB permission instructions) to ensure there is an empty line before the opening
```bash and an empty line after the closing ``` so the block is separated from
surrounding text and the commands ("sudo udevadm control --reload-rules" / "sudo
udevadm trigger") are on their own fenced block lines.
In `@src/plugins/manus/tools/CMakeLists.txt`:
- Around line 4-5: CMake now marks Vulkan and X11 as REQUIRED via
find_package(Vulkan REQUIRED) and find_package(X11 REQUIRED), but the installer
scripts don't install the corresponding dev packages; update the package lists
in install_manus.sh and install-dependencies.sh to include libvulkan-dev and
libx11-dev (and any distro-specific aliases if your script supports multiple
distros) so CMake can find those libraries during configure; ensure you add them
to the same apt/yum/dnf install lines used for other dev libs and update any
package-check or dependency arrays the scripts use.
In `@src/plugins/manus/tools/manus_hand_tracker_printer.cpp`:
- Around line 28-40: The detached vis_thread captures tracker by reference which
risks accessing a destroyed singleton; replace the detached std::thread with a
std::jthread (or a joinable std::thread plus a shutdown signal) and ensure the
visualizer gets a safe owning or lifetime-managed handle (e.g., pass a
std::shared_ptr to tracker or copy any needed state) instead of a raw reference;
also update plugins::manus::HandVisualizer::run to accept a std::stop_token (or
a custom shutdown flag) and periodically check it so the jthread can request
shutdown cleanly before destruction.
In `@src/plugins/manus/tools/manus_hand_visualizer.hpp`:
- Around line 271-289: The HandVisualizer() constructor can throw after creating
native resources (initXlib(), initVulkan(), createSwapchain(),
createRenderPass(), createPipeline(), createFramebuffers(),
createCommandBuffers(), createSyncObjects(), createVertexBuffer()) and so the
destructor won't run; add a dedicated cleanup helper (e.g.,
cleanupPartialResources() or teardownResources()) that releases X11/Vulkan
handles created so far and call it from the catch block before rethrowing, or
better yet replace the raw handles with RAII wrapper members so partial
construction is safely undone automatically; keep the destructor for full
cleanup but ensure the catch path explicitly invokes the shared teardown helper
or relies on RAII members to avoid leaks.
- Around line 988-1002: Move the vkResetFences(m_dev, 1, &m_fence_in_flight)
call so it happens only after a successful vkAcquireNextImageKHR() (i.e., after
acquiring img_idx), not before acquisition, to avoid leaving the fence
unsignaled when vkAcquireNextImageKHR() returns VK_ERROR_OUT_OF_DATE_KHR; also,
after the swapchain recreation paths that call destroySwapchainResources(),
createSwapchain(), and createFramebuffers() (both in the vkAcquireNextImageKHR()
error branch and in the vkQueuePresentKHR() error branch), call
createCommandBuffers() to rebuild m_cmd_bufs to match the new m_sc_images size
so subsequent accesses like m_cmd_bufs[img_idx] cannot go out-of-bounds.
---
Outside diff comments:
In `@src/plugins/manus/tools/CMakeLists.txt`:
- Around line 23-39: The file contains two identical set_target_properties
blocks for the target manus_hand_tracker_printer; remove the duplicate block so
the properties are only set once. Locate the two set_target_properties(...)
invocations that mention manus_hand_tracker_printer and delete one of them,
ensuring the remaining block retains CXX_VISIBILITY_PRESET,
VISIBILITY_INLINES_HIDDEN, BUILD_WITH_INSTALL_RPATH, SKIP_BUILD_RPATH,
BUILD_RPATH and RUNTIME_OUTPUT_DIRECTORY settings.
---
Duplicate comments:
In `@docs/source/device/manus.rst`:
- Around line 100-105: The docs currently show manual export commands
(NV_CXR_RUNTIME_DIR and XR_RUNTIME_JSON) in one place and instruct readers to
source scripts/setup_cloudxr_env.sh elsewhere; make these consistent by choosing
one canonical setup flow and updating both locations to match: either replace
the manual export block with a note telling users to source
scripts/setup_cloudxr_env.sh (and mention that the script sets
NV_CXR_RUNTIME_DIR and XR_RUNTIME_JSON), or insert the explicit export commands
into the later troubleshooting section; update any text that references “source
the CloudXR environment” to use the same phrasing (e.g., “source
scripts/setup_cloudxr_env.sh” or “export NV_CXR_RUNTIME_DIR and
XR_RUNTIME_JSON”) so both the main run instructions and the troubleshooting note
are identical.
In `@src/plugins/manus/install_manus.sh`:
- Line 188: The cmake build invocation uses an unquoted command substitution for
the parallel jobs flag; update the cmake command that builds the manus targets
(the line invoking cmake --build with targets manus_hand_plugin and
manus_hand_tracker_printer) to quote the $(nproc) substitution (e.g. use
-j"$(nproc)" or -j "$(nproc)") so the shell treats the expansion consistently
and safely.
In `@src/plugins/manus/install-dependencies.sh`:
- Line 84: The make invocation uses unquoted command substitution (make
-j$(nproc)) which static analysis flagged; update every occurrence of the
job-count invocation (instances of the literal string "make -j$(nproc)") to
quote the substitution as make -j"$(nproc)" (apply the same change to all
occurrences, e.g., the ones around the build/check/install commands that include
ldconfig), ensuring you only change the quoting around $(nproc) and not other
flags or commands.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: c80c078e-0ac4-489c-a944-6254a5229aeb
📒 Files selected for processing (11)
docs/source/device/index.rstdocs/source/device/manus.rstscripts/run_isaac_lab.shsrc/plugins/manus/README.mdsrc/plugins/manus/core/manus_hand_tracking_plugin.cppsrc/plugins/manus/inc/core/manus_hand_tracking_plugin.hppsrc/plugins/manus/install-dependencies.shsrc/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
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
docs/source/device/manus.rst (1)
136-138:⚠️ Potential issue | 🟡 MinorRecheck the XDev fallback wording.
This sentence says controller fallback happens whenever the optical wrist is not actively tracked. If
src/plugins/manus/core/manus_hand_tracking_plugin.cppstill falls back only when XDev fails to return a valid wrist pose, this wording is too strict and will mislead troubleshooting. If the branch is keyed offXR_SPACE_LOCATION_POSITION_VALID_BITrather thanXR_SPACE_LOCATION_POSITION_TRACKED_BIT, please document that exact condition here.#!/bin/bash # Inspect the wrist-pose selection logic used by the Manus plugin. rg -n -C5 'inject_hand_data|get_controller_wrist_pose|XR_SPACE_LOCATION_POSITION_VALID_BIT|XR_SPACE_LOCATION_POSITION_TRACKED_BIT|m_xdev_available' src/plugins/manus/core/manus_hand_tracking_plugin.cpp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/source/device/manus.rst` around lines 136 - 138, Confirm the actual wrist-pose selection logic in manus_hand_tracking_plugin.cpp (check functions inject_hand_data, get_controller_wrist_pose and the m_xdev_available flag) and update the docs text to state the precise fallback condition: whether the plugin falls back to controller pose when XR_MNDX_xdev_space is unavailable (m_xdev_available==false) or when the runtime returns a wrist pose lacking XR_SPACE_LOCATION_POSITION_VALID_BIT versus XR_SPACE_LOCATION_POSITION_TRACKED_BIT; change the sentence to explicitly name which bit is checked (POSITION_VALID_BIT or POSITION_TRACKED_BIT) and/or that the fallback occurs only when m_xdev_available is false, matching the code exactly.
🤖 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 use MODE:="0666" which makes matched
USB and hidraw devices world-writable; update the rules matching
SUBSYSTEMS=="usb" with ATTRS{idVendor}=="3325" and ATTRS{idVendor}=="1915",
ATTRS{idProduct}=="83fd" and the KERNEL=="hidraw*" rule to remove MODE:="0666"
and instead assign a group and restrictive mode (for example set MODE:="0660"
and GROUP:="plugdev" or GROUP:="manus"), and optionally add TAG+="uaccess" if
you want the active logged-in user to get access; ensure all three rule lines
use the same GROUP and MODE change so devices are not world-writable.
In `@src/plugins/manus/install_manus.sh`:
- Around line 41-46: The script only installs
/etc/udev/rules.d/70-manus-hid.rules when the file is missing; change it to
detect and refresh the installed rules when the bundled file at
"$SCRIPT_DIR/70-manus-hid.rules" differs (or is newer) so updates self-repair
USB access; implement this by comparing the source and destination (e.g., using
cmp -s or checksums) and if they differ, copy the new file with sudo (preserving
mode/ownership as needed) and run sudo udevadm control --reload-rules (and
optionally sudo udevadm trigger) to apply the change, referencing the existing
variables and commands (SCRIPT_DIR, 70-manus-hid.rules, udevadm control
--reload-rules).
- Around line 145-147: The install script uses "cmake --install build
--component manus" but none of the Manus CMake install rules declare COMPONENT
manus (targets: manus_hand_plugin, manus_plugin_core, SDK library in
src/plugins/manus/CMakeLists.txt and plugin files in
src/plugins/manus/app/CMakeLists.txt) and the built tool
manus_hand_tracker_printer has no install rule; fix by either removing the
--component manus filter from install_manus.sh or by adding COMPONENT manus to
all relevant install(...) calls and adding an install(...) rule for
manus_hand_tracker_printer in src/plugins/manus/tools/CMakeLists.txt so the
cmake install command actually installs the expected artifacts.
In `@src/plugins/manus/README.md`:
- Around line 66-70: The manual build instructions only build the
manus_hand_plugin target but later verification expects the
manus_hand_tracker_printer binary; update the commands to also build (and
install if needed) the printer target by running cmake --build build --target
manus_hand_tracker_printer -j (or build both targets or the appropriate
component during cmake --install) so that manus_hand_tracker_printer exists for
the verification step; update the README steps that currently reference
manus_hand_plugin to include manus_hand_tracker_printer.
---
Duplicate comments:
In `@docs/source/device/manus.rst`:
- Around line 136-138: Confirm the actual wrist-pose selection logic in
manus_hand_tracking_plugin.cpp (check functions inject_hand_data,
get_controller_wrist_pose and the m_xdev_available flag) and update the docs
text to state the precise fallback condition: whether the plugin falls back to
controller pose when XR_MNDX_xdev_space is unavailable (m_xdev_available==false)
or when the runtime returns a wrist pose lacking
XR_SPACE_LOCATION_POSITION_VALID_BIT versus
XR_SPACE_LOCATION_POSITION_TRACKED_BIT; change the sentence to explicitly name
which bit is checked (POSITION_VALID_BIT or POSITION_TRACKED_BIT) and/or that
the fallback occurs only when m_xdev_available is false, matching the code
exactly.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: edb58f2b-32ce-479e-a621-732ba2e1afaa
📒 Files selected for processing (4)
docs/source/device/manus.rstsrc/plugins/manus/70-manus-hid.rulessrc/plugins/manus/README.mdsrc/plugins/manus/install_manus.sh
Comment on lines
+2
to
+5
| SUBSYSTEMS=="usb", ATTRS{idVendor}=="3325", MODE:="0666" | ||
| SUBSYSTEMS=="usb", ATTRS{idVendor}=="1915", ATTRS{idProduct}=="83fd", MODE:="0666" | ||
| # HIDAPI/hidraw | ||
| KERNEL=="hidraw*", ATTRS{idVendor}=="3325", MODE:="0666" |
There was a problem hiding this comment.
Avoid world-writable udev rules.
MODE:="0666" grants every local user raw read/write access to the matched USB and hidraw devices. Because install_manus.sh installs this file into /etc/udev/rules.d, that becomes the host-wide default. Please scope access to the active user or a dedicated group instead of making the devices universally writable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/plugins/manus/70-manus-hid.rules` around lines 2 - 5, The udev rules
currently use MODE:="0666" which makes matched USB and hidraw devices
world-writable; update the rules matching SUBSYSTEMS=="usb" with
ATTRS{idVendor}=="3325" and ATTRS{idVendor}=="1915", ATTRS{idProduct}=="83fd"
and the KERNEL=="hidraw*" rule to remove MODE:="0666" and instead assign a group
and restrictive mode (for example set MODE:="0660" and GROUP:="plugdev" or
GROUP:="manus"), and optionally add TAG+="uaccess" if you want the active
logged-in user to get access; ensure all three rule lines use the same GROUP and
MODE change so devices are not world-writable.
jiwenc-nv
approved these changes
Apr 5, 2026
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
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.
HMD Optical Hand Tracking
Summary
This PR adds support for using the HMD's built-in optical hand tracking (via the
XR_MNDX_xdev_spaceextension) as the wrist position source when using Manus gloves, along with automated installation scripts and improved documentation.Changes
HMD optical hand tracking wrist source (
XR_MNDX_xdev_space)XR_MNDX_xdev_spaceandXR_EXT_hand_trackingextension function pointers. If both are available, it creates two nativeXrHandTrackerEXThandles backed by the HMD's optical tracking ("Head Device (0)" = left, "Head Device (1)" = right).inject_hand_data()now auto-selects the wrist pose source: HMD optical tracking when available (m_xdev_available == true), falling back silently to controller aim-pose + wrist offset when not.[Manus] Initialized with wrist source: HandTrackingorControllers.initialize_xdev_hand_trackers(),cleanup_xdev_hand_trackers(),update_xdev_hand(),get_controller_wrist_pose().m_handlesis now stored as a member so it can be reused across the new helper methods without fetching it repeatedly.HandTrackeris now registered alongsideControllerTrackerin theDeviceIOSession.XR_MNDX_xdev_spaceis requested as a required extension at session creation time.std::coutlog messages prefixed with[Manus]for consistent filtering.manus_hand_tracker_printer.cpp
[Manus]to match the plugin log format.install_manus.sh (new file)
All-in-one installation script:
install-dependencies.sh (new file)
Full dependency installer for the Remote mode:
abseil-cpppatch required for GCC 11+ compatibility.ldconfig.README.md
install_manus.sh.setup_cloudxr_env.sh,run_cloudxr.sh) as a prerequisite to running the plugin.Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores