Skip to content

feat(transport): add Rust subscribe-all API - #3935

Open
omarespejel wants to merge 2 commits into
dimensionalOS:mainfrom
omarespejel:feat/rust-subscribe-all
Open

feat(transport): add Rust subscribe-all API#3935
omarespejel wants to merge 2 commits into
dimensionalOS:mainfrom
omarespejel:feat/rust-subscribe-all

Conversation

@omarespejel

Copy link
Copy Markdown
Contributor

Contribution path

Problem

Rust modules can only subscribe through fixed input ports. The Rerun bridge and similar modules need every topic, including topics created after startup.

Solution

Add Builder::subscribe_all through the existing transport API.

LCM reuses its receive loop and filters LCM_SELF_TEST. Zenoh subscribes to dimos/**. Callbacks receive the raw payload and topic name, run on the transport delivery path, and remain registered for the transport lifetime.

The default implementation returns Unsupported, so existing transports keep compiling.

How to Test

From native/rust:

cargo test -p dimos-module subscribe_all
cargo test -p dimos-module -- --test-threads=1
cargo clippy -p dimos-module --all-targets -- -D warnings
cargo check --workspace --all-targets
cargo fmt --all -- --check

3 focused tests and all 143 crate tests pass. Clippy, workspace checks, and formatting are clean.

AI assistance

Codex with GPT-5 was used for implementation and testing.

Checklist

  • I have read and approved the CLA.

@github-actions github-actions Bot added the first-time-contributor PR opened by an author who had not previously committed to this repository label Sep 4, 2026
@omarespejel

Copy link
Copy Markdown
Contributor Author

@greptile please review

@greptile-apps

greptile-apps Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds a native Rust API for receiving every DimOS topic, including topics that are created after a module starts. Zenoh wildcard delivery was exercised successfully with the exact topic name and payload preserved.

Confidence Score: 5/5

Safe to merge.

No actionable issues were identified. The focused Rust coverage passed for wildcard subscription delivery on a topic created after registration.

Files Needing Attention: None.

T-Rex T-Rex Logs

What T-Rex did

  • Ran the subscribe_all test with cargo test --manifest-path native/rust/dimos-module/Cargo.toml subscribe_all -- --nocapture to execute the subscribe_all tests, which finished with 3 passed, 0 failed, 0 ignored, 140 filtered out in 0.51s.
  • The Zenoh transport registered a wildcard callback, published to a newly generated DimOS topic, and verified that the callback received the exact topic with the new channel payload.
  • The test runner reported the subscribe_all test suite, including the subscribe_all_receives_new_channels case, as ok, matching the overall pass result of 3 passed, 0 failed, 0 ignored, 140 filtered out in 0.51s.
  • Reviewed the code paths exercised by subscribe_all, including declarations of DimOS in the zenoh module and the callback registration, topic publishing, and delivery verification.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "feat(rust): add subscribe-all API" | Re-trigger Greptile

@omarespejel

Copy link
Copy Markdown
Contributor Author

so you got nothin' @greptile?

@greptile-apps

greptile-apps Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Not nothing — this is a clean pass. I found no actionable correctness or compatibility issues.

subscribe_all is wired through Transport/SharedTransport, the builder routes are panic-isolated, and both Zenoh (dimos/**) and LCM (including the LCM_SELF_TEST filter) retain callbacks for the transport lifetime. The focused coverage verifies late-topic delivery, payload/topic preservation, and LCM filtering.

Safe to merge.

@omarespejel

Copy link
Copy Markdown
Contributor Author

hey @spomichter can please run the pending ci?

@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

@@            Coverage Diff             @@
##             main    #3935      +/-   ##
==========================================
- Coverage   78.19%   78.19%   -0.01%     
==========================================
  Files        1430     1430              
  Lines      135238   135238              
  Branches    11713    11713              
==========================================
- Hits       105753   105751       -2     
- Misses      26165    26170       +5     
+ Partials     3320     3317       -3     
Components Coverage Δ
Tests 94.62% <ø> (-0.01%) ⬇️
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.52% <ø> (ø)
OS-ubuntu-latest 75.08% <ø> (-0.01%) ⬇️
Py-3.10 75.08% <ø> (-0.01%) ⬇️
Py-3.11 75.08% <ø> (-0.01%) ⬇️
Py-3.12 ?
Py-3.13 75.08% <ø> (-0.01%) ⬇️
Py-3.14 75.08% <ø> (-0.01%) ⬇️
Py-3.14t ?

Flags with carried forward coverage won't be shown. Click here to find out more.
see 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@omarespejel

Copy link
Copy Markdown
Contributor Author

hey @aclauer, CI is green now. can you take a look when you get a chance? 🙏

@omarespejel
omarespejel force-pushed the feat/rust-subscribe-all branch from 4886207 to b21c306 Compare September 6, 2026 04:25
@omarespejel

Copy link
Copy Markdown
Contributor Author

@spomichter 🙏 please review when possible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

first-time-contributor PR opened by an author who had not previously committed to this repository

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subscribe-all API in rust

1 participant