fix(bpp): overwrite turn signal by latter module #7045

satoshi-ota · 2024-05-17T07:09:02Z

Description

Sometimes, bpp module outputs unnecessary turn signal during multiple module execution. When multiple modules are running in series, the latter module is dominant for the path followed by Ego, so if there is an overlap in the turn signal section, it is overwritten by the latter module's signal.

simplescreenrecorder-2024-06-03_10.55.45.mp4

Ticket: TIER IV INTERNAL LINK

Tests performed

PASS TIER IV INTERNAL SCENARIOS

Effects on system behavior

Fix turn signal inconsistency.

Interface changes

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

I've confirmed the contribution guidelines.
The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

The PR follows the pull request guidelines.

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.

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

danielsanchezaran · 2024-06-03T04:29:58Z

planning/behavior_path_planner_common/src/turn_signal_decider.cpp

@@ -669,14 +704,16 @@ std::pair<TurnSignalInfo, bool> TurnSignalDecider::getBehaviorTurnSignalInfo(

  // If shift length is shorter than the threshold, it does not need to turn on blinkers
  if (std::fabs(relative_shift_length) < p.turn_signal_shift_length_threshold) {
-    return std::make_pair(TurnSignalInfo{}, true);
+    return std::make_pair(


Since path.path.points.front() and path.path.points.back() are called so often to make a pose, would you consider making a variable for them?

const auto first_path_pose =getPose(path.path.points.front()) and const auto last_path_pose = getPose(path.path.points.back())

shmpwk

LGTM

zulfaqar-azmi-t4 · 2024-06-03T05:28:10Z

planning/autoware_behavior_path_static_obstacle_avoidance_module/src/scene.cpp

@@ -915,11 +917,21 @@ BehaviorModuleOutput StaticObstacleAvoidanceModule::plan()
    ignore_signal_ = is_ignore ? std::make_optional(uuid) : std::nullopt;
  };

+  const auto is_large_deviation = [this](const auto & path) {
+    constexpr double THRESHOLD = 1.0;


clang-tidy will trigger a readability warning due to the variable name being all upper case.
probably better to use lowercase to remove the warning.

fixed in ef6b54c

zulfaqar-azmi-t4 · 2024-06-03T05:30:13Z

...ng/behavior_path_planner_common/include/behavior_path_planner_common/turn_signal_decider.hpp

@@ -93,6 +104,11 @@ class TurnSignalDecider
    const TurnSignalInfo & intersection_signal_info, const TurnSignalInfo & behavior_signal_info,
    const double nearest_dist_threshold, const double nearest_yaw_threshold);

+  TurnSignalInfo overwrite_turn_signal(


the function can be const

zulfaqar-azmi-t4 · 2024-06-03T05:33:26Z

planning/behavior_path_planner_common/src/turn_signal_decider.cpp

  // If shift length is shorter than the threshold, it does not need to turn on blinkers
  if (std::fabs(relative_shift_length) < p.turn_signal_shift_length_threshold) {
-    return std::make_pair(TurnSignalInfo{}, true);
+    return std::make_pair(TurnSignalInfo(getPose(p_path_start), getPose(p_path_end)), true);


Ota-san has already called getPose from the path points and assigned them to p_path_start and p_path_end.
Is there any reason why you called getPose again on these two variables?

:tashikani:

zulfaqar-azmi-t4 · 2024-06-03T05:41:38Z

planning/behavior_path_planner_common/src/turn_signal_decider.cpp

+  const double dist_to_original_desired_end =
+    get_distance(original_desired_end_point) - base_link2front_;
+  const double dist_to_new_desired_start = get_distance(new_desired_start_point) - base_link2front_;


since both subtract base_link2front_ from the get_distance, I am curious why it is not substract from within the get_distance's lambda directly?

:tashikani:

Signed-off-by: satoshi-ota <[email protected]>

… state Signed-off-by: satoshi-ota <[email protected]>

…eation Signed-off-by: satoshi-ota <[email protected]>

Signed-off-by: satoshi-ota <[email protected]>

…7045) * fix(bpp): overwrite turn signal by latter module Signed-off-by: satoshi-ota <[email protected]> * fix(avoidance): return previous module turn signal if it's in success state Signed-off-by: satoshi-ota <[email protected]> * fix(avoidance): don't output turn signal if there is huge lateral deveation Signed-off-by: satoshi-ota <[email protected]> * refactor: small update Signed-off-by: satoshi-ota <[email protected]> * chore: use lower case Signed-off-by: satoshi-ota <[email protected]> * refactor: remove redundant function call Signed-off-by: satoshi-ota <[email protected]> --------- Signed-off-by: satoshi-ota <[email protected]>

…ndation#7045)" This reverts commit bf0e56f.

…7045) * fix(bpp): overwrite turn signal by latter module Signed-off-by: satoshi-ota <[email protected]> * fix(avoidance): return previous module turn signal if it's in success state Signed-off-by: satoshi-ota <[email protected]> * fix(avoidance): don't output turn signal if there is huge lateral deveation Signed-off-by: satoshi-ota <[email protected]> * refactor: small update Signed-off-by: satoshi-ota <[email protected]> * chore: use lower case Signed-off-by: satoshi-ota <[email protected]> * refactor: remove redundant function call Signed-off-by: satoshi-ota <[email protected]> --------- Signed-off-by: satoshi-ota <[email protected]>

* fix(bpp): overwrite turn signal by latter module Signed-off-by: satoshi-ota <[email protected]> * fix(avoidance): return previous module turn signal if it's in success state Signed-off-by: satoshi-ota <[email protected]> * fix(avoidance): don't output turn signal if there is huge lateral deveation Signed-off-by: satoshi-ota <[email protected]> * refactor: small update Signed-off-by: satoshi-ota <[email protected]> * chore: use lower case Signed-off-by: satoshi-ota <[email protected]> * refactor: remove redundant function call Signed-off-by: satoshi-ota <[email protected]> --------- Signed-off-by: satoshi-ota <[email protected]>

github-actions bot added the component:planning Route planning, decision-making, and navigation. (auto-assigned) label May 17, 2024

satoshi-ota force-pushed the fix/turn-signal-consistency branch from 6298906 to e46b708 Compare May 20, 2024 01:09

satoshi-ota force-pushed the fix/turn-signal-consistency branch from e46b708 to adb1c7f Compare June 3, 2024 01:48

github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:launch Launch files, scripts and initialization tools. (auto-assigned) component:evaluator Evaluation tools for planning, localization etc. (auto-assigned) labels Jun 3, 2024

satoshi-ota force-pushed the fix/turn-signal-consistency branch from adb1c7f to 3d57a61 Compare June 3, 2024 04:03

satoshi-ota marked this pull request as ready for review June 3, 2024 04:03

satoshi-ota requested review from danielsanchezaran, rej55, go-sakayori, kosuke55, soblin, maxime-clem, shmpwk, TakaHoribe, takayuki5168, tkimura4, zulfaqar-azmi-t4, TomohitoAndo and kyoichi-sugahara as code owners June 3, 2024 04:03

github-actions bot removed type:documentation Creating or refining documentation. (auto-assigned) component:launch Launch files, scripts and initialization tools. (auto-assigned) component:evaluator Evaluation tools for planning, localization etc. (auto-assigned) labels Jun 3, 2024

danielsanchezaran reviewed Jun 3, 2024

View reviewed changes

shmpwk approved these changes Jun 3, 2024

View reviewed changes

shmpwk added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Jun 3, 2024

zulfaqar-azmi-t4 reviewed Jun 3, 2024

View reviewed changes

satoshi-ota added run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) and removed run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) labels Jun 3, 2024

