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

Add mobile robot kinematics 101 and improve steering library docs #954

Merged
merged 17 commits into from
Jun 5, 2024

Conversation

christophfroehlich
Copy link
Contributor

@christophfroehlich christophfroehlich commented Dec 30, 2023

This is the first step of #931, but I focused only on steering on the front axle for now.
Edit: I also added the diff-drive kinematics to have a complete documentation.

After peer-reviewing of the equations I'd start to rename the variables of the library to match the doc and fix possible bugs.

rendered docs (updated 5d6aee7)

image

@christophfroehlich
Copy link
Contributor Author

christophfroehlich commented Dec 30, 2023

Copy link

codecov bot commented Dec 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.43%. Comparing base (0b43291) to head (ba808b1).
Report is 23 commits behind head on master.

Current head ba808b1 differs from pull request most recent head 5d6aee7

Please upload reports for the commit 5d6aee7 to get more accurate results.

Additional details and impacted files
@@             Coverage Diff             @@
##           master     #954       +/-   ##
===========================================
- Coverage   71.86%   47.43%   -24.43%     
===========================================
  Files          41       41               
  Lines        3650     3862      +212     
  Branches     1794     1816       +22     
===========================================
- Hits         2623     1832      -791     
- Misses        707      769       +62     
- Partials      320     1261      +941     
Flag Coverage Δ
unittests 47.43% <ø> (-24.43%) ⬇️

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

see 39 files with indirect coverage changes

@christophfroehlich
Copy link
Contributor Author

Ad odometry of ackermann steering: There is a not negligible error imho. (average is the equation implemented in the library, plotted for l = 1, w = 1; the equation from above gives the correct angle phi)
image

@roncapat
Copy link
Contributor

Impressive job! I think this is very valuable. At least for now everything seems ok to me, I would maybe suggest to be more explicit in the derivation of some formulas, maybe with collapsable sections.

For example, for the unicycle:
immagine this implies at least one or two intermediate steps and to make the description more appealing to readers I may tend to add detail (so the attention of the reader is shifted from understanding the formulas to trusting them) even if sometimes it seems trivial :)

The point is... there are not so many places on the internet where you can find a clean explanation. This can become one of these 👍🏻

About the "overdetermined" odometry solution of ackermann with traction, I would start to think about a set of recommendations or methods to find the best solution. It gets even worse with 4-wheel-4-steer vehicles and becomes an hell when considering mars-like rovers (6 wheeled).

@christophfroehlich
Copy link
Contributor Author

Impressive job! I think this is very valuable. At least for now everything seems ok to me, I would maybe suggest to be more explicit in the derivation of some formulas, maybe with collapsable sections.

For example, for the unicycle: immagine this implies at least one or two intermediate steps and to make the description more appealing to readers I may tend to add detail (so the attention of the reader is shifted from understanding the formulas to trusting them) even if sometimes it seems trivial :)

The point is... there are not so many places on the internet where you can find a clean explanation. This can become one of these 👍🏻

Thanks! I was not sure where to start and where to end. I think about it.

About the "overdetermined" odometry solution of ackermann with traction, I would start to think about a set of recommendations or methods to find the best solution. It gets even worse with 4-wheel-4-steer vehicles and becomes an hell when considering mars-like rovers (6 wheeled).

We should at least assume, that ever actuated joint has a feedback sensor (e.g., no combination of traction joints on the rear axle but speed sensors on the front axle) -> this would get very complicated to parameterize for every combination.

But as far as I understood: the claim of this lib at the current state was bicycle, tricycle, ackermann; all with front or back steering; and with the traction axle being the other than the steering axle? (the steering+traction ackermann description is not part of this then, but part of this proposal of a "general" library #692 (comment))
This is the first real difference to the tricycle_controller for #850, which expects the steering wheel be the only traction wheel, and the two behind are just following.

@christophfroehlich christophfroehlich changed the title Improve steering library docs Add mobile robot kinematics 101 and improve steering library docs Dec 31, 2023
@christophfroehlich
Copy link
Contributor Author

Am I right that for a trailing steering axis (front_steering=false) one has just to use a negative value for the wheel-base l, and that's it?

@bmagyar
Copy link
Member

bmagyar commented Feb 14, 2024

@petkovich this may be interesting to you

Copy link
Contributor

mergify bot commented May 8, 2024

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

@MCFurry
Copy link
Contributor

MCFurry commented May 14, 2024

Very nice @christophfroehlich this looks like a very good documentation page! Nice work!
Looks alright to me.
IMHO I would finish and merge this PR.
The documentation page will then be available to the public and more sets of eyes can look at it and possibly spot errors or recommend improvements?

@christophfroehlich
Copy link
Contributor Author

Very nice @christophfroehlich this looks like a very good documentation page! Nice work! Looks alright to me. IMHO I would finish and merge this PR. The documentation page will then be available to the public and more sets of eyes can look at it and possibly spot errors or recommend improvements?

Thanks! Would you mind adding a review in the UI? (Below "Files Changed" tab -> Review Changes). Yes, this will be published on control.ros.org.

Copy link
Contributor

@MCFurry MCFurry left a comment

Choose a reason for hiding this comment

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

Nice work! LGTM!

@bmagyar bmagyar merged commit d076ea2 into ros-controls:master Jun 5, 2024
5 checks passed
@christophfroehlich christophfroehlich deleted the steering_lib_docs branch June 5, 2024 17:40
@christophfroehlich christophfroehlich added backport-humble This label should be used by maintaines only! Label triggers PR backport to ROS2 humble. backport-iron This label should be used by maintaines only! Label triggers PR backport to ROS2 Iron. labels Jun 5, 2024
mergify bot pushed a commit that referenced this pull request Jun 5, 2024
mergify bot pushed a commit that referenced this pull request Jun 5, 2024
(cherry picked from commit d076ea2)

# Conflicts:
#	steering_controllers_library/doc/userdoc.rst
#	tricycle_controller/doc/userdoc.rst
christophfroehlich added a commit that referenced this pull request Jun 5, 2024
…) (#1161)

(cherry picked from commit d076ea2)

Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
christophfroehlich added a commit that referenced this pull request Jun 5, 2024
…) (#1160)

Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
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 maintaines only! Label triggers PR backport to ROS2 humble. backport-iron This label should be used by maintaines only! Label triggers PR backport to ROS2 Iron.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants