-
Notifications
You must be signed in to change notification settings - Fork 684
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
fix(route_handler): route handler overlap removal is too conservative #7156
fix(route_handler): route handler overlap removal is too conservative #7156
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7156 +/- ##
==========================================
- Coverage 15.10% 14.65% -0.45%
==========================================
Files 1922 2049 +127
Lines 134796 145844 +11048
Branches 43473 46635 +3162
==========================================
+ Hits 20364 21379 +1015
- Misses 91780 101076 +9296
- Partials 22652 23389 +737
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
…sest lanelet Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
…ction Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
…rlap Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
752f27b
to
14553d1
Compare
…-is-too-conservative Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
…-is-too-conservative
…-is-too-conservative
…-is-too-conservative
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and it works well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
…#7156) * add flag to enable/disable loop check in getLaneletSequence functions Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * implement function to get closest route lanelet based on previous closest lanelet Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * refactor DefaultPlanner::plan function Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * modify loop check logic in getLaneletSequenceUpTo function Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * improve logic in isEgoOutOfRoute function Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * fix format Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * check if prev lanelet is a goal lanelet in getLaneletSequenceUpTo function Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * separate function to update current route lanelet in planner manager Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * rename function and add docstring Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * modify functions extendNextLane and extendPrevLane to account for overlap Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * refactor function getClosestRouteLaneletFromLanelet Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * add route handler unit tests for overlapping route case Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * fix function getClosestRouteLaneletFromLanelet Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * format fix Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> * move test map to autoware_test_utils Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp> --------- Signed-off-by: mohammad alqudah <alqudah.mohammad@tier4.jp>
Description
Currently, overlap handling is not great, in some corner cases, a broken route might be generated, like the one shown below.
This is happening because, in the function getLaneletSequenceUpTo, if at least one of the previous lanelets is same as the inut lanelet, then it is considered a loop and search will stop.
Another issue is the logic used to get the closest lanelet within route does not respect the history of the lanelets traversed, it will just return the closest route lanelet satisfying the distance and yaw constraints, so the closest lanelet can jump suddenly in case of partially overlapping lanelets causing a break in the route, as shown below.
Screencast_2024-05-29_14-18-00.mp4
Changes
(route_handler)
Modified the loop check logic in function getLaneletSequenceUpTo():
Implemented new function getClosestRouteLaneletFromCurrent() to get closest route lanelet with respect the previous computed closest route lanelets to insure continuity.
(planner_manager)
Implemented separate function updateCurrentRouteLanelet() and moved logic out of getReferencePathFunction()
Use new function getClosestRouteLaneletFromCurrent() in updateCurrentRouteLanelet()
(behavior_path_planner_common/utils)
Modified function isEgoOutOfRoute() to take closestRoadLanelet as input from planner manager rather than recomputing same logic locally
(mission_planner)
Refactored DefaultPlanner::plan() function, should not affect logic.
Tests performed
Effects on system behavior
Can generate proper route:

Closest route lanelet doesn't jump suddenly causing unexpected behavior:
Screencast_2024-05-29_12-01-13.mp4
Interface changes
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.