Krishna/issue 2463 aes#2476
Open
KrishnaH96 wants to merge 8 commits into
Open
Conversation
Unitree G1 firmware >=1.5.1 (and Go2 >=1.1.15) use a data2=3 WebRTC handshake requiring a per-device AES-128 key; without it the driver fails with "RSA key format is not supported". Thread an optional aes_128_key from the Go2/G1 connection configs (falling back to the UNITREE_AES_128_KEY env var) down to UnitreeWebRTCConnection, forwarding it to the driver only when set so robots that don't need a key are unaffected. Re-applies the source changes from #2117 onto current main. Refs #2463
The unitree-webrtc-connect-leshy fork caps at 2.0.7, which lacks the data2=3 WebRTC handshake and aes_128_key support required by the latest Go2/G1 firmware. Upstream unitree-webrtc-connect 2.1.2 has both, and the leshy fork's lidar-parser changes have since landed upstream (per #2117 discussion). Same import name, so no code changes - only the pin + lock. Validated: Go2 (AP mode) connects with camera/map/teleop on 2.1.2, no regression; uses the new con_notify LAN signaling path cleanly. Refs #2463
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes 🚀 New features to boost your workflow:
|
connect() ran the driver handshake in a fire-and-forget task and blocked on an event only set on success, so failures (e.g. AesKeyRequiredError on firmware requiring the per-device key) were swallowed and the deploy hung with no error. Use the same run_coroutine_threadsafe(...).result() bridge as the rest of the class, which propagates the exception to the caller, and stop the loop thread on failure. Removes the unused connected_event and the keepalive task. Verified on a Go2 on latest firmware: without a key the run now fails in seconds showing the driver's actionable error; with UNITREE_AES_128_KEY set it connects as before. Refs #2463
KrishnaH96
requested review from
leshy,
mustafab0,
paul-nechifor and
spomichter
as code owners
Contributor
Greptile SummaryThis PR closes a firmware compatibility gap for newer Unitree robots (G1 ≥1.5.1, Go2 ≥1.1.15) that require an AES-128 key for the
Confidence Score: 5/5Safe to merge. The AES key forwarding is additive and gated behind a falsy check, so robots that don't need a key receive a byte-identical call to before. The connect() rewrite correctly propagates driver exceptions and cleans up the background thread on failure, replacing a silent infinite hang. All three changes are well-scoped: the key-forwarding chain is consistent across Go2, G1, and fleet paths; the library swap preserves the import name so no downstream code changes were required; and the connect() error-propagation fix is backed by unit tests that exercise both the happy path and the failure path. No correctness defects were identified in the new code paths. No files require special attention. The fleet_connection.py comment honestly documents the per-device key limitation for heterogeneous fleets as a known future improvement. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller as Caller (main thread)
participant Cfg as Config (G1Config or ConnectionConfig)
participant GC as GlobalConfig
participant Conn as UnitreeWebRTCConnection
participant BG as Background asyncio thread
participant Driver as LegionConnection 2.1.2
Caller->>Cfg: construct config
Cfg->>GC: read unitree_aes_128_key via default_factory
GC-->>Cfg: key string or None
Caller->>Conn: __init__(ip, aes_128_key)
Conn->>Driver: "LegionConnection(LocalSTA, ip, aes_128_key=key if key)"
Conn->>BG: start thread running run_forever()
Conn->>BG: run_coroutine_threadsafe(async_connect()).result()
BG->>Driver: await conn.connect()
alt connect succeeds
Driver-->>BG: ok
BG->>Driver: disableTrafficSaving + set_decoder + publish_request_new
BG-->>Conn: Future resolved
Conn-->>Caller: returns (loop stays running for future commands)
else connect fails
Driver-->>BG: raises exception
BG-->>Conn: Future resolved with exception
Conn->>BG: call_soon_threadsafe(loop.stop) then thread.join()
Conn-->>Caller: re-raises exception
end
Reviews (4): Last reviewed commit: "Merge branch 'main' into krishna/issue-2..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
github-actions
Bot
added
the
ready-to-merge
leshy
reviewed
Jun 14, 2026
| # Per-device AES-128 key required by G1 firmware >= 1.5.1 (data2=3 WebRTC handshake). | ||
| # Fetch with: unitree-fetch-aes-key --email YOU --sn <serial> | ||
| if not aes_128_key: | ||
| aes_128_key = os.environ.get("UNITREE_AES_128_KEY") |
Member
There was a problem hiding this comment.
we have a config system that parses env vars already, shouldn't touch env within modules, just use config
Contributor
Author
There was a problem hiding this comment.
Done. Moved it to the config system in 392ebb9. Added unitree_aes_128_key to GlobalConfig (reads UNITREE_AES_128_KEY) and the Go2/G1 connection configs default aes_128_key from it via Field(default_factory=lambda m: m["g"].unitree_aes_128_key), same pattern as ip ← robot_ip. The module no longer touches os.environ.
Per review: dimos already parses env vars through GlobalConfig, so the connection shouldn't reach into os.environ directly. Add a unitree_aes_128_key field to GlobalConfig (reads UNITREE_AES_128_KEY) and default the Go2/G1 connection configs' aes_128_key from it, mirroring how ip defaults from robot_ip. UnitreeWebRTCConnection now only forwards the kwarg it is given. Tests retargeted from the leaf env-read to the config layer. Refs #2463
github-actions
Bot
removed
the
ready-to-merge
…ption Address review on the connect() fast-fail: self.task is no longer assigned to a live coroutine, so the field and the unreachable if self.task: cancel guards in stop()/disconnect() are removed. The connect cleanup now catches Exception rather than BaseException; the loop thread is daemonized, so a KeyboardInterrupt during connect propagates without needing our teardown. Refs #2463
github-actions
Bot
added
the
ready-to-merge
github-actions
Bot
added
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
2 participants
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.
Closes the firmware gap from #2463. Re-applies #2117 onto current
mainplus the upstream-lib switch agreed in that thread.New Unitree firmware (G1 ≥1.5.1, Go2 ≥1.1.15) uses a
data2=3WebRTC handshake requiring a per-device AES-128 key; without it dimos cannot connect.Changes
aes_128_key(orUNITREE_AES_128_KEYenv var) from the Go2/G1 connection configs down toUnitreeWebRTCConnection, passed to the driver only when set (byte-identical for robots that don't need it).unitree-webrtc-connect-leshy(caps at 2.0.7, nodata2=3) → upstreamunitree-webrtc-connect2.1.2, which has the handshake +aes_128_key. Per @leshy in feat(unitree): forward aes_128_key to WebRTC driver for G1 firmware >=1.5.1 #2117 the fork's lidar-parser changes have landed upstream; same import name → no code changes beyond the pin.connect()swallowed driver exceptions in a fire-and-forget task and hung forever (no error shown). It now uses the samerun_coroutine_threadsafe(...).result()bridge as the rest of the class, so failures raise with the driver's actionable message, and the loop thread is cleaned up on failure. Adds 2 unit tests.Validation (real hardware)
Latest firmware — Go2 updated to current firmware:
main, no key: silent infinite hang (reproduced — motivates change 3).AesKeyRequiredError: This robot speaks data2=3 … Pass aes_128_key=….unitree-fetch-aes-key --email <you> --sn <serial>): ✅ connects, camera/map/teleop work — exercises the forwarding end-to-end.No-regression — older firmware, no key:
main(also re-verified on 2.1.2, cleancon_notifysignaling).Pending: new-firmware G1 validation (will try to upgrade firmware of g1 here in the office, meantime @natcl offered, cc @natcl).
Refs #2463
Contributor License Agreement