Skip to content

Conversation

christophfroehlich
Copy link
Contributor

@christophfroehlich christophfroehlich commented Oct 13, 2025

@christophfroehlich christophfroehlich added the backport-jazzy Triggers PR backport to ROS 2 jazzy. label Oct 13, 2025
@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2025

Codecov Report

❌ Patch coverage is 84.21053% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.69%. Comparing base (b37cf44) to head (e74ec11).

Files with missing lines Patch % Lines
control_toolbox/include/control_toolbox/pid.hpp 57.14% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           ros2-master     #510      +/-   ##
===============================================
+ Coverage        82.35%   82.69%   +0.33%     
===============================================
  Files               29       29              
  Lines             1984     1982       -2     
  Branches           114      113       -1     
===============================================
+ Hits              1634     1639       +5     
+ Misses             281      274       -7     
  Partials            69       69              
Flag Coverage Δ
unittests 82.69% <84.21%> (+0.33%) ⬆️

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

Files with missing lines Coverage Δ
...ontrol_toolbox/include/control_toolbox/pid_ros.hpp 100.00% <ø> (ø)
control_toolbox/src/pid.cpp 83.73% <100.00%> (ø)
control_toolbox/src/pid_ros.cpp 83.77% <100.00%> (ø)
control_toolbox/test/pid_ros_parameters_tests.cpp 99.02% <100.00%> (+0.02%) ⬆️
control_toolbox/test/pid_tests.cpp 100.00% <ø> (ø)
control_toolbox/include/control_toolbox/pid.hpp 65.75% <57.14%> (+4.77%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christophfroehlich
Copy link
Contributor Author

@bijoua29 @ViktorCVS maybe you can have a quick look please, do you agree?

Copy link
Contributor

@bijoua29 bijoua29 left a comment

Choose a reason for hiding this comment

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

Have you checked if i_min=i_max=0, that it is internally consistent?

@christophfroehlich
Copy link
Contributor Author

Have you checked if i_min=i_max=0, that it is internally consistent?

From: https://en.cppreference.com/w/cpp/algorithm/clamp.html

If lo is greater than hi, the behavior is undefined.

If it equals, then std::clamp will just force it to zero, right?

@christophfroehlich christophfroehlich merged commit 84a0c5c into ros2-master Oct 14, 2025
25 checks passed
@christophfroehlich christophfroehlich deleted the improve/pid/validation branch October 14, 2025 20:31
mergify bot pushed a commit that referenced this pull request Oct 14, 2025
(cherry picked from commit 84a0c5c)

# Conflicts:
#	control_toolbox/include/control_toolbox/pid.hpp
#	control_toolbox/include/control_toolbox/pid_ros.hpp
#	control_toolbox/src/pid_ros.cpp
#	control_toolbox/test/pid_ros_parameters_tests.cpp
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