Skip to content
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

Change units of velocity feedback in diff_drive_controller #behaviour-change #452

Merged
merged 3 commits into from
Dec 3, 2022

Conversation

macstepien
Copy link
Contributor

Closes #411
Adds multiplying velocity feedback by wheel radius. When the position_feedback option is used, feedback is multiplied by radius (it is expected to be in rad). This PR changes velocity feedback to be also multiplied by radius, so that units used will be more consistent.

adds multiplying feedback by wheel radius, so that it can be in rad/s
@bmagyar bmagyar changed the title Change units of velocity feedback in diff_drive_controller Change units of velocity feedback in diff_drive_controller #behaviour-change Oct 20, 2022
@bmagyar
Copy link
Member

bmagyar commented Oct 20, 2022

Aka feedback is in m/s? Isn't there a 2* missing?

@macstepien
Copy link
Contributor Author

On the current master branch velocity feedback has to be in m/s for it to work properly. After this change it will be in rad/s and by multiplying by radius it will be converted to m/s for proper handling in the updateFromVelocity function.

I think that it is correct, we multiply angular velocity by radius to get linear velocity. Am I missing any other reason why there should be 2*?

@destogl destogl enabled auto-merge (squash) December 3, 2022 11:59
@codecov-commenter
Copy link

Codecov Report

Merging #452 (b98da97) into master (e7f9962) will decrease coverage by 6.07%.
The diff coverage is 20.15%.

@@            Coverage Diff             @@
##           master     #452      +/-   ##
==========================================
- Coverage   35.78%   29.70%   -6.08%     
==========================================
  Files         189        7     -182     
  Lines       17570      744   -16826     
  Branches    11592      429   -11163     
==========================================
- Hits         6287      221    -6066     
+ Misses        994      163     -831     
+ Partials    10289      360    -9929     
Flag Coverage Δ
unittests 29.70% <20.15%> (-6.08%) ⬇️

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

Impacted Files Coverage Δ
...de/diff_drive_controller/diff_drive_controller.hpp 100.00% <ø> (ø)
...ontroller/test/test_load_diff_drive_controller.cpp 12.50% <0.00%> (ø)
diff_drive_controller/src/odometry.cpp 42.16% <11.11%> (ø)
...ive_controller/test/test_diff_drive_controller.cpp 17.62% <12.08%> (ø)
diff_drive_controller/src/speed_limiter.cpp 46.55% <13.33%> (ø)
...troller/include/diff_drive_controller/odometry.hpp 20.00% <20.00%> (ø)
...iff_drive_controller/src/diff_drive_controller.cpp 32.04% <24.01%> (ø)
...mand_controller/src/forward_command_controller.cpp
...ller/test/test_load_forward_command_controller.cpp
...ntroller/test/test_load_joint_state_controller.cpp
... and 192 more

Copy link
Member

@destogl destogl left a comment

Choose a reason for hiding this comment

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

Great!

@destogl destogl merged commit 5a367e2 into ros-controls:master Dec 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Units of velocity feedback in diff_drive_controller
4 participants