From 16f313b55070293ef6e544297b1de007850ad1ea Mon Sep 17 00:00:00 2001 From: yuanyuyuan Date: Fri, 21 Aug 2026 13:49:35 +0800 Subject: [PATCH 1/3] feat(hiroz): embed the bundled message definitions A downloaded hu could not decode a topic even when a publisher was live. Measured against a running talker on /chatter: discovery fell through and the .msg fallback had nowhere to look, so echo reported no .msg for std_msgs/msg/String (advertised by /chatter) was found on HIROZ_MSG_PATH and exited 1. A release ships no message definitions, so that is the default outcome, not an edge case. build.rs now embeds every bundled .msg as source text and the schema loader consults it when HIROZ_MSG_PATH yields nothing. Disk still wins: a user who sets that variable means it, and a stale embedded copy must not beat the definitions their publisher was built from. The whole jazzy set is ~122 KB of text, so nothing is subsetted. Embedded text goes through the same parser as a file, via parse_msg_string, so the two cannot diverge. --- crates/hiroz/build.rs | 87 +++++++++++++++++ crates/hiroz/src/dynamic/registry.rs | 138 ++++++++++++++++++++++++++- 2 files changed, 222 insertions(+), 3 deletions(-) diff --git a/crates/hiroz/build.rs b/crates/hiroz/build.rs index 6d0bc02b7..bc2c4b35b 100644 --- a/crates/hiroz/build.rs +++ b/crates/hiroz/build.rs @@ -16,6 +16,8 @@ fn main() { println!("cargo:rerun-if-changed=src/config.rs"); println!("cargo:rerun-if-env-changed=HIROZ_CONFIG_OUTPUT_DIR"); + embed_bundled_msgs(); + // Generate C FFI header when ffi feature is enabled #[cfg(feature = "ffi")] { @@ -118,3 +120,88 @@ fn main() { ); } } + +/// Embed the bundled `.msg` definitions as source text, so a binary with no +/// `HIROZ_MSG_PATH` and no live publisher can still resolve a schema. +/// +/// Emits `$OUT_DIR/embedded_msgs.rs`: a sorted `&[(&str, &str)]` of +/// `pkg/msg/Name` to the file's contents, via `include_str!` so the bytes are +/// the ones on disk at build time and cannot drift from them. +/// +/// The whole jazzy set is ~122 KB of text. Subsetting it would only trade a +/// fraction of a percent of binary size for "why is my type missing?", so +/// everything bundled is embedded. +/// +/// Reads `../hiroz-codegen/assets/{distro}` directly rather than calling +/// `hiroz_codegen::bundled_assets_dir`, which would make hiroz-codegen a +/// build-dependency for a directory walk. If the directory is absent the table +/// is empty and schema resolution behaves exactly as it did before. +fn embed_bundled_msgs() { + use std::fmt::Write as _; + + let manifest = std::path::PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap()); + let distro = if std::env::var_os("CARGO_FEATURE_HUMBLE").is_some() { + "humble" + } else { + "jazzy" + }; + let assets = manifest + .join("..") + .join("hiroz-codegen") + .join("assets") + .join(distro); + println!("cargo:rerun-if-changed={}", assets.display()); + + let mut entries: Vec<(String, std::path::PathBuf)> = Vec::new(); + if let Ok(packages) = std::fs::read_dir(&assets) { + for pkg in packages.flatten() { + let pkg_path = pkg.path(); + if !pkg_path.is_dir() { + continue; + } + let Some(pkg_name) = pkg_path.file_name().and_then(|n| n.to_str()) else { + continue; + }; + let msg_dir = pkg_path.join("msg"); + let Ok(msgs) = std::fs::read_dir(&msg_dir) else { + continue; + }; + for m in msgs.flatten() { + let path = m.path(); + if path.extension().and_then(|e| e.to_str()) != Some("msg") { + continue; + } + let Some(stem) = path.file_stem().and_then(|n| n.to_str()) else { + continue; + }; + entries.push((format!("{pkg_name}/msg/{stem}"), path)); + } + } + } else { + println!( + "cargo:warning=bundled assets not found at {}; embedded schema fallback will be empty", + assets.display() + ); + } + + // Sorted, so the lookup can binary-search. + entries.sort_by(|a, b| a.0.cmp(&b.0)); + + let mut out = String::from( + "// @generated by build.rs - do not edit\npub static EMBEDDED_MSGS: &[(&str, &str)] = &[\n", + ); + for (type_name, path) in &entries { + writeln!( + out, + " ({type_name:?}, include_str!({:?})),", + path.display().to_string() + ) + .unwrap(); + } + out.push_str("];\n"); + + let dest = std::path::PathBuf::from(std::env::var("OUT_DIR").unwrap()).join("embedded_msgs.rs"); + if let Err(e) = std::fs::write(&dest, out) { + println!("cargo:warning=failed to write {}: {e}", dest.display()); + } +} diff --git a/crates/hiroz/src/dynamic/registry.rs b/crates/hiroz/src/dynamic/registry.rs index 050563367..156e57c06 100644 --- a/crates/hiroz/src/dynamic/registry.rs +++ b/crates/hiroz/src/dynamic/registry.rs @@ -220,15 +220,30 @@ fn load_schema_inner( // File not on disk is a legitimate "try the next source" (live discovery), // so return None quietly. Errors *after* a file is found are logged below, // since a broken `.msg` masquerading as "not found" would be misleading. - let path = find_msg_file(&package, &name)?; + // Disk first, so a user pointing HIROZ_MSG_PATH at their own definitions + // always wins over what this binary happens to have been built with. + let source = match find_msg_file(&package, &name) { + Some(path) => MsgSource::File(path), + None => MsgSource::Embedded(embedded_msg_source(&package, &name)?), + }; if !in_progress.borrow_mut().insert(type_name.to_string()) { tracing::warn!("cyclic .msg definition for {type_name}; skipping schema load"); return None; } - let mut parsed = match hiroz_codegen::parser::msg::parse_msg_file(&path, &package) { + let parse_result = match &source { + MsgSource::File(path) => hiroz_codegen::parser::msg::parse_msg_file(path, &package), + // The path argument is only used for diagnostics; the bytes come from + // the embedded table. + MsgSource::Embedded(text) => hiroz_codegen::parser::msg::parse_msg_string( + text, + &package, + std::path::Path::new(&format!("/{package}/msg/{name}.msg")), + ), + }; + let mut parsed = match parse_result { Ok(parsed) => parsed, Err(e) => { - tracing::warn!("failed to parse .msg file {}: {e}", path.display()); + tracing::warn!("failed to parse .msg for {type_name} from {source}: {e}"); in_progress.borrow_mut().remove(type_name); return None; } @@ -261,6 +276,49 @@ fn load_schema_inner( Some(register_schema(schema)) } +/// Where a `.msg` definition came from, for diagnostics. +#[cfg(feature = "dynamic-schema-loader")] +enum MsgSource { + File(std::path::PathBuf), + Embedded(&'static str), +} + +#[cfg(feature = "dynamic-schema-loader")] +impl std::fmt::Display for MsgSource { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + MsgSource::File(p) => write!(f, "{}", p.display()), + MsgSource::Embedded(_) => write!(f, "the definitions built into this binary"), + } + } +} + +/// The bundled `.msg` definitions, embedded as source text at build time. +/// +/// Sorted by `pkg/msg/Name`, so the lookup below can binary-search. +#[cfg(feature = "dynamic-schema-loader")] +mod embedded { + include!(concat!(env!("OUT_DIR"), "/embedded_msgs.rs")); +} + +/// Look up `/msg/` among the definitions built into this binary. +/// +/// This is what lets a downloaded `hu` decode a topic with no `HIROZ_MSG_PATH` +/// set and no reachable type-description service — the case where discovery +/// yields a type name and the disk has nothing to resolve it with. +/// +/// Consulted **after** `HIROZ_MSG_PATH`, never before: a user who points that +/// variable at their own definitions means it, and a stale embedded copy must +/// not silently win over the messages their publisher was actually built from. +#[cfg(feature = "dynamic-schema-loader")] +fn embedded_msg_source(package: &str, name: &str) -> Option<&'static str> { + let key = format!("{package}/msg/{name}"); + embedded::EMBEDDED_MSGS + .binary_search_by(|(k, _)| (*k).cmp(key.as_str())) + .ok() + .map(|i| embedded::EMBEDDED_MSGS[i].1) +} + /// Split `pkg/msg/Name` (or the shorthand `pkg/Name`) into `(package, name)`. #[cfg(feature = "dynamic-schema-loader")] fn split_msg_type(type_name: &str) -> Option<(String, String)> { @@ -304,3 +362,77 @@ fn find_msg_file(package: &str, name: &str) -> Option { } None } + +#[cfg(all(test, feature = "dynamic-schema-loader"))] +mod embedded_tests { + use super::*; + + /// The table is what makes a downloaded `hu` able to decode anything, so an + /// empty one is a silent regression: every lookup would simply miss and the + /// behaviour would fall back to today's "no .msg found". + #[test] + fn the_embedded_table_is_not_empty_and_is_sorted() { + assert!( + !embedded::EMBEDDED_MSGS.is_empty(), + "no bundled .msg definitions were embedded; \ + the build script found no assets directory" + ); + assert!( + embedded::EMBEDDED_MSGS.windows(2).all(|w| w[0].0 < w[1].0), + "the embedded table must be sorted and duplicate-free: binary_search relies on it" + ); + } + + /// std_msgs/msg/String is the type the documented quick start echoes, and + /// the one the G2 measurement showed failing on a default install. + #[test] + fn a_common_type_resolves_from_the_embedded_definitions() { + let src = embedded_msg_source("std_msgs", "String") + .expect("std_msgs/msg/String must be embedded"); + assert!( + src.contains("string data"), + "embedded source does not look like the real definition: {src:?}" + ); + } + + #[test] + fn an_unknown_type_is_a_miss_not_a_panic() { + assert!(embedded_msg_source("no_such_pkg", "Nope").is_none()); + } + + /// Disk must win over the embedded copy. A user who sets HIROZ_MSG_PATH + /// means it, and their publisher may have been built from definitions that + /// differ from the ones this binary was compiled with. + #[test] + #[serial_test::serial] + fn a_definition_on_disk_wins_over_the_embedded_one() { + let dir = std::env::temp_dir().join(format!("hiroz-embed-{}", std::process::id())); + let msg_dir = dir.join("std_msgs").join("msg"); + std::fs::create_dir_all(&msg_dir).unwrap(); + // Deliberately NOT the real definition, so resolving it proves the disk + // copy was used rather than the embedded one. + std::fs::write(msg_dir.join("String.msg"), "string data\nint32 sentinel_field\n").unwrap(); + + let found = find_msg_file("std_msgs", "String"); + let restore = std::env::var("HIROZ_MSG_PATH").ok(); + assert!( + found.is_none() || restore.is_some(), + "test environment already has HIROZ_MSG_PATH pointing somewhere" + ); + + unsafe { std::env::set_var("HIROZ_MSG_PATH", &dir) }; + let path = find_msg_file("std_msgs", "String") + .expect("the on-disk definition must be found first"); + let text = std::fs::read_to_string(&path).unwrap(); + assert!( + text.contains("sentinel_field"), + "HIROZ_MSG_PATH did not take precedence over the embedded table" + ); + + match restore { + Some(v) => unsafe { std::env::set_var("HIROZ_MSG_PATH", v) }, + None => unsafe { std::env::remove_var("HIROZ_MSG_PATH") }, + } + let _ = std::fs::remove_dir_all(&dir); + } +} From 22cc52aa8d82b90ec3f5878065a122e32c6abe1c Mon Sep 17 00:00:00 2001 From: yuanyuyuan Date: Fri, 21 Aug 2026 13:54:34 +0800 Subject: [PATCH 2/3] style(hiroz): format the embedded-msgs test as stable rustfmt wants Local `cargo fmt --all --check` passed and CI's failed on the same commit. The repo's rustfmt.toml sets nightly-only keys including skip_children; a nightly local toolchain honours it and skips child modules, while CI's stable rustfmt ignores it and formats them. Local fmt-green is therefore weaker evidence than it looks. --- crates/hiroz/src/dynamic/registry.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/hiroz/src/dynamic/registry.rs b/crates/hiroz/src/dynamic/registry.rs index 156e57c06..0489aa0dc 100644 --- a/crates/hiroz/src/dynamic/registry.rs +++ b/crates/hiroz/src/dynamic/registry.rs @@ -411,7 +411,11 @@ mod embedded_tests { std::fs::create_dir_all(&msg_dir).unwrap(); // Deliberately NOT the real definition, so resolving it proves the disk // copy was used rather than the embedded one. - std::fs::write(msg_dir.join("String.msg"), "string data\nint32 sentinel_field\n").unwrap(); + std::fs::write( + msg_dir.join("String.msg"), + "string data\nint32 sentinel_field\n", + ) + .unwrap(); let found = find_msg_file("std_msgs", "String"); let restore = std::env::var("HIROZ_MSG_PATH").ok(); From e2c5d6d167c3392806bb66e76483b9acf0e831ff Mon Sep 17 00:00:00 2001 From: yuanyuyuan Date: Fri, 21 Aug 2026 14:34:00 +0800 Subject: [PATCH 3/3] fix(hu): compare the live topic type in ROS form, not DDS form `hu meter pub --msg-type std_msgs/msg/String` to a topic with a live endpoint failed with topic /pub_target carries std_msgs::msg::dds_::String_, not the requested std_msgs/msg/String The graph reports a DDS-mangled name; --msg-type is the ROS form. The guard compared them raw, so a correct publish was rejected. The branch was unreachable until a schema could resolve without a live node: it needs both a resolvable schema and a live endpoint, and the one test that resolves from disk publishes to an empty topic, where there is no live type to compare against. Embedding the definitions made the combination reachable and the latent defect fired. Normalises with the same helper the discovery branch's names already come in as, rather than adding a second copy of the mangling rules. --- crates/hiroz-union/src/plugin/wasm/host/ros.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/hiroz-union/src/plugin/wasm/host/ros.rs b/crates/hiroz-union/src/plugin/wasm/host/ros.rs index 04dc425f0..43907409e 100644 --- a/crates/hiroz-union/src/plugin/wasm/host/ros.rs +++ b/crates/hiroz-union/src/plugin/wasm/host/ros.rs @@ -389,8 +389,15 @@ impl hu::plugin::ros::Host for PluginState { // bytes onto that endpoint's key — reject instead (same guard as // the discovery branch below). An empty topic has no live type, // so publishing to it still works, like `ros2 topic pub`. + // The graph reports a DDS-mangled name + // (`std_msgs::msg::dds_::String_`) while `--msg-type` is the ROS + // form (`std_msgs/msg/String`). Comparing them raw rejects a + // correct publish. This branch only became reachable once a + // schema could resolve without a live node, so nothing exercised + // it before: the one test that resolves from disk publishes to an + // *empty* topic, where there is no live type to compare against. if let Some(live) = self.live_topic_type_info(&topic) - && live.name != type_name + && hiroz::dynamic::ros_type_name_from_dds(&live.name) != type_name { return Err(PluginError::Invalid(format!( "topic {topic} carries {}, not the requested {type_name}",