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

Extend the dwb_local_planner with a split_path option #50

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mintar
Copy link
Contributor

@mintar mintar commented Jan 13, 2020

When split_path is set to true, upon receiving a new path from the global planner, dwb will now split the path into segments of the same movement direction (roughly forwards/backwards/in-place-rotation). The segments will be treated as if they were given by the global planner one by one. This avoids taking shortcuts during complex maneuvers.

This is especially useful when used with the SBPL global planner. For a non-holonomic robot, SBPL will plan multiple segments (driving forward, then switching driving direction to backward, sometimes turning in place). Ideally, you want each "intermediate goal" where the driving direction changes to be treated like the global goal would be: the PathAlign critic should stop trying to keep the nose close to the path, the RotateToGoal critic and the goal checker should activate and so on. Instead of modifying each critic, it's much easier to simply split the path into segments and send them to the critics one by one, which is exactly what this PR does.

The split_path option is disabled by default, so the default behavior is not changed by this PR.

Here's an example of an SBPL path where the driving direction changes (from the mir_robot Gazebo simulation):

rviz_screenshot_2020_01_13-11_03_55_2

@mintar
Copy link
Contributor Author

mintar commented Jan 13, 2020

FYI @niniemann

@mintar
Copy link
Contributor Author

mintar commented Jan 13, 2020

Note that this PR also includes the commit from #27. I've changed my mind on this; the critics really should be reset whenever the global planner replans, not when the user sends a new goal. This is important in situations where e.g. the robot is already in the goal position, but cannot turn to the final orientation because of obstacles. The new global plan might require the robot to leave the goal position again, and so the critics (like for example the RotateToGoal critic) need to be reset.

Here's a picture of such a situation (beware my mad MS Paint skillz!):
replanning

@mintar
Copy link
Contributor Author

mintar commented Feb 28, 2020

@ros-pull-request-builder retest this please

@SteveMacenski
Copy link

Its easier to retrigger by closing and reopening again (or adding a dummy commit)

@mintar
Copy link
Contributor Author

mintar commented Mar 3, 2020

Closing and reopening to retrigger tests.

@mintar mintar closed this Mar 3, 2020
@mintar mintar reopened this Mar 3, 2020
@mintar
Copy link
Contributor Author

mintar commented Mar 3, 2020

The tests were failing because of roslint style checking. I've fixed the style and now they should pass.

@DLu : Any comments on the PR?

@SteveMacenski
Copy link

SteveMacenski commented Mar 3, 2020

If you want to move to ROS2, I’d merge that into our Nav2 DWB version 😉

@DLu
Copy link
Collaborator

DLu commented Jul 2, 2020

I disagree a little bit with you on where this should be handled. If it were me, what I would do is to add this logic to the onNewGlobalPlan method within locomotor and then pass each piece to the local planner in turn.

However, I acknowledge that you may not be using locomotor and might be more constrained by the inflexible fist of move_base.

Could you start by extracting the main grouping logic and placing it in a method in nav_2d_utils/path_ops.h with parameter(s) replacing the constants?

mintar and others added 3 commits July 3, 2020 12:58
The documentation on `reset()` says:

> called at the beginning of every new navigation, i.e. when we get a
> new global plan

The `reset()` functions were called in `setGoalPose()`, i.e. when the
user gives a new goal. However, when the local planner fails and the
global planner replans, `setGoalPose()` is not called, but `setPlan()`
is. This commit makes sure that `reset()` is also called in this case.
When split_path is set to true, upon receiving a new path from the
global planner, dwb will now split the path into segments of the same
movement direction (roughly forwards/backwards/in-place-rotation). The
segments will be treated as if they were given by the global planner one
by one. This avoids taking shortcuts during complex maneuvers.
@mintar
Copy link
Contributor Author

mintar commented Jul 3, 2020

I disagree a little bit with you on where this should be handled. If it were me, what I would do is to add this logic to the onNewGlobalPlan method within locomotor and then pass each piece to the local planner in turn.

I fully agree with that; in the long run, it would probably be better to handle it in the navigation framework, this way not just dwb_local_planner but any other local planner would automatically benefit from this.

However, I acknowledge that you may not be using locomotor and might be more constrained by the inflexible fist of move_base.

Alas, that's the problem. And because I'm firmly in the grasp of that fist for now I've put it into the dwb_local_planner.

Could you start by extracting the main grouping logic and placing it in a method in nav_2d_utils/path_ops.h with parameter(s) replacing the constants?

Done! I've also added a test for this new function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants