Zenoh is the default transport, pinned to localhost - #3617
Open
leshy wants to merge 10 commits into
Open
Conversation
Every platform now defaults to zenoh; --transport=lcm is the opt-out. A stock peer used to listen on tcp/*:<ephemeral>, reachable from the LAN, even though its multicast scouting was already loopback-only. The listener now follows the same rule the scouting interface does: pinned to tcp/127.0.0.1:0 while discovery never leaves loopback, handed back to zenoh's default the moment scouting is turned outward. Dialing out is untouched, so --robot-ip still reaches a robot. A client has no listener to pin, and a router still has to name its own. Zenoh also needed the isolation LCM has had: conftest gives every xdist worker its own bus so sibling workers can't leak into each other, and zenoh's loopback discovery group was shared by all of them. ZENOH_SCOUT_ADDR moves each worker onto its own group, which is a session setting like any other -- so it rides the wire to native modules too, and the Rust side parses and applies it (deny_unknown_fields would have refused the launch otherwise).
leshy
requested review from
Dreamsorcerer,
mustafab0,
paul-nechifor and
spomichter
as code owners
August 22, 2026 23:51
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
A module travels to its worker process by pickle, and getattr on it hands back a bound method -- which drags the whole instance, transports and RPC service included. LCMService has always shed its runtime handles for this; the zenoh classes never did, so the worker died on `cannot pickle 'builtins.Session'` the moment zenoh became the default. ZenohService, ZenohPubSubBase and ZenohRPC now drop the same kind of state (session, publishers, subscribers, queryables, call pool, locks) and rebuild it on the far side. The reason that took so long to see: _worker_loop caught BrokenPipeError and EOFError around conn.send, so a TypeError there killed the worker mid-send and left the coordinator blocked on a pipe nobody would answer. An unsendable response is now reported to the caller instead. Two fixes to the commit before this one. ZENOH_SCOUT_ADDR was set with setdefault, but the pytest controller runs conftest first and exports it, so every worker inherited one group and the isolation never fired -- the neighbouring LCM and MCP lines assign, and so does this one now. And the thread monitor flagged zenoh's pyo3-closure threads: they belong to a pooled session that outlives the test on purpose, since sharing one session is the point of the pool and closing it per test would cut module-scoped fixtures off from their transports.
These tests each pinned one backend while the code under test followed the configured one, so the two halves landed on different buses the moment the default moved. The async-module fixtures built pLCMTransport by hand, published into it, and waited on a queue their zenoh-side modules never fed; they use the transport factory now, same as production. test_remapping and test_fleet_blueprint asserted LCM-shaped topic strings, which are only correct for one backend -- transport_topic spells them for whichever is active. test_autoconnect and the webrtc raw-pin test are about pinning itself, so their pins now sit on the active backend, where the backend switch leaves them alone and the tests keep meaning something either way. A native module's arg is the transport's channel, so the factory produces the expectation rather than a literal. e2e_tests is pinned to LCM instead: LcmSpy sniffs the raw LCM bus and observes nothing at all under zenoh, so the package cannot be ported without a zenoh spy. The env var reaches the dimos CLI these tests spawn.
The session-wide ZENOH_SCOUT_ADDR export leaked into "stock default" configs; the wire goldens had to dodge it by hand. Pin it like every other zenoh field.
Two stale claims said TF stayed on LCM; the bridge listens only on the configured backend (bridge.py), dual-listen is an explicit override.
leshy
force-pushed
the
ivan/feat/zenoh-default
branch
from
August 23, 2026 00:09
d477c7f to
0e2265f
Compare
leshy
changed the title
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #3617 +/- ##
==========================================
- Coverage 77.26% 77.25% -0.02%
==========================================
Files 1262 1262
Lines 120302 120423 +121
Branches 10600 10607 +7
==========================================
+ Hits 92953 93030 +77
- Misses 24284 24330 +46
+ Partials 3065 3063 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 9 files with indirect coverage changes 🚀 New features to boost your workflow:
|
When the default flipped to zenoh, these tests followed it and LCM silently lost its only module/coordinator/RPC coverage. An each_transport fixture pins the backend per param; the bus-coupled fixtures depend on it, the in-process main() tests stay single.
Contributor
Greptile SummaryZenoh now defaults to localhost-scoped discovery while preserving local communication and explicitly configured robot connections. The WebSocket server reports restart as unsupported, but stopping it while a viewer remains connected can leave the listening port bound and prevent a replacement server from starting. The earlier report that restart uses a closed loop is no longer present: the exercised restart path raises the documented Confidence Score: 4/5Do not merge until WebSocket shutdown reliably releases its listening socket when a viewer is connected. A connected viewer can leave the configured WebSocket port unavailable after shutdown, blocking a replacement viewer service from binding it. Files Needing Attention: dimos/visualization/rerun/websocket_server.py
 T-Rex LogsWhat T-Rex did
Reviews (4): Last reviewed commit: "rerun: a stopped websocket server says s..." | Re-trigger Greptile |
A session's callback threads are non-daemon, so any script that touched the default pool hung past its last line (the executable docs timed out at 120s). atexit can't help -- interpreter shutdown joins those threads first -- so the pool close rides threading's own shutdown hook, which runs before the join. Default pool only; an explicit pool has an owner. The refreshed doc results show the zenoh-era config output.
loopback_session() predates scout_addr, and the rust side deliberately has no defaults, so the host rejected its launch config and exited before listening. The listener wait now surfaces the host's stderr in the assertion, which is the diagnostic CI was missing.
stop() raced its own teardown: Module.stop() killed the event loop before the serve coroutine closed the listening socket, leaking the bound port for the life of the process. Zenoh's stop timing loses that race reliably (LCM won it, which is why main never saw this). The next test in the same xdist worker then hit EADDRINUSE -- swallowed in a never-checked future -- and start() waited on _server_ready without a timeout until pytest-timeout shot the worker: 'node down' in CI. stop() now waits for the serve future so the port is free on return, start() surfaces the bind error and bounds the wait, and the tests bind port 0 and read back bound_port, retiring the shared hardcoded ports.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
github-actions
Bot
added
the
ready-to-merge
Restart was never a module lifecycle -- the loop dies with stop() -- but a second start() failed on a bare assert while stale state (_server_ready, bound_port, client_connected) kept claiming a listener existed. Now stop() clears them and start() on a stopped server raises with words.
github-actions
Bot
removed
the
ready-to-merge
github-actions
Bot
added
the
ready-to-merge
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.
dimos/core/transport_factory.pywill need a rewrite, we need dimos normalized qos, would be nice to start removing pickling and defining custom lcm_msg everywhere, but will do this later.rpc also needs conversion to json