ros-controls / ros2_controllers

Generic robotic controllers to accompany ros2_control
https://control.ros.org
Apache License 2.0
355 stars 319 forks source link

WIP: Add AckermannDriveStamped control to steering library #1171

Open wittenator opened 3 months ago

wittenator commented 3 months ago

This PR adds the option to use steering angle and linear velocity for controllers that inherit from the steering library. In anticipation of the merging and backport of the fixes in the ackermann controller, I branched off of the fix/steering_controllers_library_kinematics branch and cherry-picked the changes on top of the iron branch.

Status:

mergify[bot] commented 3 months ago

@wittenator, all pull requests must be targeted towards the master development branch. Once merged into master, it is possible to backport to iron, but it must be in master to have these changes reflected into new distributions.

wittenator commented 3 months ago

Ah hmm, I can't move our software stack to Jazzy (due to Nvidia dependencies) and therefore have to develop these changes on Iron. Should I develop the feature completely on Iron and then cherry-pick the changes on top of the master? The drawback is that I can't really test in on the master besides a few unit tests maybe.

christophfroehlich commented 3 months ago

Let's merge https://github.com/ros-controls/ros2_controllers/pull/1150 first, and then proceed with this one here. Could you please review it and give your feedback (Files changed -> Review changes)?

mergify[bot] commented 3 months ago

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

mergify[bot] commented 3 months ago

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

christophfroehlich commented 2 months ago

@wittenator is this WIP or ready for review?

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 22.72727% with 51 lines in your changes missing coverage. Please review.

Project coverage is 86.83%. Comparing base (c824345) to head (12d3fa3).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## iron #1171 +/- ## ========================================== - Coverage 87.30% 86.83% -0.48% ========================================== Files 92 92 Lines 8408 8449 +41 Branches 701 711 +10 ========================================== - Hits 7341 7337 -4 - Misses 812 852 +40 - Partials 255 260 +5 ``` | [Flag](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1171/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1171/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls) | `86.83% <22.72%> (-0.48%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1171?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls) | Coverage Δ | | |---|---|---| | [...steering\_controllers\_library/steering\_odometry.hpp](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1171?src=pr&el=tree&filepath=steering_controllers_library%2Finclude%2Fsteering_controllers_library%2Fsteering_odometry.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#diff-c3RlZXJpbmdfY29udHJvbGxlcnNfbGlicmFyeS9pbmNsdWRlL3N0ZWVyaW5nX2NvbnRyb2xsZXJzX2xpYnJhcnkvc3RlZXJpbmdfb2RvbWV0cnkuaHBw) | `100.00% <ø> (ø)` | | | [...g\_controller/src/ackermann\_steering\_controller.cpp](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1171?src=pr&el=tree&filepath=ackermann_steering_controller%2Fsrc%2Fackermann_steering_controller.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#diff-YWNrZXJtYW5uX3N0ZWVyaW5nX2NvbnRyb2xsZXIvc3JjL2Fja2VybWFubl9zdGVlcmluZ19jb250cm9sbGVyLmNwcA==) | `80.00% <0.00%> (ø)` | | | [...ing\_controller/src/bicycle\_steering\_controller.cpp](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1171?src=pr&el=tree&filepath=bicycle_steering_controller%2Fsrc%2Fbicycle_steering_controller.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#diff-YmljeWNsZV9zdGVlcmluZ19jb250cm9sbGVyL3NyYy9iaWN5Y2xlX3N0ZWVyaW5nX2NvbnRyb2xsZXIuY3Bw) | `75.00% <0.00%> (ø)` | | | [...ng\_controller/src/tricycle\_steering\_controller.cpp](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1171?src=pr&el=tree&filepath=tricycle_steering_controller%2Fsrc%2Ftricycle_steering_controller.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#diff-dHJpY3ljbGVfc3RlZXJpbmdfY29udHJvbGxlci9zcmMvdHJpY3ljbGVfc3RlZXJpbmdfY29udHJvbGxlci5jcHA=) | `78.57% <0.00%> (ø)` | | | [...ring\_controllers\_library/src/steering\_odometry.cpp](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1171?src=pr&el=tree&filepath=steering_controllers_library%2Fsrc%2Fsteering_odometry.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#diff-c3RlZXJpbmdfY29udHJvbGxlcnNfbGlicmFyeS9zcmMvc3RlZXJpbmdfb2RvbWV0cnkuY3Bw) | `80.12% <33.33%> (-1.29%)` | :arrow_down: | | [...llers\_library/src/steering\_controllers\_library.cpp](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1171?src=pr&el=tree&filepath=steering_controllers_library%2Fsrc%2Fsteering_controllers_library.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#diff-c3RlZXJpbmdfY29udHJvbGxlcnNfbGlicmFyeS9zcmMvc3RlZXJpbmdfY29udHJvbGxlcnNfbGlicmFyeS5jcHA=) | `63.34% <23.33%> (-10.83%)` | :arrow_down: | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1171/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls)
christophfroehlich commented 2 months ago

@wittenator please have a look at the failing pre-commit test, and add tests for your new feature because it is decreasing test coverage. Ping me again if you need guidance on how to write tests for the lib.