Skip to content

Conversation

@domire8
Copy link
Member

@domire8 domire8 commented Oct 1, 2025

Description

The controller base class from ROS has two parameters update_rate and is_async. https://github.com/ros-controls/ros2_control/blob/aefbad38e4371aff499c39f53c8b296a557142cd/controller_interface/src/controller_interface_base.cpp#L37

Because of how our add/set parameter logic works, it is currently not possible to set those parameters on the node because the modulo base class doesn't know about them. By adding them to the internal parameter map, it becomes possible to set them (and not fail validation). For the is_async this is all good.

For the update_rate it is slightly more annoying because we usually use the name rate and type double for this. Keeping in mind that I marked both parameters internal, do you want me to investigate further how we could handle this? It would most likely be yet another parameter that we can then map to update_rate but it's not sure this is a clean and smooth solution.

Review guidelines

Estimated Time of Review: 5 minutes

Checklist before merging:

  • Confirm that the relevant changelog(s) are up-to-date in case of any user-facing changes

Copy link
Member

@eeberhard eeberhard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to keep it as update_rate for controllers to match the ros2_control API. While it's internal we can still change it later if desired

@domire8 domire8 merged commit 3761a2d into main Oct 3, 2025
4 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 3, 2025
@domire8 domire8 deleted the feat/async-params branch October 3, 2025 06:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants