Repository navigation
fix(examples): make retargeting demo shutdown explicit - #1188
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 3.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 5 files. (5 skipped: 5… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled. Title check ✅ Passed The title clearly describes the retargeting examples’ explicit shutdown behavior, which is a central change in the pull request. Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 3.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 5 files. (5 skipped: 5 unsupported.)
✨ Finishing Touches 💡 2
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Run the gripper and SE3 examples until Ctrl+C by default, with optional timed runs and completion messages after cleanup. Preserve stream and teardown failures, correct the binary gripper documentation, and cover example lifecycle behavior with regression tests. Signed-off-by: Farbod Motlagh <fmotlagh@nvidia.com>
b9e9093 to
ece5401
Compare
ivany-nv
left a comment
There was a problem hiding this comment.
Summary
The gripper and SE(3) examples now share a loop helper that runs until Ctrl+C by default, with an optional monotonic --duration, and Session closed. is printed only after both the launcher and session contexts unwind cleanly. The red build-ubuntu jobs are infra: Configure CMake failed downloading the SDL2 tarball and the rest were fail-fast cancelled, so CI never reached the new test target; locally test_retargeting_examples.py passes 50/50. _example_loop.py ships with the examples via the *.py glob in InstallPythonExample.cmake, the runtime's own session keeps terminal Ctrl+C from reaching it, and the quick-start gripper claims match GripperRetargeter. No changes needed; a CI re-run is still needed for a green build, and headset validation remains open as the description says.
Legend: 🚫 BLOCKER = merge-blocking | 💡 SUGGESTION = meaningful improvement | 🧹 NIT = precise small correction
| Severity | Finding |
|---|---|
| 🚫 | none |
| 💡 | none |
| 🧹 | none |
…nges Signed-off-by: Farbod Motlagh <fmotlagh@nvidia.com>
Run the gripper and SE3 examples until Ctrl+C by default, with optional timed runs and completion messages after cleanup. Preserve stream and teardown failures, correct the binary gripper documentation, and cover example lifecycle behavior with regression tests. (cherry picked from commit 0075fa7) Signed-off-by: Farbod Motlagh <fmotlagh@nvidia.com>
Description
The gripper and SE(3) examples silently stopped after 20 seconds, and Ctrl+C could produce a traceback. The quickstart also showed gradual gripper values even though
GripperRetargeterintentionally outputs binary open/closed commands.This change:
--duration 20preserves the previous timed behavior.+1.00open /-1.00closed and the default trigger threshold.Related to CXR-4301. The headset/browser “unexpected stream error” has not been reproduced, so this PR does not claim to resolve that symptom.
Type of change
Testing
Validated on macOS with Python 3.12:
python -m pytest tests/python/examples/teleop -q: 50 cases passed using simulated input and mocked native session boundaries.make current-docs: passed with warnings treated as errors.CloudXR/headset validation remains pending.
Checklist
SKIP=check-copyright-year pre-commit run --all-filesgit commit -s) per the DCOSummary by CodeRabbit