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

Issue with Bidirectional Acceleration and Deceleration Settings in MPPI Controller #4601

Open
osama-z-salah opened this issue Aug 7, 2024 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@osama-z-salah
Copy link

I am using an MPPI controller to drive a robot capable of traveling bidirectionally with equal efficiency. I want to have different values for acceleration and deceleration. To achieve this, I set the acceleration by setting ax_max to 0.5 and the deceleration by setting ax_min to -1.5. These values are respected when the robot is traveling forward. However, when the robot is traveling backward, the acceleration value becomes the deceleration value, and vice versa.

@SteveMacenski
Copy link
Member

SteveMacenski commented Aug 7, 2024

This is the block of code of interest https://github.com/ros-navigation/navigation2/blob/main/nav2_mppi_controller/include/nav2_mppi_controller/motion_models.hpp#L85-L113

What you mention here is very sensible and I think that should be enable-able. I'd be open to your suggestions, but I think a potential solution would be to have a new parameter option for bidirectionality - similar to mode in PathAngleCritic - which in this block can be checked and flip the signs as needed. The parameter should be stored in control_constraints_.

Optionally, we could make that default behavior and just have the logic in that block for checking if we're slowing our total speed (apply deceleration) or increasing our total speed (apply acceleration), regardless of signed direction when applying limits.

Something to take care about is that this block is very performance sensitive, so when prototyping solutions, we need to benchmark before and after compute times of the controller to make sure we're not manifestly dropping performance. This should be possible without any performance dings I think.

I imagine that's what @doisyg would also want. @tonynajjar do you think that's reasonable to have it default have that behavior as a general case for your non-bidirectional case? I'd always prefer less parameters if it can be avoided, but it does seem like it should be applied in this way for all cases I think since its more about 'speed' than direction.

@doisyg
Copy link
Contributor

doisyg commented Aug 9, 2024

I imagine that's what @doisyg would also want.

Yes!
Maybe some logic to reuse from this: #3529

@SteveMacenski
Copy link
Member

I think that is precisely code to reuse here! @osama-z-salah can you create a PR for this? Happy to merge!

@SteveMacenski SteveMacenski added enhancement New feature or request good first issue Good for newcomers labels Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants