feat(webrtc): send robot_id through the broker transport#2492
feat(webrtc): send robot_id through the broker transport#2492spomichter wants to merge 1 commit into
Conversation
spomichter
requested review from
leshy,
mustafab0 and
paul-nechifor
as code owners
Greptile SummaryThis PR removes the
Confidence Score: 4/5Safe to merge as-is, but the comment example in blueprints.py actively promotes a usage pattern that silently splits data and video into two separate broker sessions if followed literally. The broker.py change itself is clean and intentional. The concern is the new comment in blueprints.py: it shows robot_id on CloudflareTransport without mentioning that CloudflareVideoTransport must carry the same robot_id to share the provider singleton. A user who follows the example will end up with two separate broker sessions — one carrying data channels and one carrying the video track — with no error, just silently broken teleop video. dimos/teleop/quest_hosted/blueprints.py — the comment example needs a corresponding CloudflareVideoTransport(robot_id=...) to avoid misleading callers. Important Files Changed
Sequence DiagramsequenceDiagram
participant Blueprint as Blueprint Definition
participant CT as CloudflareTransport(robot_id="go2-a")
participant CVT as CloudflareVideoTransport(no robot_id)
participant BC1 as BrokerConfig(robot_id="go2-a")
participant BC2 as BrokerConfig(robot_id="")
participant Cache as Provider Cache (keyed by config equality)
participant BP1 as BrokerProvider A (session A)
participant BP2 as BrokerProvider B (session B)
participant Broker as dimensional-teleop Broker
Blueprint->>CT: "CloudflareTransport(robot_id="go2-a")"
Blueprint->>CVT: CloudflareVideoTransport()
CT->>BC1: "BrokerConfig(robot_id="go2-a")"
CVT->>BC2: "BrokerConfig(robot_id="")"
BC1->>Cache: cache.get(BC1) miss
Cache->>BP1: create BrokerProvider
BC2->>Cache: cache.get(BC2) miss (different key)
Cache->>BP2: create BrokerProvider
BP1->>Broker: "POST /sessions {robot_id: go2-a, sdp_offer: ...}"
BP2->>Broker: "POST /sessions {robot_id: empty, sdp_offer: ...}"
Note over BP1,BP2: Two separate broker sessions - data channels on A, video on B
Reviews (3): Last reviewed commit: "feat(webrtc): robot_id as a per-transpor..." | Re-trigger Greptile |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 56 files with indirect coverage changes 🚀 New features to boost your workflow:
|
spomichter
force-pushed
the
feat/webrtc-robot-id
branch
from
938df64 to
7bb950e
Compare
robot_id rides the pickled BrokerConfig to the provider, so multiple
robots under one key — even two transports in one process — stay
distinct: CloudflareTransport("cmd_unreliable", TwistStamped, robot_id="go2-a").
A process/env-global id can't distinguish them, so no GlobalConfig field
or env bridge; BrokerProvider reads config.robot_id only (TELEOP_ROBOT_ID
dropped).
spomichter
force-pushed
the
feat/webrtc-robot-id
branch
from
7bb950e to
06e8cee
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # robot_id is a per-transport kwarg (rides the pickled config to the provider), | ||
| # so running several robots off one key just means a distinct id per blueprint — | ||
| # e.g. CloudflareTransport("cmd_unreliable", TwistStamped, robot_id="go2-a"). | ||
| # Omitted here → empty; the broker still keeps sessions distinct by session id. |
There was a problem hiding this comment.
robot_id only on CloudflareTransport while leaving CloudflareVideoTransport() with no robot_id. Because ProviderConfig.provider() is a per-process singleton keyed on config equality, a data transport with BrokerConfig(robot_id="go2-a") and a video transport with BrokerConfig(robot_id="") resolve to two different providers — two separate broker sessions. Video lands on one session; data channels on the other, silently breaking teleop. The robot_id must be set identically on every transport in the blueprint that should share a session.
| # robot_id is a per-transport kwarg (rides the pickled config to the provider), | |
| # so running several robots off one key just means a distinct id per blueprint — | |
| # e.g. CloudflareTransport("cmd_unreliable", TwistStamped, robot_id="go2-a"). | |
| # Omitted here → empty; the broker still keeps sessions distinct by session id. | |
| # robot_id is a per-blueprint label: every transport in the blueprint must carry | |
| # the same robot_id so they all resolve to the same per-process provider | |
| # singleton (configs are compared by equality — a mismatch creates two separate | |
| # broker sessions, splitting data channels from the video track). | |
| # e.g. CloudflareTransport("cmd_unreliable", TwistStamped, robot_id="go2-a"), | |
| # CloudflareVideoTransport(robot_id="go2-a") | |
| # Omitted here → empty; the broker still keeps sessions distinct by session id. |
1 participant
Stacks on #2048 (base =
feat/webrtc-transport) — does not modify it.What
Wires
robot_idend-to-end on the transport path so one API key across multiple robots registers as distinct robots:GlobalConfig.robot_id(per-machine id; auto--robot-idflag; distinct fromrobot_model).runCLI exportsROBOT_IDto env before workers spawn — aBrokerProviderisn't a Module, so it gets nog; env is the only channel that reaches a forkserver worker.BrokerProviderreadsglobal_config.robot_id(hydrated fromROBOT_IDin the worker) and sends it asbody.robot_idon session create.TELEOP_ROBOT_IDdropped.Why
The broker (dimensional-teleop, deployed) already consumes
body.robot_idfor tenant-scoped dedup — but nothing populated it from the new field, so it wasn't actually sent. This is the client half.Test
ROBOT_ID=lab-go2-7→GlobalConfig.robot_id→BrokerProvider._robot_id→ sent asbody.robot_id. Verified (assert).Run:
dimos --robot-id lab-go2-7 run teleop-hosted-go2-transport. Supersedes #2490 (the standalone field PR off main) — close that.