Add Noitom full-body teleop, recording, and replay support#742
Add Noitom full-body teleop, recording, and replay support#742Astrorix wants to merge 10 commits into
Conversation
Signed-off-by: maji <maji@connect.hku.hk>
…replay) Signed-off-by: maji <maji@connect.hku.hk>
Signed-off-by: maji <maji@connect.hku.hk>
Signed-off-by: maji <maji@connect.hku.hk>
Signed-off-by: maji <maji@connect.hku.hk>
Signed-off-by: maji <maji@connect.hku.hk> # Conflicts: # CMakeLists.txt # src/core/replay_trackers/cpp/replay_deviceio_factory.cpp
Signed-off-by: maji <maji@connect.hku.hk>
Signed-off-by: maji <maji@connect.hku.hk>
|
📝 Docs preview is not auto-deployed for fork PRs. A maintainer with write access to |
📝 WalkthroughWalkthroughThis PR adds full support for Noitom Perception Neuron mocap devices: a new FullBodyPosePN FlatBuffers schema with C++/Python bindings, deviceio/live/replay tracker implementations, a standalone Linux plugin bundling the vendored MocapApi SDK, retargeting math mapping mocap data to a G1 robot's wrists/elbows/shoulders, an Isaac Lab example task with Pink IK wiring, and record/replay MCAP CLI/visualization tools. Build options, REUSE/license metadata, and pre-commit exclusions were also added for the vendored SDK. Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant NoitomMocapPlugin
participant SchemaPusher
participant FullBodyTrackerPN
participant NoitomG1Retargeter
participant NoitomG1ActionSource
NoitomMocapPlugin->>SchemaPusher: push_frame(FullBodyPosePN)
FullBodyTrackerPN->>NoitomG1ActionSource: get_body_pose() via DeviceIO
NoitomG1ActionSource->>NoitomG1Retargeter: calibrate(frame) / retarget(frame)
NoitomG1Retargeter-->>NoitomG1ActionSource: ArmIkTargets
NoitomG1ActionSource->>NoitomG1ActionSource: _make_action(targets) -> G1 action vector
Estimated code review effort5 (Critical) | ~120 minutes Related Issues: None referenced in the provided information. Related PRs: None referenced in the provided information. Suggested labels: feature, plugin, python, cpp, third-party Suggested reviewers: None specified in the provided information. 🐇 A mocap suit, a robot's grace, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds end-to-end Noitom Perception Neuron full-body support to IsaacTeleop: a Linux plugin that publishes FullBodyPosePN frames over OpenXR tensor data, plus schema/deviceio plumbing, MCAP record/replay integration, and a G1 retargeting example workflow.
Changes:
- Introduce the
FullBodyPosePNFlatBuffer schema + C++/Python bindings, and wire a newFullBodyTrackerPNthrough live and replay DeviceIO stacks (including MCAP recording traits). - Add the
noitom_mocapplugin (C++ executable + CMake + plugin.yaml) that connects to the Noitom MocapApi SDK and pushesfull_body_pnsamples. - Add Noitom record/replay + G1 teleop example scripts and documentation (with GIFs), plus REUSE licensing annotations for the bundled SDK.
Reviewed changes
Copilot reviewed 53 out of 62 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/plugins/noitom_mocap/tools/noitom_mocap_printer.py | CLI tool to print live PN full-body frames. |
| src/plugins/noitom_mocap/third_party/mocap_api/include/MocapApi/MocapApi.h | Bundled vendor SDK header (Noitom MocapApi). |
| src/plugins/noitom_mocap/third_party/mocap_api/init.py | Python package export surface for vendor SDK bindings. |
| src/plugins/noitom_mocap/third_party/.gitignore | Third-party subtree ignore rules for the SDK drop. |
| src/plugins/noitom_mocap/README.md | Plugin build/run instructions and quick workflows. |
| src/plugins/noitom_mocap/plugin.yaml | Plugin manifest for discovery/launch. |
| src/plugins/noitom_mocap/noitom_mocap_plugin.hpp | Plugin interface/config and SDK API handles. |
| src/plugins/noitom_mocap/noitom_mocap_plugin.cpp | Noitom SDK polling, joint mapping, OpenXR tensor push. |
| src/plugins/noitom_mocap/main.cpp | Plugin CLI + main loop + signal handling. |
| src/plugins/noitom_mocap/CMakeLists.txt | Linux-only build/install for noitom_mocap_plugin and SDK .so. |
| src/core/schema/python/schema_module.cpp | Registers PN schema bindings in _schema module. |
| src/core/schema/python/schema_init.py | Exposes PN schema types in isaacteleop.schema. |
| src/core/schema/python/full_body_pn_bindings.h | Pybind bindings for PN schema object API types. |
| src/core/schema/python/CMakeLists.txt | Adds PN bindings header to schema py module build. |
| src/core/schema/fbs/full_body_pn.fbs | Defines PN joint enum, pose tables, and record wrapper. |
| src/core/schema_tests/python/test_full_body.py | Python tests for PN schema binding basics. |
| src/core/schema_tests/cpp/test_full_body.cpp | C++ FlatBuffers serialization test for PN record wrapper. |
| src/core/retargeting_engine/python/deviceio_source_nodes/deviceio_tensor_types.py | Adds PN tracked tensor type + group type. |
| src/core/retargeting_engine/python/deviceio_source_nodes/init.py | Exports PN tracked tensor type/group. |
| src/core/replay_trackers/cpp/replay_full_body_tracker_pn_impl.hpp | Replay impl interface for PN full-body tracker. |
| src/core/replay_trackers/cpp/replay_full_body_tracker_pn_impl.cpp | Replays PN body frames from MCAP channels. |
| src/core/replay_trackers/cpp/replay_deviceio_factory.cpp | Creates PN tracker impls for replay sessions. |
| src/core/replay_trackers/cpp/inc/replay_trackers/replay_deviceio_factory.hpp | Factory API extended for PN tracker impl creation. |
| src/core/replay_trackers/cpp/CMakeLists.txt | Adds PN replay impl sources to replay_trackers lib. |
| src/core/python/deviceio_init.py | Exposes FullBodyTrackerPN in isaacteleop.deviceio. |
| src/core/mcap/cpp/inc/mcap/recording_traits.hpp | Adds PN recording/replay channel traits. |
| src/core/live_trackers/cpp/live_full_body_tracker_pn_impl.hpp | Live schema-tracker impl for PN full-body tracker. |
| src/core/live_trackers/cpp/live_full_body_tracker_pn_impl.cpp | Live PN schema update + optional MCAP recording channels. |
| src/core/live_trackers/cpp/live_full_body_tracker_pico_impl.hpp | Minor include update alongside PN additions. |
| src/core/live_trackers/cpp/live_deviceio_factory.cpp | Creates PN tracker impls for live sessions. |
| src/core/live_trackers/cpp/inc/live_trackers/live_deviceio_factory.hpp | Factory API extended for PN tracker impl creation. |
| src/core/live_trackers/cpp/CMakeLists.txt | Adds PN live impl sources to live_trackers lib. |
| src/core/deviceio_trackers/python/tracker_bindings.cpp | Adds pybind wrapper for FullBodyTrackerPN. |
| src/core/deviceio_trackers/python/deviceio_trackers_init.py | Exposes FullBodyTrackerPN in python tracker package. |
| src/core/deviceio_trackers/cpp/inc/deviceio_trackers/full_body_tracker_pn.hpp | New tracker front-end for PN schema-tracked poses. |
| src/core/deviceio_trackers/cpp/full_body_tracker_pn.cpp | Implements PN tracker session dispatch. |
| src/core/deviceio_trackers/cpp/full_body_tracker_pico.cpp | Removes redundant comment block (minor cleanup). |
| src/core/deviceio_trackers/cpp/CMakeLists.txt | Adds PN tracker sources/headers to deviceio_trackers lib. |
| src/core/deviceio_base/cpp/inc/deviceio_base/full_body_tracker_pn_base.hpp | Adds abstract impl interface for PN full-body tracker. |
| REUSE.toml | Adds REUSE annotation for bundled MocapApi SDK subtree. |
| LICENSES/LicenseRef-Noitom-MocapApi-SDK.txt | Adds license reference stub for the SDK drop. |
| examples/noitom/replay_noitom_full_body.py | Headless replay + visualization of PN MCAP recordings. |
| examples/noitom/record_noitom_full_body.py | Records PN frames to MCAP via DeviceIO tracker. |
| examples/noitom/README.md | End-user teleop + record/replay workflows + behavior notes. |
| examples/noitom/noitom_tasks.py | External Isaac Lab task using PN full-body retargeting. |
| examples/noitom/noitom_reference_draw.py | Shared reference skeleton placement/drawing utilities. |
| examples/noitom/assets/teleop.gif | Teleop demo asset (LFS). |
| examples/noitom/assets/replay.gif | Replay demo asset (LFS). |
| examples/noitom/assets/record.gif | Recording demo asset (LFS). |
| CMakeLists.txt | Adds build option and subdir for noitom_mocap plugin. |
| .pre-commit-config.yaml | Excludes bundled SDK subtree from formatting/lint hooks. |
| if (*slot == core::BodyJointPN_LEFT_WRIST) | ||
| { | ||
| frame_.joints->mutable_joints()->Mutate(core::BodyJointPN_LEFT_HAND, joint_pose); | ||
| seen[core::BodyJointPN_LEFT_HAND] = valid; | ||
| } | ||
| else if (*slot == core::BodyJointPN_RIGHT_WRIST) | ||
| { | ||
| frame_.joints->mutable_joints()->Mutate(core::BodyJointPN_RIGHT_HAND, joint_pose); | ||
| seen[core::BodyJointPN_RIGHT_HAND] = valid; | ||
| } | ||
| else if (*slot == core::BodyJointPN_LEFT_ANKLE) | ||
| { | ||
| frame_.joints->mutable_joints()->Mutate(core::BodyJointPN_LEFT_FOOT, joint_pose); | ||
| seen[core::BodyJointPN_LEFT_FOOT] = valid; | ||
| } | ||
| else if (*slot == core::BodyJointPN_RIGHT_ANKLE) | ||
| { | ||
| frame_.joints->mutable_joints()->Mutate(core::BodyJointPN_RIGHT_FOOT, joint_pose); | ||
| seen[core::BodyJointPN_RIGHT_FOOT] = valid; | ||
| } |
| `replay_noitom_full_body.py` uses the Python `mcap` package to infer playback | ||
| duration from the file summary. If that package is unavailable in your test | ||
| environment, pass a duration explicitly: | ||
|
|
||
| ```bash | ||
| python examples/noitom/replay_noitom_full_body.py /tmp/noitom_full_body.mcap --duration 10 | ||
| ``` |
| # The Noitom MocapApi SDK | ||
| !mocap_api/lib/arm64/libMocapApi.so | ||
| !mocap_api/lib/x86_64/libMocapApi.so |
There was a problem hiding this comment.
+1 to this. probably it's not your intension to check in the mocap_api sdk into Isaac Teleop repo, right?
probably good to do something like https://github.com/NVIDIA/IsaacTeleop/blob/main/src/plugins/manus/install_manus.sh to download your from Noitom hosted sites.
| if os.name == 'nt': | ||
| os.add_dll_directory(str(mocap_lib_file.parent)) | ||
|
|
||
| print(f'mocap_lib_file={mocap_lib_file}') |
| By default the recorder reads collection `noitom_mocap`, writes MCAP channel | ||
| base `full_body_pn`, and stores the file under `examples/noitom/recordings/`. | ||
| Override paths or ids as needed: | ||
|
|
||
| ```bash | ||
| python examples/noitom/record_noitom_full_body.py \ | ||
| --collection-id noitom_mocap \ | ||
| --channel-name full_body_pn \ | ||
| --duration 10 \ | ||
| --output /tmp/noitom_full_body.mcap | ||
| ``` |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@src/core/replay_trackers/cpp/replay_full_body_tracker_pn_impl.cpp`:
- Around line 30-42: The update() method in ReplayFullBodyTrackerPNImpl is
logging the missing body-data case on every tick after read(0) starts returning
std::nullopt, which spams stderr. Update the ReplayFullBodyTrackerPNImpl::update
logic to suppress repeated EOF/miss logging by tracking whether the miss has
already been reported, or otherwise rate-limit the message so it only logs once
when replay is exhausted.
In `@src/plugins/noitom_mocap/third_party/mocap_api/mocap_api.py`:
- Around line 1275-1278: The two API wrapper methods reference non-existent
ctypes fields, so update the call sites in set_bvh_displacement and
get_rotating_direction to use the actual MCPSettingsApi members defined on the
struct. Replace the mismatched SetSettingsBvhDisplacement access with the
corresponding existing BVH settings function, and change GetRotatingDirection to
the correctly named GetRotationDirection so both wrappers invoke valid API
symbols and no longer raise AttributeError.
- Line 317: The joint-position error path is using the misspelled
MCPError._fileds lookup, which can throw AttributeError instead of the intended
RuntimeError. Update the error message construction in the joint position lookup
handling to use MCPError._fields in the raise RuntimeError call so the failure
reports the correct mapped error text.
- Around line 629-634: In get_count_of_support_poses, the
GetCalibrateMotionProgressCountOfSupportPoses call is passing count by value
instead of as a POINTER(c_uint32), which will fail at runtime. Update the
invocation in mocap_api.py to pass a wrapped pointer (using byref(count) or
pointer(count)) while keeping the existing error handling and return of
count.value intact.
- Around line 985-1021: The GetSensorModules binding and call in
MCPSystemApi/get_sensor_modules use the wrong ctypes signature and argument
order, so the native call receives corrupted parameters. Update the CFUNCTYPE
declaration for GetSensorModules to match the C++ signature in MocapApi.h, using
pointer-based arguments for the sensor module output and count, and fix
get_sensor_modules to pass the correct argument types instead of a c_char_p
buffer. Keep the changes localized to MCPSystemApi and get_sensor_modules so the
Python wrapper matches the native API exactly.
- Around line 731-734: `create_command` is passing `self.handle` by value into
`CreateCommand`, but that API expects a `POINTER(MCPCommandHandle)`, so the
ctypes call fails immediately. Update `create_command` in `mocap_api.py` to pass
the command handle by pointer using the existing
`self.handle`/`MCPCommandHandle` symbol, and keep the existing error check after
the call so `queue_command` and `queue_command_with_extra_long` can work
correctly.
🪄 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: 791d3eb6-11ec-41c0-a331-09a3a10dac65
⛔ Files ignored due to path filters (6)
examples/noitom/assets/record.gifis excluded by!**/*.gifexamples/noitom/assets/replay.gifis excluded by!**/*.gifexamples/noitom/assets/teleop.gifis excluded by!**/*.gifsrc/plugins/noitom_mocap/third_party/mocap_api/lib/amd64/MocapApi.dllis excluded by!**/*.dllsrc/plugins/noitom_mocap/third_party/mocap_api/lib/arm64/libMocapApi.sois excluded by!**/*.sosrc/plugins/noitom_mocap/third_party/mocap_api/lib/x86_64/libMocapApi.sois excluded by!**/*.so
📒 Files selected for processing (56)
.pre-commit-config.yamlCMakeLists.txtLICENSES/LicenseRef-Noitom-MocapApi-SDK.txtREUSE.tomlapplicationInfo.apiVersion:applicationInfo.applicationName:applicationInfo.applicationVersion:applicationInfo.engineName:applicationInfo.engineVersion:examples/noitom/README.mdexamples/noitom/noitom_reference_draw.pyexamples/noitom/noitom_retargeting.pyexamples/noitom/noitom_tasks.pyexamples/noitom/record_noitom_full_body.pyexamples/noitom/replay_noitom_full_body.pysrc/core/deviceio_base/cpp/inc/deviceio_base/full_body_tracker_pn_base.hppsrc/core/deviceio_trackers/cpp/CMakeLists.txtsrc/core/deviceio_trackers/cpp/full_body_tracker_pico.cppsrc/core/deviceio_trackers/cpp/full_body_tracker_pn.cppsrc/core/deviceio_trackers/cpp/inc/deviceio_trackers/full_body_tracker_pn.hppsrc/core/deviceio_trackers/python/deviceio_trackers_init.pysrc/core/deviceio_trackers/python/tracker_bindings.cppsrc/core/live_trackers/cpp/CMakeLists.txtsrc/core/live_trackers/cpp/inc/live_trackers/live_deviceio_factory.hppsrc/core/live_trackers/cpp/live_deviceio_factory.cppsrc/core/live_trackers/cpp/live_full_body_tracker_pico_impl.hppsrc/core/live_trackers/cpp/live_full_body_tracker_pn_impl.cppsrc/core/live_trackers/cpp/live_full_body_tracker_pn_impl.hppsrc/core/mcap/cpp/inc/mcap/recording_traits.hppsrc/core/python/deviceio_init.pysrc/core/replay_trackers/cpp/CMakeLists.txtsrc/core/replay_trackers/cpp/inc/replay_trackers/replay_deviceio_factory.hppsrc/core/replay_trackers/cpp/replay_deviceio_factory.cppsrc/core/replay_trackers/cpp/replay_full_body_tracker_pn_impl.cppsrc/core/replay_trackers/cpp/replay_full_body_tracker_pn_impl.hppsrc/core/retargeting_engine/python/deviceio_source_nodes/__init__.pysrc/core/retargeting_engine/python/deviceio_source_nodes/deviceio_tensor_types.pysrc/core/schema/fbs/full_body_pn.fbssrc/core/schema/python/CMakeLists.txtsrc/core/schema/python/full_body_pn_bindings.hsrc/core/schema/python/schema_init.pysrc/core/schema/python/schema_module.cppsrc/core/schema_tests/cpp/test_full_body.cppsrc/core/schema_tests/python/test_full_body.pysrc/plugins/noitom_mocap/CMakeLists.txtsrc/plugins/noitom_mocap/README.mdsrc/plugins/noitom_mocap/main.cppsrc/plugins/noitom_mocap/noitom_mocap_plugin.cppsrc/plugins/noitom_mocap/noitom_mocap_plugin.hppsrc/plugins/noitom_mocap/plugin.yamlsrc/plugins/noitom_mocap/third_party/.gitignoresrc/plugins/noitom_mocap/third_party/mocap_api/__init__.pysrc/plugins/noitom_mocap/third_party/mocap_api/include/MocapApi/MocapApi.hsrc/plugins/noitom_mocap/third_party/mocap_api/lib/amd64/MocapApi.libsrc/plugins/noitom_mocap/third_party/mocap_api/mocap_api.pysrc/plugins/noitom_mocap/tools/noitom_mocap_printer.py
💤 Files with no reviewable changes (1)
- src/core/deviceio_trackers/cpp/full_body_tracker_pico.cpp
| void ReplayFullBodyTrackerPNImpl::update(int64_t /*monotonic_time_ns*/) | ||
| { | ||
| auto record = mcap_viewers_->read(0); | ||
| if (record) | ||
| { | ||
| tracked_.data = std::move(record->data); | ||
| } | ||
| else | ||
| { | ||
| std::cerr << "ReplayFullBodyTrackerPNImpl: body data not found" << std::endl; | ||
| tracked_.data.reset(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect McapTrackerViewers::read semantics and how other trackers (e.g. Pico) handle a miss.
rg -n "class McapTrackerViewers" -A 30 src/core/mcap/cpp/inc/mcap/tracker_channels.hpp
rg -n "read(0)|body data not found|cerr" src/core/replay_trackers/cpp/*.cppRepository: NVIDIA/IsaacTeleop
Length of output: 2985
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== McapTrackerViewers read implementation =="
rg -n "read\\(" src/core/mcap/cpp/inc/mcap/tracker_channels.hpp src/core/mcap/cpp -A 80 -B 20
echo
echo "== ReplayFullBodyTrackerPNImpl and peer tracker patterns =="
for f in \
src/core/replay_trackers/cpp/replay_full_body_tracker_pn_impl.cpp \
src/core/replay_trackers/cpp/replay_full_body_tracker_pico_impl.cpp \
src/core/replay_trackers/cpp/replay_head_tracker_impl.cpp \
src/core/replay_trackers/cpp/replay_hand_tracker_impl.cpp \
src/core/replay_trackers/cpp/replay_joint_state_tracker_impl.cpp \
src/core/replay_trackers/cpp/replay_controller_tracker_impl.cpp \
src/core/replay_trackers/cpp/replay_generic_3axis_pedal_tracker_impl.cpp
do
echo "--- $f ---"
cat -n "$f" | sed -n '1,140p'
doneRepository: NVIDIA/IsaacTeleop
Length of output: 42236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== read() implementation and EOF behavior =="
ast-grep outline src/core/mcap/cpp/inc/mcap/tracker_channels.hpp --view expanded || true
rg -n "read\\s*\\(" src/core/mcap/cpp -A 120 -B 40
echo
echo "== replay callers / loops that invoke update() =="
rg -n "update\\(.*monotonic_time_ns|update\\(" src/core/replay_trackers src/core -A 4 -B 4 | sed -n '1,220p'
echo
echo "== example / documentation references to replay ticking =="
rg -n "replay|monotonic_time_ns|body data not found|no record|EOF|end of data" . -g '!**/build/**' -g '!**/dist/**' -g '!**/.git/**' -A 2 -B 2 | sed -n '1,240p'Repository: NVIDIA/IsaacTeleop
Length of output: 50374
Suppress repeated EOF logging here. read(0) returns std::nullopt once the replay is exhausted, and update() keeps running every replay tick, so this will spam and flush stderr for the rest of playback. Rate-limit it or log only on the first miss.
🤖 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/replay_trackers/cpp/replay_full_body_tracker_pn_impl.cpp` around
lines 30 - 42, The update() method in ReplayFullBodyTrackerPNImpl is logging the
missing body-data case on every tick after read(0) starts returning
std::nullopt, which spams stderr. Update the ReplayFullBodyTrackerPNImpl::update
logic to suppress repeated EOF/miss logging by tracking whether the miss has
already been reported, or otherwise rate-limit the message so it only logs once
when replay is exhausted.
| z = c_float() | ||
| err = self.api.contents.GetJointPosition(pointer(x), pointer(y), pointer(z), self.handle) | ||
| if err != MCPError.NoError: | ||
| raise RuntimeError('Can not get joint position:{0}'.format(MCPError._fileds[err])) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Typo _fileds masks the real error.
MCPError._fileds[err] should be MCPError._fields[err]; as written this raises AttributeError instead of the intended RuntimeError with a useful message whenever a joint-position lookup fails.
🐛 Proposed fix
- raise RuntimeError('Can not get joint position:{0}'.format(MCPError._fileds[err]))
+ raise RuntimeError('Can not get joint position:{0}'.format(MCPError._fields[err]))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raise RuntimeError('Can not get joint position:{0}'.format(MCPError._fileds[err])) | |
| raise RuntimeError('Can not get joint position:{0}'.format(MCPError._fields[err])) |
🤖 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/plugins/noitom_mocap/third_party/mocap_api/mocap_api.py` at line 317, The
joint-position error path is using the misspelled MCPError._fileds lookup, which
can throw AttributeError instead of the intended RuntimeError. Update the error
message construction in the joint position lookup handling to use
MCPError._fields in the raise RuntimeError call so the failure reports the
correct mapped error text.
Source: Path instructions
| def get_count_of_support_poses(self): | ||
| count = c_uint32() | ||
| err = self.api.contents.GetCalibrateMotionProgressCountOfSupportPoses(count, self.handle) | ||
| if err != MCPError.NoError: | ||
| raise RuntimeError('GetProgress Count Of Support Poses with error {0}'.format(MCPError._fields[err])) | ||
| return count.value |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Missing pointer wrap on GetCalibrateMotionProgressCountOfSupportPoses call.
Field is declared CFUNCTYPE(c_int32, POINTER(c_uint32), MCPCalibrateMotionProgressHandle) (Line 611), but count is passed by value, not byref(count)/pointer(count) — will raise ctypes.ArgumentError if this method is ever called.
🐛 Proposed fix
def get_count_of_support_poses(self):
count = c_uint32()
- err = self.api.contents.GetCalibrateMotionProgressCountOfSupportPoses(count, self.handle)
+ err = self.api.contents.GetCalibrateMotionProgressCountOfSupportPoses(byref(count), self.handle)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_count_of_support_poses(self): | |
| count = c_uint32() | |
| err = self.api.contents.GetCalibrateMotionProgressCountOfSupportPoses(count, self.handle) | |
| if err != MCPError.NoError: | |
| raise RuntimeError('GetProgress Count Of Support Poses with error {0}'.format(MCPError._fields[err])) | |
| return count.value | |
| def get_count_of_support_poses(self): | |
| count = c_uint32() | |
| err = self.api.contents.GetCalibrateMotionProgressCountOfSupportPoses(byref(count), self.handle) | |
| if err != MCPError.NoError: | |
| raise RuntimeError('GetProgress Count Of Support Poses with error {0}'.format(MCPError._fields[err])) | |
| return count.value |
🤖 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/plugins/noitom_mocap/third_party/mocap_api/mocap_api.py` around lines 629
- 634, In get_count_of_support_poses, the
GetCalibrateMotionProgressCountOfSupportPoses call is passing count by value
instead of as a POINTER(c_uint32), which will fail at runtime. Update the
invocation in mocap_api.py to pass a wrapped pointer (using byref(count) or
pointer(count)) while keeping the existing error handling and return of
count.value intact.
Source: Path instructions
| def create_command(self, commandType): | ||
| err = self.api.contents.CreateCommand(commandType, self.handle) | ||
| if err != MCPError.NoError: | ||
| raise RuntimeError('CreateCommand failed with error {0}'.format(MCPError._fields[err])) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
create_command will crash immediately — missing pointer() wrap.
CreateCommand is declared CFUNCTYPE(c_int32, c_int32, POINTER(MCPCommandHandle)) (Line 716), but self.handle (a plain MCPCommandHandle/c_uint64 instance) is passed by value instead of by pointer. ctypes strictly checks POINTER argtypes, so this raises ctypes.ArgumentError on every call — breaking queue_command/queue_command_with_extra_long (start/stop capture, calibration, etc.).
🐛 Proposed fix
def create_command(self, commandType):
- err = self.api.contents.CreateCommand(commandType, self.handle)
+ err = self.api.contents.CreateCommand(commandType, pointer(self.handle))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def create_command(self, commandType): | |
| err = self.api.contents.CreateCommand(commandType, self.handle) | |
| if err != MCPError.NoError: | |
| raise RuntimeError('CreateCommand failed with error {0}'.format(MCPError._fields[err])) | |
| def create_command(self, commandType): | |
| err = self.api.contents.CreateCommand(commandType, pointer(self.handle)) | |
| if err != MCPError.NoError: | |
| raise RuntimeError('CreateCommand failed with error {0}'.format(MCPError._fields[err])) |
🤖 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/plugins/noitom_mocap/third_party/mocap_api/mocap_api.py` around lines 731
- 734, `create_command` is passing `self.handle` by value into `CreateCommand`,
but that API expects a `POINTER(MCPCommandHandle)`, so the ctypes call fails
immediately. Update `create_command` in `mocap_api.py` to pass the command
handle by pointer using the existing `self.handle`/`MCPCommandHandle` symbol,
and keep the existing error check after the call so `queue_command` and
`queue_command_with_extra_long` can work correctly.
Source: Path instructions
| class MCPSystemApi(Structure): | ||
| _fields_ = [ | ||
| ('GetMasterVersion', CFUNCTYPE(c_int32, POINTER(c_char_p), MCPSystemHandle)), | ||
| ('GetMasterSerialNumber', CFUNCTYPE(c_int32, POINTER(c_char_p), MCPSystemHandle)), | ||
| ('GetSensorModules', CFUNCTYPE(c_int32, MCPSensorModuleHandle, c_int32, POINTER(MCPSystemHandle))), | ||
| ] | ||
|
|
||
| api = POINTER(MCPSystemApi)() | ||
|
|
||
| def __init__(self, system_handle): | ||
| if not self.api: | ||
| self.api = POINTER(self.MCPSystemApi)() | ||
| err = MocapApi.MCPGetGenericInterface(self.IMCPSystem_Version, pointer(self.api)) | ||
| if err != MCPError.NoError: | ||
| raise RuntimeError('Cannot get MCPSystemApi: {0}'.format(MCPError._fields[err])) | ||
| self.handle = system_handle | ||
|
|
||
| def get_master_version(self): | ||
| pstr = c_char_p() | ||
| err = self.api.contents.GetMasterVersion(pointer(pstr), self.handle) | ||
| if err != MCPError.NoError: | ||
| raise RuntimeError('Can not get master version: {0}'.format(MCPError._fields[err])) | ||
| return str(pstr.value, encoding='utf8') | ||
|
|
||
| def get_master_serial_number(self): | ||
| pstr = c_char_p() | ||
| err = self.api.contents.GetMasterSerialNumber(pointer(pstr), self.handle) | ||
| if err != MCPError.NoError: | ||
| raise RuntimeError('Can not get master serial number: {0}'.format(MCPError._fields[err])) | ||
| return str(pstr.value, encoding='utf8') | ||
|
|
||
| def get_sensor_modules(self, sensor_module_type): | ||
| pstr = c_char_p() | ||
| err = self.api.contents.GetSensorModules(sensor_module_type, pointer(pstr), pointer(self.handle)) | ||
| if err != MCPError.NoError: | ||
| raise RuntimeError('Can not get sensor modules: {0}'.format(MCPError._fields[err])) | ||
| return str(pstr.value, encoding='utf8') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
GetSensorModules/get_sensor_modules argument types don't match either the declared CFUNCTYPE or the C++ signature.
MCPSystemApi.GetSensorModules is declared CFUNCTYPE(c_int32, MCPSensorModuleHandle, c_int32, POINTER(MCPSystemHandle)) (Line 989), but get_sensor_modules calls it with (sensor_module_type, pointer(pstr), pointer(self.handle)) — mismatched types/sizes on the middle argument, and neither matches the actual C++ signature GetSensorModules(MCPSensorModuleHandle_t*, uint32_t*, MCPSystemHandle_t) in MocapApi.h (lines 361-362). Calling this will pass corrupted arguments into native code.
🐛 Proposed fix
class MCPSystemApi(Structure):
_fields_ = [
('GetMasterVersion', CFUNCTYPE(c_int32, POINTER(c_char_p), MCPSystemHandle)),
('GetMasterSerialNumber', CFUNCTYPE(c_int32, POINTER(c_char_p), MCPSystemHandle)),
- ('GetSensorModules', CFUNCTYPE(c_int32, MCPSensorModuleHandle, c_int32, POINTER(MCPSystemHandle))),
+ ('GetSensorModules', CFUNCTYPE(c_int32, POINTER(MCPSensorModuleHandle), POINTER(c_uint32), MCPSystemHandle)),
]
...
def get_sensor_modules(self, count=None):
- pstr = c_char_p()
- err = self.api.contents.GetSensorModules(sensor_module_type, pointer(pstr), pointer(self.handle))
+ count_ = c_uint32(count or 0)
+ handles = (MCPSensorModuleHandle * count_.value)() if count else None
+ err = self.api.contents.GetSensorModules(handles, byref(count_), self.handle)🤖 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/plugins/noitom_mocap/third_party/mocap_api/mocap_api.py` around lines 985
- 1021, The GetSensorModules binding and call in MCPSystemApi/get_sensor_modules
use the wrong ctypes signature and argument order, so the native call receives
corrupted parameters. Update the CFUNCTYPE declaration for GetSensorModules to
match the C++ signature in MocapApi.h, using pointer-based arguments for the
sensor module output and count, and fix get_sensor_modules to pass the correct
argument types instead of a c_char_p buffer. Keep the changes localized to
MCPSystemApi and get_sensor_modules so the Python wrapper matches the native API
exactly.
Source: Path instructions
| def set_bvh_displacement(self, bvh_transformation): | ||
| err = self.api.contents.SetSettingsBvhDisplacement(c_int32(bvh_transformation), self.handle) | ||
| if err != MCPError.NoError: | ||
| raise RuntimeError('Can not set bvh displacement: {0}'.format(MCPError._fields[err])) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Two more attribute-name mismatches will AttributeError on call.
set_bvh_displacement(Lines 1275-1278) callsself.api.contents.SetSettingsBvhDisplacement, butMCPSettingsApionly definesSetSettingsBvhTransformation/SetSettingsBvhData— no such field exists.get_rotating_direction(Lines 1433-1438) callsself.api.contents.GetRotatingDirection, but the struct field is namedGetRotationDirection(Line 1360).
Both will raise AttributeError if invoked.
🐛 Proposed fixes
def set_bvh_displacement(self, bvh_transformation):
- err = self.api.contents.SetSettingsBvhDisplacement(c_int32(bvh_transformation), self.handle)
+ err = self.api.contents.SetSettingsBvhTransformation(c_int32(bvh_transformation), self.handle) def get_rotating_direction(self):
rotating_direction = c_int32()
- err = self.api.contents.GetRotatingDirection(pointer(rotating_direction), self.handle)
+ err = self.api.contents.GetRotationDirection(pointer(rotating_direction), self.handle)Also applies to: 1433-1438
🤖 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/plugins/noitom_mocap/third_party/mocap_api/mocap_api.py` around lines
1275 - 1278, The two API wrapper methods reference non-existent ctypes fields,
so update the call sites in set_bvh_displacement and get_rotating_direction to
use the actual MCPSettingsApi members defined on the struct. Replace the
mismatched SetSettingsBvhDisplacement access with the corresponding existing BVH
settings function, and change GetRotatingDirection to the correctly named
GetRotationDirection so both wrappers invoke valid API symbols and no longer
raise AttributeError.
Source: Path instructions
There was a problem hiding this comment.
i think this is a rebase error. didn't seem you need to change anything here.
There was a problem hiding this comment.
Likely caused by the recent force push to main. @Astrorix Please rebase the PR, appologies for disruption
|
|
||
| // Body joint indices for Noitom Perception Neuron full-body mocap samples. | ||
| // The order intentionally mirrors the 24 slots consumed by the Noitom G1 example. | ||
| enum BodyJointPN : uint8 { |
There was a problem hiding this comment.
This extra schema seems a bit problematic.
maybe our add device documentation was clear enough: but the intention is to reuse the same data schema for the same device category. can we discuss how we can
also I think we probably did't put the correct name for BodyJointPico it's really just SMPL that Pico happen to be the first adopter in our system. We can probably just rename it to BodyJointSMPL
There was a problem hiding this comment.
@Astrorix can you try reuse BodyJointPico schema (and the relevant tracker for now)? Once you land your PR, we will do a renaming, so that the branding will not be wrong.
cc: @qingsi-at-nv @yanziz-nvidia @nv-mhaselton for viz
|
|
||
| // Body joint indices for Noitom Perception Neuron full-body mocap samples. | ||
| // The order intentionally mirrors the 24 slots consumed by the Noitom G1 example. | ||
| enum BodyJointPN : uint8 { |
There was a problem hiding this comment.
@Astrorix can you try reuse BodyJointPico schema (and the relevant tracker for now)? Once you land your PR, we will do a renaming, so that the branding will not be wrong.
cc: @qingsi-at-nv @yanziz-nvidia @nv-mhaselton for viz
| --cloudxr_env none --no-auto_launch_cloudxr | ||
| ``` | ||
|
|
||
| ### Example 2: Record and replay |
There was a problem hiding this comment.
iiuc, this example can reuse the same full_body tracking record & replay scripts from:
https://github.com/NVIDIA/IsaacTeleop/tree/main/examples/mcap_record_replay/python
Though you probably still want to add the docs for the building and launching notion plugins.
|
@Astrorix really appreciate the contribution here and the effort to make Noitom body tracker to work end to end in Isaac Lab is a great milesone! I added a couple high level comments (most importantly requesting to reuse the same full body tracking schema). I also have asked help from @qingsi-at-nv and @aristarkhovNV to chime in to do a bit more detailed review for this. |
There was a problem hiding this comment.
Likely caused by the recent force push to main. @Astrorix Please rebase the PR, appologies for disruption
| Terminal 1, keep CloudXR running: | ||
|
|
||
| ```bash | ||
| conda activate teleop |
There was a problem hiding this comment.
Although conda should be supported, this repo standartizes on using uv in all example code for consistency
| ```bash | ||
| source ~/.cloudxr/run/cloudxr.env | ||
| cd ~/IsaacTeleop | ||
| ./install/plugins/noitom_mocap/noitom_mocap_plugin --host=<hds-host> --port=<hds-port> |
There was a problem hiding this comment.
Could the plugin be picked up automatically via the plugin manager so that users would not need any manual steps in Terminal 2?
| ```bash | ||
| conda activate teleop | ||
| cd ~/IsaacTeleop | ||
| python -m isaacteleop.cloudxr |
There was a problem hiding this comment.
Could CloudXRLauncher be of any help to avoid the manual Terminal 1 step like it's done in other examples?
| ``` | ||
|
|
||
| If CloudXR is already running in Terminal 1 and IsaacLab should not auto-launch | ||
| another runtime, add: |
There was a problem hiding this comment.
auto-launch should be the default. Put manual launch steps under an optional advanced setup section.
| cd ~/IsaacTeleop | ||
| cmake --build build --target python_package noitom_mocap_plugin --parallel | ||
|
|
||
| PYTHONPATH=$PWD/build/python_package/Release \ |
There was a problem hiding this comment.
Do not promote using raw build outputs. IsaacTeleop modules should be consumed via a built wheel or from the install location
| tracker = FullBodyTrackerPN() | ||
| required_extensions = DeviceIOSession.get_required_extensions([tracker]) | ||
| mcap_config = McapRecordingConfig(str(mcap_path), [(tracker, "full_body_pn")]) | ||
|
|
||
| with OpenXRSession( | ||
| "NoitomFullBodyRecordExample", required_extensions | ||
| ) as oxr_session: | ||
| handles = oxr_session.get_handles() | ||
| with DeviceIOSession.run([tracker], handles, mcap_config) as session: |
There was a problem hiding this comment.
Consider upgrading to TeleopSession helper class that incapsulates most of the low level plumbing
| Noitom MocapApi SDK License Reference | ||
|
|
||
| The files under src/plugins/noitom_mocap/third_party/mocap_api/ are bundled | ||
| third-party Noitom MocapApi SDK artifacts. |
There was a problem hiding this comment.
Avoid hosting 3rd party IP within the repo. Is there a way to fetch 3rd party headers from their official location using cmake's FetchContent or similar?
Description
Adds Noitom Perception Neuron full-body support, including the Noitom mocap plugin, FullBodyPosePN schema/deviceio plumbing, G1 retargeting example task, and MCAP record/replay utilities.
Also updates the Noitom README with two example workflows:
The README includes GIFs demonstrating teleop, recording, and replay.
Fixes #N/A
Type of change
Testing
Ran
SKIP=check-copyright-year uvx pre-commit run --all-files; all checks passed.Manual validation: Noitom teleop, record, and replay workflows were captured and added as README GIF examples.
Checklist
Summary by CodeRabbit
New Features
Bug Fixes