Repository navigation
JUST TRYING THING OUT refactor(rpc): replace pickle with CBOR in RPC and the worker pipe - #4821
Open
paul-nechifor wants to merge 3 commits into
Open
paul-nechifor wants to merge 3 commits into
paul-nechifor wants to merge 3 commits into
Conversation
ZenohRPC, LCMRPC and the forkserver worker pipe now serialize with a CBOR codec (dimos/protocol/encode/cbor.py) instead of pickle. Tuples, NamedTuples, sets, numpy arrays and scalars, enums, dataclasses, pydantic models, Path, message types (class ref plus LCM bytes), classes, generic aliases, module-level functions and functools.partial round-trip as their original types. Anything else raises TypeError at the call site naming the type and its path in the payload; classes opt in with cbor_encode()/cbor_decode() or register_codec(). Transports, RPCClient and its proxies, Actor, streams, Blueprint, system configurators, LCM and Zenoh services, the relay-bridge metaclass, lazy asset paths and grasp candidates gain the hooks; their pickle hooks are removed. StreamDescriptor carries the real type and transport, HostFragment encodes its payload with the codec, and the pickled pubsub transports keep pickle. Decode failures surface at the caller instead of timing out, a worker that cannot decode a request replies with an error, and the worker encodes before taking its lock. Also fixes start/stop on DemoCalculatorSkill and OsmSkill (missing @rpc, so pickle ran them on a parent-side copy) and gives dimos Header its own lcm_decode.
paul-nechifor
requested review from
Dreamsorcerer,
TomCC7,
aclauer,
bogwi,
jeff-hykin,
leshy,
mustafab0 and
spomichter
as code owners
October 24, 2026 18:45
❌ 9 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Contributor
 [High impact] The change reaches shared RPC and worker communication across Dimos features. Not safe to merge until image roundtrips and DemoRobot startup are fixed; the blueprint-test coverage concern is non-blocking. FindingsSummaryReplaces pickle with CBOR for RPC, worker messages, and hosted fragments, adding typed encoding and custom support for proxies, streams, transports, and lazy paths. Two compatibility regressions must be fixed before merging: floating-point RGB images fail to decode and ordinary images become read-only; DemoRobot startup fails because its plain lifecycle methods cannot cross the new worker pipe. The new blueprint test also has a non-blocking coverage gap: unexpected import errors become skips instead of test failures.
Reviews (1) · Last reviewed commit: "refactor(rpc): replace pickle with CBOR ..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Review fixes for the pickle-to-CBOR commit. Decode no longer calls anything the payload names. A registered codec travels as its type plus the registering module; the receiver imports that module and takes the decoder from its own registry, so a payload naming builtins:eval is refused. StreamDescriptor carries a TransportSpec instead of a live transport, so list_streams and dimos shell no longer open an LCM socket per stream on every call. TransportSpec moves to dimos/core/stream.py and describes a live transport with TransportSpec.of(). Every transport's cbor_encode() returns constructor kwargs and PubSubTransport.cbor_decode is cls(**data); ZenohTransport and pZenohTransport take a qos kwarg, DDSTransport accepts the qos dict it encodes, and ROSTransport drops its unused kwargs. LCM RPC wraps bodies as bytes in a primitives-only envelope with a wire version: an undecodable request gets an error reply, an undecodable reply raises at the caller instead of timing out, and traffic from another dimos version is logged and ignored. Codec: encode() only raises TypeError and hook failures keep their path, enums travel by value so Flag combinations survive, typing.List, Optional, Union, Literal and Any are accepted, union and generic-alias payloads must hold types, a missing dataclass field names the class and field, and the fields-only dataclass rule and the pydantic private-state rule are documented and tested. The worker decodes replies outside its lock. Obstacle.points is an (N, 3) ndarray with an array-aware __eq__, so the voxel map crosses RPC as one tag instead of one tuple per cell. A cbor2 native-encoder hybrid was measured and rejected; _prepare says why. Also: isolated_python sends its config over stdin as CBOR, stale pickle comments and deepcopy-hook docstrings are updated, and the LCM and Zenoh service, Zenoh pubsub and DDS transport round trips are tested again.
paul-nechifor
added
the
backport:skip
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
go2 replay realtime (arm64)
Details
| Benchmark suite | Current: 66a52ab | Previous: bedd25b | Ratio |
|---|---|---|---|
peak memory |
1750.379 MB |
1715.012 MB |
1.02 |
peak threads |
379 threads |
376 threads |
1.01 |
network (transport) |
2726.593 MB |
2728.384 MB |
1.00 |
disk write |
1.473 MB |
1.695 MB |
0.87 |
instructions |
137.522 G |
137.518 G |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
…ides fail clearly Review fixes for PR #4821. Image travelled as its LCM form, which labels every RGB image rgb8 and decodes through np.frombuffer: float32 RGB images failed to reshape and uint8 images came back read-only. Image now has cbor_encode/cbor_decode built on its pickle state, so dtype, shape, format, timestamp and writability survive. Module.rpcs only sees methods carrying the @rpc marker, so a plain start() override drops out of the proxy's RPC set and the host fetched the bound method from the worker, which CBOR rejects (pickle shipped a module copy and ran start() in the parent). DemoRobot and the image encoding demo modules get @rpc, the proxy raises an AttributeError that names the missing decorator, and a codebase check fails any built-in blueprint whose module overrides an @rpc method without it. The blueprint codec test skipped on any exception; it now uses load_blueprint and skips only on OptionalDependencyError, with the self_hosted marks and robot_ips monkeypatch the sibling tests use.
This branch was successfully deployed
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
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.
No description provided.