ros2_controllers
ros2_controllers copied to clipboard
Use Generic Steering Controller State Message in Steering Controllers Library
I was reading through the code as a part of learning how to write a new steering controller. I understood that a new steering controller inherits from the SteeringControllersLibrary. But I also found the use of Ackermannxxx
in the base class code for a status message and the same was being used in other steering controllers such as BicycleSteeringController
and TricycleSteeringController
.
If I understood the inheritance and the code structure correctly, the SteerinControllersLibrary should be agnostic to the specifics of a steering controller that derives from it. Therefore, this PR introduces the following changes:
- Remove the use of
Ackermannxxx
declarations in theSteeringControllersLibrary
base class and rename it toSteeringControllerStateMsg
- Modify the tests and state publishers to use the renamed declaration
- Remove
ackermann_msgs
dependency fromsteering_controllers_library
package.
All tests for the modified packages were run successfully (locally).
P.S: Sorry, this took longer than planned. It took me some time to figure out the relevant workspace overlays to make sure the correct version of controller_interface
was used for the build to succeed.
Codecov Report
Attention: Patch coverage is 0%
with 1 lines
in your changes are missing coverage. Please review.
Project coverage is 47.69%. Comparing base (
0b43291
) to head (fd71b75
). Report is 8 commits behind head on master.
:exclamation: Current head fd71b75 differs from pull request most recent head bfc78b8. Consider uploading reports for the commit bfc78b8 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## master #836 +/- ##
===========================================
- Coverage 71.86% 47.69% -24.17%
===========================================
Files 41 41
Lines 3650 3862 +212
Branches 1794 1816 +22
===========================================
- Hits 2623 1842 -781
- Misses 707 758 +51
- Partials 320 1262 +942
Flag | Coverage Δ | |
---|---|---|
unittests | 47.69% <0.00%> (-24.17%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Files | Coverage Δ | |
---|---|---|
...llers_library/src/steering_controllers_library.cpp | 41.69% <0.00%> (-30.96%) |
:arrow_down: |
The build failures I am seeing on the workflow with rolling binaries are exactly the same I had because the latest master
of controller_interface
pkg in ros2_control
wasn't yet available on rolling
binaries as of yesterday. Basically, this change is not yet available on rolling
binaries.
I am not sure how the changes I made have impacted code coverage because I mainly renamed declarations and removed unused declarations. If that needs to be fixed, I could use some help on the same. Thank you!
We know of the failing CI of the binary builds, and the coverage diff of 0.05% is some "noise" you don't have to worry about.
We know of the failing CI of the binary builds, and the coverage diff of 0.05% is some "noise" you don't have to worry about.
Alright, thanks!