feat(lane_change): reduce prepare duration when blinker has been activated #9185

mkquda · 2024-10-30T05:28:00Z

Description

Currently, a constant prepare duration (parameterized) is being used when generating LC path candidates, this was necessary because until recently turn signal was activated only after LC path was approved.

However, LC turn signal logic was recently changed to activate as soon as we have the intention to change lanes (i.e: when LC module is run). Therefore it is no longer necessary to have a constant long prepare duration, and we can reduce the prepare duration as the turn signal on duration increases.

Changes

Add member variable signal_activation_time_ to LaneChangeBase class
Add member variable lane_change_prepare_duration to TransientData struct
Add function calc_actual_prepare_duration to calculate actual needed prepare duration based on current velocity and signal on duration
Modify function update_transient_data to set lane_change_prepare_duration

How was this PR tested?

PSIM
TIER IV Internal Evaluation (pending)

Notes for reviewers

None.

Interface changes

None.

ROS Parameter Changes

Additions and removals

Change type	Parameter Name	Type	Default Value	Description
Added	`min_prepare_duration`	`double`	`1.0`	Minimum preparation time for the ego vehicle to be ready to perform lane change

Modifications

Version	Parameter Name	Type	Default Value	Description
Old	`prepare_duration`	`double`	`4.0`	Preparation time for the ego vehicle to be ready to perform lane change
New	`max_prepare_duration`	`double`	`4.0`	Maximum preparation time for the ego vehicle to be ready to perform lane change

Effects on system behavior

When LC module activates, initially prepare duration will be max, as signal activation duration increases, prepare duration will be gradually reduced to minimum, allowing a shorter LC path to be found.

Signed-off-by: mohammad alqudah <[email protected]>

…blinker-has-been-activated

github-actions · 2024-10-30T05:28:21Z

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

You've checked our contribution guidelines.
Your PR follows our pull request guidelines.
All required CI checks pass before marking the PR ready for review.

…blinker-has-been-activated

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions <[email protected]>

…owarefoundation#9173) * refactor(time_utils): prefix package and namespace with autoware Signed-off-by: Esteve Fernandez <[email protected]> * refactor(time_utils): prefix package and namespace with autoware Signed-off-by: Esteve Fernandez <[email protected]> * style(pre-commit): autofix --------- Signed-off-by: Esteve Fernandez <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* add requested feature Signed-off-by: Go Sakayori <[email protected]> * Update planning/autoware_rtc_interface/test/test_rtc_interface.cpp Co-authored-by: Satoshi OTA <[email protected]> --------- Signed-off-by: Go Sakayori <[email protected]> Co-authored-by: Satoshi OTA <[email protected]>

