feat(microduck): simulation cockpit and hosted-world extension points - #3943
Open
aromeoes wants to merge 27 commits into
Open
feat(microduck): simulation cockpit and hosted-world extension points#3943aromeoes wants to merge 27 commits into
aromeoes wants to merge 27 commits into
Conversation
…+ agent Adds dimos/robot/microduck: the open-source Microduck biped walking in a small MuJoCo room, driven end to end from humancli. - MicroduckSimModule(MujocoSimModule): composes room + robot MJCF (adding trunk raycast-lidar cameras), runs the pretrained alpha_walking.onnx in the engine step hook at 50 Hz behind a cmd_vel Twist input. No ControlCoordinator; one module is the whole robot. - gait.py implements the 61-dim alpha observation contract (per upstream duck-control/src/obs.rs), reading joint order/home pose/action scale from ONNX metadata. - Command shaping for the policy's ~2.5x velocity undershoot and its yaw deadband (pure turns below ~1 rad/s do nothing); auto-stand fall recovery with a nudge toward open floor (no recovery policy ships). - Room scene with four colored objects; ground-truth object skills (go_to_object/list_objects/move_to/...) over NavigationInterfaceSpec. - Blueprints: microduck-sim (nav + frontier explorer) and microduck-agentic-sim[-ollama] (McpServer/McpClient + skills). - Robot meshes + policy download-on-first-use from pinned upstream commits into ~/.cache/dimos/microduck (CC BY-NC-SA meshes are not vendored). Verified end to end on macOS (headless): explore roams the room, agent (ollama qwen3:8b) sequences begin_exploration/wait/end_exploration/ go_to_object(red_ball) and arrives next to the ball; humancli channel path answers position queries. Claude-Session: https://claude.ai/code/session_01P6rSRXW7dtEabSrQjib6Q9
…oling Replace the four-object room with exactly a red_box and blue_box for the scripted video demo, and add the one-off record/replay tooling used to capture the conversation and re-render it offline into an mp4 (paths are machine-specific). Claude-Session: https://claude.ai/code/session_01WSKm8cPntfux73NeJ7rgZy
…laces + click-to-goal nav
Adds the `microduck-cockpit-sim` blueprint: a four-room MuJoCo flat
(kitchen/living/bedroom/office = spaces A-D around a central hub) driven
from the dimos web cockpit on localhost.
Backend
- DuckControlModule: teleop | agent arbitration (tele_cmd_vel vs the
planner's nav_cmd_vel -> cmd_vel), ui_command routing (set_mode,
cancel_nav, policy toggles), mode / nav_state / policy_state JSON for
the UI, nav cancel through the NavigationInterfaceSpec RPCs.
- Policy registry (policies.py): all RL policies (walk, kicks, roulade,
sit/stand, ...) selectable per variant; sim module runs them in the
loop, exposes one-shot vs locked policies and the fallen/seated state.
- Places memory (places.py + four_room_scene.xml): rooms with aliases
("space A", "lounge"...), landmark objects, remembered spots; skills
go_to_room / go_to_object / go_to_place / remember_place / list_places
/ where_am_i / perform / sit / stand_up / move_to / stop_moving / wait.
ObserveSkill wired for the head camera.
- Nav: goals from map clicks or skills go to ReplanningAStarPlanner on
goal_request; planner config gains optional stuck_time_window /
stuck_threshold (defaults unchanged) because the 0.06 m/s gait tripped
the default stuck detector on every leg.
- Sim command shaping (_shape_twist): linear/angular gains into the
gait's trained range and a yaw deadband bump - pure turns below the
top of WZ_RANGE barely rotate (~3-9 deg/s at 1.0 vs 25-31 at 1.5).
- Cockpit/manifest/protocol: chat, navmap and control panel kinds;
reliable tx datagrams (goal_request, ui_command, human_input).
Frontend (web/cockpit, sdk, relay, shared)
- ControlPanel: teleop/agent toggle + one button per policy with
pending/active/locked states.
- NavMapPanel: costmap + rooms/objects/places overlay, planner path and
goal, click-to-goal, room-label click, Esc/chip cancel; pose drawn on
the overlay so the duck stays visible above room labels.
- ChatPanel: humancli-style transcript (agent/tool rows, thinking
spinner) shown in agent mode, input with retry on failed delivery.
- Teleop panel titled "Teleop (WASDQE)".
Tests: microduck/planner/web pytest suites, cockpit vitest, sdk vitest,
deno shared/relay tests, plus a headless-Chrome E2E used during
development (not committed).
Claude-Session: https://claude.ai/code/session_01WSKm8cPntfux73NeJ7rgZy
Launch, panels, rooms/spaces, agent phrases and the teleop/agent + policy lock semantics for microduck-cockpit-sim. Claude-Session: https://claude.ai/code/session_01WSKm8cPntfux73NeJ7rgZy
Catches the branch up with main (#3568 subs on a reliable carrier, #3569 pickle-safe dynamic bridge classes, #3570 the channel authoring API and web codec registry) and rebuilds our cockpit's plumbing on top of it instead of beside it. What moved onto main's API - The eight microduck encoders leave relay_bridge_module.py's hand-kept CHANNELS table for dimos/robot/microduck/web_codecs.py, registered with @web_encoder. The four JSON-string streams share one function under four encoding ids (the id is what picks a panel slot). The generic bridge no longer imports langchain: the transcript encoder is on the duck's side of the fence now, where langchain is already a dependency. - The blueprint declares its own streams with Channel(...) and passes them to cockpit(channels=...), so cockpit() generates a bridge subclass with a typed port per stream. RelayBridgeModule is back to main's three built-in rx ports. This also removes a real hazard: our `path: In[Path]` port matched by autoconnect name+type against three planner modules that publish `path: Out[Path]`, so any other cockpit blueprint containing a planner was silently wiring it to the web bridge. What main could not express yet, so it grew a little - Channel/RuntimeChannelSpec gain resend_on_subscribe (main hardcoded it False for authored channels; a cockpit opened mid-run must show the duck's current mode, places and nav state), rate_gate (an event stream's skipped sample is lost data, not a dropped frame) and replay_depth (the agent transcript must survive a page reload). - The transcript entry number is the bridge's, not the message's, and main's encoder signature cannot produce it. The bridge now ships it as frame meta and the cockpit registers a chat.json.v1 decoder that folds it back into the record - the decoder-registry pattern from the Web SDK tutorial. The chat panel is untouched. What stayed ours - The whole generic tx stack (Tx wire message, TxChannelDef, _on_wire_tx, the relay's tx allowlist, TeleopHooks.tx): main's Channel still refuses dir="tx" pending its publish ticket (W7), so the three tx Outs stay static on the bridge and move onto Channel when that lands. - The three React panels, DuckControlModule, and everything under dimos/robot/microduck. Tests: the pure-encoder tests move to dimos/robot/microduck/test_web_codecs.py against the new signatures; the replay/rate-gate tests now assert the entry number from the frame meta; the channel-table tests run the real blueprint stack instead of assuming built-ins. Nothing was dropped. Verified: pytest 1264 passed (remaining failures are pre-existing Git-LFS fetch errors in untouched files), cockpit vitest 139, sdk vitest 314, deno relay+shared 133, ruff and deno lint clean; and the live stack passes the 13-step headless cockpit run (teleop, all policies, mode switch, agent "go to the bedroom", click-to-goal, cancel). Claude-Session: https://claude.ai/code/session_01WSKm8cPntfux73NeJ7rgZy
…the cameras
Three fixes found by actually running the thing after the main merge.
1. `dimos run microduck-cockpit-sim` failed with "Unknown blueprint" while
the ollama variant resolved fine. The blueprint registry is generated by
a static AST scan that only recognises `autoconnect(...)` by name or an
expression ending in a builder method; `microduck_cockpit_sim = _stack({})`
is neither, so it was never discovered. The ollama variant only worked
because it happens to end in `.requirements()`. Moving `.global_config()`
out of the `_stack` helper to the assignment sites makes both visible, and
the generator now emits the entry on its own instead of needing the line
restored by hand every time the generation test runs.
2. The chase camera lost ~30% of its frames. The bridge's rate gate drops any
frame arriving less than 1/max_hz after the last, and the cap had been
pinned to the render rate itself (12 Hz), so publisher jitter aliased
against it: 11.5 Hz published, 8.4 fps delivered. Caps now sit above the
render rates.
3. Cameras raised to 30 fps. Measured on an M4: ~25 fps delivered on both
cams (from 11.2 and 4.6), ~540 kB/s (from ~190), sim worker 59% of one
core (from 37%). The simulation is unaffected - real-time factor stays
1.00 - and the 13-step headless cockpit run still passes, so the gait,
policies and nav are all intact at the higher render load. The rationale
and the numbers are in the constants' comment, including why a truer 30
means fewer pixels rather than a higher rate.
Claude-Session: https://claude.ai/code/session_01WSKm8cPntfux73NeJ7rgZy
The cockpit's video collapsed from ~11 fps to ~3.5 fps the moment the duck
started moving. It was never the Mac, the simulation or the browser: the sim
published a steady 27.7 Hz throughout (real-time factor 1.00), the relay
forwarded a steady 25.5 fps, and the page held 60 fps paint with zero long
tasks and 500 fps of spare JPEG-decode headroom. The frames died between the
relay and the browser.
Cause: every `latest` frame costs one relay->viewer QUIC stream, held until
the reaper frees it (web/README.md bug 12), and a real viewer sustains only
~30 latest-frames per second IN TOTAL across all latest channels. The budget
is counted in streams, not bytes - a 500 byte state frame costs exactly what
a 15 kB JPEG costs. The proof was already in /api/stats: nav_state (latest,
710 bps) shed 38% of its frames while mode (reliable, 506 bps) shed 0% at the
identical 4.4 fps in the same session. Four JSON state channels were spending
~7 fps of the video budget, and when the duck moved they got busier and took
more.
- nav_state and policy_state become `reliable`, joining mode and places. They
are state, not frames: one persistent stream, no per-frame cost, and no
drops. This is also more correct - both already set rate_gate=False ("every
sample matters") while `latest` permits dropping them in transit, which
could leave a chip showing a state the robot had already left.
- The head camera drops to 6 fps and the chase camera to 480 x 270. The big
panel should get the budget, and a smaller frame is worth more delivered
fps than a higher requested rate.
Measured, browser-side (the panel's own badge, not the relay's optimism):
chase 3.5 -> 17.5 fps while walking, and it no longer degrades with motion.
Deliberately NOT changed, after measuring: LATEST_STALE_MS (dropping it to
150 ms raises the relay's send rate but resets streams before the browser can
read them - badges fell to 5.5 fps), and the decreasing per-frame sendOrder
(README bug 7 records that equal priority makes quinn round-robin and deliver
in ~1 s waves). The relay's design is sound; it was being asked to carry
state on a video-shaped channel.
Claude-Session: https://claude.ai/code/session_01WSKm8cPntfux73NeJ7rgZy
A UI audit of the microduck cockpit found the panels tracking their state correctly and then not telling anyone. Six fixes, all feedback rather than mechanics. - The teleop pad armed in agent mode. It went green, said "armed - WASD drive, QE strafe, Space stop", lit the keycaps and counted vx up to 0.15 while DuckControl discarded every twist (control_module._on_teleop), so Space - advertised as "stop" - was a no-op too. This was the worst of them: a control that looks live and does nothing reads as a broken robot, and ChatPanel already handles the mirror case gracefully. Teleop gains an optional `mode` role: params-only, so the "teleop binds exactly one channel" manifest rule and every other robot's cockpit are untouched, and an unbound pad behaves exactly as before. Bound, it refuses to arm, says "agent mode - switch to Teleop to drive", and drops the lease if the mode flips while it is armed. - The map's click note never cleared. "goal -> (1.20, 1.00)" sat under the map for the rest of the session, describing a goal several goals ago. - "no path" - a navigation that failed - rendered in the same neutral grey as "idle" and "reached". The chip now takes a tone from the phase. - Cancel was unavailable in the window between accepting a goal and the planner picking it up, where the robot reports "idle" WITH a goal. Terminal phases clear the goal first, so they still cannot be cancelled. - Nav state was drawn twice, in the control strip and on the map, each with its own cancel. The map keeps it, beside the goal marker and path it describes. - Refusals showed the SDK's internal codes: "command not sent (not_tx)". ChatPanel already had a plain-English map; it moves to a shared module and the other two panels use it. Also reverts the chase camera to 640 x 360. The cockpit's video canvas scales down to its panel but never up, so 480 x 270 just drew a smaller picture inside a larger black panel - and it was only worth ~7% more delivered fps. Claude-Session: https://claude.ai/code/session_01WSKm8cPntfux73NeJ7rgZy
The cockpit led with the chase camera; the first-person view was a thumbnail.
Swapped: the duck's view is the big panel and gets the frame budget (30 fps,
640 x 360), the chase camera becomes the small second view (6 fps, 320 x 180)
- still worth keeping, because a first-person duck cannot see its own feet.
That swap exposed a real bug. The robot MJCF's stock `head_camera` is mounted
on the jaw with `quat="0 0 -1 0"`, so it looks along body -x: BACKWARDS, into
the duck's own head. Measured on the composed model, 28 of the duck's own
geoms sit in front of that lens, the nearest at 0.000 m. Rendering it gives a
close-up of the inside of a beak - which is what the cockpit showed, and what
the agent's `observe` skill has been reading all along, so every "what do you
see?" was answered from a picture of jaw.
Adds POV_CAMERA_NAME ("pov_camera"): same eye position as the stock camera
(its offset expressed in the trunk frame), mounted on the trunk rather than
the jaw so the head policies do not shake it, and pointed along body +x with
the existing _camera_quat_wxyz helper. Own-geoms in front of the lens go 28
-> 2. The cockpit blueprint renders it for color_image; the stock camera is
left in place and the other microduck blueprints keep their current default,
so nothing else changes behaviour.
Verified live: the duck view shows the room ahead - floor, walls, the yellow
pillar, the ball - at 26 fps, matching the heading the nav map draws.
Claude-Session: https://claude.ai/code/session_01WSKm8cPntfux73NeJ7rgZy
…d as "stale"
Three things, one of them a real fix.
THE STALLS. The video did not have a low frame rate; it had freezes. Hooking
the canvas the video sink draws into and recording every inter-frame gap for
80 s showed p50 = 35 ms (a steady 28 fps) but a worst gap of 5.3 SECONDS, with
12 gaps over a second and 4 over the 2 s that trips the panel's stale badge.
The average of 21.8 fps hid all of it, which is why earlier fps measurements
were so misleading.
The freezes come from stream-credit exhaustion: every `latest` frame opens a
relay->viewer QUIC stream, held until a reaper frees it, and
createUnidirectionalStream({waitUntilAvailable: true}) has no timeout - when
credit runs dry it blocks silently and unboundedly. Fewer streams per second
means fewer exhaustions, so the two cameras now spend 26 renders/s between
them instead of ~31, allocated to the panel actually being looked at.
Measured again over the same 80 s: worst gap 5261 -> 1764 ms, gaps over 1 s
12 -> 1, gaps over 2 s 4 -> 0, and MORE frames delivered (1743 -> 1873)
because fewer are lost to freezes.
PICTURE-IN-PICTURE. `Video` gains an optional `inset` naming a second feed,
drawn over the corner of the first with its own rate and quality. The
manifest rule for video panels widens from "exactly one channel" to "one, or
two for an inset" in both manifest.py and manifest.ts - a widening, so every
existing single-feed panel stays valid. The cockpit now leads with the chase
camera (you need to see the duck to drive it) and insets the duck's own view,
rather than the two trading places.
Rates follow from what actually costs. Render cost here is geometry, not
pixels - the duck's own 215k-vertex body dominates, so 64x48 measures ~27 ms
against ~29 ms for 640x360 - so the inset keeps full resolution (it is also
the frame `observe` reads) and pays only in rate: 20 fps for the main view,
6 for the thumbnail.
THE "LOADING" GLITCH. An empty transcript said "waiting for the agent...",
which reads as a stuck spinner when the agent is up and simply has nothing to
say yet. It now invites a message. The agent column also gets more width
(shares 5/4/3 -> 5/3/4): the transcript wraps tool calls and their results.
Claude-Session: https://claude.ai/code/session_01WSKm8cPntfux73NeJ7rgZy
Two different bugs wearing the same badge, both found by measuring the live
stack rather than the averages, which hide everything.
THE CHASE CAM'S STALL WAS SELF-INFLICTED. startVideoSink deliberately decodes
nothing while document.hidden - correct, a backgrounded panel should not burn
CPU - but health.lastDrawOkAtMs kept ageing while it sat there. PanelFrame
reads exactly that to decide "frames are arriving but nothing draws", so a tab
flipped away and back always came back reading "stalled", for a gap the panel
itself chose. The mount path already guards this ("a fresh mount is never
instantly stalled"); the visibilitychange path did not. It now resets the same
way when the tab becomes visible again. This is why it looked constant while
testing - every switch to another window and back produced one.
THE NAV MAP'S THRESHOLD WAS VIDEO-SHAPED. Its badge is driven solely by the
costmap, which runs at about 2 Hz, against MapPanel's 5 s threshold - roughly
8 frames of slack, where the video at 17 fps gets 34. Any brief transport
hiccup tripped the map long before it troubled the cameras. It is also the
wrong thing to alarm about: the flat is static, so a grid a few seconds old is
exactly as correct as a fresh one, and the panel keeps drawing rooms, path and
the live pose (odom, reliable, never dropped) regardless. NavMapPanel now uses
its own 15 s threshold, which still catches a genuinely dead mapper.
Measured over 70 s of real inter-frame gaps, hooking the canvas the sink draws
into: 21.3 fps average, p50 46 ms, worst gap 1427 ms, and zero gaps over the
2 s that trips the badge - against a worst of 5261 ms and four stale episodes
before this line of work started.
Claude-Session: https://claude.ai/code/session_01WSKm8cPntfux73NeJ7rgZy
Blue header = a panel kind dimos ships (video, map2d, teleop); orange = one this cockpit added (chat, navmap, control - the three registered under the "Microduck cockpit" comment in panels/registry.tsx). Makes it obvious at a glance which surface is ours when reading or demoing the cockpit. PanelFrame stamps data-panel-kind on the frame and PanelFrame.module.css does the rest, so removing the tint is deleting one attribute and one CSS block. Deliberately NOT data-kind: ChatPanel already uses that on transcript rows for the message role, and overloading it broke a test that selects rows by it. Claude-Session: https://claude.ai/code/session_01WSKm8cPntfux73NeJ7rgZy
…wser leg Debugged from the browser this time, driving the duck rather than watching it idle, and the earlier diagnosis was only half right. WHERE THE FREEZE IS. Recording real inter-frame gaps at the canvas while the duck walked showed every panel stalling at the SAME instants (worst 4319 ms chase, 4395 inset, 4930 costmap) - all channels together, which is not what a per-stream problem looks like. Measuring each hop separately settled it: bus (sim -> anyone) p50 56 ms, worst 79 ms, ZERO gaps over 1 s robot -> relay never flat while the relay kept receiving relay -> browser frozen 2.1-4.1 s, four times in 30 s of movement So the sim is blameless and so is the bridge; the relay->browser leg freezes connection-wide. WHAT IT IS. Bytes, not stream count. Halving JPEG quality leaves the stream count identical and cut the freezes to 0.8-1.3 s; shrinking the inset, worth about 7% of the bytes, barely moved them. That is connection-level flow control, and it explains the synchronisation: one blocked write stops every channel. It also explains why movement is worse - a changing scene compresses worse, so the same frame rate ships more bytes. WHAT CHANGED. The chase camera is ~90% of the byte budget, so it takes the cut: quality 70 -> 40, the highest measured to keep freezes under the 2 s that trips the badge. The head camera drops to 320x180, which it can afford as a 260 px inset and which costs nothing in render time (geometry-bound). Frame RATE is untouched, so movement still runs at a steady 17.9 fps. Measured over 87 s of navigating and driving: stale badges 9 -> 1, worst gap 4930 -> 2563 ms, p50 unchanged at 56 ms. This is a workaround. The freeze is the relay blocking in createUnidirectionalStream (web/relay/session.ts) with waitUntilAvailable and no timeout, stopping every channel instead of dropping one frame on a latest-wins video feed. Fixed there, these numbers could all go back up. Claude-Session: https://claude.ai/code/session_01WSKm8cPntfux73NeJ7rgZy
…g unread depth A headed Chrome drew the microduck chase cam at 10.6 fps with 263 drops and multi-second freezes while the robot walked, against a 16 fps source. Two independent causes, both measured end-to-end in a real browser: 1. `latest` delivery opened one uni stream per frame. That is exactly the pattern ReliableChannel's own docstring indicts: uni-stream credit is only replenished when streams complete, latest streams are never FIN'd (README bug 12), so a 20 Hz video channel spends most of its time wedged in createUnidirectionalStream and sheds every frame offered meanwhile. Add LatestPersistentChannel: same depth-1 newest-wins contract, carried on one persistent stream. `delivery` stays "latest", so the frozen manifest and the wire format are untouched - the viewer never reads the field. A persistent stream drops two things the old design got for free, so reap() rebuilds both: `aborted` stays the suspended-viewer gauge (it now counts reap ticks with a write outstanding past staleMs), and past 2 s the stream is reset so a resuming tab sees fresh video instead of replaying a connection window of stale frames. The healthy path reaches neither. 2. Every camera rendered depth as a second full render of the scene. Nothing reads it for either microduck camera - the nav stack is lidar -> voxels -> 2D costmap, and the chase cam is a JPEG video feed - so the chase cam was spending a 640x360 depth render per frame on the sim thread for nobody. CameraConfig.render_depth (default True, so no behaviour change elsewhere) drives it; the primary camera keeps depth exactly when enable_depth or the RGB-D pointcloud path needs it, extra cameras opt out. Measured in a real headed Chrome, idle / nav / teleop, on a machine already at load average 10: before 10.6 fps, 263 drops, multi-second freezes while walking (1) 17.7 fps, zero drops, zero frames >1s, no stale badges (1)+(2) ~29 fps steady (p50 33-35 ms, p95 67-85 ms), still zero >1s Sim RTF stays 1.000, so the extra frames are not bought from physics. Tests: server_test's backpressure round asserted the old onset (~100 uni streams of credit); a persistent stream wedges on the connection's byte window instead, so it now pumps 128 KB frames and caps its budget below the robot leg's ~100 bidi streams - a regression fails on the assertion rather than a RangeError that says nothing about the relay. Four new forward_test cases cover the policy, including that our own reset does not kick the viewer while a real write failure still does.
A user recording the cockpit with OBS reported the sim going into slow motion - "like time moves slower". It is not the video pipeline: the frame rate stayed fine, simulated time itself fell behind the wall clock. Cameras render inline on the sim thread, and one render costs several times the whole per-step budget (640x360 is ~10 ms against a 5 ms step). The loop had no notion of its own schedule - it just slept whatever was left of dt, so an overrun was silently absorbed by the simulation clock. Measured on the microduck model, physics is 0.5% of the sim thread's second and rendering is essentially all of it, which left only ~2.4x of headroom before ANY of today's changes: enough that ordinary use was fine and a screen recorder competing for the GPU (renders measured 4.2x slower under contention) pushed it straight into slow motion. Track the wall-clock schedule and skip a cycle's camera renders when the loop is already more than RENDER_LAG_BUDGET behind. Contention now costs frame rate, which recovers by itself, instead of simulated time, which does not. Debt past MAX_LAG_CATCHUP (a host suspend, a long GC pause) is abandoned rather than repaid, so one stall cannot suppress rendering forever after. Lidar is deliberately not skipped: it feeds navigation, runs at ~1 Hz, and is a rounding error next to the cameras. Also drop the microduck chase cam 40 -> 30 fps. 40 was mine from the previous commit and, while the depth-render removal had paid for most of it (2.38x tolerance before today vs 2.13x after), 30 fps buys back a real margin at 2.72x for a frame rate the renderer could not sustain under load anyway. Verified: the new test drives the loop with a renderer ~8x slower than real time. Without the governor it measures RTF 0.111 - a 9x slow motion, the reported symptom. With it, RTF > 0.9 and the renders are throttled instead. On the live stack, RTF holds 1.000 under 12 CPU burners with the chase source at 28.4 fps.
web/_bench_ingest.ts is a throwaway ingest probe from diagnosing the video lag; `git add -A` swept it into d2f37f8. Not part of the change.
A parallel review found a second defect in the old pacer, independent of the render overruns the governor addresses: it computed `sleep_time = dt - elapsed` and started each iteration from scratch, with no accumulated schedule. time.sleep() overshoots - 5 ms typically returns after ~6 - so that overshoot was thrown away every single step. The loop therefore could not reach real time even with nothing rendering: measured RTF 0.836 idle, against the 0.83 ceiling that 5/6 ms predicts. The `next_step_at` schedule added in d2f37f8 fixes this as a side effect of fixing the render overruns. This test pins it directly, since the two failure modes are separable and only one of them was deliberate. Also records why the assertion reads engine.data.time rather than the odom timestamp: mujoco_sim_module stamps odom with time.time(), so an odom-ts-vs-wall RTF is wall-vs-wall and reads 1.000 however far behind the simulation is. That instrument reported a healthy 1.000 while the live stack was stepping at 29 Hz of a nominal 200 - a true RTF of ~0.145. Verified against the live stack with a valid instrument (odom message RATE, which is the step rate since odom publishes unthrottled from the after-step hook): 200.0 Hz of 200 nominal, RTF 1.000, chase source 28.6 fps.
…unters
A Codex review (gpt-5.6-sol, read-only) raised three findings against the
previous two commits. Two were plainly right and are fixed here; the third was
a real gap in the EVIDENCE, though not the defect it claimed, and is now
measured by a test.
1. The sim loop paced on time.time(). A wall clock is steerable - an NTP step
or a manual change would make the loop sleep for minutes or sprint through
its debt, and would distort the watchdog and report intervals. Pacing now
uses time.monotonic(); frame and sensor timestamps stay wall-clock, which
is what their consumers expect. The two RTF tests measure elapsed time the
same way.
2. LatestPersistentChannel.aborted counted watchdog TICKS, so one 2 s stall
reported three or more "aborts" in /api/stats, contradicting both the
ChannelPolicy contract ("streams reset while backpressured") and the
field's own comment two lines above, which still said "fixed 0". It now
counts one abort per stream actually reset. The two tests that encoded the
tick count were rewritten around the property.
3. The review's headline finding was that a stale backlog can grow UNBOUNDED
below the depth-1 queue, since write() resolving means "accepted into the
QUIC send buffer", not "delivered", and frames on one ordered stream cannot
be superseded once buffered - and it recommended reverting to the
per-frame LatestChannel until measured.
The mechanism is real and I had no evidence either way: every measurement
so far was of inter-frame GAPS, which cannot see a steady stream of stale
frames. So this adds the missing test - a deliberately slow reader, with
the lag measured in frames. Result: a reader ~4x too slow peaks at 67
frames (~2 s at 30 fps) and STAYS there, shedding 115 of 270 frames. The
backlog is bounded, by QUIC flow control plus the stall watchdog, so the
revert is not warranted: it would return the multi-second freezes and 10.6
fps that the persistent stream fixed.
But the review was right that the class overclaimed. Its docstring said a
slow viewer "sees dropped frames, never lag". It does see lag, ~2 s in the
worst case; what is bounded is how far it falls behind, and the bound comes
from flow control, not from the queue depth. The docstring now says that,
with the measurement and where to find it.
One sub-claim of the finding is wrong and worth recording: the browser does
NOT have to decode every stale frame. startVideoSink draws only the newest
value in the channel store and skips versions it never drew, so stale
frames are overwritten undecoded. They still have to be RECEIVED, which is
what causes the lag.
Not done, deliberately: tightening the ~2 s bound (it needs a byte-budget
reset policy, which is a design change wanting its own measurement), and the
geom-group camera masking - the review declined to propose it without a
benchmark harness, and I agree hiding the duck from the CHASE camera would
defeat that camera's purpose.
Tests: 969 python + 11 mujoco-marked + 103 relay, ruff and deno lint clean.
…ants
The nav footprint numbers lived as three loose constants in BOTH microduck
blueprints, so tuning one silently desynced the other - and their comment
claimed they were the duck's physical size ("~25 cm tall and ~12 cm wide")
while actually carrying margin, which made every reader mis-estimate the
safety budget by centimetres in both directions.
dimos already has the pattern: dimos/robot/unitree/g1/config.py holds a frozen
G1Config with honestly-named `*_clearance` fields that its blueprints read.
Add the microduck equivalent. Values are unchanged (0.2 / 0.28 / 0.3), so this
is a pure refactor - the point is that they now live in one place, under names
that say what they are, with the margin written down.
Measured the real robot to make the docs true: AABB over the body subtree of
the trunk free joint, in the rest pose, is 14 cm wide, 26 cm tall, with a
20 cm footprint circle. So the clearances carry roughly +6, +2 and +10 cm.
(The naive measurement is badly wrong and worth warning about in the helper:
geom_rbound is a bounding-SPHERE radius, which reads ~1.3 m for this
mesh-heavy model.)
Two tests: one mujoco-marked, asserting each clearance still covers the
dimension it clears and has not silently grown into a different robot; one
plain, asserting neither blueprint has grown a local copy back.
Not derived from the MJCF at run time on purpose: a blueprint is a static
declaration evaluated long before the model is loaded, and neither CostMapper
nor ReplanningAStarPlanner accepts a footprint at start-up. That justifies
constants, not duplicated or mis-named ones.
Every dimos package named after a specific robot model sits two levels deep,
under its vendor or class: unitree/{b1,g1,go2}, manipulators/{a1z,a750,
dual_openyam,openarm,openyam,piper,xarm}, diy/alfred - eleven of eleven. Ours
was the only one placed flat, and the flat path also discarded provenance our
own code already knows: assets_fetch.py pulls from pollen-robotics/microduck
and pollen-robotics/microduck_rl.
PR #3937, which adds Microduck support independently, puts it at
dimos/robot/pollen/microduck/. That is the unitree/g1 shape and it is right, so
follow it. A single-tenant vendor directory has precedent in diy/alfred, so the
"extra empty level" objection does not apply. No __init__.py: the repo uses
implicit namespace packages throughout dimos/robot.
Mechanical: git mv plus 49 import references and 5 path strings; the blueprint
registry regenerated to match. Blueprint and module resolution verified for all
six registry entries.
Also fixes a fragility a review flagged in the render_depth change: depth_needed
was bound AFTER the add_camera closure that reads it. It worked only because
the sole call site sits below the assignment, so any future caller placed above
it would have raised NameError. Bound before the closure now, and primary_needed
reduced to `enable_color or depth_needed` - it had been spelling out the same
two conditions a second time.
Tests: 970 python + 24 mujoco-marked, ruff clean.
The last item from the Codex review worth taking. CameraFrame.depth became optional when render_depth landed, and two call sites build an Image from it: the depth_image publisher (gated on enable_depth) and the RGB-D pointcloud back-projection (gated on the raycast lidar being off). Both gates are exactly the conditions that make depth_needed true in start(), so today neither can see a None - the invariant holds by construction, in a derivation living 500 lines away from its two consumers. That is the kind of coupling that rots quietly: change how depth_needed is derived and the failure becomes Image(data=None) surfacing somewhere else, or not at all. Both sites now name the invariant and raise if it is broken. Codex called this "not presently a functional defect", which is right - this buys a clear failure instead of a confusing one, nothing more.
aromeoes
requested review from
Dreamsorcerer,
aclauer,
leshy,
mustafab0,
paul-nechifor and
spomichter
as code owners
September 4, 2026 20:26
github-actions
Bot
added
the
first-time-contributor
Contributor
Greptile SummaryThis change adds generic browser-to-robot command forwarding, including chat, navigation goals, and UI commands. Before merging, the relay must protect motion-affecting generic commands with exclusive authorization; the bridge should also isolate command sequence tracking so one viewer cannot suppress another viewer's commands. Confidence Score: 3/5Not safe to merge until generic motion-affecting commands enforce exclusive authorization. The sequence-isolation problem is non-blocking but should be corrected to avoid losing valid commands when multiple viewers are active. The authorization failure and the command-loss behavior were both reproduced with focused executions against the changed command paths. Files Needing Attention: web/relay/registry.ts must authorize generic motion-affecting commands; dimos/web/relay_bridge/relay_bridge_module.py must associate sequence state with the sending viewer or session.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+368
to
+396
| case "tx": { | ||
| // Generic command to a non-twist tx channel of the watched robot. No | ||
| // lease: chat text, goals and UI commands are not motion (twist/stop | ||
| // stay lease-gated). Manifest-gated so a viewer cannot invent robot | ||
| // stream names, and size-gated so the robot-ward datagram always | ||
| // fits. Dropped silently like a gated twist: the SDK runs the same | ||
| // checks before sending, so a drop here is a manifest race or a | ||
| // misbehaving client, and an error reply would only surface as a | ||
| // session-level banner. | ||
| const entry = viewer.watched === null ? undefined : this.#robots.get(viewer.watched); | ||
| if (entry === undefined) { | ||
| this.#dropTx(viewer, "not watching a live robot"); | ||
| break; | ||
| } | ||
| if (!entry.txChs.has(msg.ch)) { | ||
| // msg.ch is already bounded (TX_CH_PATTERN, <= 64 chars) by the | ||
| // wire decoder, so it is safe to echo. | ||
| this.#dropTx(viewer, `no tx channel ${msg.ch} on ${viewer.watched}`); | ||
| break; | ||
| } | ||
| if (encodeDatagram(msg).byteLength > MAX_TX_MSG_BYTES) { | ||
| this.#dropTx(viewer, `tx on ${msg.ch} over ${MAX_TX_MSG_BYTES} B`); | ||
| break; | ||
| } | ||
| // Same lossy robot-ward leg as twist/stop (the relay never writes on | ||
| // robot-opened streams, see session.ts); the bridge's per-channel | ||
| // seq lets it discard a reordered command. Forwarded as-is: TxMsg | ||
| // admits no extra keys, so there is no relay stamp to add. | ||
| entry.peer.sendMsg(msg); |
Contributor
There was a problem hiding this comment.
The generic tx route forwards declared goal_request and ui_command messages after checking only that the sender watches the robot, the channel is declared, and the message fits. It never checks that the sender owns the teleoperation lease. Another viewer can therefore send navigation goals or control-mode actions while a different viewer holds exclusive teleoperation, allowing conflicting robot actions outside the exclusive-control boundary. Require the lease, or an equivalent command-specific authorization check, before forwarding motion-affecting channels.
How this was verified: With viewer 1 holding teleoperation, viewer 2's goal and UI commands were both forwarded to the robot while the lease remained assigned to viewer 1.
Knowledge Base Used:
Comment on lines
+1141
to
+1143
| last_rx = self._tx_last_rx.get(msg.ch, -math.inf) | ||
| if msg.seq <= self._tx_last_seq.get(msg.ch, -1) and now - last_rx < _TX_SEQ_WINDOW_S: | ||
| return |
Contributor
There was a problem hiding this comment.
_on_wire_tx stores each channel's sequence high-water mark only by msg.ch. Separate browser sessions maintain independent counters, so a reloaded page or another active viewer beginning at sequence 1 is treated as reordered traffic after a recent higher sequence from a different viewer. Its valid chat, goal, or UI command is silently dropped for up to one second. This is a non-blocking reliability concern, but it makes multi-viewer command delivery appear successful when the robot never receives the command. Scope sequence state to the originating viewer or session, or assign the sequence at the relay.
Knowledge Base Used: Web relay protocol and sessions
Artifacts
- This authored executable extracts and runs the current `_on_wire_tx` method body with two browser sequence scenarios, showing the channel-only high-water behavior.
- The executed first-viewer capture shows `seq=50` publishing `viewer-one` and establishing a channel high-water of 50.
- The executed second-viewer capture shows immediate `seq=1` does not add `viewer-two` to the published output while the high-water remains 50, proving the command is silently dropped.
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
aromeoes
changed the title
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
1 participant
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.
A web cockpit for Pollen Robotics' Microduck in MuJoCo, using DimOS navigation, robot policies, and an MCP agent. The browser can drive the duck, select locomotion policies, navigate to places or map clicks, and view camera feeds and agent activity over WebTransport.
The Microduck stack lives under
dimos/robot/pollen/microduck. Navigation composes the existing voxel mapper, cost mapper and replanning A* modules. Run the bundled scene withuv run dimos run microduck-cockpit-sim --local-relay.This branch also exposes the shared extension points used by a separately packaged hosted world:
Earlier changes on this branch correct simulation deadline pacing, use persistent newest-frame QUIC delivery, and avoid unnecessary depth renders. The hosted application's lobby, independent robot blueprints, Three.js renderer, scene assets, head-camera correction and manual respawn UI remain in the external
microduck-worldproject. They are not copied into DimOS demos.Checkpoint: merged
mainat3d65f28(acknowledged shared publishing). The existing cockpit command path and bounded replay coexist with upstream publishing; the SDK, relay and bridge reject legacy commands aimed at acknowledged-publish channels. Explicit publish declarations can retype a legacy command port without overriding motion or module methods. Regression tests cover this boundary. The merge also fixes reported license/section-marker/export-list checks and separates Microduck bridge tests from the general relay suite.Validation for this checkpoint:
robot_allcollisions.xmlmodel; both stayed upright and the measurements were unchanged.The Omarchy application is independently committed at
579c6e7in/home/tule/projects/microduck-world. Its lobby, per-duck agents/maps, Three.js world, football room and physics assets remain external to this PR. The hosted runtime stays on its tested vendor pin; this merge does not silently upgrade the running service.Remaining integration considerations: assets are fetched from pinned Pollen commits and retain their upstream license; the policy executor is simulation-specific and has not been validated on physical hardware. This overlaps the independent Microduck work in #3937 and needs reconciliation before merge. MuJoCo-marked suites are not selected by the default CI test command.