-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 option to use open-loop control with Rotation Shim #4880
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some pretty reasonable blocks of code without coverage, so I think this needs some tests to cover those gaps by enabling the setting and checking for the expected outputs without odometry feedback
Otherwise, LTGM!
nav2_rotation_shim_controller/src/nav2_rotation_shim_controller.cpp
Outdated
Show resolved
Hide resolved
nav2_rotation_shim_controller/src/nav2_rotation_shim_controller.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: RBT22 <rozgonyibalint@gmail.com>
Signed-off-by: RBT22 <rozgonyibalint@gmail.com>
Signed-off-by: RBT22 <rozgonyibalint@gmail.com>
Signed-off-by: RBT22 <rozgonyibalint@gmail.com>
d959cf0
to
6a22187
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small docs nitpick and I can merge the pair!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, missed one.
If you have a goal then stop navigating for some time, then start navigating again, the state will be stale. If the goal is canceled mis-spin, then the last value will not be reset
We should override reset()
in the controller and reset the value to max
there as well. The reset()
was added to the controller API a little while ago for exactly these kinds of between-request state clearing operations
Basic Info
Description of contribution in a few bullet points
Description of documentation updates required from your changes
New parameter description needed and migration guide.
Future work that may be required in bullet points
For Maintainers: