-
Notifications
You must be signed in to change notification settings - Fork 26
feat/fix: PID quaternion-based DP controller #643
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
16f8668
d333822
a27998d
71df096
edcd63b
1ed1d42
98c5d57
85f9b1a
457c2af
471e32d
b4aefb9
e7cdcf9
2d71411
afab550
f1df4f7
386fd99
7c72929
9fe9af2
bf97597
d97c0f2
d69fed6
9a560f7
ae83763
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,23 @@ | ||
| /**: | ||
| ros__parameters: | ||
| Kp: [70.0, 70.0, 70.0, 12.0, 12.0, 12.0] | ||
| Ki: [2.0, 2.0, 2.0, 0.12, 0.12, 0.12] | ||
| Kd: [10.0, 10.0, 10.0, 4.0, 5.0, 4.0] | ||
| # Kp: [70.0, 70.0, 70.0, 12.0, 12.0, 12.0] | ||
| # Ki: [2.0, 2.0, 2.0, 0.12, 0.12, 0.12] | ||
| # Kd: [10.0, 10.0, 10.0, 4.0, 5.0, 4.0] | ||
| Kp_x: 10.0 | ||
| Kp_y: 0.0 | ||
| Kp_z: 0.0 | ||
| Kp_roll: 0.0 | ||
| Kp_pitch: 0.0 | ||
| Kp_yaw: 0.0 | ||
| Ki_x: 0.0 | ||
| Ki_y: 0.0 | ||
| Ki_z: 0.0 | ||
| Ki_roll: 0.0 | ||
| Ki_pitch: 0.0 | ||
| Ki_yaw: 0.0 | ||
| Kd_x: 0.0 | ||
| Kd_y: 0.0 | ||
| Kd_z: 0.0 | ||
| Kd_roll: 0.0 | ||
| Kd_pitch: 0.0 | ||
| Kd_yaw: 0.0 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it not more tidy to have the parameters in vectors here? |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| #include <geometry_msgs/msg/twist_with_covariance_stamped.hpp> | ||
| #include <geometry_msgs/msg/wrench_stamped.hpp> | ||
| #include <nav_msgs/msg/odometry.hpp> | ||
| #include <rcl_interfaces/msg/set_parameters_result.hpp> | ||
| #include <rclcpp/rclcpp.hpp> | ||
| #include <std_msgs/msg/bool.hpp> | ||
| #include <std_msgs/msg/float64_multi_array.hpp> | ||
|
|
@@ -55,6 +56,12 @@ class PIDControllerNode : public rclcpp::Node { | |
| void guidance_callback( | ||
| const vortex_msgs::msg::ReferenceFilter::SharedPtr msg); | ||
|
|
||
| // TODO: parameter callback for dynamic reconfigure of PID gains | ||
| //@brief Callback function for parameter updates | ||
| // @param parameters: vector of parameters to be set | ||
| rcl_interfaces::msg::SetParametersResult parametersCallback( | ||
| const std::vector<rclcpp::Parameter>& parameters); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using a yaml-based approach like Anbit did. Mixing this approach with a service call (Trigger) allows for the same dynamic reconfiguration, but without relying so much on ROS. |
||
|
|
||
| PIDController pid_controller_; | ||
|
|
||
| rclcpp::Subscription<std_msgs::msg::Bool>::SharedPtr killswitch_sub_; | ||
|
|
@@ -93,6 +100,8 @@ class PIDControllerNode : public rclcpp::Node { | |
| bool killswitch_on_; | ||
|
|
||
| std::string software_mode_; | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably applies to more than just this pr / package, but dealing with string-representation for stuff like this can get really annoying. if you need a string-rep at any point, i'd suggest converting to an enum (magic-enum f.ex) as early as possible in the chain
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nvm seems to have been changed in #656 |
||
| OnSetParametersCallbackHandle::SharedPtr callback_handle_; | ||
| }; | ||
|
|
||
| #endif | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,71 @@ | ||
| #include "pid_controller_dp/pid_controller.hpp" | ||
| #include "pid_controller_dp/pid_controller_utils.hpp" | ||
|
|
||
| void print_eta(const types::Eta& eta) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For all these printing utils, consider checking out |
||
| // spdlog::info("Eta values:"); | ||
| spdlog::info("Position - North: {}, East: {}, Down: {}", eta.pos[0], | ||
| eta.pos[1], eta.pos[2]); | ||
| spdlog::info("Orientation - w: {}, x: {}, y: {}, z: {}", eta.ori.w(), | ||
| eta.ori.x(), eta.ori.y(), eta.ori.z()); | ||
| } | ||
|
|
||
| void print_nu(const types::Nu& nu) { | ||
| spdlog::info("Nu values:"); | ||
| spdlog::info("Linear Speed - u: {}, v: {}, w: {}", nu.linear_speed[0], | ||
| nu.linear_speed[1], nu.linear_speed[2]); | ||
| spdlog::info("Angular Speed - p: {}, q: {}, r: {}", nu.angular_speed[0], | ||
| nu.angular_speed[1], nu.angular_speed[2]); | ||
| } | ||
|
|
||
| void print_vect_6d(const types::Vector6d& vec) { | ||
| spdlog::info("Vector6d values:"); | ||
| for (int i = 0; i < 6; ++i) { | ||
| spdlog::info("Element[{}]: {}", i, vec[i]); | ||
| } | ||
| } | ||
|
|
||
| void print_J_transformation(const types::J_transformation& J) { | ||
| spdlog::info("J_transformation:"); | ||
|
|
||
| spdlog::info("R (3x3) elements:"); | ||
| for (int i = 0; i < J.R.rows(); ++i) { | ||
| for (int j = 0; j < J.R.cols(); ++j) { | ||
| spdlog::info("R[{},{}] = {}", i, j, J.R(i, j)); | ||
| } | ||
| } | ||
|
|
||
| spdlog::info("T (4x3) elements:"); | ||
| for (int i = 0; i < J.T.rows(); ++i) { | ||
| for (int j = 0; j < J.T.cols(); ++j) { | ||
| spdlog::info("T[{},{}] = {}", i, j, J.T(i, j)); | ||
| } | ||
| } | ||
|
|
||
| spdlog::info("Combined Matrix (7x6) elements:"); | ||
| auto M = J.as_matrix(); | ||
| for (int i = 0; i < M.rows(); ++i) { | ||
| for (int j = 0; j < M.cols(); ++j) { | ||
| spdlog::info("M[{},{}] = {}", i, j, M(i, j)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| void print_Jinv_transformation(const types::Matrix6x7d& J_inv) { | ||
| spdlog::info("J_pseudo_inverse (6x7):"); | ||
| for (int i = 0; i < J_inv.rows(); ++i) { | ||
| std::string row; | ||
| row.reserve(128); | ||
| row += "["; | ||
| for (int j = 0; j < J_inv.cols(); ++j) { | ||
| row += std::to_string(J_inv(i, j)); | ||
| if (j < J_inv.cols() - 1) | ||
| row += ", "; | ||
| } | ||
| row += "]"; | ||
| spdlog::info("{}", row); | ||
| } | ||
| } | ||
|
|
||
| PIDController::PIDController() | ||
| : Kp_(types::Matrix6d::Identity()), | ||
| Ki_(types::Matrix6d::Zero()), | ||
|
|
@@ -12,21 +77,52 @@ types::Vector6d PIDController::calculate_tau(const types::Eta& eta, | |
| const types::Eta& eta_d, | ||
| const types::Nu& nu, | ||
| const types::Eta& eta_dot_d) { | ||
| types::Eta error = error_eta(eta, eta_d); | ||
| types::Eta error = error_eta(eta, eta_d); // calculate eta error | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it better to make the error 6d and change the corresponding matrices?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With 6d error you could also replace pseudo_inv with standard inverse |
||
|
|
||
| // set w = 0 | ||
| error.ori.w() = 0.0; // only use vector part of quaternion for error | ||
|
|
||
| types::Matrix6x7d J_inv = calculate_J_sudo_inv(error); | ||
| auto eta_dot_d_copy = eta_dot_d; | ||
| eta_dot_d_copy.ori.w() = 0.0; // set w = 0 for desired eta_dot | ||
| // debug | ||
| // eta_error_debug = error; | ||
| spdlog::info("Eta: "); | ||
| print_eta(eta); | ||
| spdlog::info("Eta desired: "); | ||
| print_eta(eta_d); | ||
| spdlog::info("Eta error:"); | ||
| print_eta(error); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would suggest making this spdlog::trace (you could pass in log level to the print methods). Lets you f.ex compile debug with trace log level and release with info/warn, and set different log levels for file/terminal (io is expensive) |
||
|
|
||
| types::Vector6d nu_d = J_inv * eta_dot_d.as_vector(); | ||
| types::Matrix6x7d J_inv = | ||
| calculate_J_sudo_inv(eta); // calculate J pseudo inverse | ||
| J_inv_debug = J_inv; | ||
| print_Jinv_transformation(J_inv); | ||
|
|
||
| types::Vector6d error_nu = nu.as_vector() - nu_d; | ||
| types::Vector6d nu_d = | ||
| J_inv * eta_dot_d_copy.as_vector(); // calculate velocity | ||
| // nu_d_debug = nu_d; | ||
| // print_nu(nu_d); | ||
|
|
||
| types::Vector6d P = Kp_ * J_inv * error.as_vector(); | ||
| types::Vector6d error_nu = nu.as_vector() - nu_d; // calculate vel error | ||
| // error_nu_debug = error_nu; | ||
| // print_vect_6d(error_nu); | ||
|
|
||
| types::Vector6d I = Ki_ * J_inv * integral_; | ||
| types::Vector6d P = Kp_ * J_inv * error.as_vector(); // P term | ||
| // P_debug = P; | ||
| Kp_debug = Kp_; | ||
|
|
||
| types::Vector6d D = Kd_ * error_nu; | ||
| types::Vector6d I = Ki_ * J_inv * integral_; // I term | ||
| // I_debug = I; | ||
| // Ki_debug = Ki_; | ||
|
|
||
| types::Vector6d D = Kd_ * error_nu; // D term | ||
| // D_debug = D; | ||
| // Kd_debug = Kd_; | ||
| types::Vector6d tau = -clamp_values((P + I + D), -80.0, 80.0); | ||
| // types::Vector6d tau = -clamp_values((P), -80.0, 80.0); | ||
|
|
||
| // debug: tau = 0 | ||
| // types::Vector6d tau = types::Vector6d::Zero(); | ||
|
|
||
| integral_ = anti_windup(dt_, error, integral_); | ||
|
|
||
|
|
||
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.
Sticking to the "keep non-ROS code separate from everything else", you wouldn't want to pull in anything to your lib using ament_target_dependencies. Instead, use pure cmake for the library, and link in ros-deps + the lib with ament for the final executable