fix: reset header stamps to zero in reset methods to prevent timeout by Ishan1923 · Pull Request #2197 · ros-controls/ros2_controllers

Ishan1923 · 2026-03-04T01:56:35Z

Problem: When controllers reset their internal reference messages, the initialization functions assigned the current time (now()) to the new empty message's timestamp. This caused the message 'age' calculation to yield 0 seconds, which allowed the empty message to bypass safety timeout checks.

Solution:

Modified the reset functions across the affected controllers to set the timestamp to rclcpp::Time(0) instead of the current time. This explicitly marks the reset message as an old/invalid command, ensuring the safety timeout logic halts the robot.
Applied (void)node; cast to silence -Werror=unused-parameter compiler warnings without altering existing function signatures. I was unsure to remove this const std::shared_ptr<rclcpp_lifecycle::LifecycleNode> & node as I might break ABI, but I guess this might not, as it completely separate.
Updated EXPECT_EQ to EXPECT_GT in t_est_mecanum_drive_controller.cpp_ to correctly test the timestamp update against the new 0 baseline. The old EXPECT_EQ tested against an incorrect now() baseline; EXPECT_GT correctly ensures the timestamp advanced from 0.

Affected Controllers:

admittance_controller
diff_drive_controller
gpio_controllers
mecanum_drive_controller
omni_wheel_drive_controller
steering_controllers_library

fix #1774

codecov · 2026-03-04T02:30:34Z

Codecov Report

❌ Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.62%. Comparing base (68630ae) to head (f4ed8b0).

Files with missing lines	Patch %	Lines
gpio_controllers/src/gpio_command_controller.cpp	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2197      +/-   ##
==========================================
+ Coverage   85.61%   85.62%   +0.01%     
==========================================
  Files         148      148              
  Lines       15837    15850      +13     
  Branches     1339     1341       +2     
==========================================
+ Hits        13559    13572      +13     
  Misses       1792     1792              
  Partials      486      486

Flag	Coverage Δ
unittests	`85.62% <96.77%> (+0.01%)`	⬆️

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

Files with missing lines	Coverage Δ
...dmittance_controller/src/admittance_controller.cpp	`74.65% <100.00%> (ø)`
...iff_drive_controller/src/diff_drive_controller.cpp	`80.36% <100.00%> (ø)`
..._drive_controller/src/mecanum_drive_controller.cpp	`84.63% <100.00%> (+0.18%)`	⬆️
..._controller/test/test_mecanum_drive_controller.cpp	`98.35% <ø> (ø)`
...ive_controller/src/omni_wheel_drive_controller.cpp	`83.07% <100.00%> (ø)`
...llers_library/src/steering_controllers_library.cpp	`67.58% <100.00%> (+0.70%)`	⬆️
...library/test/test_steering_controllers_library.cpp	`98.92% <100.00%> (+<0.01%)`	⬆️
gpio_controllers/src/gpio_command_controller.cpp	`83.60% <75.00%> (ø)`

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Ishan1923 · 2026-03-27T15:30:17Z

@christophfroehlich The function signatures have been updated for the rolling target.
Apologies for the delay; I was occupied with my mid-semester exams.

Removed the unused node parameter and (void)node; casts from the
reset_controller_reference_msg local helper functions in:
- admittance_controller
- gpio_controllers
- mecanum_drive_controller
- steering_controllers_library
Local colcon test passes for all modified controllers.

mergify · 2026-03-28T18:22:37Z

This pull request is in conflict. Could you fix it @Ishan1923?

…bypass Signed-off-by: Ishan1923 <ecdev4ishan@gmail.com>

… node parameter

Signed-off-by: Ishan1923 <ecdev4ishan@gmail.com>

Ishan1923 · 2026-04-02T20:52:08Z

@christophfroehlich, @thedevmystic would you please have a look at this.

