Skip to content

JTC: Use std::atomic<bool> #1720

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 31, 2025
Merged

JTC: Use std::atomic<bool> #1720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07e0a23
JTC: Use std::atomic<bool> instead
christophfroehlich May 30, 2025
5846aa9
Fix copy pasta
christophfroehlich May 30, 2025
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
5 changes: 3 additions & 2 deletions ...trajectory_controller/include/joint_trajectory_controller/joint_trajectory_controller.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#ifndef JOINT_TRAJECTORY_CONTROLLER__JOINT_TRAJECTORY_CONTROLLER_HPP_
#define JOINT_TRAJECTORY_CONTROLLER__JOINT_TRAJECTORY_CONTROLLER_HPP_

#include <atomic>
#include <functional> // for std::reference_wrapper
#include <memory>
#include <string>
Expand Down Expand Up @@ -152,7 +153,7 @@ class JointTrajectoryController : public controller_interface::ControllerInterfa
// Timeout to consider commands old
double cmd_timeout_;
// True if holding position or repeating last trajectory point in case of success
realtime_tools::RealtimeBuffer<bool> rt_is_holding_;
std::atomic<bool> rt_is_holding_;
Copy link
Member

@saikishor saikishor May 30, 2025

Choose a reason for hiding this comment

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

Suggested change
std::atomic<bool> rt_is_holding_;
std::atomic_bool rt_is_holding_;

// TODO(karsten1987): eventually activate and deactivate subscriber directly when its supported
bool subscriber_is_active_ = false;
rclcpp::Subscription<trajectory_msgs::msg::JointTrajectory>::SharedPtr joint_command_subscriber_ =
Expand All @@ -179,7 +180,7 @@ class JointTrajectoryController : public controller_interface::ControllerInterfa

rclcpp_action::Server<FollowJTrajAction>::SharedPtr action_server_;
RealtimeGoalHandleBuffer rt_active_goal_; ///< Currently active action goal, if any.
realtime_tools::RealtimeBuffer<bool> rt_has_pending_goal_; ///< Is there a pending action goal?
std::atomic<bool> rt_has_pending_goal_; ///< Is there a pending action goal?
Copy link
Member

@saikishor saikishor May 30, 2025

Choose a reason for hiding this comment

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

Suggested change
std::atomic<bool> rt_has_pending_goal_; ///< Is there a pending action goal?
std::atomic_bool rt_has_pending_goal_; ///< Is there a pending action goal?

rclcpp::TimerBase::SharedPtr goal_handle_timer_;
rclcpp::Duration action_monitor_period_ = rclcpp::Duration(50ms);

