Have rerun render shared memory topics (fix dev for MacOS)#1716
Have rerun render shared memory topics (fix dev for MacOS)#1716jeff-hykin wants to merge 12 commits intodevfrom
Conversation
….py, add typing_extensions dep Address Paul's review comment: fix remaining Self imports that used 'Any as Self' fallback on Python 3.10, and add typing_extensions as an explicit dependency in pyproject.toml.
typer.confirm raises click.Abort when stdin is not a TTY. New dimos/utils/prompt.py provides confirm() that returns the default when not interactive. system_configurator now uses it. Revert: git revert HEAD
dimos/utils/prompt.py provides: - confirm(): returns default if not TTY, else typer.confirm() - sudo_run(): prepends sudo if not root Moved sudo_run from system_configurator/base.py to prompt.py. Updated all importers (base.py, lcm.py, clock_sync.py, tests). Root cause: Ivan's 8edc995 replaced input() with typer.confirm() which raises click.Abort on non-TTY stdin. Revert: git revert HEAD
Greptile SummaryThis PR adds macOS support to the Rerun bridge by introducing a Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant BP as unitree_go2_basic
participant SS as ShmSubset
participant SHM as PickleSharedMemory
participant Bridge as RerunBridgeModule
participant RR as Rerun Viewer
BP->>Bridge: pubsubs=[LCM(), ShmSubset(["/color_image"])]
Bridge->>SS: subscribe_all(callback)
SS->>SHM: PickleSharedMemory(default_capacity=capacity)
SS->>SHM: start()
SS->>SHM: subscribe("/color_image", _cb)
SHM-->>SS: unsub fn
note over SHM: macOS robot publishes via pSHMTransport
SHM->>SS: _cb(msg, "/color_image")
SS->>Bridge: callback(msg, "/color_image")
Bridge->>Bridge: _get_entity_path("/color_image")<br/>assert startswith("/") ✓
Bridge->>RR: rr.log(entity_path, rerun_data)
Reviews (1): Last reviewed commit: "Merge branch 'dev' of github.com:dimensi..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Strip everything after # (LCM topic suffix) | ||
| topic_str = topic_str.split("#")[0] | ||
| # Ensure / separator between prefix and topic | ||
| assert topic_str.startswith("/"), f"{topic_str} doesn't start with slash" |
There was a problem hiding this comment.
assert is silently stripped when Python runs with the -O (optimise) flag, so this guard will disappear in any optimised deployment. If a topic arrives without a leading slash, the bridge will silently produce a malformed entity path instead of surfacing an error. A proper ValueError (or at minimum a logged warning) is the right tool here.
| assert topic_str.startswith("/"), f"{topic_str} doesn't start with slash" | |
| if not topic_str.startswith("/"): | |
| raise ValueError(f"{topic_str!r} doesn't start with slash; SHM topic names must begin with '/'") |
| from dimos.protocol.pubsub.impl.shmpubsub import ShmSubset | ||
|
|
||
| bridge = RerunBridgeModule( | ||
| pubsubs=[LCM(), ShmSubset(topics=[("color_image", 6220800, "pickle")])], |
There was a problem hiding this comment.
The example topic name is "color_image" (no leading /), but the bridge now asserts topic_str.startswith("/") at bridge.py:274. A developer who copies this example verbatim will trigger an AssertionError at runtime. The example should mirror the actual usage in unitree_go2_basic.py ("/color_image").
| pubsubs=[LCM(), ShmSubset(topics=[("color_image", 6220800, "pickle")])], | |
| bridge = RerunBridgeModule( | |
| pubsubs=[LCM(), ShmSubset(topics=[("/color_image", 6220800, "pickle")])], | |
| ) |
| def __reduce__(self): # type: ignore[no-untyped-def] | ||
| return (pSHMTransport, (self.topic,)) | ||
| return (pSHMTransport, (self.topic,), {"default_capacity": self._default_capacity}) | ||
|
|
||
| def __setstate__(self, state: dict) -> None: # type: ignore[no-untyped-def] | ||
| self._default_capacity = state.get("default_capacity") | ||
| self.shm = PickleSharedMemory(default_capacity=self._default_capacity) |
There was a problem hiding this comment.
With the current __reduce__ / __setstate__ split, unpickling pSHMTransport (and SHMTransport) calls __init__ first (because __reduce__ returns (pSHMTransport, (self.topic,), state)). That __init__ call constructs a PickleSharedMemory(default_capacity=None), which is immediately discarded when __setstate__ constructs the correct one. This is a minor but unnecessary allocation.
One common pattern to avoid this is to delegate to __new__ in __reduce__ and perform all initialisation in __setstate__:
def __reduce__(self):
return (
_reconstruct_pshm, # module-level helper that calls __new__
(type(self), self.topic, self._default_capacity),
)Or simply accept the redundant allocation as a known trade-off and add a comment. The same applies to SHMTransport (lines 203-208).
1 participant
Draft b/c
ShmSubsetprobably shouldn't havesubscribe_allsince its basically a shim. Better fix would be changing rerun bridge to support topics without subscribe all.Problem
MacOS doesn't render any camera images to rerun, cause rerun doesn't support shared memory transport
Solution
Breaking Changes
How to Test
Contributor License Agreement