I have updated the function signature, removed the node parameter from reset_controller_reference_msg.
Moreover, I also updated the odometry_set_service test case in steering_controllers_library which started failing after the recent open-loop odometry merge (PR Fix open_loop odometry of steering controllers #2087). Because the open-loop odometry math now correctly integrates movement, the leftover simulated wheel feedback from previous test steps caused the robot to drift slightly during the update() call.

…os-controls#2293)

… interfaces (ros-controls#2276)

… Joint State Broadcaster. (ros-controls#2082) Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com> Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com> Co-authored-by: Surya! <thedevmystic@gmail.com>

…rols#2344)

…: admittance_controller, pose_broadcaster, tricycle_steering_controller (ros-controls#2345)

…interfaces (ros-controls#2187)

…ols#2359)

…rs (ros-controls#2368)

…ntrols#2214)

mergify · 2026-05-26T21:33:53Z

This pull request is in conflict. Could you fix it @Ishan1923?

Signed-off-by: Ishan1923 <ecdev4ishan@gmail.com>

Ishan1923 · 2026-05-27T07:49:24Z

@christophfroehlich @thedevmystic merge conflict in mecanum_drive_controller.cpp has been resolved and pushed. Could you please take a look?

saikishor · 2026-05-27T08:00:29Z

@christophfroehlich @thedevmystic merge conflict in mecanum_drive_controller.cpp has been resolved and pushed. Could you please take a look?

@Ishan1923 The CI builds are failing. Can you please take a look?

bmagyar · 2026-05-27T08:00:56Z

The CI is clearly unhappy still. Please ignore the main binary jobs but semi binaries and testing binaries should pass

saikishor · 2026-05-27T08:24:53Z

@christophfroehlich @thedevmystic merge conflict in mecanum_drive_controller.cpp has been resolved and pushed. Could you please take a look?

  /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/steering_controllers_library/test/test_steering_controllers_library.cpp: In member function ‘virtual void SteeringControllersLibraryTest_odometry_set_service_Test::TestBody()’:
  /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/steering_controllers_library/test/test_steering_controllers_library.cpp:428:13: error: no match for call to ‘(SteeringControllersLibraryTest_odometry_set_service_Test::TestBody()::<lambda(double, double)>) (double, double, double)’
    428 |   move_robot(0.0, 0.0, 0.0);
        |   ~~~~~~~~~~^~~~~~~~~~~~~~~
  /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/steering_controllers_library/test/test_steering_controllers_library.cpp:409:23: note: there is 1 candidate
    409 |   auto move_robot = [&](double vx, double wz)

The code is not building for this error, can you take a look?

Signed-off-by: Ishan1923 <ecdev4ishan@gmail.com>

Ishan1923 · 2026-05-27T08:47:38Z

@christophfroehlich @thedevmystic merge conflict in mecanum_drive_controller.cpp has been resolved and pushed. Could you please take a look?

  /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/steering_controllers_library/test/test_steering_controllers_library.cpp: In member function ‘virtual void SteeringControllersLibraryTest_odometry_set_service_Test::TestBody()’:
  /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/steering_controllers_library/test/test_steering_controllers_library.cpp:428:13: error: no match for call to ‘(SteeringControllersLibraryTest_odometry_set_service_Test::TestBody()::<lambda(double, double)>) (double, double, double)’
    428 |   move_robot(0.0, 0.0, 0.0);
        |   ~~~~~~~~~~^~~~~~~~~~~~~~~
  /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/steering_controllers_library/test/test_steering_controllers_library.cpp:409:23: note: there is 1 candidate
    409 |   auto move_robot = [&](double vx, double wz)

The code is not building for this error, can you take a look?

The move_robot() function only takes 2 arguments:
auto move_robot = [&](double vx, double wz) {...};,

the error was from passing three arguments to it :
move_robot(0.0, 0.0, 0.0);

Ishan1923 · 2026-05-27T09:20:05Z

The CI is clearly unhappy still. Please ignore the main binary jobs but semi binaries and testing binaries should pass

The semi-binary(s) failures are infra failures :-

lyrical RHEL and Debian failing due to missing images (mainfest_unkown).
rolling RHEL semi-binary fails in realtime_tools due to a FastDDS ABI mismatch in upstream packages.

could you please confirm these are known infra issues

christophfroehlich

The 4 remaining failing jobs are known and not related.
Changes from this PR LGTM.

saikishor

Please take a look https://docs.ros.org/en/rolling/p/rclcpp/generated/classrclcpp_1_1Time.html#_CPPv4N6rclcpp4Time4TimeE7int64_t16rcl_clock_type_t

saikishor · 2026-06-09T11:22:30Z

+void reset_wrench_msg(geometry_msgs::msg::WrenchStamped & msg)
 {
-  msg.header.stamp = node->now();
+  msg.header.stamp = rclcpp::Time(0);


Suggested change

msg.header.stamp = rclcpp::Time(0);

msg.header.stamp = rclcpp::Time(0, node->get_clock()->get_clock_type());

Maybe this is needed, seems like default is systemtime?

saikishor · 2026-06-09T11:22:39Z

  // Fill RealtimeBox with NaNs so it will contain a known value
  // but still indicate that no command has yet been sent.
-  command_msg_.header.stamp = get_node()->now();
+  command_msg_.header.stamp = rclcpp::Time(0);


Suggested change

command_msg_.header.stamp = rclcpp::Time(0);

command_msg_.header.stamp = rclcpp::Time(0, node->get_clock()->get_clock_type());

saikishor · 2026-06-09T11:22:50Z

+void reset_controller_reference_msg(gpio_controllers::CmdType & msg)
 {
-  msg.header.stamp = node->now();
+  msg.header.stamp = rclcpp::Time(0);


Suggested change

msg.header.stamp = rclcpp::Time(0);

msg.header.stamp = rclcpp::Time(0, node->get_clock()->get_clock_type());

github-actions Bot requested review from bmagyar, christophfroehlich, homalozoa, mamueluth and xguay March 4, 2026 01:56

Ishan1923 changed the title ~~fix: reset header stamps to zero in reset methods to prevent timeout …~~ fix #1774: reset header stamps to zero in reset methods to prevent timeout Mar 4, 2026

Ishan1923 changed the title ~~fix #1774: reset header stamps to zero in reset methods to prevent timeout~~ fix: reset header stamps to zero in reset methods to prevent timeout #1774 Mar 4, 2026

Ishan1923 changed the title ~~fix: reset header stamps to zero in reset methods to prevent timeout #1774~~ fix: reset header stamps to zero in reset methods to prevent timeout Mar 4, 2026

thedevmystic reviewed Mar 13, 2026

View reviewed changes

Comment thread admittance_controller/src/admittance_controller.cpp Outdated

Ishan1923 requested a review from thedevmystic March 27, 2026 15:30

thedevmystic reviewed Mar 28, 2026

View reviewed changes

Comment thread steering_controllers_library/src/steering_controllers_library.cpp

Ishan1923 requested a review from thedevmystic April 2, 2026 16:49

Ishan1923 added 3 commits April 2, 2026 22:22

fix: reset header stamps to zero in reset methods to prevent timeout …

d433d58

…bypass Signed-off-by: Ishan1923 <ecdev4ishan@gmail.com>

fix: change reset_controller_reference_msg signature to remove unused…

50b2493

… node parameter

style: pre-commit run

cc93ee3

Signed-off-by: Ishan1923 <ecdev4ishan@gmail.com>

Ishan1923 force-pushed the fix/1774-reset-header-stamps branch from 28d2417 to cc93ee3 Compare April 2, 2026 17:08

test: halt robot to clear wheel feedback before odometry reset test

42104e3

Signed-off-by: Ishan1923 <ecdev4ishan@gmail.com>

christophfroehlich and others added 9 commits April 17, 2026 20:42

Merge branch 'master' into fix/1774-reset-header-stamps

1eb067b

Fix segfault in jtc if joint name not in urdf (ros-controls#2321)

fad4d2e

Update changelogs

7debce0

6.6.0

36e0276

[rqt_jtc] Use urdf_parser_py (ros-controls#2254)

4aaf4e7

Remove deprecated controller specializations (ros-controls#2016)

6643956

fix JTC userdoc YAML indentation and stray quote (ros-controls#2327)

e8be012

diff_drive: set parameters as read_only or dynamically update them (r…

fdd51ae

…os-controls#2293)

fix(joint_state_broadcaster): suppress confusing warning for standard…

bdd8433

… interfaces (ros-controls#2276)

github-actions Bot and others added 15 commits May 27, 2026 02:53

Bump version of pre-commit hooks (ros-controls#2337)

2b34b67

Suppress cppcheck errors from macros from version.h (ros-controls#2346)

cd4e227

Add lyrical workflows, update README, and fix gcc-15 issues (ros-cont…

7554821

…rols#2344)

Update changelogs

5d808d6

6.7.0

567c1a6

More general initialization of state from command (ros-controls#2294)

7fdd9ea

Test fix - call appropriate lifecycle transitions in controller tests…

5e0f88c

…: admittance_controller, pose_broadcaster, tricycle_steering_controller (ros-controls#2345)

[JSB] Remove dynamic_joint_states and use INDIVIDUAL_BEST_EFFORT for …

a8bfbb0

…interfaces (ros-controls#2187)

Added velocity limiting to the mecanum controller. (ros-controls#2313)

ee5bab0

[JTC] Fix segfault when last trajectory segment is skipped (ros-contr…

d5ec4be

…ols#2359)

Fix motion_primitive_controller TOC in docs (ros-controls#2221)

aa4079f

Remove broken toctree entries for deleted position/velocity controlle…

b074eb4

…rs (ros-controls#2368)

Add broadcaster for magnetic field values from a magnetometer (ros-co…

ad79f26

…ntrols#2214)

Fix SPDX license expression (ros-controls#2370)

e89c342

Ishan1923 added 2 commits May 27, 2026 03:06

fix: resolve merge conflict in mecanum_drive_controller

54893c2

Signed-off-by: Ishan1923 <ecdev4ishan@gmail.com>

fix: resolve merge conflicts

fb0b8e0

Signed-off-by: Ishan1923 <ecdev4ishan@gmail.com>

fix: pass correct arguments for move_robot()

e29f848

Signed-off-by: Ishan1923 <ecdev4ishan@gmail.com>

Merge branch 'master' into fix/1774-reset-header-stamps

f4ed8b0

christophfroehlich approved these changes Jun 8, 2026

View reviewed changes

christophfroehlich requested review from saikishor and removed request for thedevmystic June 8, 2026 20:48

saikishor requested changes Jun 9, 2026

View reviewed changes

	msg.header.stamp = rclcpp::Time(0);
	msg.header.stamp = rclcpp::Time(0, node->get_clock()->get_clock_type());

	command_msg_.header.stamp = rclcpp::Time(0);
	command_msg_.header.stamp = rclcpp::Time(0, node->get_clock()->get_clock_type());

Conversation

Ishan1923 commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Ishan1923 commented Mar 27, 2026

Uh oh!

Uh oh!

mergify Bot commented Mar 28, 2026

Uh oh!

Ishan1923 commented Apr 2, 2026

Uh oh!

mergify Bot commented May 26, 2026

Uh oh!

Ishan1923 commented May 27, 2026

Uh oh!

saikishor commented May 27, 2026

Uh oh!

bmagyar commented May 27, 2026

Uh oh!

saikishor commented May 27, 2026

Uh oh!

Ishan1923 commented May 27, 2026

Uh oh!

Ishan1923 commented May 27, 2026

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

saikishor left a comment

Choose a reason for hiding this comment

Uh oh!

saikishor Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

saikishor Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

saikishor Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

Ishan1923 commented Mar 4, 2026 •

edited

Loading

codecov Bot commented Mar 4, 2026 •

edited

Loading