-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[RPP] Prevent collision check premature termination with scaled_lookahead #5677
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
base: main
Are you sure you want to change the base?
[RPP] Prevent collision check premature termination with scaled_lookahead #5677
Conversation
Signed-off-by: EricoMeger <[email protected]>
Co-authored-by: Steve Macenski <[email protected]> Signed-off-by: Érico Meger <[email protected]> Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
Signed-off-by: EricoMeger <[email protected]>
|
This PR should address the cases we discussed. |
Codecov Report❌ Patch coverage is
... and 24 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
Sorry for the delay - ROSCon + vacation delayed me and getting back up to everything :-) Looking now. Can you handle the merge conflict GitHub is reporting? |
|
@doisyg and @mini-1235 were involved in the discussions leading to this PR. Can you guys review / give you opinion on this vs the other PRs that were reverted to make sure it fixes the need? |
nav2_regulated_pure_pursuit_controller/src/collision_checker.cpp
Outdated
Show resolved
Hide resolved
I will review tomorrow / Friday |
|
No worries about the delay. I'm currently in the middle of my finals at uni, so for the next two weeks, my time will be quite limited, so I won't be able to pay close attention to this PR. I will, however, resolve the merge conflict as soon as possible I missed the code coverage message, but I will be looking into how to write the tests for this. Do you guys have any recommendation for a starting point? And a huge congratulations to @mini-1235 on joining the core team! 👏 👏 |
Signed-off-by: Érico Meger <[email protected]>
Co-authored-by: Steve Macenski <[email protected]> Signed-off-by: Érico Meger <[email protected]>
Basic Info
Description of contribution in a few bullet points
Description of documentation updates required from your changes
Added new parameter, so need to add that to default configs and documentation page
Description of how this change was tested
Ran multiple tests on both a physical platform and simulation. Tests included different maps, routes, and parameter configurations (e.g., varying footprint sizes, velocities, and safety distances)
Future work that may be required in bullet points
For Maintainers:
backport-*.