feat(msgs): functional lyrical distro support via feature-selected asset trees - #344
Open
richardw347 wants to merge 1 commit into
Open
feat(msgs): functional lyrical distro support via feature-selected asset trees#344richardw347 wants to merge 1 commit into
richardw347 wants to merge 1 commit into
Conversation
…set trees The lyrical/rolling/kilted Cargo features already existed but were non-functional: bundled_assets_dir() hardcoded jazzy, build.rs selected packages via an env-probe (detect_ros_version) + a hand-maintained allow-list (get_all_packages), and there was no assets/lyrical/ tree. So `--features lyrical` silently generated jazzy messages. Make distro selection real and feature-driven: - Add a Distro enum (Humble/Jazzy/Lyrical) chosen from Cargo features via from_features() (defaults to Jazzy — no behavior change for existing users). - Replace the env-probe + allow-list with asset-tree enumeration: every package dir under hiroz-codegen/assets/<distro>/ is generated, so adding a package is just dropping its interface files in the tree. - Add hiroz_codegen::bundled_assets_dir_for(distro) and route bundled_assets_dir(is_humble) through it. - Add the stock upstream assets/lyrical/ tree (25 standard interface packages; shipped to crates.io via the existing assets/** include globs). Verified: `--features lyrical` generates 25 packages from assets/lyrical; `--features jazzy` unchanged (24 packages from assets/jazzy). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
The
lyrical(androlling/kilted) Cargo features already exist on thehirozcrate, but they are non-functional — enabling--features lyricalsilently generates jazzy messages:
hiroz_codegen::bundled_assets_dir(is_humble)only maps to"humble"/"jazzy".hiroz-msgs/build.rsselects packages via an environment probe(
detect_ros_version()readingAMENT_PREFIX_PATH) plus a hand-maintainedallow-list (
get_all_packages), neither of which is distro-feature aware.assets/lyrical/tree.This PR makes distro selection real and feature-driven, and finishes the
lyrical support the feature flags already advertise.
Changes
Distroenum (Humble/Jazzy/Lyrical) selected from Cargo features viaDistro::from_features(). Defaults toJazzywhen no distro feature isset — no behavior change for existing users.
directory under
hiroz-codegen/assets/<distro>/(one containingmsg/,srv/, oraction/) is generated. Adding a package is just dropping itsinterface files into the tree — no
build.rsedit. (test_msgs/test_interface_filesstay excluded, as before, because their wstring/Nestedfixtures don't compile.)
hiroz_codegen::bundled_assets_dir_for(distro: &str)added; the existingbundled_assets_dir(is_humble)now routes through it (kept for compatibility).assets/lyrical/— the stock upstream ROS 2 interface set (25 standardpackages:
std_msgs,sensor_msgs,geometry_msgs,nav_msgs,action_msgs,builtin_interfaces,rcl_interfaces,tf2_msgs,vision_msgs,type_description_interfaces, …). Shipped to crates.io via theexisting
assets/**include globs inhiroz-codegen/Cargo.toml.build.rsshrinks substantially (the allow-list + system-package probing +detect_ros_versionare all removed in favor of enumeration).Verification
Notes / open questions for the maintainer
jazzyto preserve current behavior. Happy to change ifyou'd prefer newest-available or an explicit-required selection.
assets/lyrical/is stock upstream interfaces only. Onepackage,
ffmpeg_image_transport_msgs, is community (not ROS-core) — includedbecause it round-trips cleanly, but easy to drop from the tree if you'd rather
keep only core/
common_interfaces.rolling/kilted: those feature flags still map to no asset tree; this PRonly adds
lyrical. The enumeration mechanism makes adding them trivial later(drop an
assets/rolling/tree + aDistroarm).