feat(oob): Adds USB ADB automation for OOB teleop#390
Merged
yanziz-nvidia merged 2 commits intomainfrom Apr 15, 2026
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request implements USB-adb automation for OOB (out-of-band) teleoperation control. When Sequence DiagramsequenceDiagram
participant User as User/CLI
participant Main as __main__.py
participant AdbCheck as oob_teleop_adb
participant Env as oob_teleop_env
participant WSS as WSS Proxy
participant ADB as adb (device)
participant Headset as Headset
User->>Main: python -m isaacteleop.cloudxr --setup-oob
Main->>AdbCheck: require_adb_on_path()
AdbCheck-->>Main: ✓ or OobAdbError
Main->>AdbCheck: assert_exactly_one_adb_device()
AdbCheck->>ADB: adb devices
ADB-->>AdbCheck: device list
AdbCheck-->>Main: ✓ or OobAdbError (zero/multiple/unauthorized)
Main->>Env: resolve_lan_host_for_oob()
Env-->>Main: LAN host IP
Main->>WSS: run(setup_oob=True, ...)
WSS->>Env: wss_proxy_port()
Env-->>WSS: port
WSS->>Env: default_initial_stream_config()
Env-->>WSS: stream config dict
WSS->>WSS: start hub + proxy
WSS->>AdbCheck: run_adb_headset_bookmark(resolved_port) [background]
AdbCheck->>Env: build_headset_bookmark_url()
Env-->>AdbCheck: bookmark URL
AdbCheck->>ADB: adb shell am start [URL]
ADB->>Headset: launch teleop page
Headset-->>ADB: ✓
AdbCheck-->>WSS: (exit_code, diagnostic)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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 |
yanziz-nvidia
requested review from
aristarkhovNV,
blongs-nv,
nvddr and
sgrizan-nv
sgrizan-nv
approved these changes
Apr 13, 2026
yanziz-nvidia
force-pushed
the
yanziz/oob-phase2
branch
from
02e3894 to
1e333cd
Compare
![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 the current code and only fix it if needed.
Inline comments:
In `@src/core/cloudxr/python/oob_teleop_adb.py`:
- Line 165: The current ADB command log in oob_teleop_adb.py uses log.info("ADB
automation: %s", " ".join(shlex.quote(c) for c in full)) which can leak
CONTROL_TOKEN if present in URL query params; update the logging to redact
sensitive tokens before logging by scanning the constructed command list/URL
entries (the variable full) and replacing any CONTROL_TOKEN query param or its
value with a placeholder like "<REDACTED>" (or omit query params entirely) prior
to joining and passing to log.info, ensuring you reference the same variable
full and keep shlex.quote usage for other args.
- Around line 93-105: The subprocess.run call that invokes ["adb", "devices"]
(inside the try block around proc = subprocess.run(...)) currently only catches
FileNotFoundError; add handling for subprocess.TimeoutExpired so timeouts are
converted to the user-facing OobAdbError. Specifically, catch
subprocess.TimeoutExpired (either in an except block or a combined except) and
raise OobAdbError with a clear message like "adb command timed out; ensure
Android Platform Tools are installed and adb is callable" (chain the original
exception with "from e") so the timeout doesn't raise an unhandled traceback.
- Around line 166-171: Wrap the subprocess.run call in a try/except and add a
timeout parameter so adb calls cannot hang: call subprocess.run(full,
capture_output=True, text=True, timeout=ADB_CMD_TIMEOUT) inside a try block,
then keep the existing return of (proc.returncode, _adb_output_text(proc)) when
proc.returncode != 0 and log.info/return 0 when successful; add an except
subprocess.TimeoutExpired that builds a diagnostic string (including the
timeout, partial stdout/stderr from the exception if available) and return a
non-zero code and that diag, and add a broad except Exception to capture
unexpected failures and return an appropriate non-zero code and diagnostic.
Reference subprocess.run, subprocess.TimeoutExpired, _adb_output_text,
proc.returncode, and log.info in the changes.
In `@src/core/cloudxr/python/oob_teleop_env.py`:
- Around line 28-33: The wss_proxy_port() function currently does int(raw)
without validating the env value; update wss_proxy_port to validate the
PROXY_PORT string (e.g., ensure it matches r'^\d+$' and is within valid port
range 1–65535) before converting, and if validation fails either log/raise a
clear OOB-specific error mentioning the env var name and the bad value or fall
back to WSS_PROXY_DEFAULT_PORT; apply the same validation and error/logging
pattern to the other port-parsing function in this file (the second port-parser
around lines 50–56) so neither int(...) call can raise an unhandled ValueError.
- Around line 142-173: The code prints bookmark_primary (built by
build_headset_bookmark_url) which can contain CONTROL_TOKEN and leak secrets;
update the printing logic near bookmark_primary/wss_primary so you do not print
the raw URL with the controlToken—either build a separate display URL without
the controlToken or redact the controlToken query value before printing (e.g.,
replace its value with "<redacted>" or remove the param) and use that safe
string in the existing print statements (look for bookmark_primary,
build_headset_bookmark_url, and the block gated by web_client_base_override).
In `@src/core/cloudxr/python/wss.py`:
- Around line 500-514: The call site uses await
asyncio.to_thread(run_adb_headset_bookmark, ...) which can block indefinitely if
the underlying subprocess.run() in run_adb_headset_bookmark hangs; update
run_adb_headset_bookmark (the function that invokes subprocess.run) to pass
timeout=30 to subprocess.run (or implement a subprocess timeout there), or
alternatively wrap the asyncio.to_thread call at this call site in
asyncio.wait_for(..., timeout=30) so the call returns on timeout and allows
shutdown to proceed; reference the run_adb_headset_bookmark function and the
asyncio.to_thread invocation in wss.py when making the change.
🪄 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: Pro Plus
Run ID: 452c92e9-e17d-41bf-b121-4fca9bdc6adc
📒 Files selected for processing (11)
docs/source/references/oob_teleop_control.rstsrc/core/cloudxr/python/CMakeLists.txtsrc/core/cloudxr/python/__main__.pysrc/core/cloudxr/python/launcher.pysrc/core/cloudxr/python/oob_teleop_adb.pysrc/core/cloudxr/python/oob_teleop_env.pysrc/core/cloudxr/python/oob_teleop_hub.pysrc/core/cloudxr/python/wss.pysrc/core/cloudxr_tests/python/conftest.pysrc/core/cloudxr_tests/python/test_oob_teleop_adb.pysrc/core/cloudxr_tests/python/test_oob_teleop_env.py
💤 Files with no reviewable changes (1)
- src/core/cloudxr/python/oob_teleop_hub.py
yanziz-nvidia
force-pushed
the
yanziz/oob-phase2
branch
2 times, most recently
from
eb4bb67 to
7df8324
Compare
yanziz-nvidia
force-pushed
the
yanziz/oob-phase2
branch
from
7df8324 to
255d022
Compare
yanziz-nvidia
changed the title
yanziz-nvidia
force-pushed
the
yanziz/oob-phase2
branch
from
255d022 to
1c87db4
Compare
yanziz-nvidia
force-pushed
the
yanziz/oob-phase2
branch
from
1c87db4 to
f010faa
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
2 participants
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.
Second phase PR on top of #388. This one adds ADB automation handling but still requires wifi or ethernet network.
Co-Authored-By: default avatarClaude Sonnet 4.6 <noreply@anthropic.com>
Summary by CodeRabbit
Release Notes
New Features
Documentation