…ws (autowarefoundation#9199) Signed-off-by: Maxime CLEMENT <[email protected]>

fix(bpp): calcDistanceToRedTrafficLight null Signed-off-by: Shumpei Wakabayashi <[email protected]>

… predictor (autowarefoundation#9201) * refactor: grouping functions Signed-off-by: Taekjin LEE <[email protected]> * refactor: grouping parameters Signed-off-by: Taekjin LEE <[email protected]> * refactor: rename member road_users_history to road_users_history_ Signed-off-by: Taekjin LEE <[email protected]> * refactor: separate util functions Signed-off-by: Taekjin LEE <[email protected]> * refactor: Add predictor_vru.cpp and utils.cpp to map_based_prediction_node Signed-off-by: Taekjin LEE <[email protected]> * refactor: Add explicit template instantiation for removeOldObjectsHistory function Signed-off-by: Taekjin LEE <[email protected]> * refactor: Add tf2_geometry_msgs to data_structure Signed-off-by: Taekjin LEE <[email protected]> * refactor: Remove unused variables and functions in map_based_prediction_node.cpp Signed-off-by: Taekjin LEE <[email protected]> * Update perception/autoware_map_based_prediction/include/map_based_prediction/predictor_vru.hpp * Apply suggestions from code review * style(pre-commit): autofix --------- Signed-off-by: Taekjin LEE <[email protected]> Co-authored-by: Mamoru Sobue <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

…er (autowarefoundation#8912) * Moved ndt_omp into ndt_scan_matcher Signed-off-by: Shintaro Sakoda <[email protected]> * Added Copyright Signed-off-by: Shintaro Sakoda <[email protected]> * style(pre-commit): autofix * Fixed include Signed-off-by: Shintaro Sakoda <[email protected]> * Fixed cast style Signed-off-by: Shintaro Sakoda <[email protected]> * Fixed include Signed-off-by: Shintaro Sakoda <[email protected]> * Fixed honorific title Signed-off-by: Shintaro Sakoda <[email protected]> * Fixed honorific title Signed-off-by: Shintaro Sakoda <[email protected]> * style(pre-commit): autofix * Fixed include hierarchy Signed-off-by: Shintaro Sakoda <[email protected]> * style(pre-commit): autofix * Fixed include hierarchy Signed-off-by: Shintaro Sakoda <[email protected]> * style(pre-commit): autofix * Fixed hierarchy Signed-off-by: Shintaro Sakoda <[email protected]> * Fixed NVTP to NVTL Signed-off-by: Shintaro Sakoda <[email protected]> * Added cspell:ignore Signed-off-by: Shintaro Sakoda <[email protected]> * Fixed miss spell Signed-off-by: Shintaro Sakoda <[email protected]> * style(pre-commit): autofix * Fixed include Signed-off-by: Shintaro Sakoda <[email protected]> * Renamed applyFilter Signed-off-by: Shintaro Sakoda <[email protected]> * Moved ***_impl.hpp from include/ to src/ Signed-off-by: Shintaro Sakoda <[email protected]> * style(pre-commit): autofix * Fixed variable scope Signed-off-by: Shintaro Sakoda <[email protected]> * Fixed to pass by reference Signed-off-by: Shintaro Sakoda <[email protected]> --------- Signed-off-by: Shintaro Sakoda <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

…ndation#9177) Signed-off-by: Mamoru Sobue <[email protected]>

…blinker-has-been-activated

…transient data Signed-off-by: mohammad alqudah <[email protected]>

…blinker-has-been-activated

…pproved path prepare duration Signed-off-by: mohammad alqudah <[email protected]>

Signed-off-by: mohammad alqudah <[email protected]>

codecov · 2024-11-06T05:20:03Z

Codecov Report

Attention: Patch coverage is 51.92308% with 25 lines in your changes missing coverage. Please review.

Project coverage is 29.22%. Comparing base (391beda) to head (ed8cf38).

Files with missing lines	Patch %	Lines
...are_behavior_path_lane_change_module/src/scene.cpp	28.00%	18 Missing ⚠️
..._path_lane_change_module/src/utils/calculation.cpp	75.00%	3 Missing and 1 partial ⚠️
...behavior_path_lane_change_module/src/interface.cpp	33.33%	2 Missing ⚠️
...havior_path_lane_change_module/src/utils/utils.cpp	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9185      +/-   ##
==========================================
- Coverage   29.29%   29.22%   -0.08%     
==========================================
  Files        1334     1335       +1     
  Lines      102798   103668     +870     
  Branches    39985    40404     +419     
==========================================
+ Hits        30119    30297     +178     
- Misses      69811    70457     +646     
- Partials     2868     2914      +46

Flag	Coverage Δ		*Carryforward flag
differential	`20.10% <52.38%> (?)`
total	`29.20% <52.50%> (-0.10%)`	⬇️	Carriedforward from 809ccf9

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

maxime-clem

The code changes look good, but there are degradation in the evaluation.
The commit history appears to contain some extra commits but I am not sure if this related.

maxime-clem · 2024-11-06T12:03:40Z

...h_lane_change_module/include/autoware/behavior_path_lane_change_module/utils/calculation.hpp

@@ -130,6 +130,10 @@ std::vector<double> calc_lon_acceleration_samples(
  const CommonDataPtr & common_data_ptr, const double max_path_velocity,
  const double prepare_duration);

+double calc_actual_prepare_duration(


It would be nice to add some docstring to explain the function.

…blinker-has-been-activated

zulfaqar-azmi-t4 · 2024-11-08T09:48:47Z

planning/behavior_path_planner/autoware_behavior_path_lane_change_module/src/scene.cpp

+  if (turn_signal_info.turn_signal.command != terminal_turn_signal_info.turn_signal.command) {
+    signal_activation_time_ = std::nullopt;
+  } else if (!signal_activation_time_) {
+    signal_activation_time_ = clock_.now();
+  }


This is repeated in line 436, maybe wrapping it in a function would be cleaner?

zulfaqar-azmi-t4 · 2024-11-08T09:50:14Z

...ng/behavior_path_planner/autoware_behavior_path_lane_change_module/src/utils/calculation.cpp

+    if (current_velocity >= min_lc_velocity) {
+      return common_data_ptr->lc_param_ptr->minimum_prepare_duration;
+    }
+    const auto max_acc = common_data_ptr->bpp_param_ptr->max_acc;


we have max acc from bpp param and max acc from lane change param.
Just curious why only max acc from bpp param is considered.

zulfaqar-azmi-t4 · 2024-11-08T09:51:25Z

...ng/behavior_path_planner/autoware_behavior_path_lane_change_module/src/utils/calculation.cpp

+      return common_data_ptr->lc_param_ptr->minimum_prepare_duration;
+    }
+    const auto max_acc = common_data_ptr->bpp_param_ptr->max_acc;
+    return (min_lc_velocity - current_velocity) / max_acc;


there mighttt be possibility max_acc = 0.0? should we place a guard or something?

zulfaqar-azmi-t4 · 2024-11-08T09:51:57Z

planning/behavior_path_planner/autoware_behavior_path_lane_change_module/src/utils/utils.cpp

+  const auto & time_resolution = lc_param_ptr->prediction_time_resolution;
+  const auto & velocity_threshold = lc_param_ptr->stopped_object_velocity_threshold;
+  const auto & prepare_duration = common_data_ptr->transient_data.lane_change_prepare_duration;


Suggested change

const auto & time_resolution = lc_param_ptr->prediction_time_resolution;

const auto & velocity_threshold = lc_param_ptr->stopped_object_velocity_threshold;

const auto & prepare_duration = common_data_ptr->transient_data.lane_change_prepare_duration;

const auto time_resolution = lc_param_ptr->prediction_time_resolution;

const auto velocity_threshold = lc_param_ptr->stopped_object_velocity_threshold;

const auto prepare_duration = common_data_ptr->transient_data.lane_change_prepare_duration;

…blinker-has-been-activated

…-prepare-duration-when-blinker-has-been-activated

mkquda added 4 commits October 25, 2024 16:35

add minimum prepare duration parameter

e7b994e

Signed-off-by: mohammad alqudah <[email protected]>

reduce prepare duration according to signal activation time

5da415d

Signed-off-by: mohammad alqudah <[email protected]>

Merge branch 'awf-latest' into RT1-8204-reduce-prepare-duration-when-…

19d7caa

…blinker-has-been-activated

Merge branch 'awf-latest' into RT1-8204-reduce-prepare-duration-when-…

ed271cb

…blinker-has-been-activated

github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) labels Oct 30, 2024

mkquda and others added 10 commits October 31, 2024 09:46

Merge branch 'awf-latest' into RT1-8204-reduce-prepare-duration-when-…

dd0cba0

…blinker-has-been-activated

chore: update CODEOWNERS (autowarefoundation#9203)

c12034a

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions <[email protected]>

fix(mpc_lateral_controller): correctly resample the MPC trajectory ya…

0ed5613

…ws (autowarefoundation#9199) Signed-off-by: Maxime CLEMENT <[email protected]>

fix(bpp): prevent accessing nullopt (autowarefoundation#9204)

092f5ae

fix(bpp): calcDistanceToRedTrafficLight null Signed-off-by: Shumpei Wakabayashi <[email protected]>

feat(autoware_test_utils): add traffic light msgs parser (autowarefou…

9d4b5b4

…ndation#9177) Signed-off-by: Mamoru Sobue <[email protected]>

Merge branch 'awf-latest' into RT1-8204-reduce-prepare-duration-when-…

79009b6

…blinker-has-been-activated

modify implementation to compute and keep actual prepare duration in …

0277683

…transient data Signed-off-by: mohammad alqudah <[email protected]>

mkquda added 4 commits November 5, 2024 08:01

Merge branch 'awf-latest' into RT1-8204-reduce-prepare-duration-when-…

c19c534

…blinker-has-been-activated

Merge branch 'awf-latest' into RT1-8204-reduce-prepare-duration-when-…

b9afb08

…blinker-has-been-activated

if LC path is approved, set prepare duration in transient data from a…

2853e4c

…pproved path prepare duration Signed-off-by: mohammad alqudah <[email protected]>

change default value of LC param min_prepare_duration

e0ffba8

Signed-off-by: mohammad alqudah <[email protected]>

mkquda marked this pull request as ready for review November 6, 2024 04:56

mkquda requested review from rej55, kosuke55, maxime-clem, satoshi-ota, shmpwk, TomohitoAndo, tkimura4 and zulfaqar-azmi-t4 as code owners November 6, 2024 04:56

mkquda assigned maxime-clem and zulfaqar-azmi-t4 Nov 6, 2024

mkquda added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Nov 6, 2024

maxime-clem reviewed Nov 6, 2024

View reviewed changes

Merge branch 'awf-latest' into RT1-8204-reduce-prepare-duration-when-…

bcc0e8e

…blinker-has-been-activated

zulfaqar-azmi-t4 reviewed Nov 8, 2024

View reviewed changes

Merge branch 'awf-latest' into RT1-8204-reduce-prepare-duration-when-…

d23249a

…blinker-has-been-activated

mkquda mentioned this pull request Nov 13, 2024

feat(lane_change): add new lane change parameter autowarefoundation/autoware_launch#1224

Open

4 tasks

Merge remote-tracking branch 'origin/awf-latest' into RT1-8204-reduce…

ed8cf38

…-prepare-duration-when-blinker-has-been-activated

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(lane_change): reduce prepare duration when blinker has been activated #9185

feat(lane_change): reduce prepare duration when blinker has been activated #9185

mkquda commented Oct 30, 2024 •

edited

Loading

github-actions bot commented Oct 30, 2024 •

edited

Loading

codecov bot commented Nov 6, 2024 •

edited

Loading

maxime-clem left a comment

maxime-clem Nov 6, 2024

zulfaqar-azmi-t4 Nov 8, 2024

zulfaqar-azmi-t4 Nov 8, 2024

zulfaqar-azmi-t4 Nov 8, 2024

zulfaqar-azmi-t4 Nov 8, 2024

feat(lane_change): reduce prepare duration when blinker has been activated #9185

Are you sure you want to change the base?

feat(lane_change): reduce prepare duration when blinker has been activated #9185

Conversation

mkquda commented Oct 30, 2024 • edited Loading

Description

Changes

Related links

How was this PR tested?

Notes for reviewers

Interface changes

ROS Parameter Changes

Additions and removals

Modifications

Effects on system behavior

github-actions bot commented Oct 30, 2024 • edited Loading

codecov bot commented Nov 6, 2024 • edited Loading

Codecov Report

maxime-clem left a comment

Choose a reason for hiding this comment

maxime-clem Nov 6, 2024

Choose a reason for hiding this comment

zulfaqar-azmi-t4 Nov 8, 2024

Choose a reason for hiding this comment

zulfaqar-azmi-t4 Nov 8, 2024

Choose a reason for hiding this comment

zulfaqar-azmi-t4 Nov 8, 2024

Choose a reason for hiding this comment

zulfaqar-azmi-t4 Nov 8, 2024

Choose a reason for hiding this comment

mkquda commented Oct 30, 2024 •

edited

Loading

github-actions bot commented Oct 30, 2024 •

edited

Loading

codecov bot commented Nov 6, 2024 •

edited

Loading