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

fix(lane_change): use current lane for num to preferred lane input #2615

Merged

Conversation

zulfaqar-azmi-t4
Copy link
Contributor

@zulfaqar-azmi-t4 zulfaqar-azmi-t4 commented Jan 6, 2023

Description

This PR aims to fix triple lane change issue. To standardize some computations, the current available functions are also refactored.

Before PR

cap-.2023-01-06-13-01-02.mp4.mp4

After PR

temp-branch-check-.2023-01-06-12-04-37.mp4.mp4

Related links

Tests performed

Perform triple lane changes scenario .

Notes for reviewers

The following parameters in lane_change.param.yaml need to be modified as follows

backward_length_buffer_for_end_of_lane > lane_change_finish_judge_buffer + 1
backward_length_buffer_for_end_of_lane: 3.0
lane_change_finish_judge_buffer: 2.0      # [m]

Rationale

For lane change change more than 1, because lane changing module have to be complete in order for the second lane change to start. And lane change completion depends on finish judge buffer.

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.

  • The PR follows the pull request guidelines.
  • The PR has been properly tested.
  • The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.
  • The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

@github-actions github-actions bot added the component:planning Route planning, decision-making, and navigation. (auto-assigned) label Jan 6, 2023
@zulfaqar-azmi-t4 zulfaqar-azmi-t4 changed the title Refactor lane change fix(lane_change): use current lane for num to preferred lane input Jan 6, 2023
@zulfaqar-azmi-t4 zulfaqar-azmi-t4 self-assigned this Jan 6, 2023
@zulfaqar-azmi-t4 zulfaqar-azmi-t4 marked this pull request as ready for review January 6, 2023 04:17
@codecov
Copy link

codecov bot commented Jan 6, 2023

Codecov Report

Base: 11.48% // Head: 11.42% // Decreases project coverage by -0.05% ⚠️

Coverage data is based on head (935f93c) compared to base (7247041).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2615      +/-   ##
==========================================
- Coverage   11.48%   11.42%   -0.06%     
==========================================
  Files        1271     1272       +1     
  Lines       88503    88909     +406     
  Branches    23320    23625     +305     
==========================================
+ Hits        10161    10162       +1     
- Misses      67572    67951     +379     
- Partials    10770    10796      +26     
Flag Coverage Δ *Carryforward flag
differential 5.13% <0.00%> (?)
total 11.48% <0.00%> (ø) Carriedforward from 7247041

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
...ene_module/lane_change/lane_change_module_data.hpp 0.00% <0.00%> (ø)
...or_path_planner/src/behavior_path_planner_node.cpp 0.26% <0.00%> (+0.08%) ⬆️
...path_planner/src/scene_module/lane_change/util.cpp 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@TakaHoribe
Copy link
Contributor

@zulfaqar-azmi-t4 I suggest having a check if these parameters meet the condition. If not, it'd be better to modify the parameter automatically with some warnings.

The following parameters in lane_change.param.yaml need to be modified as follows

backward_length_buffer_for_end_of_lane > lane_change_finish_judge_buffer + 1

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>
Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>
Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>
Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>
Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>
@zulfaqar-azmi-t4
Copy link
Contributor Author

@TakaHoribe
Thank you very much for the review.
The changes is reflected in 935f93c

Copy link
Contributor

@TakaHoribe TakaHoribe left a comment

Choose a reason for hiding this comment

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

All comments are addressed. LGTM.

@zulfaqar-azmi-t4 zulfaqar-azmi-t4 enabled auto-merge (squash) January 11, 2023 05:59
@zulfaqar-azmi-t4 zulfaqar-azmi-t4 merged commit d94d532 into autowarefoundation:main Jan 11, 2023
@zulfaqar-azmi-t4 zulfaqar-azmi-t4 deleted the refactor-lane-change branch January 11, 2023 06:31
tkimura4 pushed a commit to tier4/autoware.universe that referenced this pull request Jan 24, 2023
…utowarefoundation#2615)

* fix(lane_change): use current lane for num to preferred lane input

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* fix lane change distance from deadend

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Make separate function to compute resampling interval

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Change default config

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Make phase info data structure

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Added error for finish judge buffer

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Fix rebase

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* ci(pre-commit): autofix

