Skip to content

fix(teleop): run the keyboard teleop window in a child process on macOS - #4950

Merged
TomCC7 merged 2 commits into
mainfrom
tule/fix/keyboard-teleop-macos
Oct 31, 2026
Merged

TomCC7 merged 2 commits into
mainfrom
tule/fix/keyboard-teleop-macos

Conversation

@aromeoes

@aromeoes aromeoes commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

What is this feature?

KeyboardTeleopModule now works on macOS.

  • X11 is forced only on Linux.
  • On macOS, the existing _pygame_loop runs unchanged in a child process: the module re-runs itself with python -m, and publishes the twist / gripper lines that process prints.
  • Linux keeps the original thread, unchanged.

Why do we need this?

On macOS the module failed in two ways:

  1. It forced SDL's X11 driver, which macOS doesn't have (x11 not available).
  2. It opened its pygame window from a worker thread. Cocoa only allows windows on a process's main thread, so the worker died with an NSException.

As a result, every arm keyboard-teleop-* blueprint was unusable on Mac.

How to Test

On macOS:

dimos run keyboard-teleop-xarm7

Uses mock hardware. Click the Keyboard Teleop window and hold W/A/S/D/Q/E; the arm moves in Viser (http://127.0.0.1:8095) and stops on release.

Checklist

  • I have read and approved the CLA.

KeyboardTeleopModule failed on macOS in two ways: it forced SDL's X11
driver, which macOS lacks, and it opened its pygame window from a worker
thread, which Cocoa rejects with an NSException that kills the worker.

Force X11 only on Linux. On macOS, run the existing _pygame_loop in a
child process (this module run with -m), where it owns the main thread,
and publish the commands it prints. Linux keeps the thread unchanged.
@github-actions github-actions Bot added the first-time-contributor PR opened by an author who had not previously committed to this repository label Oct 30, 2026

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go2 replay realtime (arm64)

Details
Benchmark suite Current: b16b012 Previous: 1505f35 Ratio
peak memory 1735.781 MB 1735.602 MB 1.00
peak threads 374 threads 372 threads 1.01
network (transport) 2728.404 MB 2728.511 MB 1.00
disk write 1.742 MB 1.898 MB 0.92
instructions 138.085 G 136.483 G 1.01

This comment was automatically generated by workflow using github-action-benchmark.

@greptile-apps

greptile-apps Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The change reaches macOS users of a shared keyboard teleop feature.

The subprocess-cleanup requirement must be satisfied before merging. The separate regression-test gap is non-blocking.

Findings

  1. P2 Stopped children keep resources ▶
  2. P2 Mac controls lack regression tests ▶

Summary

Moves the macOS keyboard window into a child process so pygame runs on its main thread.

  • macOS keyboard teleop opens its window in a child process.

Reviews (1) · Last reviewed commit: "fix(teleop): run the keyboard teleop win..." · Reviewed by Greptile

@codecov

codecov Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 36.36364% with 21 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/teleop/keyboard/keyboard_teleop_module.py 36.36% 19 Missing and 2 partials ⚠️
@@            Coverage Diff             @@
##             main    #4950      +/-   ##
==========================================
- Coverage   83.15%   82.90%   -0.26%     
==========================================
  Files        1743     1743              
  Lines      167653   167684      +31     
  Branches    13870    13877       +7     
==========================================
- Hits       139418   139014     -404     
- Misses      24485    24905     +420     
- Partials     3750     3765      +15     
Components Coverage Δ
Tests 96.54% <ø> (-0.61%) ⬇️
Flag Coverage Δ
OS-macos-latest 77.26% <30.30%> (+<0.01%) ⬆️
OS-ubuntu-24.04-arm 77.05% <33.33%> (-0.01%) ⬇️
OS-ubuntu-latest 77.43% <33.33%> (-0.01%) ⬇️
Py-3.10 77.42% <33.33%> (-0.01%) ⬇️
Py-3.11 77.41% <33.33%> (?)
Py-3.12 77.42% <33.33%> (-0.01%) ⬇️
SelfHosted-Linux 40.36% <33.33%> (-0.22%) ⬇️
SelfHosted-macOS 40.08% <30.30%> (-0.77%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/teleop/keyboard/keyboard_teleop_module.py 58.75% <36.36%> (+6.03%) ⬆️

... and 34 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Oct 30, 2026
@TomCC7
TomCC7 enabled auto-merge (squash) October 31, 2026 00:27
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Oct 31, 2026
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Oct 31, 2026
@TomCC7
TomCC7 merged commit c339308 into main Oct 31, 2026
39 of 41 checks passed
@TomCC7
TomCC7 deleted the tule/fix/keyboard-teleop-macos branch October 31, 2026 00:49
@mustafab0

Copy link
Copy Markdown
Contributor

🙏
@aromeoes

This branch was previously deployed

1 inactive deployment
cachix — b16b0129 Deployed Oct 31, 2026 by TomCC7 via cachix-build-macos #15301
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

first-time-contributor PR opened by an author who had not previously committed to this repository ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants