fix(rmw-zenoh-rs): publish on the DDS-mangled type name, as the wire requires - #324
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Aligns rmw-zenoh-rs topic and service keys with DDS-mangled type names used by hiroz and rmw_zenoh_cpp.
Changes:
- Adds a shared DDS type-name formatter.
- Uses mangled names for message and service wire metadata.
- Adds topic/service formatting tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
crates/rmw-zenoh-rs/src/type_support.rs |
Applies DDS names to runtime type information. |
crates/hiroz-protocol/src/format/rmw_zenoh.rs |
Adds the formatter and regression tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
YuanYuYuan
force-pushed
the
fix/rmw-zenoh-rs-dds-type-name
branch
3 times, most recently
from
August 25, 2026 03:14
b43d336 to
bccecb3
Compare
YuanYuYuan
force-pushed
the
fix/rmw-zenoh-rs-dds-type-name
branch
6 times, most recently
from
August 25, 2026 07:24
14f59ed to
a67f7f9
Compare
…requires A hiroz library node and a ROS 2 node running hiroz's own rmw_zenoh_rs could not exchange a single message, in either direction, silently. Same repository, same revision, same domain, byte-identical type hash. Only the type-name segment of the key expression differed: hiroz library : 0/chatter/std_msgs::msg::dds_::String_/RIHS01_df668c74... rmw_zenoh_rs : 0/chatter/std_msgs/msg/String/RIHS01_df668c74... Measured which form is correct rather than inferring it. Upstream rmw_zenoh_cpp (ros-jazzy-rmw-zenoh-cpp) running demo_nodes_cpp talker, with a raw zenoh subscriber on `**` attached to its router, declares: 0/chatter/std_msgs::msg::dds_::String_/RIHS01_df668c740482bbd48fb39d76a70dfd4bd59db1288021743503259e948f6b1a18 The DDS-mangled form. The library agrees with the wire; rmw_zenoh_rs was the outlier, so rmw_zenoh_rs is what changes here. The cause was two type-name derivations sitting next to each other in type_support.rs. get_type_prefix() produced the DDS-mangled name and get_ros_type_name() produced the slash-separated one; get_type_info(), which feeds the key expression and the liveliness token, called the second. The same file was already internally inconsistent -- MessageTypeInfo::type_name_dyn() returned the mangled form while type_info_dyn() returned the slash form. The mangling rule now lives once, in hiroz-protocol's rmw_zenoh format module next to the formatter that consumes it, as dds_type_name(). hiroz-codegen bakes the same string into generated messages, so the regression test can compare the two derivations directly. get_ros_type_name() is deleted rather than left as a second source of truth. The service key strips the response suffix from the mangled name, so AddTwoInts_Response_ yields AddTwoInts_. Why CI was green: two workflows that never meet. test.yml exercises the library against rmw_zenoh_cpp; rmw-zenoh-rs.yml exercises rmw_zenoh_rs against ROS packages. Nothing tested the library against rmw_zenoh_rs. The added unit test closes that gap without needing an integration harness. Evidence, all on the remote worker: - RED: library_and_rmw_derive_the_same_topic_key failed on the assertion (left "std_msgs/msg/String", right "std_msgs::msg::dds_::String_") with the other 33 tests in the module passing, so it was a real failure and not a compile error. - GREEN: 35 passed, 0 failed. - End to end through one router, both directions: a hiroz library talker was heard 35 times by demo_nodes_cpp listener under rmw_zenoh_rs, and a demo_nodes_cpp talker under rmw_zenoh_rs was heard 29 times by a hiroz library listener. The sniffed key was 0/dir_b/std_msgs::msg::dds_::String_/RIHS01_df668c74..., matching the rmw_zenoh_cpp measurement exactly. - Re-falsified: reverting dds_type_name to the slash form turned the unit tests red again and dropped the end-to-end listener to 0 received out of 22 published. Restored. Not fixed here: hiroz peers are not brokered by a router, so the end-to-end runs used the library's client mode (--mode client --endpoint tcp/...) to work around it. That is a separate defect and is being handled separately.
YuanYuYuan
force-pushed
the
fix/rmw-zenoh-rs-dds-type-name
branch
from
August 25, 2026 07:34
a67f7f9 to
e85e1e9
Compare
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.
Summary
rmw_zenoh_rsderives the topic key expression from a slash-separated type name. The hiroz library, and upstreamrmw_zenoh_cpp, use the DDS-mangled form. They differ in one segment, so a hiroz library node and a ROS 2 node runningrmw_zenoh_rsmatch no topic and exchange nothing. Neither side reports an error.0/chatter/std_msgs::msg::dds_::String_/RIHS01_df668c74…rmw_zenoh_rs0/chatter/std_msgs/msg/String/RIHS01_df668c74…The domain matches. The type hash matches byte for byte. Only the type name differs.
get_type_info()feeds both the key expression and the liveliness token. It readget_ros_type_name(), whileget_type_prefix()sat beside it in the same file and produced the mangled form.Which side is wrong was measured
A
demo_nodes_cpptalker ran underRMW_IMPLEMENTATION=rmw_zenoh_cpp. A raw zenoh subscriber on**recorded the key it declared:The library matches the wire.
rmw_zenoh_rsis the outlier, sormw_zenoh_rsis what changes here. Patching the library instead would have broken thetest.ymlleg that passes today.The key expression is also the only difference. Raw CDR bytes published on the
rmw_zenoh_rskey, produced by no ROS code and carrying no attachment, reached ademo_nodes_cpplistener. The two stacks already agree on domain, type hash, encapsulation and envelope.Why CI is green
A missing edge, not a weak test.
test.ymlpairs the library withrmw_zenoh_cpp.rmw-zenoh-rs.ymlpairsrmw_zenoh_rswith ROS packages. No leg pairs the library withrmw_zenoh_rs.What this changes
get_type_info()get_ros_type_name()get_type_prefix()get_ros_type_name()type_name_dyn()vstype_info_dyn()…/std_msgs/msg/String/……/std_msgs::msg::dds_::String_/…rmw_zenoh_rsnoderos2 topic list -t, local topicros2 topic list -t, remote topicstd_msgs::msg::dds_::String_std_msgs/msg/StringThe graph leak on the last row is older than this change. hiroz builds the graph from liveliness tokens. Those tokens already carry the mangled form for every hiroz peer and every
rmw_zenoh_cpppeer. Sormw_get_topic_names_and_typesalready reports the mangled name for a remote publisher onmaintoday. On its own this change would extend that to local topics as well.ros_from_dds_strictcloses both halves. It is a port of_demangle_if_ros_typeinrmw_zenoh_cpp'sgraph_cache.cpp, applied at all eight sites that copy a graph type name into an RMW result.One rule, ten call sites
Review asked why several derivations existed at all. The rule now lives once, in
hiroz_schema::type_name, and ten production sites call it:hiroz-protocol, bothhiroz-codegengenerators,hiroz-derive, twohiroz::dynamicpaths, two action blocks inhiroz::node,hiroz-py, andhiroz-union. Four of those were independent inverses.dds_::now appears in no productionformat!orreplaceoutside that module.hiroz-schemais the home because it already owns the canonical form, and because it is a leaf crate. Nothing pullszenohinto a build script or a proc macro.Two inverses remain, on purpose.
ros_from_dds_strictmatches the reference byte for byte, which an RMW boundary requires.ros_from_ddsfalls back for a name that has nodds_::segment, because such a name must still resolve against the schema registry. Both differences were accidental before. A test now pins each.What fails without this
library_and_rmw_derive_the_same_topic_keyfails on the assertion:left: "std_msgs/msg/String",right: "std_msgs::msg::dds_::String_"cargo fmt --checkcleandemo_nodes_cpplistener underrmw_zenoh_rs: 35 received. Reverse: 29 received. The sniffed key matches thermw_zenoh_cppmeasurementTests
All 15 tests this change adds live in dedicated files, not an inline
mod tests. Both crates already carried atests/directory.crates/hiroz-schema/tests/type_name.rscrates/hiroz-protocol/tests/type_name_agreement.rsThe pre-existing inline module in
rmw_zenoh.rskeeps its own 33 tests. Moving those too would add churn to a defect fix; that belongs in its own pull request.Breaking changes
MessageTypeSupport::get_ros_type_name()is removedstd_msgs/msg/String→ callget_type_prefix()get_type_prefix()rmw_zenoh_rspublishes on changes…/std_msgs/msg/String/…→…/std_msgs::msg::dds_::String_/…rmw_zenoh_cppand the library already usestd_msgs::msg::dds_::String_→std_msgs/msg/StringNo caller of
get_ros_type_name()exists in this repository beyond the two this change rewrites.Coverage
rmw_zenoh_cpp, four distrosInterop Tests, which runshiroz-testswithros-interopno-type-hashconfiguration--no-default-features --features ros-interop,humblermw_zenoh_rsagainst ROS packagesrmw_zenoh_rs with ROS 2 Jazzytest-ros.nucheck, run by that same workflowThe check lives in
scripts/test-ros.nu, which already registers this project's ROS-dependent tests. It starts a talker underRMW_IMPLEMENTATION=rmw_zenoh_rs, then assertsros2 topic list -tprintsstd_msgs/msg/Stringand nodds_::. It also asserts/chatterappeared at all, so a probe that saw nothing fails instead of passing.It stays out of the default pipeline, because it needs
rmw_zenoh_rsbuilt and on the RMW search path. Only thermw_zenoh_rsworkflow sets that up, and it runs the check by name.That required one fix to the script.
mainvalidated a requested test against the pipeline, so any test registered in the map but left out of the default order was unrunnable. It now validates against the map, and--listmarks which tests the default run skips.The detector was proven to detect, against a stub
ros2:std_msgs/msg/Stringstd_msgs::msg::dds_::String_The check starts its own
rmw_zenohd, because rmw_zenoh peers need a router and nothing else in that step starts one. It guards bothjob killcalls: a talker that already exited makes an unguarded kill throw "Job N not found", which hides the real diagnosis behind a Nushell error.Not verified
The end-to-end runs used
std_msgs/Stringonly. Tests cover the service path, the action interfaces and the liveliness token at the formatter. No live ROS service exercises them.This touches no file that #325 touches, so the two can land in either order.