Add zenoh transport to the C++ native SDK - #3846
Draft
jeff-hykin wants to merge 14 commits into
Draft
Conversation
C++ native modules could only speak LCM, so a blueprint running on zenoh had to be all-rust. ZenohTransport is the peer of native/rust/dimos-module/src/zenoh.rs: it reads the same launch line, maps channels to keys the same way, and applies the same per-channel publisher QoS, so a C++ and a rust module in one blueprint land on the same wire. zenoh-cpp is header-only over zenoh-c, a compiled rust library, so unlike the rest of the SDK it cannot be vendored. It is detected rather than required: existing LCM-only modules build unchanged, and a module that asks for zenoh without it fails at startup naming the fix. The launch line is now read before the transport is constructed, because zenoh opens its session from it.
zenoh is a test dependency of the cpp job the same way liblcm is, so it belongs in the same install step. It has no apt package, so it comes from the upstream release instead, dropping the gh CLI and its token.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #3846 +/- ##
==========================================
+ Coverage 77.58% 77.60% +0.01%
==========================================
Files 1315 1317 +2
Lines 124697 124794 +97
Branches 10867 10886 +19
==========================================
+ Hits 96748 96841 +93
- Misses 24816 24820 +4
Partials 3133 3133
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Publishing runs on a worker thread that has no catch of its own, and zenoh-cpp reports every failure by throwing, so a rejected key expression or a put error escaped the thread and called std::terminate. The LCM transport logs a failed publish and keeps running; so does the rust zenoh transport. Match them.
as_vector() allocated and copied every inbound message, which the LCM transport does not do and the rust zenoh transport does not do either. Zenoh holds a payload as one contiguous slice in the common case, so that slice is passed straight through and only a fragmented payload is joined.
The rust LcmTransport warns that a session_local topic still goes out on the multicast bus (lcm.rs:104). The C++ one inherited the silent no-op, so a baked host would look like it had hidden an internal hop when it had not.
The config test called zenoh_config and then CHECK(true), so a key written to the wrong path still passed. It now reads every key back, including the two that must stay unset rather than be written as empty. Adds coverage for name resolution in endpoint_addresses and for the await_connect deadline.
nlohmann casts rather than rejects, so a negative connect_timeout_ms became 584 million years and await_connect never returned. Rust's u64 field errors on the same input.
The host/port split kept the brackets a locator writes, which getaddrinfo rejects, and getnameinfo hands back a bare host, which no link ever reports. An ipv6 endpoint therefore never resolved and await_connect always waited out its timeout. Rust gets this from tokio's lookup_host.
Rust logs it (zenoh.rs:252) and it is what identifies one peer among several on a shared bus, so a C++ module was the one process you could not pick out.
rfind(c, 0) == 0 is the pre-C++20 spelling and this SDK is C++20.
Every C++ module flake now applies a shared overlay and asks for the zenoh transport, so a module built from nix speaks the same two transports the CI build does. nixpkgs' zenoh-c is unusable as it ships, in three separate ways, so the overlay lives in one file rather than being repeated four times. CMake also now names the version it needs: zenoh-c renamed its locality enum after 1.4.0, and the old one failed as a page of compiler errors.
Five cases asserted a field survived a round trip through nlohmann rather than anything about the transport, and each was already covered where the value has an effect: the settings dump is asserted on the zenoh config it builds, the locality strings on the parsed QoS, and locator_address through every endpoint_addresses case. A sixth checked a base class the compiler already checks.
Most of them restated the line below or the name above. What is left is the part a reader cannot get from the code: why a negative timeout has to be rejected, why an unset key is not the same as an empty one, and why the session is waited on after it opens.
A GitHub release asset can be replaced in place, so the tag alone does not say which bytes the C++ transport was tested against.
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
1 participant
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.
add zenoh to C++