Dynamic param patterns #4971
Dynamic param patterns #4971
Conversation
…oller Signed-off-by: Nils-ChristianIseke <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Otherwise, looks good to me! It might be valuable as part of this work to also add in other parameter validations while we're making these changes and auditing dynamic parameters. For example:
- Are all parameters set as dynamic parameters? If not, should those missing ones be dynamic?
- Are there other range checks that should be made for parameters?
It might be worth adding to the ticket a checklist of all the nodes that need updating and we can have that discussion for each to make sure we fill any obvious gaps currently missing due to updates over time where parameters were mistakenly not added or some validation checks that might be good to have
nav2_regulated_pure_pursuit_controller/src/parameter_handler.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Nils-ChristianIseke <[email protected]>
|
|
||
| rcl_interfaces::msg::SetParametersResult | ||
| ParameterHandler::dynamicParametersCallback( | ||
| // Follow up on this example: https://github.com/ros2/demos/blob/rolling/demo_nodes_cpp/src/parameters/set_parameters_callback.cpp |
Signed-off-by: Nils-ChristianIseke <[email protected]>
Nils-ChristianIseke
force-pushed
the
DynamicParamPatterns
branch
from
a61b571 to
589474c
Compare
|
@SteveMacenski i am not sure if i like the approach of 589474c. What is your opinion on it? |
This reverts commit 589474c. Signed-off-by: Nils-ChristianIseke <[email protected]>
Signed-off-by: Nils-ChristianIseke <[email protected]>
|
@Nils-ChristianIseke, your PR has failed to build. Please check CI outputs and resolve issues. |
|
@Nils-ChristianIseke, your PR has failed to build. Please check CI outputs and resolve issues. |
Basic Info
Description of contribution in a few bullet points
Description of documentation updates required from your changes
Description of how this change was tested
Future work that may be required in bullet points
For Maintainers:
@SteveMacenski I hope I understood the ticket correctly. I will continue to work on the rest of the codebase if these changes are the expected ones :).