Skip to content

fixed on_set_chained_mode implementations to avoid cpplint warnings #1564

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

Merged

Conversation

bhagyeshagresar
Copy link
Contributor

@bhagyeshagresar bhagyeshagresar commented Mar 2, 2025

Fixed warnings generated by cpplint/cpplint#131 on some of the controllers that implement the chained mode.

@@ -689,8 +689,9 @@ controller_interface::CallbackReturn DiffDriveController::configure_side(

bool DiffDriveController::on_set_chained_mode(bool chained_mode)
{
// Always accept switch to/from chained mode (without linting type-cast error)
return true || chained_mode;
// Fix to adhere to CppLint standards
Copy link
Member

@bmagyar bmagyar Mar 2, 2025

Choose a reason for hiding this comment

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

A moor reference isn't really adhering to cppkint standards ;)

Copy link
Member

Choose a reason for hiding this comment

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

You can achieve the same by commenting out the name of the argument. It tricks the parser and also the compiler. We already use that approach in the codebase, just look for "/*" in ros2_control

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I don't think so. See #1491 and the linked cpplint issue.

Copy link
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 contribution! Great that you found out this. Please take a look at the solution I recommended.

@@ -689,8 +689,9 @@ controller_interface::CallbackReturn DiffDriveController::configure_side(

bool DiffDriveController::on_set_chained_mode(bool chained_mode)
{
// Always accept switch to/from chained mode (without linting type-cast error)
return true || chained_mode;
// Fix to adhere to CppLint standards
Copy link
Member

Choose a reason for hiding this comment

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

You can achieve the same by commenting out the name of the argument. It tricks the parser and also the compiler. We already use that approach in the codebase, just look for "/*" in ros2_control

@christophfroehlich christophfroehlich linked an issue Mar 2, 2025 that may be closed by this pull request
8 tasks
Copy link

codecov bot commented May 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.61%. Comparing base (0095ef1) to head (6538cfc).
Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1564   +/-   ##
=======================================
  Coverage   85.60%   85.61%           
=======================================
  Files         123      123           
  Lines       11873    11860   -13     
  Branches     1016     1015    -1     
=======================================
- Hits        10164    10154   -10     
+ Misses       1384     1383    -1     
+ Partials      325      323    -2     
Flag Coverage Δ
unittests 85.61% <100.00%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
...iff_drive_controller/src/diff_drive_controller.cpp 81.29% <100.00%> (-0.07%) ⬇️
..._drive_controller/src/mecanum_drive_controller.cpp 90.68% <100.00%> (-0.04%) ⬇️
pid_controller/src/pid_controller.cpp 72.08% <100.00%> (-0.12%) ⬇️
...llers_library/src/steering_controllers_library.cpp 70.37% <100.00%> (-0.11%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christophfroehlich christophfroehlich added backport-humble This label should be used by maintainers only! Label triggers PR backport to ROS2 humble. backport-jazzy labels May 17, 2025
@christophfroehlich christophfroehlich merged commit d8dc4b1 into ros-controls:master May 17, 2025
21 of 26 checks passed
mergify bot pushed a commit that referenced this pull request May 17, 2025
…ngs (#1564)

(cherry picked from commit d8dc4b1)

# Conflicts:
#	diff_drive_controller/src/diff_drive_controller.cpp
#	pid_controller/src/pid_controller.cpp
mergify bot pushed a commit that referenced this pull request May 17, 2025
christophfroehlich pushed a commit that referenced this pull request May 17, 2025
christophfroehlich pushed a commit that referenced this pull request May 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble This label should be used by maintainers only! Label triggers PR backport to ROS2 humble. backport-jazzy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix cpplint warning
4 participants