Add Pirate Go2 vision-guided object interaction hackathon submission#2290
Add Pirate Go2 vision-guided object interaction hackathon submission#22900xmandy wants to merge 4 commits into
Conversation
Add hackathon/pirate/README.md linking the Go2 vision-guided object interaction project. DimOS core files are not modified by this submission. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
0xmandy
requested review from
leshy,
mustafab0,
paul-nechifor and
spomichter
as code owners
Greptile SummaryThis PR pairs a hackathon README for the Pirate YOLO vision-servoing project with five core DimOS infrastructure changes needed to support SHM-based high-bandwidth streaming for the Go2 robot.
Confidence Score: 4/5Safe to merge; no data-loss or crash paths introduced. The transport-pickling change is functionally correct and the Rerun wiring follows the established lifecycle. The transport and bridge changes are functionally correct. The redundant SHM object created during unpickling is an inefficiency rather than a runtime failure, and the implicit self.quality dependency in JpegShmTransport.setstate holds under standard pickle but could silently break under atypical reconstruction paths. The coordinator deferred import of RerunBridgeModule couples core coordination to the visualization layer but is guarded at runtime. dimos/core/transport.py — the setstate / reduce interaction warrants a second look for the implicit quality dependency and the double-construction pattern. Important Files Changed
Sequence DiagramsequenceDiagram
participant B as ModuleCoordinator.build()
participant D as _deploy_all_modules
participant W as _connect_streams
participant C as _configure_rerun_bridge_visual_transports
participant R as RerunBridgeModule
participant S as SHMTransport
B->>D: deploy all modules incl. RerunBridgeModule
B->>W: wire streams populate _transport_registry
W->>C: called at end of _connect_streams
C->>C: filter _transport_registry for SHM transports
C->>R: set_visual_transports(transports) RPC
R->>R: store in config.visual_transports
B->>R: build_all_modules / start_all_modules
R->>R: "start() sets _started=True"
R->>R: _subscribe_visual_transports()
R->>S: transport.start()
R->>S: transport.subscribe(on_visual_message)
S-->>R: unsub callable registered as disposable
S-->>R: on_visual_message(msg, transport_topic)
R->>R: _on_message to _get_entity_path to rr.log
Reviews (1): Last reviewed commit: "feat: add Pirate hackathon submission po..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def __reduce__(self): # type: ignore[no-untyped-def] | ||
| return (pSHMTransport, (self.topic,)) | ||
| # Preserve sizing options such as default_capacity when the coordinator | ||
| # sends this transport to workers or to Rerun. | ||
| return (pSHMTransport, (self.topic,), self._kwargs) | ||
|
|
||
| def __setstate__(self, state: dict[str, Any]) -> None: | ||
| self._kwargs = state | ||
| self.shm = PickleSharedMemory(**state) |
There was a problem hiding this comment.
pSHMTransport it first calls pSHMTransport(topic) (which runs __init__ and creates PickleSharedMemory() with the default 3.5 MiB capacity), then immediately calls __setstate__ which creates a second PickleSharedMemory(**state) with the correct capacity and replaces self.shm. The first object is abandoned and GC'd. While no OS-level shared memory is allocated until .start(), the unnecessary construction can be avoided by having __reduce__ return a factory callable that bypasses __init__.
| def __reduce__(self): # type: ignore[no-untyped-def] | |
| return (pSHMTransport, (self.topic,)) | |
| # Preserve sizing options such as default_capacity when the coordinator | |
| # sends this transport to workers or to Rerun. | |
| return (pSHMTransport, (self.topic,), self._kwargs) | |
| def __setstate__(self, state: dict[str, Any]) -> None: | |
| self._kwargs = state | |
| self.shm = PickleSharedMemory(**state) | |
| def __reduce__(self): # type: ignore[no-untyped-def] | |
| # Preserve sizing options such as default_capacity when the coordinator | |
| # sends this transport to workers or to Rerun. | |
| return (_rebuild_pshm, (self.topic, self._kwargs)) | |
| def __setstate__(self, state: dict[str, Any]) -> None: | |
| self._kwargs = state | |
| self.shm = PickleSharedMemory(**state) | |
| def _rebuild_pshm(topic: str, kwargs: dict) -> "pSHMTransport": | |
| """Pickle helper that constructs pSHMTransport without a double-init.""" | |
| obj = object.__new__(pSHMTransport) | |
| pSHMTransport.__init__(obj, topic, **kwargs) | |
| return obj |
| def __setstate__(self, state: dict[str, Any]) -> None: | ||
| from dimos.protocol.pubsub.impl.jpeg_shm import ( | ||
| JpegSharedMemory, | ||
| ) # deferred to avoid pulling in Image/cv2/rerun | ||
|
|
||
| self._kwargs = state | ||
| self.shm = JpegSharedMemory(quality=self.quality, **state) |
There was a problem hiding this comment.
self.quality is implicitly required before __setstate__ runs. __setstate__ references self.quality (line 268), which is only guaranteed to be set because pickle always calls __init__ (via the args tuple in __reduce__) before calling __setstate__. If the reconstruction path ever changes (e.g., __new__ + __setstate__ directly, as in some frameworks), this would raise AttributeError. Storing quality in the state dict alongside _kwargs would make the dependency explicit and the reconstruction self-contained.
| def _configure_rerun_bridge_visual_transports(coordinator: ModuleCoordinator) -> None: | ||
| """Send resolved SHM transports to an active Rerun bridge. | ||
|
|
||
| RerunBridgeModule subscribes to configured pubsubs directly. For SHM | ||
| streams, the coordinator forwards the concrete transport objects after | ||
| stream wiring has selected them. | ||
| """ | ||
| from dimos.visualization.rerun.bridge import RerunBridgeModule | ||
|
|
||
| if RerunBridgeModule not in coordinator._deployed_modules: | ||
| return | ||
|
|
||
| # LCM transports are already visible through RerunBridgeModule.config.pubsubs. | ||
| transports = [ | ||
| transport | ||
| for transport in coordinator._transport_registry.values() | ||
| if isinstance(transport, SHMTransport | pSHMTransport | JpegShmTransport) | ||
| ] | ||
| if not transports: | ||
| return | ||
|
|
||
| bridge = coordinator.get_instance(RerunBridgeModule) | ||
| bridge.set_visual_transports(transports) |
There was a problem hiding this comment.
_configure_rerun_bridge_visual_transports does a deferred from dimos.visualization.rerun.bridge import RerunBridgeModule inside the core coordinator module. This creates a layering dependency: the coordination layer now has knowledge of—and a runtime dependency on—the visualization layer. The runtime guard (if RerunBridgeModule not in coordinator._deployed_modules) keeps headless deployments safe, but the right long-term home for this wiring is likely a post-wire hook registered by the bridge itself, keeping core coordination decoupled from visualization concerns.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
3 participants
Summary
Submission Links
What it demonstrates
Current results
Validation
make check— all scripts compilemake sim-run— MuJoCo sim push succeeds with ball displacement confirmedNotes
This PR intentionally does not vendor the full external project into DimOS. The full
implementation, artifacts, decision traces, and demo video live in the project repo above.