Skip to content

Added velocity limiting to the mecanum controller.#2313

Open
tonybaltovski wants to merge 6 commits intoros-controls:masterfrom
clearpathrobotics:feature/add-velocity-limiting-mecanum
Open

Added velocity limiting to the mecanum controller.#2313
tonybaltovski wants to merge 6 commits intoros-controls:masterfrom
clearpathrobotics:feature/add-velocity-limiting-mecanum

Conversation

@tonybaltovski
Copy link
Copy Markdown
Contributor

Description

Added velocity limiting to linear x,y direction and angular yaw. Also, added tests to verify them.

Fixes # (issue)

Is this user-facing behavior change?

Users can set velocity limits but if they are not set it will not change behaviors.

Did you use Generative AI?

No.

Additional Information

TODOs

To send us a pull request, please:

  • Fork the repository.
  • Modify the source; please focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
  • Ensure local tests pass. (colcon test and pre-commit run (requires you to install pre-commit by pip3 install pre-commit)
  • Commit to your fork using clear commit messages.
  • Send a pull request, answering any default questions in the pull request interface.
  • Pay attention to any automated CI failures reported in the pull request, and stay involved in the conversation.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 16, 2026

Codecov Report

❌ Patch coverage is 93.69748% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.24%. Comparing base (5a74f29) to head (15551f6).
⚠️ Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
..._drive_controller/src/mecanum_drive_controller.cpp 83.63% 9 Missing ⚠️
..._controller/test/test_mecanum_drive_controller.cpp 96.72% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2313      +/-   ##
==========================================
+ Coverage   85.13%   85.24%   +0.11%     
==========================================
  Files         154      154              
  Lines       15417    15331      -86     
  Branches     1334     1283      -51     
==========================================
- Hits        13125    13069      -56     
+ Misses       1800     1787      -13     
+ Partials      492      475      -17     
Flag Coverage Δ
unittests 85.24% <93.69%> (+0.11%) ⬆️

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

Files with missing lines Coverage Δ
..._controller/test/test_mecanum_drive_controller.hpp 87.50% <ø> (ø)
..._controller/test/test_mecanum_drive_controller.cpp 98.36% <96.72%> (-0.99%) ⬇️
..._drive_controller/src/mecanum_drive_controller.cpp 84.93% <83.63%> (-0.83%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread mecanum_drive_controller/src/mecanum_drive_controller.cpp Outdated
Comment thread mecanum_drive_controller/src/mecanum_drive_controller.cpp Outdated
Copy link
Copy Markdown
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

Thank you!
Left a few notes but I'm mostly concerned about the reset.

Copy link
Copy Markdown
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

Thank you for the quick fixes!

@tonybaltovski
Copy link
Copy Markdown
Contributor Author

Thank you for the quick fixes!

No worries but let me get these changes test on a robot before merging though.

@christophfroehlich christophfroehlich added the hold Holding off merging this PR until some condition label Apr 22, 2026
Copy link
Copy Markdown
Member

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

As the limiter parameters are not read_only, we should also update them in active state
This is also the way we want o go with diff_drive_controller

@bmagyar bmagyar added backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels May 5, 2026
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. backport-kilted Triggers PR backport to ROS 2 kilted. hold Holding off merging this PR until some condition

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants