Skip to content

Conversation

@christophfroehlich
Copy link
Member

@christophfroehlich christophfroehlich commented May 30, 2025

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

  • I changed all Boxes to use the messages itself, instead of a smart pointer to it.
  • Messages get pre-allocated and saved later for the case that any try_get fails in the RT thread
  • I refactored the steering_controllers_lib a bit, to have the same timeout logic that the mecanum_controller has. This needed some updates of the tests, too.
  • Apart from that, the behavior of all controllers should not change at all

@codecov
Copy link

codecov bot commented May 30, 2025

Codecov Report

❌ Patch coverage is 87.24490% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.38%. Comparing base (4a42b1f) to head (1747c95).
⚠️ Report is 67 commits behind head on master.

Files with missing lines Patch % Lines
...roller/parallel_gripper_action_controller_impl.hpp 4.76% 20 Missing ⚠️
pid_controller/src/pid_controller.cpp 66.66% 8 Missing and 3 partials ⚠️
...llers_library/src/steering_controllers_library.cpp 78.12% 3 Missing and 4 partials ⚠️
...dmittance_controller/src/admittance_controller.cpp 83.33% 1 Missing and 3 partials ⚠️
gpio_controllers/src/gpio_command_controller.cpp 78.94% 2 Missing and 2 partials ⚠️
...ommand_controller/src/forward_controllers_base.cpp 87.50% 0 Missing and 2 partials ⚠️
...iff_drive_controller/src/diff_drive_controller.cpp 93.75% 0 Missing and 1 partial ⚠️
..._drive_controller/src/mecanum_drive_controller.cpp 97.05% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1721      +/-   ##
==========================================
- Coverage   86.47%   86.38%   -0.09%     
==========================================
  Files         123      123              
  Lines       12133    12240     +107     
  Branches     1008     1024      +16     
==========================================
+ Hits        10492    10574      +82     
- Misses       1335     1348      +13     
- Partials      306      318      +12     
Flag Coverage Δ
unittests 86.38% <87.24%> (-0.09%) ⬇️

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

Files with missing lines Coverage Δ
...roller/test/test_ackermann_steering_controller.cpp 100.00% <100.00%> (ø)
...ntroller/test/test_bicycle_steering_controller.cpp 100.00% <100.00%> (ø)
...ive_controller/test/test_diff_drive_controller.cpp 94.58% <ø> (ø)
...ollers/test/test_joint_group_effort_controller.cpp 98.43% <100.00%> (ø)
...rd_command_controller/forward_controllers_base.hpp 100.00% <ø> (ø)
...ontroller/test/test_forward_command_controller.cpp 99.24% <100.00%> (-0.01%) ⬇️
...est_multi_interface_forward_command_controller.cpp 99.13% <100.00%> (-0.01%) ⬇️
..._controllers/test/test_gpio_command_controller.cpp 99.00% <100.00%> (ø)
...ller/include/mecanum_drive_controller/odometry.hpp 100.00% <ø> (ø)
..._controller/test/test_mecanum_drive_controller.cpp 100.00% <100.00%> (ø)
... and 15 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christophfroehlich christophfroehlich marked this pull request as ready for review June 3, 2025 20:25
@github-actions github-actions bot requested review from peterdavidfagan and xguay June 3, 2025 20:27
@mergify
Copy link
Contributor

mergify bot commented Jun 5, 2025

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

@mergify
Copy link
Contributor

mergify bot commented Jun 7, 2025

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

@mergify
Copy link
Contributor

mergify bot commented Jun 11, 2025

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

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

joints_command_subscriber_ = get_node()->create_subscription<CmdType>(
"~/commands", rclcpp::SystemDefaultsQoS(),
[this](const CmdType::SharedPtr msg) { rt_command_ptr_.writeFromNonRT(msg); });
[this](const CmdType::SharedPtr msg) { rt_command_.set(*msg); });
Copy link
Member

Choose a reason for hiding this comment

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

May be we should check here with std::all_of that all of the data elements are finite values

Copy link
Member Author

Choose a reason for hiding this comment

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

This would be a behavior change right? maybe we can do that in a follow-up PR?

@christophfroehlich christophfroehlich merged commit a88bf0a into master Jun 25, 2025
20 of 26 checks passed
@christophfroehlich christophfroehlich deleted the update/rt branch June 25, 2025 08:44
@saikishor saikishor added the backport-jazzy Triggers PR backport to ROS 2 jazzy. label Sep 27, 2025
mergify bot pushed a commit that referenced this pull request Sep 27, 2025
(cherry picked from commit a88bf0a)

# Conflicts:
#	ackermann_steering_controller/test/test_ackermann_steering_controller.cpp
#	bicycle_steering_controller/test/test_bicycle_steering_controller.cpp
#	forward_command_controller/src/forward_controllers_base.cpp
#	pid_controller/src/pid_controller.cpp
#	pid_controller/test/test_pid_controller.cpp
#	pid_controller/test/test_pid_controller.hpp
#	tricycle_steering_controller/test/test_tricycle_steering_controller.cpp
christophfroehlich added a commit that referenced this pull request Oct 2, 2025
saikishor pushed a commit that referenced this pull request Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-jazzy Triggers PR backport to ROS 2 jazzy.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants