-
Notifications
You must be signed in to change notification settings - Fork 345
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
[pid_controller] Update tests #1538
[pid_controller] Update tests #1538
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1538 +/- ##
==========================================
+ Coverage 84.27% 84.38% +0.10%
==========================================
Files 123 124 +1
Lines 11359 11386 +27
Branches 961 959 -2
==========================================
+ Hits 9573 9608 +35
+ Misses 1470 1462 -8
Partials 316 316
Flags with carried forward coverage won't be shown. Click here to find out more.
|
FYI: some of the changes are already included in #1437. I marked that ready for review now, as it got released and merged into ros2-testing. |
This pull request is in conflict. Could you fix it @Juliaj? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx for adding more tests and the cleanup!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I just added a convenience function set_reference
to TestablePidController
, we have repeated that 10times already.
b24b310
into
ros-controls:master
(cherry picked from commit b24b310) # Conflicts: # pid_controller/src/pid_controller.cpp # pid_controller/test/pid_controller_params.yaml # pid_controller/test/test_pid_controller.cpp # pid_controller/test/test_pid_controller.hpp
Main changes