Skip to content

Fix - #4400 keepout speed zones #5129

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Fix - #4400 keepout speed zones #5129

wants to merge 2 commits into from

Conversation

rohinraj
Copy link

@rohinraj rohinraj commented May 6, 2025

Basic Info

Info Please fill out this column
Ticket(s) this addresses (add tickets here #4400)
Primary OS tested on Ubuntu
Robotic platform tested on Turtlebot 4 simulation
Does this PR contain AI generated software? No

Description of contribution in a few bullet points

  • Add keepout filter for tb4 simulation
  • Add speed filter for tb4 simulation
  • Add seperate launch files and params file for better references.

Description of documentation updates required from your changes

Description of how this change was tested

  • Tested in simulation enviornment in latest jazzy-desktop-full docker image.

Future work that may be required in bullet points

  • I think there need some better visualisation to be added for speed filter

For Maintainers:

  • Check that any new parameters added are updated in docs.nav2.org
  • Check that any significant change is added to the migration guide
  • Check that any new features OR changes to existing behaviors are reflected in the tuning guide
  • Check that any new functions have Doxygen added
  • Check that any new features have test coverage
  • Check that any new plugins is added to the plugins page
  • If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

rohinraj added 2 commits May 6, 2025 10:55
@rohinraj
Add Keepout Simulation
349a86a 
* Add keepout filter launch file
* Add keepout navigation parameters
* Add keepout warehouse map and yaml files
@rohinraj
Add Speed Filter Simulation
b891ee6
@rohinraj rohinraj mentioned this pull request May 6, 2025
Closed
7 tasks
SteveMacenski
SteveMacenski reviewed May 6, 2025
View reviewed changes
Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

There's some clear overlap with #5125 that I think you and @leander-dsouza should coordinate on! Please look over that PRs review as well for some more hints. There's alot of duplication in this PR with new launch files, config files, etc that I think we should be integrating and not replacing with to add these features!

nav2_bringup/params/speed_filter_params.yaml
@@ -0,0 +1,16 @@
costmap_filter_info_server:
Copy link
Member

Choose a reason for hiding this comment

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

Please add parameters to the main nav2_params.yaml file

nav2_bringup/maps/speed_filter_warehouse.pgm
Copy link
Member

Choose a reason for hiding this comment

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

Lets do the depot first since that's the default bringup

@leander-dsouza
Copy link
Contributor

There's some clear overlap with #5125 that I think you and @leander-dsouza should coordinate on! Please look over that PRs review as well for some more hints.

@rohinraj, Please feel free to have a look over at the PR #5125 to check if I have missed any crucial behaviour. Currently, it only has the keepout filter regions. I plan to add the speed limit regions in a future PR.

@SteveMacenski
Copy link
Member

Thanks for the contribution @rohinraj but I'm going to close in favor of @leander-dsouz's implementation since they're responding to reviews and we've gone through several design processes already. Thanks for the help and I look forward to your contribution in other potential fixes in the future!

@rohinraj
Copy link
Author

Sure, Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SteveMacenski SteveMacenski SteveMacenski left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rohinraj @leander-dsouza @SteveMacenski