Skip to content

Remove deprecated publish_rate from diff_drive_controller#2259

Merged
christophfroehlich merged 1 commit intoros-controls:masterfrom
Bhavin-umatiya:remove/publish-rate-diff-drive
Apr 30, 2026
Merged

Remove deprecated publish_rate from diff_drive_controller#2259
christophfroehlich merged 1 commit intoros-controls:masterfrom
Bhavin-umatiya:remove/publish-rate-diff-drive

Conversation

@Bhavin-umatiya
Copy link
Copy Markdown
Contributor

Description

Following the deprecation in PR #2245, this follow-up PR removes the deprecated publish_rate parameter and its associated logic from diff_drive_controller.

This includes:

  1. Removal of publish_rate from diff_drive_controller_parameter.yaml.
  2. Removal of publish_rate_, publish_period_, and previous_publish_timestamp_ from the controller header.
  3. Removal of publication throttling logic from the control loop; it now publishes at every cycle.

This PR is set as a Draft as requested by @christophfroehlich, to be merged after the next project sync.

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Mar 29, 2026

This pull request is in conflict. Could you fix it @Bhavin-umatiya?

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.

Thank you. Please rebase this on top of master as the deprecation PR got merged now.

@Bhavin-umatiya Bhavin-umatiya force-pushed the remove/publish-rate-diff-drive branch from 9fe719a to 1bbdc98 Compare March 29, 2026 09:46
@Bhavin-umatiya
Copy link
Copy Markdown
Contributor Author

Thank you, @christophfroehlich. I have rebased the branch on the latest master and resolved the merge conflicts. The PR is now up to date with the merged deprecation code.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 29, 2026

Codecov Report

❌ Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.00%. Comparing base (27ea539) to head (b16e2b9).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...iff_drive_controller/src/diff_drive_controller.cpp 95.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2259   +/-   ##
=======================================
  Coverage   84.99%   85.00%           
=======================================
  Files         145      145           
  Lines       15186    15169   -17     
  Branches     1331     1328    -3     
=======================================
- Hits        12907    12894   -13     
+ Misses       1789     1787    -2     
+ Partials      490      488    -2     
Flag Coverage Δ
unittests 85.00% <95.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 80.61% <95.00%> (+0.49%) ⬆️

... and 4 files with indirect coverage changes

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

@Bhavin-umatiya Bhavin-umatiya force-pushed the remove/publish-rate-diff-drive branch from 1bbdc98 to 238cc7c Compare April 20, 2026 05:14
@Bhavin-umatiya Bhavin-umatiya marked this pull request as ready for review April 20, 2026 05:16
@Bhavin-umatiya
Copy link
Copy Markdown
Contributor Author

@christophfroehlich I have rebased this on top of master and resolved the conflicts as requested. The PR is now ready for review.

@Bhavin-umatiya Bhavin-umatiya changed the title [Draft] Remove deprecated publish_rate from diff_drive_controller Remove deprecated publish_rate from diff_drive_controller Apr 21, 2026
@Bhavin-umatiya Bhavin-umatiya force-pushed the remove/publish-rate-diff-drive branch from 238cc7c to b16e2b9 Compare April 25, 2026 16:41
@Bhavin-umatiya Bhavin-umatiya force-pushed the remove/publish-rate-diff-drive branch from b16e2b9 to 21cf0e8 Compare April 28, 2026 12:49
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.

LGTM, but can you add a note here?

# Conflicts:
#	diff_drive_controller/src/diff_drive_controller_parameter.yaml
@Bhavin-umatiya Bhavin-umatiya force-pushed the remove/publish-rate-diff-drive branch from 21cf0e8 to 6d79d93 Compare April 30, 2026 04:45
@Bhavin-umatiya
Copy link
Copy Markdown
Contributor Author

@christophfroehlich I have added the note to the release notes as requested!

@christophfroehlich
Copy link
Copy Markdown
Member

christophfroehlich commented Apr 30, 2026

Please never do force pushes on PRs which have been reviewed already. For reference, see our contribution guidelines

@christophfroehlich christophfroehlich merged commit 2482f04 into ros-controls:master Apr 30, 2026
14 of 18 checks passed
@github-project-automation github-project-automation Bot moved this from ROS-L to Done in Roadmap / Features Apr 30, 2026
@Bhavin-umatiya
Copy link
Copy Markdown
Contributor Author

Thank you for the guidance and for merging the PR! @christophfroehlich.. My apologies for the force push I understand now that it disrupts the review history. I will make sure to use incremental commits for review updates in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants