Skip to content

Defer CloudXR headset connection until the OpenXR app is ready - #1226

Merged
farbod-nv merged 5 commits into
mainfrom
fix/televiz-cloudxr-startup-order
Oct 12, 2026
Merged

farbod-nv merged 5 commits into
mainfrom
fix/televiz-cloudxr-startup-order

Conversation

@farbod-nv

@farbod-nv farbod-nv commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Description

CloudXR can exhibit poor performance when the web client connects before the OpenXR application starts. This change allows embedded applications to enforce runtime → OpenXR application → headset connection ordering.

  • Add opt-in defer_connect=True to hold OOB automation and WebSocket signaling until connect_headset() is called, including connections from already-open client pages.
  • Add oxr_session_factory so an external renderer can create a shared OpenXR session with all required tracker, sink, and plugin extensions.
  • Unblock pending connection waits during shutdown.
  • Preserve immediate connection and existing session creation by default.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Testing

  • macOS, Python 3.12: 95 tests passed across the CloudXR launcher, service lifecycle, and static-client suites, using a source-only package bootstrap.
  • Coverage includes deferred connection, cancellation, early WebSocket signaling, and default startup behavior.
  • SKIP=check-copyright-year pre-commit run --all-files — passed.
  • Added session-factory ownership tests. Native OpenXR tests and headset performance validation remain pending on Ubuntu with an NVIDIA GPU; native bindings were unavailable locally.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the linter and formatter with SKIP=check-copyright-year pre-commit run --all-files
  • I have made corresponding changes to the documentation (API docstrings)
  • I have added tests that prove my fix/feature works (or explained why not)
  • I have signed off all my commits (git commit -s) per the DCO

Summary by CodeRabbit

  • New Features
    • CloudXR launches can now defer headset connection until explicitly requested, with validation and shutdown checks.
    • Live teleoperation sessions can use a custom OpenXR session factory to provide external session handles.
  • Bug Fixes
    • Improved handling of early WebSocket connections so clients wait for the headset connection request or service shutdown.
    • Prevented CloudXR test setups from requiring an installed native SDK.

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/IsaacCapture/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: cf05ea5e-a236-4160-895f-bae7ccfd0aeb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/IsaacCapture/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 3907760f-6f92-464e-bd2a-9806281f96d3

📥 Commits

Reviewing files that changed from the base of the PR and between e7bd63b and bf2906a.


📒 Files selected for processing (9)
  • src/python/isaaccapture/cloudxr/launcher.py
  • src/python/isaaccapture/cloudxr/service/_service.py
  • src/python/isaaccapture/cloudxr/wss.py
  • src/python/isaaccapture/teleop_session_manager/config.py
  • src/python/isaaccapture/teleop_session_manager/teleop_session.py
  • tests/python/core/cloudxr/conftest.py
  • tests/python/core/cloudxr/test_launcher.py
  • tests/python/core/cloudxr/test_service.py
  • tests/python/core/teleop_session_manager/test_external_oxr_handles.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.



📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

CloudXR adds an option to defer headset connection until connect_headset() is called. The WSS proxy waits for that request or shutdown before proxying backend connections or starting OOB ADB automation. Teleop session configuration adds an optional factory that supplies external OpenXR handles for live sessions, and session setup passes those handles to DeviceIO.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CloudXRLauncher
  participant CloudXRService
  participant WSSrun
  participant WebSocketHandler
  CloudXRLauncher->>CloudXRService: Construct with defer_connect
  CloudXRService->>WSSrun: Pass connect_requested event
  CloudXRLauncher->>CloudXRService: Call connect_headset()
  CloudXRService->>WSSrun: Schedule connection event
  WSSrun->>WebSocketHandler: Allow backend proxying after request
Loading
sequenceDiagram
  participant TeleopSessionConfig
  participant TeleopSession
  participant oxr_session_factory
  participant DeviceIO
  TeleopSession->>oxr_session_factory: Request handles with required extensions
  oxr_session_factory-->>TeleopSession: Return OpenXR handles
  TeleopSession->>DeviceIO: Start with returned handles
Loading

Merge Risk: ⚪ Minimal · up to bf290

No concrete issue remains that would prevent merging after normal checks. Native headset validation is still pending.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 9 files. 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 and concisely describes the primary change: delaying CloudXR headset connection until the OpenXR application is ready.
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.

✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch fix/televiz-cloudxr-startup-order

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Comment thread src/python/isaaccapture/cloudxr/wss.py Fixed
Comment thread tests/python/core/cloudxr/test_service.py Fixed
@farbod-nv
farbod-nv force-pushed the fix/televiz-cloudxr-startup-order branch from bf2906a to 3906700 Compare October 9, 2026 15:49
Comment thread tests/python/core/cloudxr/test_service.py Dismissed
@yanziz-nvidia

Copy link
Copy Markdown
Collaborator

reviewed by yanziz-review-bot

Summary

Other than the existing findings in previous comments, I found one new connection-lifecycle defect. CI is green and 128 focused CloudXR tests passed locally; native OpenXR/headset validation was not run.

Legend: 🚫 BLOCKER = merge-blocking | 💡 SUGGESTION = meaningful improvement | 🧹 NIT = precise small correction

Severity Finding
🚫 BLOCKER wss.py:690 — Disconnected clients survive the deferred gate and replay stale signaling. If a headset page disconnects or reloads before connect_headset(), its handler keeps waiting because the gate ignores socket closure. Repeated attempts accumulate handlers and buffered messages. A real-WebSocket reproduction retained three disconnected clients; releasing the gate then opened three backend connections and forwarded all three stale offers. Race the gate against ws.wait_closed() and exit without proxying when the client has closed.
💡 SUGGESTION none
🧹 NIT none

Actionables (copy-paste-ready for implementation agents)

Validate these suggestions in context before applying them. Skip anything already addressed, incorrect, or not worth the churn.

  • src/python/isaaccapture/cloudxr/wss.py:690 — Include client closure in the deferred wait, clean up per-client wait tasks without cancelling the shared shutdown future, and add a real-socket regression test proving disconnected clients neither remain waiting nor forward buffered signaling after release.

Comment thread src/python/isaaccapture/cloudxr/service/_service.py Outdated
Comment thread src/python/isaaccapture/cloudxr/service/_service.py Outdated
@farbod-nv

Copy link
Copy Markdown
Contributor Author

PTAL

@yanziz-nvidia

Copy link
Copy Markdown
Collaborator
At the latest SHA, the reproduction retained three disconnected clients, then opened three backend connections and forwarded all three stale offers.
The fix is to also wait for ws.wait_closed() and exit without proxying if the client disconnects.

Can you fix the disconnected client issue?

@farbod-nv

Copy link
Copy Markdown
Contributor Author
At the latest SHA, the reproduction retained three disconnected clients, then opened three backend connections and forwarded all three stale offers.
The fix is to also wait for ws.wait_closed() and exit without proxying if the client disconnects.

Can you fix the disconnected client issue?

fixed

Add an opt-in gate for OOB automation and WebSocket signaling, with
immediate connection remaining the default. Allow an external OpenXR
session factory to receive the full set of tracker and plugin extensions.

Cover deferred connection, cancellation, early signaling and session
factory ownership; isolate lifecycle tests from the installed native SDK.

Signed-off-by: Farbod Motlagh <fmotlagh@nvidia.com>
Signed-off-by: Farbod Motlagh <fmotlagh@nvidia.com>
Signed-off-by: Farbod Motlagh <fmotlagh@nvidia.com>
Signed-off-by: Farbod Motlagh <fmotlagh@nvidia.com>
Signed-off-by: Farbod Motlagh <fmotlagh@nvidia.com>
@farbod-nv
farbod-nv force-pushed the fix/televiz-cloudxr-startup-order branch from 0db19d7 to a15f2fd Compare October 12, 2026 20:55
@farbod-nv
farbod-nv merged commit 2fe1bbe into main Oct 12, 2026
38 checks passed
@farbod-nv
farbod-nv deleted the fix/televiz-cloudxr-startup-order branch October 12, 2026 21:35
github-actions Bot added a commit that referenced this pull request Oct 12, 2026

This branch was previously deployed

1 inactive deployment
dev — a15f2fdb Deployed Oct 12, 2026 by farbod-nv via publish-wheel #5542
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants