feat(parameters): derive descriptor constraints from the parameter type - #690
Open
azerupi wants to merge 1 commit into
Open
feat(parameters): derive descriptor constraints from the parameter type#690azerupi wants to merge 1 commit into
azerupi wants to merge 1 commit into
Conversation
additional_constraints in a parameter descriptor was only ever what the declaration passed to .constraints(). For a type with a closed set of valid values, such as a string-backed enum, that meant every declaration site restating the type's rules, with nothing keeping the text in sync as variants are added -- and an empty descriptor field when the call was forgotten, leaving operators no way to discover what a value may be. ParameterVariant::type_constraints() lets a type describe itself once. A declaration that sets no constraints of its own inherits it, so `ros2 param describe` reports the valid values for free. An explicit .constraints() still takes precedence for rules that belong to one declaration rather than to the type. The text is carried by the declaration's ParameterConversion, which is where a parameter's representation is already described, so a conversion written by hand can say what its values may be through with_constraints() without a type to hang the rules on. Assisted-by: Claude:claude-opus-5 [Claude Code]
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.
The code changes in this PR have been assisted by Claude Code but I have reviewed and iterated on the code.
Problem
additional_constraintsin a parameter descriptor was only ever whatever the declaration passed to.constraints(). For a type with a closed set of valid values, a string-backed enum being the obvious case, that means every declaration site has to restate the type's rules as free text:Nothing keeps that text in sync between the different declarations and with the type. As the type gains variants it is very easy to forget to update each declaration and that is when the user adds
.constraints()to begin with. So often there would be no constraints or the constraints would be outdated and a user runningros2 param describewould not be helped.Solution
Add a way for a type to describes its own constraints once:
and every declaration that sets no constraints of its own inherits it:
An explicit
.constraints()still wins, for a rule that belongs to one declaration rather than to the type:The constraint rides on the declaration's
ParameterConversion(added in a prior PR), which is already where a parameter's representation is described.of_variant()fills it fromT::type_constraints(), so a type with aParameterVariantimpl gets the behaviour above for free.That placement also means a hand-written conversion can describe its values with no type to hang the rules on:
To make this work the API grows to have
ParameterVariant::type_constraints() -> Option<Arc<str>>with a default implementation that defaults toNone. This is purely additive and not a breaking change. Existing code continues to compile.ParameterConversion::with_constraints()and::constraints().