Robot twin: tests - #999
Draft
jiwenc-nv wants to merge 3 commits into
Draft
Conversation
Contributor
|
Important Review skippedDraft detected. 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:
Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| "LocomotionRootCmdRetargeter", | ||
| "LocomotionRootCmdRetargeterConfig", | ||
| # Manipulator retargeters | ||
| "ControllerPoseSource", |
| "LocomotionRootCmdRetargeterConfig", | ||
| # Manipulator retargeters | ||
| "ControllerPoseSource", | ||
| "EngageAlignmentGate", |
| # Manipulator retargeters | ||
| "ControllerPoseSource", | ||
| "EngageAlignmentGate", | ||
| "EngageGateConfig", |
| "ControllerPoseSource", | ||
| "EngageAlignmentGate", | ||
| "EngageGateConfig", | ||
| "GateVerdict", |
| "JointMap", | ||
| "RobotTwin", | ||
| "RobotTwinPublisher", | ||
| "SceneTwin", |
| # handles, so destroying the session first would pull them out from | ||
| # under it. `destroy()` is what releases this. | ||
| self._destroy.wait() | ||
| except BaseException as error: # noqa: BLE001 -- reported to the owner |
The MuJoCo-backed digital twin was buried in examples/mujoco_xr, compiled as part of the example and reachable only by installing that example. It is a Televiz capability, not an example, so the backend moves to src/viz/robot_twin and the Python surface to isaacteleop.viz.robot, both shipping in the wheel. What is left of the example is the app itself, renamed examples/robot_viz. The twin no longer depends on a `mujoco` wheel. cmake/Mujoco.cmake fetches MuJoCo and builds it under a private name, reached by dlopen/dlsym so the extension carries no undefined mj* for a foreign libmujoco to answer; the example's version-pin cross-check goes with it. Also here: the engage gate and controller pose source under isaacteleop.retargeters, and the twin runner in teleop_session_manager. Tests are stripped from this branch and land in a follow-up PR. Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
MuJoCo was fetched from src/viz/CMakeLists.txt, the one dependency not declared where every other one is. Its FetchContent_Declare now sits beside glfw and glm under the same BUILD_VIZ gate, and the isolation contract it exists for moves next to it as deps/third_party/Mujoco.cmake. Declaration order relative to every other dependency is unchanged. ISAACTELEOP_MUJOCO_VERSION becomes a cache variable because src/viz/robot_twin_tests is now a sibling of where it is set. Two mechanisms did not survive being asked to justify themselves. `-Wl,--exclude-libs,ALL` is redundant against the version script, a whitelist and so strictly stronger: on robot_twin_py the version script alone leaves 1 export and --exclude-libs alone leaves 11. The BUILD_SHARED_LIBS guard cannot fire -- upstream's add_library is unconditionally SHARED off Emscripten -- and the failure it converted was already loud. include_guard(GLOBAL) goes with the single include site. The OUTPUT_NAME comment had the interposition backwards. The wheel's extension modules carry DT_NEEDED on libmujoco.so.3.x, so it is ours answering theirs: loaded first under an unrenamed SONAME, our copy is the only libmujoco the process maps and the user's `import mujoco` silently runs on it. Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
The follow-up half of #995, which carries the twin with every test stripped out. All 20 files land here: src/viz/robot_twin_tests, examples/robot_viz/tests, the four src/viz/python_tests/test_robot_*.py, test_engage_gate.py and test_robot_twin_session.py, plus the CMake that registers them and the example README's Tests section. test_symbol_isolation.py runs against the shipped _robot_twin rather than a test-only extension carrying a second private MuJoCo. Review asked why the tests reached MuJoCo directly, and they should not have: that probe's two unique assertions exercised its own reimplemented longjmp handler rather than mj_guard.cpp, and its ELF assertions were the weaker copy of ones the shipped module already gets -- --version-script has nothing to hide in a module linking no static archive, and the shipped one links cudart_static. Two replacements do cover shipped code: that install_mujoco_handlers() wrote the twin's own mju_user_error and not the wheel's, and that an unguarded mju_error reaches mj_guard.cpp's abort rather than MuJoCo's exit(1). The example's tests are keyed on TARGET robot_twin_py, which says the actual dependency instead of restating where the twin gets built. Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
jiwenc-nv
force-pushed
the
jiwenc-nv/robot-viz-tests
branch
from
August 23, 2026 22:10
06f1e02 to
cdc3ff7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #995, which carries the robot twin itself with every test stripped out. This is the one commit that adds them back:
src/viz/robot_twin_tests/,examples/robot_viz/tests/, the foursrc/viz/python_tests/test_robot_*.py,test_engage_gate.pyandtest_robot_twin_session.py, plus the CMake that registers them and the example README's Tests section. Nothing outside those:deps/third_party/is untouched, and the only non-test edits are three wiring lines and the README.Merge #995 first. Its base is
main, so until it merges this PR's diff renders #995's contents too; the branch itself is a single commit on top of #995.Addressing review feedback on #995:
robot_twin_testsno longer carries its own MuJoCo. Themujoco_probeextension is gone andtest_symbol_isolation.pyruns against the shipped_robot_twin. The probe's two unique tests exercised its own reimplemented longjmp handler rather thanmj_guard.cpp, and its ELF assertions were the weaker copy of ones already made against the shipped module —--version-scripthas nothing to hide in a module that links no static archive, and the shipped one linkscudart_static. Two replacements do cover shipped code: thatinstall_mujoco_handlers()wrote the twin's ownmju_user_errorand not the wheel's, and that an unguardedmju_errorreachesmj_guard.cpp's abort rather than MuJoCo'sexit(1).The example's tests are keyed on
TARGET robot_twin_pyrather than on a platform check.Type of change
Testing
cteston Linux + CUDA: 330 tests, 0 failures.test_symbol_isolation.pyis 11 tests, none skipped. #995 alone is 316, 0 failures, with norobot_twin_*orrobot_viz_*entries registered.Checklist
SKIP=check-copyright-year pre-commit run --all-filesgit commit -s) per the DCO