-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[RPP] Adding support for rotate in place cusps #3934
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SteveMacenski
changed the title
Adding support for rotate in place cusps
[RPP] Adding support for rotate in place cusps
Nov 2, 2023
Sounds reasonable enough. |
@Aposhian do you see any regressions in this for your application? I want to make sure there aren't unintended consequences given your larger application |
Actually we aren't using RPP at the moment, so I can't say if it causes regressions for us. |
ajtudela
pushed a commit
to grupo-avispa/navigation2
that referenced
this pull request
Nov 17, 2023
jwallace42
pushed a commit
to jwallace42/navigation2
that referenced
this pull request
Jan 3, 2024
Signed-off-by: gg <[email protected]>
SteveMacenski
added a commit
that referenced
this pull request
Jan 24, 2024
SteveMacenski
added a commit
that referenced
this pull request
Jan 24, 2024
* collision_monitor: dynamic polygon and source enable/disable (#3825) * Rename PushRosNamespace to PushROSNamespace * Fix min_points checking * initial * fix * fix * remove unrelated change * reset * fix format * PR fixes * Add test * fix comments * add to params * publish only if enabled * Add source dynamic enable/disable * add enabled param to sources * fix * add same to collision detector * Update README.md: fix typo (#3842) * Update package.xml * fix typo (#3850) * adjust link to point to v3.8 of behavior tree docs (#3851) BT.CPP_v3 is used, thereby the correct docs should be linked * Fix bug in nav2_behavior_tree/bt_action_node (#3849) * Fix bug in nav2_behavior_tree/bt_action_node * Fixed the bug in halt function inside nav2_behavior_tree/plugin/action/bt_action_node.hpp * Added new case to nav2_behavior_tree/plugin/action/bt_action_node.hpp for testing the scenario to cancel * Refactored existing cases in nav2_behavior_tree/plugin/action/bt_action_node.hpp Signed-off-by: CihatAltiparmak <[email protected]> * Fix bug in nav2_behavior_tree/bt_action_node * Fixed the bug in halt function inside nav2_behavior_tree/plugin/action/bt_action_node.hpp * Added new case to nav2_behavior_tree/plugin/action/bt_action_node.hpp for testing the scenario to cancel * Refactored existing cases in nav2_behavior_tree/plugin/action/bt_action_node.hpp Signed-off-by: CihatAltiparmak <[email protected]> --------- Signed-off-by: CihatAltiparmak <[email protected]> * Fix action test failure due to rate after Rolling sync API change (#3852) * Update CMakeLists.txt (#3843) * add option for sse4 and avs512 (#3853) * Remove all exit(-1) crash conditions (#3846) * Update transform_available_condition.cpp * wrapping all examples of get_plugin_type_param in exceptions and making that throw instead of crash * some linting * fix typo * Update controller.cpp * Update nav2_params.yaml * Update nav2_params.yaml * simplication of lattice_generator.py, fixes #3858 (#3859) * simplification of equation to compute the max_value/outer edge of the lattice based on number of headings. * Stop error diagnostics when pausing nav (#3830) * Added nodestate enum and a variable to keep track of current state of managed nodes. * Updating state_of_managed_nodes_ when switching states and using it to determine an accurate diagnostics message. * Fixing bugs. * Updated/added docstrings. * Publishing OK status when nodes are unconfigured. Changed if-else chain to switch case. * Renamed NodeState PAUSED to INACTIVE, state_of_managed_nodes_ to managed_nodes_state_ and replaced system_active_ with an inline method. * Bugfix. --------- Co-authored-by: Pekka Myller <[email protected]> * Add a timeout to the wait for transforms step of the costmap activation. (#3866) * Add a timeout to the wait for transforms step of the costmap activation. Signed-off-by: Fabian König <[email protected]> * Rename wait_for_transforms_timeout to initial_transform_timeout * Activate costmap publishers only after transforms are checked * Check if controller server activation was succesful in planner_server * Add unittest for costmap activation Signed-off-by: Fabian König <[email protected]> --------- Signed-off-by: Fabian König <[email protected]> * Fix class doxygen * fix minor typos (#3892) Signed-off-by: Anton Kesy <[email protected]> * Publish collision points for debug purposes (#3879) * Rename PushRosNamespace to PushROSNamespace * Fix min_points checking * . * fixes * add to collision detector * fix * fix * . * fixes * add namespace to topic * fixes * fix use after free (#3910) * fix build mppi (#3927) Signed-off-by: kevin <[email protected]> Co-authored-by: kevin <[email protected]> * Removing old TODOs * protect invalid max_velocity min_velocity (#3953) Co-authored-by: Guillaume Doisy <[email protected]> * protect properly max_accel and max_decel (#3952) Co-authored-by: Guillaume Doisy <[email protected]> * Fixed links for install and build in README (#3963) Currently the readme is linking to an invalida page in the docs (404 error). * adding support for rotate in place cusps (#3934) * Fix linting error (#3969) * Fix linting error * Update regulated_pure_pursuit_controller.cpp * fix a few outdated comments in smac planners (#3978) * adding soft realtime prioritization for collision monitor and velocity smoother (#3979) * adding soft realtime prioritization for collision monitor and velocity smoother * refactor simple action server to use new utils API * Update README.md * Synchronize map size information during map initialization (#4015) * Update costmap size configuration This commit updates the costmap_2d.cpp file to fix a bug where the costmap size wasn't appropriately updated. Two new lines of code have been added to ensure the size of the costmap is correctly configured each time it's instantiated. * Refactor costmap size assignment in Costmap2D class The code refactor eliminates the direct mutation of the size_x_ and size_y_ attributes in the Costmap2D class. Instead, the class uses the size of cells provided during initialization and calculation from map coordinates for better encapsulation and clarity. * check width&height params (#4017) Co-authored-by: GoesM <[email protected]> * Fix SimpleActionServer nullprt callback (#4025) * add check before calling completion_callback_ * Fix lint * footprint checks (#4030) * footprint checks Signed-off-by: gg <[email protected]> * lint fix Signed-off-by: gg <[email protected]> --------- Signed-off-by: gg <[email protected]> * Is path valid doc (#4032) * docs Signed-off-by: gg <[email protected]> * update Signed-off-by: gg <[email protected]> --------- Signed-off-by: gg <[email protected]> * Update vision_opencv's branch for rolling Signed-off-by: Steve Macenski <[email protected]> * handle dynamically changes in parameters. (#4046) Signed-off-by: Sebastian Solarte <[email protected]> * Add inflation_layer_name param (#4047) Signed-off-by: Renan Salles <[email protected]> * missing urdf dep (#4050) Signed-off-by: Guillaume Doisy <[email protected]> Co-authored-by: Guillaume Doisy <[email protected]> * bump to 1.2.6 for release --------- Signed-off-by: CihatAltiparmak <[email protected]> Signed-off-by: Fabian König <[email protected]> Signed-off-by: Anton Kesy <[email protected]> Signed-off-by: kevin <[email protected]> Signed-off-by: gg <[email protected]> Signed-off-by: Steve Macenski <[email protected]> Signed-off-by: Sebastian Solarte <[email protected]> Signed-off-by: Renan Salles <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]> Co-authored-by: Tony Najjar <[email protected]> Co-authored-by: thandal <[email protected]> Co-authored-by: Anton Kesy <[email protected]> Co-authored-by: CihatAltiparmak <[email protected]> Co-authored-by: Anil Kumar Chavali <[email protected]> Co-authored-by: Plaqueoff <[email protected]> Co-authored-by: Pekka Myller <[email protected]> Co-authored-by: Fabian König <[email protected]> Co-authored-by: 정찬희 <[email protected]> Co-authored-by: kevin <[email protected]> Co-authored-by: Guillaume Doisy <[email protected]> Co-authored-by: Guillaume Doisy <[email protected]> Co-authored-by: Abiel Fernandez <[email protected]> Co-authored-by: Michael Ferguson <[email protected]> Co-authored-by: Hao-Li-Bachelorarbeit <[email protected]> Co-authored-by: GoesM <[email protected]> Co-authored-by: GoesM <[email protected]> Co-authored-by: BriceRenaudeau <[email protected]> Co-authored-by: Joshua Wallace <[email protected]> Co-authored-by: Sebastian Solarte <[email protected]> Co-authored-by: Renan Salles <[email protected]>
enricosutera
pushed a commit
to enricosutera/navigation2
that referenced
this pull request
May 19, 2024
Signed-off-by: enricosutera <[email protected]>
Marc-Morcos
pushed a commit
to Marc-Morcos/navigation2
that referenced
this pull request
Jul 4, 2024
Marc-Morcos
pushed a commit
to Marc-Morcos/navigation2
that referenced
this pull request
Jul 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@Aposhian @vinnnyr @doisyg I could use a sanity check here as I'm proposing a solution to #3089.
Previously, cusps that involve in-place rotations from the state lattice planner are ignored. This is an attempted solution which I don't think has any negative side effects, but its subtle and worth expanding upon for visibility and understandability.
Previously, we check the dot project is negative, meaning that we have a discontinuity in the path that is a direction change. Since in place rotations have the same position, the dot product is
0
which isn't triggered. That's a problem.So, I adjusted a new check if we're
0
and that the orientations of the poses are different -- that way if we simply have 2 overlapping exactly same points, this won't trigger (only true rotate in place represntations). Rather than parameterizing it, this always triggers with the assumption that if you're providing a feasible path with an in-place rotation, your robot can handle it since you gave the planner a motion model that can do it. Also in the case that the cusp direction changes (not just a 'going forward -> rotate -> going forward' still), you still want to identify the cusp. Its now that 'going forward -> rotate -> going forward' will also now trigger.So now we've identified the cusp with rotate in place & general rotate in places. Cool. That'll impact the lookahead point selection as any cusp would that we know works, so no need to beat that analysis to death, other than to mention that the
min_by
finds the closes path point to the robot to prune all path points prior to.To support this, I needed to change the comparison so that we find the last, closest point, instead of the first, closest point when there are N points the same distance away (e.g. rotate in place, same position). That way, we prune to the final path-pose in the rotation sequence, not the first.
In the ticket, we discussed adding a
shouldRotateAlongPath
, but actually I think that's not necessary. TheshouldRotateToPath
should actually implicitly do what we need to do. If rotate-to-path is valid and the error is sufficiently large, we do it. I think that covers our rotate to path heading needs for inplace rotations. When that rotation is very small - - then we don't really functionally need to do it here and it'll take care of itself in the immediate path tracking request curvature velocities. The sameuse_rotate_to_heading
param applies.So, I think this fully resolves the issue - with relatively minor changes. What do you think?