[rwiltz]

Description

Adds a keyboard input API to visualizers, so code can read the keys pressed in a visualizer's window, e.g. for keyboard teleoperation or script hotkeys, independent of Kit/carb.

• BaseVisualizer.key_event_source returns a KeyEventSource for the visualizer's window, or None when it has none (the default; also before initialization, after close, and headless). It is created
  on first access and closed with the visualizer.
• KeyEventSource.add_key_listener(on_key, on_focus_lost) returns a closable KeyboardSubscription. Presses and releases arrive as W3C KeyboardEvent.code strings ("KeyW"), forwarded as the window
  reports them; on_focus_lost runs on blur, when the visualizer's UI takes the keyboard, and on close. Subscribing, closing and callbacks all happen on the visualizer's thread.
• BaseVisualizer.set_camera_keys_enabled(enabled) lets the caller turn off keyboard camera movement (Newton WASD/QE and arrow keys) while it uses those keys; a no-op by default.
• Implemented for Kit (physical keys; Kit visualizers share the app window, so each reports the same keys) and for Newton GL and Newton RTX (one pyglet viewer window; US-layout key mapping, presses
  typed into the ImGui UI are withheld). Viser, Rerun and headless runs return None.
• Each visualizer closes its keyboard source first in close(), while the window is still open; a failure there is logged and the rest of the teardown still runs.

Opt-in: nothing is hooked until a listener subscribes, and existing visualizer behavior is unchanged.

Used by #8797 (keyboard, gamepad and SpaceMouse teleop through Isaac Capture) and #8798 (remaining scripts and deprecation of the isaaclab.devices keyboard, gamepad and SpaceMouse devices).

Fixes # (issue)

Type of change

• New feature (non-breaking change which adds functionality)

Release backport

• Backport this pull request to the active release branch after it merges into develop

Checklist

Docker and GPU tests run on demand. Push the commits you want tested, then
comment run-ci on the pull request.

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with uv run isaaclab --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have validated the changed behavior with focused coverage and applied the test-audit criteria to any test changes
• I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package (do not edit CHANGELOG.rst or bump package versions — CI handles that)
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[rwiltz]

run-ci

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Adds keyboard input capture to visualizer windows.

The PR appears safe to merge; no new actionable issue was found.

What we checked:

• Test runs only fixed code: The test builds code from fixed strings and passes it directly to Python without a shell.
• Closed listeners reject later keys: Final focus loss marks each subscription closed. Its key callback then rejects further events, even from an earlier snapshot.

Summary

Adds opt-in keyboard input for Kit and Newton GL visualizer windows.

• Shares listener, focus-loss, and capture behavior through KeyEventSource.
• Filters Newton UI input and suspends its keyboard camera movement while captured.
• Keeps the closing source available during shutdown callbacks, preventing replacement sources.
• Both earlier, unnumbered findings are addressed: camera capture now exercises _update_camera, and shutdown callbacks cannot create a replacement source.

Tests were inspected but not run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Window[Focused window] --> Backend[Kit or Newton GL]
  Backend --> Source[KeyEventSource]
  Source --> Listener[Key and focus-loss callbacks]
  Capture[Keyboard capture] --> Camera[Newton camera keys suspended]
  Close[Visualizer closes] --> Shutdown[Close source before dropping references]

Loading

_{Reviews (3) · Last reviewed commit: "Report keys from the Kit and Newton GL v..." · Reviewed by Greptile}

[isaaclab-review-bot]

Isaac Lab Review Bot

Needs attention: Scope and over-engineering (2), Implementation (3), Style consistency (2), Test quality (2). Other reviewed categories look good.

• PR description: The body falsely claims a Viser implementation although the diff and capability table return None; remove that claim and the empty Fixes #, then list focused validation commands.
• Design and architecture: Architecture looks good.
• Scope and over-engineering: Minimal implementation: extend BaseVisualizer, KitVisualizer, and NewtonVisualizer; keep key_event_source.py, keep kit_key_event_source.py but inline its one-use _kit() wrapper, and keep newton_key_event_source.py for pyglet mapping and capture. Delete both redundant _key_input_closed flags; retain lazy creation, backend hooks, event delivery, and visualizer-owned cleanup. Scope is cohesive; no split recommended.
• API: API changes look good.
• Compatibility and deprecation: No breaking changes identified.
• Implementation: Backend cleanup is not failure-safe: Kit can remain marked listening after losing hooks, and Newton can retain its frame callback after unhook failure; preserve consistent state and clear callbacks during release.
• Style consistency: The new property and capabilities declaration violate required class organization: move properties before public methods and annotate backend-shared capabilities with ClassVar[KeyboardCapabilities].
• Test quality: The added suites cover distinct lifecycle and backend behavior, but capability-literal assertions and mirrored shift-order rows duplicate stronger behavioral coverage; remove those cases.

Minor fixes needed. Posted 9 inline findings.

Automated review; human maintainers own approval decisions.

[rwiltz]

@greptile

[rwiltz]

@greptile

[rwiltz]

run-ci

[ooctipus]

The goal makes sense, but I’m concerned about how much responsibility is going into KeyEventSource. It handles window events, tracks held keys for each listener, manages requests to disable camera controls, and coordinates subscriptions and cleanup across threads.

Could we first connect this to Se2Keyboard or Se3Keyboard and use that example to define the smallest interface we need? Both still subscribe directly to Carb, so the PR does not yet show how the new API fits the existing keyboard controls.

I’d expect the ownership to be roughly:

• The visualizer receives and translates key/focus events and cleans up its window hooks.
• The keyboard controller tracks the keys it uses and turns them into robot commands.
• The viewer owns camera keyboard controls, with the caller deciding when to disable them.

The shared source may need some bookkeeping to deliver reliable events, and focus-loss handling is important. But which actual callers need subscription/cleanup from different threads and multiple independent keyboard capture handles? Would requiring operations on the visualizer thread and providing a simple camera-keyboard toggle cover the current use case?

I’d like us to validate those boundaries with one real controller before committing to the larger public API.

[rwiltz]

The goal makes sense, but I’m concerned about how much responsibility is going into KeyEventSource. It handles window events, tracks held keys for each listener, manages requests to disable camera controls, and coordinates subscriptions and cleanup across threads.

Could we first connect this to Se2Keyboard or Se3Keyboard and use that example to define the smallest interface we need? Both still subscribe directly to Carb, so the PR does not yet show how the new API fits the existing keyboard controls.

I’d expect the ownership to be roughly:

• The visualizer receives and translates key/focus events and cleans up its window hooks.
• The keyboard controller tracks the keys it uses and turns them into robot commands.
• The viewer owns camera keyboard controls, with the caller deciding when to disable them.

The shared source may need some bookkeeping to deliver reliable events, and focus-loss handling is important. But which actual callers need subscription/cleanup from different threads and multiple independent keyboard capture handles? Would requiring operations on the visualizer thread and providing a simple camera-keyboard toggle cover the current use case?

I’d like us to validate those boundaries with one real controller before committing to the larger public API.

Thanks Octi, agreed on validating against a real controller. There is one: the follow-up branch rwiltz/keyboard-focus-surfaces moves the keyboard teleop to isaaclab_teleop on top of this API and deprecates Se2Keyboard/Se3Keyboard (which still subscribe to Carb directly). I'll open it as a draft PR stacked on this one so both can be reviewed together.

The ownership you describe matches how that branch splits it:

• Visualizer: hooks its window, translates keys to W3C codes, reports focus loss (blur, its UI taking the keyboard, close), and unhooks on close.
• Controller: Isaac Capture's KeyboardSource tracks held keys and turns them into commands; the teleop session attaches it to each visualizer's key_event_source.
• Viewer: owns its camera keys; the caller decides when to turn them off.

On your questions, the real caller doesn't need everything this PR provides:

• Threads: no. The teleop session attaches and detaches on the sim thread (session start/stop and Kit pre-shutdown), the same thread that steps the visualizers. I'll drop the locks and document that subscribing, closing and callbacks all happen on the visualizer thread.
• Per-listener held keys: the controller already does this. Isaac Capture's keyboard ignores repeated presses and releases everything on focus loss. I'll remove that bookkeeping from the source, so it only forwards events and focus loss.
• Multiple capture handles: no real need. The session is the one place that decides camera keys should yield while a keyboard is attached. I'll replace capture_keyboard() with a simple camera-keys toggle on the visualizer, and keep the handle shape Isaac Capture expects in a small adapter in isaaclab_teleop, where that decision is made.

That reduces the public API to key_event_source with add_key_listener(on_key, on_focus_lost) returning a closable subscription, plus the camera-keys toggle. Does that match what you had in mind? If so, I'll push it here and open the stacked PR to show it working end to end.

[ooctipus]

Yes, that matches what I had in mind. Thanks for clarifying.

Let's simplify this PR as you described: keep key/focus events and a closable subscription, remove the locks and per-listener held-key tracking, and replace capture handles with a camera-keyboard toggle.

Please open the controller integration as a follow-up draft PR so we can review both together. That should make the ownership much clearer.

[rwiltz]

run-ci

[ooctipus]

run-ci

[ooctipus]

run-ci