Expand Down
36 changes: 17 additions & 19 deletions joint_trajectory_controller/src/joint_trajectory_controller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,7 @@ controller_interface::return_type JointTrajectoryController::update(
auto new_external_msg = new_trajectory_msg_.readFromRT();
// Discard, if a goal is pending but still not active (somewhere stuck in goal_handle_timer_)
if (
current_trajectory_msg != *new_external_msg &&
(*(rt_has_pending_goal_.readFromRT()) && !active_goal) == false)
current_trajectory_msg != *new_external_msg && (rt_has_pending_goal_ && !active_goal) == false)
{
fill_partial_goal(*new_external_msg);
sort_to_local_joint_order(*new_external_msg);
Expand Down Expand Up @@ -230,7 +229,7 @@ controller_interface::return_type JointTrajectoryController::update(
// have we reached the end, are not holding position, and is a timeout configured?
// Check independently of other tolerances
if (
!before_last_point && *(rt_is_holding_.readFromRT()) == false && cmd_timeout_ > 0.0 &&
!before_last_point && !rt_is_holding_ && cmd_timeout_ > 0.0 &&
time_difference > cmd_timeout_)
{
RCLCPP_WARN(logger, "Aborted due to command timeout");
Expand All @@ -248,15 +247,15 @@ controller_interface::return_type JointTrajectoryController::update(
// is the last point
// print output per default, goal will be aborted afterwards
if (
(before_last_point || first_sample) && *(rt_is_holding_.readFromRT()) == false &&
(before_last_point || first_sample) && !rt_is_holding_ &&
!check_state_tolerance_per_joint(
state_error_, index, active_tol->state_tolerance[index], true /* show_errors */))
{
tolerance_violated_while_moving = true;
}
// past the final point, check that we end up inside goal tolerance
if (
!before_last_point && *(rt_is_holding_.readFromRT()) == false &&
!before_last_point && !rt_is_holding_ &&
!check_state_tolerance_per_joint(
state_error_, index, active_tol->goal_state_tolerance[index], false /* show_errors */))
{
Expand Down Expand Up @@ -357,7 +356,7 @@ controller_interface::return_type JointTrajectoryController::update(
// TODO(matthew-reynolds): Need a lock-free write here
// See https://github.com/ros-controls/ros2_controllers/issues/168
rt_active_goal_.writeFromNonRT(RealtimeGoalHandlePtr());
rt_has_pending_goal_.writeFromNonRT(false);
rt_has_pending_goal_ = false;

RCLCPP_WARN(logger, "Aborted due to state tolerance violation");

Expand All @@ -376,7 +375,7 @@ controller_interface::return_type JointTrajectoryController::update(
// TODO(matthew-reynolds): Need a lock-free write here
// See https://github.com/ros-controls/ros2_controllers/issues/168
rt_active_goal_.writeFromNonRT(RealtimeGoalHandlePtr());
rt_has_pending_goal_.writeFromNonRT(false);
rt_has_pending_goal_ = false;

RCLCPP_INFO(logger, "Goal reached, success!");

Expand All @@ -395,7 +394,7 @@ controller_interface::return_type JointTrajectoryController::update(
// TODO(matthew-reynolds): Need a lock-free write here
// See https://github.com/ros-controls/ros2_controllers/issues/168
rt_active_goal_.writeFromNonRT(RealtimeGoalHandlePtr());
rt_has_pending_goal_.writeFromNonRT(false);
rt_has_pending_goal_ = false;

RCLCPP_WARN(logger, "%s", error_string.c_str());

Expand All @@ -404,16 +403,15 @@ controller_interface::return_type JointTrajectoryController::update(
}
}
}
else if (tolerance_violated_while_moving && *(rt_has_pending_goal_.readFromRT()) == false)
else if (tolerance_violated_while_moving && !rt_has_pending_goal_)
{
// we need to ensure that there is no pending goal -> we get a race condition otherwise
RCLCPP_ERROR(logger, "Holding position due to state tolerance violation");

new_trajectory_msg_.reset();
new_trajectory_msg_.initRT(set_hold_position());
}
else if (
!before_last_point && !within_goal_time && *(rt_has_pending_goal_.readFromRT()) == false)
else if (!before_last_point && !within_goal_time && !rt_has_pending_goal_)
{
RCLCPP_ERROR(logger, "Exceeded goal_time_tolerance: holding position...");

Expand Down Expand Up @@ -1028,7 +1026,7 @@ controller_interface::CallbackReturn JointTrajectoryController::on_activate(

// The controller should start by holding position at the beginning of active state
add_new_trajectory_msg(set_hold_position());
rt_is_holding_.writeFromNonRT(true);
rt_is_holding_ = true;

// parse timeout parameter
if (params_.cmd_timeout > 0.0)
Expand Down Expand Up @@ -1060,7 +1058,7 @@ controller_interface::CallbackReturn JointTrajectoryController::on_deactivate(
const auto active_goal = *rt_active_goal_.readFromNonRT();
if (active_goal)
{
rt_has_pending_goal_.writeFromNonRT(false);
rt_has_pending_goal_ = false;
auto action_res = std::make_shared<FollowJTrajAction::Result>();
action_res->set__error_code(FollowJTrajAction::Result::INVALID_GOAL);
action_res->set__error_string("Current goal cancelled during deactivate transition.");
Expand Down Expand Up @@ -1178,7 +1176,7 @@ void JointTrajectoryController::topic_callback(
if (subscriber_is_active_)
{
add_new_trajectory_msg(msg);
rt_is_holding_.writeFromNonRT(false);
rt_is_holding_ = false;
}
};

Expand Down Expand Up @@ -1217,7 +1215,7 @@ rclcpp_action::CancelResponse JointTrajectoryController::goal_cancelled_callback
get_node()->get_logger(), "Canceling active action goal because cancel callback received.");

// Mark the current goal as canceled
rt_has_pending_goal_.writeFromNonRT(false);
rt_has_pending_goal_ = false;
auto action_res = std::make_shared<FollowJTrajAction::Result>();
active_goal->setCanceled(action_res);
rt_active_goal_.writeFromNonRT(RealtimeGoalHandlePtr());
Expand All @@ -1232,7 +1230,7 @@ void JointTrajectoryController::goal_accepted_callback(
std::shared_ptr<rclcpp_action::ServerGoalHandle<FollowJTrajAction>> goal_handle)
{
// mark a pending goal
rt_has_pending_goal_.writeFromNonRT(true);
rt_has_pending_goal_ = true;

// Update new trajectory
{
Expand All @@ -1241,7 +1239,7 @@ void JointTrajectoryController::goal_accepted_callback(
std::make_shared<trajectory_msgs::msg::JointTrajectory>(goal_handle->get_goal()->trajectory);

add_new_trajectory_msg(traj_msg);
rt_is_holding_.writeFromNonRT(false);
rt_is_holding_ = false;
}

// Update the active goal
Expand Down Expand Up @@ -1593,7 +1591,7 @@ JointTrajectoryController::set_hold_position()
hold_position_msg_ptr_->points[0].positions = state_current_.positions;

// set flag, otherwise tolerances will be checked with holding position too
rt_is_holding_.writeFromNonRT(true);
rt_is_holding_ = true;

return hold_position_msg_ptr_;
}
Expand All @@ -1607,7 +1605,7 @@ JointTrajectoryController::set_success_trajectory_point()
hold_position_msg_ptr_->points[0].time_from_start = rclcpp::Duration(0, 0);

// set flag, otherwise tolerances will be checked with success_trajectory_point too
rt_is_holding_.writeFromNonRT(true);
rt_is_holding_ = true;

return hold_position_msg_ptr_;
}
Expand Down
Loading