Skip to content

Conversation

ashwinsushil
Copy link
Contributor

Convert last_movement_time_ from direct rclcpp::Time to RealtimeThreadSafeBox to ensure thread-safe communication between realtime and non-realtime contexts.

Changes:

  • Replace rclcpp::Time last_movement_time_ with RealtimeThreadSafeBoxrclcpp::Time
  • Initialize RealtimeThreadSafeBox in on_activate() method

Fixes potential realtime violations in gripper stall timeout mechanism.

closes #1853

Convert last_movement_time_ from direct rclcpp::Time to RealtimeThreadSafeBox
to ensure thread-safe communication between realtime and non-realtime contexts.

Changes:
- Replace rclcpp::Time last_movement_time_ with RealtimeThreadSafeBox<rclcpp::Time>
- Initialize RealtimeThreadSafeBox in on_activate() method

Fixes potential realtime violations in gripper stall timeout mechanism.
@ashwinsushil ashwinsushil marked this pull request as draft August 7, 2025 07:53
@ashwinsushil ashwinsushil marked this pull request as ready for review August 7, 2025 07:55
Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, some comments below

Copy link

codecov bot commented Aug 7, 2025

Codecov Report

❌ Patch coverage is 4.76190% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.97%. Comparing base (a01fd0b) to head (c373c15).

Files with missing lines Patch % Lines
...roller/parallel_gripper_action_controller_impl.hpp 4.76% 20 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1855      +/-   ##
==========================================
- Coverage   85.98%   85.97%   -0.02%     
==========================================
  Files         129      129              
  Lines       13112    13116       +4     
  Branches     1144     1144              
==========================================
+ Hits        11275    11276       +1     
- Misses       1469     1472       +3     
  Partials      368      368              
Flag Coverage Δ
unittests 85.97% <4.76%> (-0.02%) ⬇️

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

Files with missing lines Coverage Δ
...roller/parallel_gripper_action_controller_impl.hpp 25.61% <4.76%> (-0.02%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Ashwin Sushil added 2 commits August 7, 2025 12:52
Replace blocking set() with non-blocking try_set() for last_movement_time_
initialization in on_activate() method, as this lifecycle callback can be
executed from the realtime thread in some ROS2 control implementations.
Use try_get/try_set for rt_active_goal_ operations in check_for_success()
since this method runs in the realtime control loop. Prevents potential
blocking during goal completion and stall detection.
Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

LGTM

saikishor
saikishor previously approved these changes Aug 13, 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.

Rest looks good to me

Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com>
@christophfroehlich christophfroehlich merged commit 14c4cc4 into ros-controls:master Aug 24, 2025
18 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential realtime thread safety violations in parallel gripper action controller
3 participants