Open
Conversation
4 tasks
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.
This is an alternative implementation of #516.
The goal of this PR is not to dismiss or negate the work done in #516. But while reviewing that PR, because I'm excited about this feature, I felt like there was a better approach and it felt easier to explore an alternative design by trying another implementation from scratch (with AI assistance) rather than posting a bunch of review comments on the initial PR.
Anyone working on or reviewing #516 should feel free to dismiss this PR entirely or incorporate any ideas from it into the original PR if desired.
Key differences from #516
StructuredParameters)ParameterSet)StructuredParametersMeta<DefaultForbidden>trait impl that callsdeclare_structured_()passing all builder options as function arguments. The actual builder logic lives in hand-written trait impls onMandatoryParameter<T>,OptionalParameter<T>, etc.ParameterBuildermethod chains (.default().description().mandatory()) directly, with no intermediate traits.DefaultForbiddenmarker type withunreachable!()impls to satisfy the type system for nested structs.ParameterSet::declare()calls.declare_structured()call with all builder options passed as arguments. For nested structs, options likedefault,description,rangeare silently ignored. Leaf-only attributes on nested fields are not caught at compile time.node.declare_parameters("")on aRobotstruct with a nestedsensors: SensorConfigfield producesspeed,sensors.rate. Withnode.declare_parameters("robot"):robot._speed,robot._sensors._rate.node.declare_parameter_set::<Robot>()producesrobot.speed,robot.sensors.rate. Overridable with#[parameters(namespace = "...")]or#[parameters(flatten)].#[param(flatten)]on fields skips the field name in the namespace;#[parameters(flatten)]on structs skips the struct name. Flatten is not recursive — it only affects the level it's applied to.range = rclrs::ParameterRange { lower: Some(1.0), ..Default::default() }range(lower = 1.0)— parsed intoParameterRangeby the macro.impl.rsfile (~160 lines).struct_attrs.rs,field_attrs.rs,field_info.rs,codegen.rs+mod.rs(~550 lines).dev-dependenciesin rclrs and not re-exported, the derive macro is only usable by adding the proc-macro crate as a dependency in downstream crates.dependenciesand re-exported viapub use rclrs_macros::ParameterSet— downstream crates use#[derive(ParameterSet)]fromrclrsdirectlyWhat this PR adds
rclrs-macroscrate — proc-macro crate with#[derive(ParameterSet)]ParameterSettrait in rclrs withdefault_namespace()anddeclare()methodsNodeStateconvenience methods —declare_parameter_set()anddeclare_parameter_set_with_prefix()#[parameters(flatten)],#[parameters(namespace = "...")]#[param(default, description, constraints, range(...), ignore_override, discard_mismatching_prior_value, discriminate, mandatory, optional, read_only, flatten)]