satoshi-ota added 6 commits June 5, 2024 21:14

fix(bpp): overwrite turn signal by latter module

7bb66d1

Signed-off-by: satoshi-ota <[email protected]>

fix(avoidance): return previous module turn signal if it's in success…

14d3a10

… state Signed-off-by: satoshi-ota <[email protected]>

fix(avoidance): don't output turn signal if there is huge lateral dev…

84da875

…eation Signed-off-by: satoshi-ota <[email protected]>

refactor: small update

2206306

Signed-off-by: satoshi-ota <[email protected]>

chore: use lower case

d2a8f6f

Signed-off-by: satoshi-ota <[email protected]>

refactor: remove redundant function call

2fc8362

Signed-off-by: satoshi-ota <[email protected]>

satoshi-ota force-pushed the fix/turn-signal-consistency branch from 1959d3f to 2fc8362 Compare June 5, 2024 12:14

satoshi-ota enabled auto-merge (squash) June 5, 2024 12:15

satoshi-ota merged commit 4daa6a9 into autowarefoundation:main Jun 5, 2024
20 of 22 checks passed

satoshi-ota deleted the fix/turn-signal-consistency branch June 5, 2024 14:18

shmpwk added a commit to tier4/autoware.universe that referenced this pull request Jun 11, 2024

Revert "fix(bpp): overwrite turn signal by latter module (autowarefou…

3f1ae9a

…ndation#7045)" This reverts commit bf0e56f.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(bpp): overwrite turn signal by latter module #7045

fix(bpp): overwrite turn signal by latter module #7045

satoshi-ota commented May 17, 2024 •

edited

Loading

danielsanchezaran Jun 3, 2024

shmpwk left a comment

zulfaqar-azmi-t4 Jun 3, 2024

satoshi-ota Jun 3, 2024

zulfaqar-azmi-t4 Jun 3, 2024

satoshi-ota Jun 3, 2024

zulfaqar-azmi-t4 Jun 3, 2024

satoshi-ota Jun 3, 2024

satoshi-ota Jun 3, 2024

zulfaqar-azmi-t4 Jun 3, 2024

satoshi-ota Jun 3, 2024

satoshi-ota Jun 3, 2024

fix(bpp): overwrite turn signal by latter module #7045

fix(bpp): overwrite turn signal by latter module #7045

Conversation

satoshi-ota commented May 17, 2024 • edited Loading

Description

Tests performed

Effects on system behavior

Interface changes

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

Choose a reason for hiding this comment

shmpwk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

satoshi-ota commented May 17, 2024 •

edited

Loading