Skip to content

Reset the PIDs states when controller is at rest #1666

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pid_controller/include/pid_controller/pid_controller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ class PidController : public controller_interface::ChainableControllerInterface
// Feed-forward velocity weight factor when calculating closed loop pid adapter's command
std::vector<double> feedforward_gain_;

double reset_pid_time_;

// Command subscribers and Controller State publisher
rclcpp::Subscription<ControllerReferenceMsg>::SharedPtr ref_subscriber_ = nullptr;
realtime_tools::RealtimeBuffer<std::shared_ptr<ControllerReferenceMsg>> input_ref_;
Expand Down
17 changes: 17 additions & 0 deletions pid_controller/src/pid_controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,23 @@ controller_interface::return_type PidController::update_and_write_commands(
}

double error = reference_interfaces_[i] - measured_state_values_[i];

const auto zero_threshold = params_.gains.dof_names_map[params_.dof_names[i]].zero_threshold;
if (std::abs(reference_interfaces_[i]) < zero_threshold && std::abs(error) < zero_threshold)
Comment on lines +537 to +538
Copy link
Member

Choose a reason for hiding this comment

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

I agree with @christophfroehlich

Zero itself is a valid reference. IMHO using that to reset the PIDs is not a smart decision

Copy link
Author

Choose a reason for hiding this comment

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

I agree, using reference values is not a perfect solution, but I can't think of a better way to detect this.

There was once an idea to add emergency-stop feature in ros2_control that can be set in hardware component and propagated to all controllers (as a flag if I remember correctly). That would be perfect in this case. What happened to that idea?

Copy link
Contributor

Choose a reason for hiding this comment

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

is “Configuration/Movement/Safety-critical" interfaces definition ( ros-controls/roadmap#51) relevant for this scenario ?

Copy link
Author

Choose a reason for hiding this comment

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

If I understand this correctly this will only allow to configure command interfaces depending if hardware component is active/inactive. In controller we would like to be able to read hardware interface state and act if it was deactivated/halted, or have some global flag that indicates hardware e-stop was triggerd

{
reset_pid_time_ += period.seconds();
if (
reset_pid_time_ >
params_.gains.dof_names_map[params_.dof_names[i]].zero_threshold_duration)
{
pids_[i]->reset();
}
}
else
{
reset_pid_time_ = 0.0;
}

if (params_.gains.dof_names_map[params_.dof_names[i]].angle_wraparound)
{
// for continuous angles the error is normalized between -pi<error<pi
Expand Down
14 changes: 14 additions & 0 deletions pid_controller/src/pid_controller.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,17 @@ pid_controller:
default_value: true,
description: "Indicating if integral term is retained after re-activation"
}
zero_threshold: {
type: double,
default_value: 0.0,
description: "If the output is below this threshold for a duration specified by
zero_threshold_duration, the PID will be reset. If set to 0.0, this functionality is
disabled. To reset integral term, set the save_i_term parameter to false."
}
zero_threshold_duration: {
type: double,
default_value: 0.5,
description: "Duration in seconds for which the output must be below the zero_threshold
before the PID will be reset. This is used to prevent the PID from resetting when the
output is only briefly below the threshold."
}
12 changes: 12 additions & 0 deletions pid_controller/test/pid_controller_params.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,15 @@ test_save_i_term_on:

gains:
joint1: {p: 1.0, i: 2.0, d: 3.0, i_clamp_max: 5.0, i_clamp_min: -5.0, save_i_term: true}

test_zero_threshold:
ros__parameters:
dof_names:
- joint1

command_interface: position

reference_and_state_interfaces: ["position"]

gains:
joint1: {p: 1.0, i: 2.0, d: 3.0, i_clamp_max: 5.0, i_clamp_min: -5.0, save_i_term: false, zero_threshold: 0.1, zero_threshold_duration: 0.01}
53 changes: 53 additions & 0 deletions pid_controller/test/test_pid_controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,59 @@ TEST_F(PidControllerTest, test_save_i_term_on)
EXPECT_NEAR(actual_value, 2.00002, 1e-5); // i_term from above
}

/**
* @brief Test if zero_threshold is working
*
*/

TEST_F(PidControllerTest, test_zero_threshold)
{
SetUpController("test_zero_threshold");
rclcpp::executors::MultiThreadedExecutor executor;
executor.add_node(controller_->get_node()->get_node_base_interface());
executor.add_node(service_caller_node_->get_node_base_interface());

ASSERT_EQ(controller_->on_configure(rclcpp_lifecycle::State()), NODE_SUCCESS);
controller_->set_chained_mode(false);
for (const auto & dof_name : dof_names_)
{
ASSERT_NE(controller_->params_.gains.dof_names_map[dof_name].zero_threshold, 0.0);
}
ASSERT_EQ(controller_->on_activate(rclcpp_lifecycle::State()), NODE_SUCCESS);
ASSERT_FALSE(controller_->is_in_chained_mode());

controller_->set_reference(dof_command_values_);

ASSERT_EQ(
controller_->update(rclcpp::Time(0), rclcpp::Duration::from_seconds(0.01)),
controller_interface::return_type::OK);

// check the command value
// error = ref - state = 100.001, error_dot = error/ds = 10000.1,
// p_term = 100.001 * 1, i_term = 1.00001 * 2 = 2.00002, d_term = error/ds = 10000.1 * 3
// feedforward OFF -> cmd = p_term + i_term + d_term = 30102.3
const double expected_command_value = 30102.30102;

double actual_value =
std::round(controller_->command_interfaces_[0].get_optional().value() * 1e5) / 1e5;
EXPECT_NEAR(actual_value, expected_command_value, 1e-5);

// set command and state to a value slightly less than zero threshold
const auto near_zero_value =
controller_->params_.gains.dof_names_map[dof_names_[0]].zero_threshold * 0.9;
controller_->set_reference({near_zero_value});
dof_state_values_[0] = near_zero_value;

// set the duration to a value larger than the zero threshold duration
const auto duration =
controller_->params_.gains.dof_names_map[dof_names_[0]].zero_threshold_duration * 1.1;
ASSERT_EQ(
controller_->update(rclcpp::Time(0), rclcpp::Duration::from_seconds(duration)),
controller_interface::return_type::OK);
actual_value = std::round(controller_->command_interfaces_[0].get_optional().value() * 1e5) / 1e5;
EXPECT_NEAR(actual_value, 0.0, 1e-5);
}

int main(int argc, char ** argv)
{
::testing::InitGoogleTest(&argc, argv);
Expand Down
1 change: 1 addition & 0 deletions pid_controller/test/test_pid_controller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class TestablePidController : public pid_controller::PidController
FRIEND_TEST(PidControllerDualInterfaceTest, test_chained_feedforward_with_gain_dual_interface);
FRIEND_TEST(PidControllerTest, test_save_i_term_on);
FRIEND_TEST(PidControllerTest, test_save_i_term_off);
FRIEND_TEST(PidControllerTest, test_zero_threshold);

public:
controller_interface::CallbackReturn on_configure(
Expand Down
Loading