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.

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

merged 2 commits into from
May 31, 2025

Conversation

christophfroehlich
Copy link
Contributor

Apply changes from the guidelines added with ros-controls/realtime_tools#347

@christophfroehlich christophfroehlich added backport-humble This label should be used by maintainers only! Label triggers PR backport to ROS2 humble. backport-jazzy labels May 30, 2025
Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

Just some minor nitpicks. Rest looks great

@@ -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

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_;

@@ -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

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?

Co-authored-by: Sai Kishor Kothakota <[email protected]>
Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

LGTM
I think CI will be happy too

Copy link

codecov bot commented May 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.69%. Comparing base (483d6ac) to head (5846aa9).
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1720      +/-   ##
==========================================
- Coverage   85.69%   85.69%   -0.01%     
==========================================
  Files         123      123              
  Lines       11897    11895       -2     
  Branches     1015     1015              
==========================================
- Hits        10195    10193       -2     
  Misses       1379     1379              
  Partials      323      323              
Flag Coverage Δ
unittests 85.69% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...jectory_controller/joint_trajectory_controller.hpp 100.00% <ø> (ø)
...ory_controller/src/joint_trajectory_controller.cpp 85.97% <100.00%> (-0.04%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christophfroehlich christophfroehlich merged commit 0dcdd04 into master May 31, 2025
21 of 26 checks passed
@christophfroehlich christophfroehlich deleted the update/jtc/rt branch May 31, 2025 07:28
mergify bot pushed a commit that referenced this pull request May 31, 2025
Co-authored-by: Sai Kishor Kothakota <[email protected]>
(cherry picked from commit 0dcdd04)

# Conflicts:
#	joint_trajectory_controller/include/joint_trajectory_controller/joint_trajectory_controller.hpp
#	joint_trajectory_controller/src/joint_trajectory_controller.cpp
mergify bot pushed a commit that referenced this pull request May 31, 2025
Co-authored-by: Sai Kishor Kothakota <[email protected]>
(cherry picked from commit 0dcdd04)
christophfroehlich added a commit that referenced this pull request May 31, 2025
Co-authored-by: Sai Kishor Kothakota <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble This label should be used by maintainers only! Label triggers PR backport to ROS2 humble. backport-jazzy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants