-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for getting PID parameters from loaded parameters #374
Add support for getting PID parameters from loaded parameters #374
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mind to add an example and document how to set up the pid values ?
"Parameter '%s' has wrong type: %s", entry.c_str(), ex.what()); | ||
return false; | ||
} | ||
return std::isfinite(value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include <cmath>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I will add it. I just wanted to leave it for initial review. Thank you for the review. I'll get it done soon
@ahcorde I've just added it an example, let me know how it looks If this ros-simulation/gazebo_ros_pkgs#1546 is merged, we can have parameters tag as how we had in gazebo_ros2_control currently and we can make changes in such a way, we declare once and both same the params file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look fine for me.
gazebo_ros2_control_demos/launch/vertical_cart_example_position_pids_in_yaml.launch.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Christoph Fröhlich <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
https://github.com/Mergifyio backport humble iron |
✅ Backports have been created
|
Co-authored-by: Christoph Fröhlich <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> (cherry picked from commit 6a4cc84)
Co-authored-by: Christoph Fröhlich <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> (cherry picked from commit 6a4cc84)
…375) Co-authored-by: Christoph Fröhlich <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> (cherry picked from commit 6a4cc84) Co-authored-by: Sai Kishor Kothakota <[email protected]>
…376) Co-authored-by: Christoph Fröhlich <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> (cherry picked from commit 6a4cc84) Co-authored-by: Sai Kishor Kothakota <[email protected]>
Hello!
The issue #369 will be fixed once this is merged. It seems that for our Humanoids we will be needing this PID feature to make it work using position.
This PR:
position
interface, because having theposition_pid
interface doesn't help to use many standard ros2 controllers, as many controllers tend to check if they belong to a subset (https://github.com/ros-controls/ros2_controllers/blob/master/joint_trajectory_controller/src/joint_trajectory_controller_parameters.yaml#L20-L29)I hope this helps someone. If this PR changes look fine. I will open the following PR in the gz_ros2_control
Defining it like this should work