* warn user of the modified values

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
tkimura4 added a commit to tier4/autoware.universe that referenced this pull request Jan 25, 2023
* fix(behavior_path_planner): lane change check default parameters (autowarefoundation#2575)

Signed-off-by: Muhammad Zulfaqar <[email protected]>

Signed-off-by: Muhammad Zulfaqar <[email protected]>

* feat(lane_change): update path generation logic to consider lateral jerk and lateral acceleration (autowarefoundation#2428)

* [lane_change] update path generation to handle lateral acceleration limit

Signed-off-by: Takamasa Horibe <[email protected]>

* remove unused code

Signed-off-by: Takamasa Horibe <[email protected]>

* remove unused code & fix precommit

Signed-off-by: Takamasa Horibe <[email protected]>

* update doc

Signed-off-by: Takamasa Horibe <[email protected]>

* update docs

Signed-off-by: Takamasa Horibe <[email protected]>

* update doc

Signed-off-by: Takamasa Horibe <[email protected]>

* update doc

Signed-off-by: Takamasa Horibe <[email protected]>

* update doc

Signed-off-by: Takamasa Horibe <[email protected]>

* move path_shifter implementation to cpp

Signed-off-by: Takamasa Horibe <[email protected]>

* Update planning/behavior_path_planner/src/scene_module/utils/path_shifter.cpp

Co-authored-by: Zulfaqar Azmi <[email protected]>

* Update planning/behavior_path_planner/behavior_path_planner_path_generation.md

Co-authored-by: Zulfaqar Azmi <[email protected]>

* update doc link

Signed-off-by: Takamasa Horibe <[email protected]>

* update doc

Signed-off-by: Takamasa Horibe <[email protected]>

* remove unused code

Signed-off-by: Takamasa Horibe <[email protected]>

* add common min distance computation

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* rearrange config and rework min distance

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* revert some changes

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* remove warning

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Update planning/behavior_path_planner/behavior_path_planner_path_generation.md

Co-authored-by: Fumiya Watanabe <[email protected]>

* Update planning/behavior_path_planner/behavior_path_planner_path_generation.md

Co-authored-by: Fumiya Watanabe <[email protected]>

* fix spell check

Co-authored-by: Fumiya Watanabe <[email protected]>
Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>
Co-authored-by: Zulfaqar Azmi <[email protected]>
Co-authored-by: Muhammad Zulfaqar Azmi <[email protected]>
Co-authored-by: Fumiya Watanabe <[email protected]>

* feat(behavior_path_planner): abort lane change function (autowarefoundation#2359)

* feat(behavior_path_planner): abort lane change function

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* change Revert -> Cancel

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Remove some unwanted functions and and STOP state

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* update steering factor (accidentally removed)

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* include is_abort_condition_satisfied_ flag

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* use only check ego in current lane

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Revert "use only check ego in current lane"

This reverts commit 4f97408.

* ci(pre-commit): autofix

* use only check ego in current lane

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* improve isAbortConditionSatisfied by using ego polygon check

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* add lateral jerk and path doesn't keep on updating anymore

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* parameterized all abort related values

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* rename abort_end -> abort_return

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* fix some parameter issue

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* check if lane change distance is enough after abort

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* improve the code flow of isAbortConditionSatisfied

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Place warning message in corresponding states.

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* fix clock and rebase

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* remove accel and jerk parameters

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* remove unnecessary parameters

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* fix param file in config

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Update planning/behavior_path_planner/src/scene_module/lane_change/util.cpp

Co-authored-by: Takamasa Horibe <[email protected]>

* remove isStopState and refactoring

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Fixed CANCEL when ego is out of lane

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* fix path reset during abort

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* fix abort path exceed goal

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* fix logger to debug

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Takamasa Horibe <[email protected]>
Signed-off-by: tomoya.kimura <[email protected]>

* feat(behavior path planner): lane change cancel/abort docs update (autowarefoundation#2599)

* feat(behavior path planner): lane change cancel/abort docs update

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Update parameters and it's config (yaml) file

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* fix(lane_change): use current lane for num to preferred lane input (autowarefoundation#2615)

* fix(lane_change): use current lane for num to preferred lane input

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* fix lane change distance from deadend

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Make separate function to compute resampling interval

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Change default config

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Make phase info data structure

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Added error for finish judge buffer

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Fix rebase

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* ci(pre-commit): autofix

* warn user of the modified values

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(behavior_path_planner): lane change turn signal during approval (autowarefoundation#2645)

* fix(behavior_path_planner): lane change turn signal during approval

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* some refactoring

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* fix(behavior_path_planner): improve isPathInLanelet function for lane change (autowarefoundation#2693)

* fix(behavior_path_planner): improve isPathInLanelet function for lane change

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* simplify the functions

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* feat(rtc_auto_mode_manager)!: add auto mode status array (autowarefoundation#2517)

* feat(rtc_aut_mode_manager)!: add auto mode status array

Signed-off-by: taikitanaka3 <[email protected]>

* chore:  planning/rtc_auto_mode_manager/src/rtc_auto_mode_manager_interface.cpp

Signed-off-by: taikitanaka3 <[email protected]>

* feat(behavior_path_planner): external request lane change (autowarefoundation#2442)

* feature(behavior_path_planner): add external request lane change module

Signed-off-by: Fumiya Watanabe <[email protected]>

feature(behavior_path_planner): fix for RTC

Signed-off-by: Fumiya Watanabe <[email protected]>

feature(behavior_path_planner): fix decision logic

Signed-off-by: Fumiya Watanabe <[email protected]>

feat(behavior_path_planner): fix behavior_path_planner_tree.xml

Signed-off-by: Fumiya Watanabe <[email protected]>

feat(behavior_path_planner): fix for rebase

Signed-off-by: Fumiya Watanabe <[email protected]>

feat(behavior_path_planner): output multiple candidate paths

Signed-off-by: Fumiya Watanabe <[email protected]>

feat(behavior_path_planner): get path candidate in behavior tree manager

Signed-off-by: Fumiya Watanabe <[email protected]>

feat(behavior_path_planner): fix for multiple candidate path

Signed-off-by: Fumiya Watanabe <[email protected]>

feat(behavior_path_planner): separate external request lane change module

Signed-off-by: Fumiya Watanabe <[email protected]>

feature(behavior_path_planner): add create publisher method

Signed-off-by: Fumiya Watanabe <[email protected]>

feature(behavior_path_planner): move publishers to node

Signed-off-by: Fumiya Watanabe <[email protected]>

feature(behavior_path_planner): remove unnecessary publisher

Signed-off-by: Fumiya Watanabe <[email protected]>

feat(behavior_path_planner): move reset path candidate function to behavior tree manager

Signed-off-by: Fumiya Watanabe <[email protected]>

feat(behavior_path_planner): add external request lane change path candidate publisher

Signed-off-by: Fumiya Watanabe <[email protected]>

feat(behavior_path_planner): apply abort lane change

Signed-off-by: Fumiya Watanabe <[email protected]>

* fix(behavior_path_planner): remove unnecessary change

Signed-off-by: Fumiya Watanabe <[email protected]>

* feat(behavior_path_planner): fix getLaneChangePaths()

Signed-off-by: Fumiya Watanabe <[email protected]>

* feat(behavior_path_planner): disable external request lane change in default tree

Signed-off-by: Fumiya Watanabe <[email protected]>

* Update rtc_auto_mode_manager.param.yaml

* fix(route_handler): remove redundant code

* fix(behavior_path_planner): fix for turn signal

Signed-off-by: Fumiya Watanabe <[email protected]>

Signed-off-by: Fumiya Watanabe <[email protected]>
Signed-off-by: tomoya.kimura <[email protected]>

* fix(motion_utils/path_shifter): modify the sampling method (autowarefoundation#2658)

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* fix(behavior_path_planner): lane change interpolated obj orientation (autowarefoundation#2727)

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* fix(behavior_path_planner): reduce obj indices call in lane change (autowarefoundation#2726)

* fix(behavior_path_planner): reduce obj indices call in lane change

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* remove unused functions

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* add continue for the intersect in current

* replace target and current lanes

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* change isLaneChangePathSafe arguments

Signed-off-by: Muhammad Zulfaqar <[email protected]>

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>
Signed-off-by: Muhammad Zulfaqar <[email protected]>

* feat(behavior_path_planner): enable lane change in intersection (autowarefoundation#2720)

fix(behavior_path_planner): enable lane change in intersection

Signed-off-by: Fumiya Watanabe <[email protected]>

Signed-off-by: Fumiya Watanabe <[email protected]>

* fix(behavior_path_planner): make lane change safety check adaptive (autowarefoundation#2704)

* fix(behavior_path_planner): make lane change safety check adaptive

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Temporarily hard code use all predicted path

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

* Revert "Temporarily hard code use all predicted path"

This reverts commit 8f92e45.

* fix external lane change request

Signed-off-by: Muhammad Zulfaqar <[email protected]>

* use prediction resolution as rounding multiplier

Signed-off-by: Muhammad Zulfaqar <[email protected]>

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>
Signed-off-by: Muhammad Zulfaqar <[email protected]>

* fix(lane_change): chattering issue when performing check (autowarefoundation#2741)

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>

Signed-off-by: Muhammad Zulfaqar <[email protected]>
Signed-off-by: Takamasa Horibe <[email protected]>
Signed-off-by: Muhammad Zulfaqar Azmi <[email protected]>
Signed-off-by: tomoya.kimura <[email protected]>
Signed-off-by: taikitanaka3 <[email protected]>
Signed-off-by: Fumiya Watanabe <[email protected]>
Co-authored-by: Zulfaqar Azmi <[email protected]>
Co-authored-by: Takamasa Horibe <[email protected]>
Co-authored-by: Muhammad Zulfaqar Azmi <[email protected]>
Co-authored-by: Fumiya Watanabe <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: taikitanaka3 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:planning Route planning, decision-making, and navigation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants