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(behavior_path_planner): if ego leaves the current lane turn the signal on #4348

Conversation

beyzanurkaya
Copy link
Contributor

@beyzanurkaya beyzanurkaya commented Jul 20, 2023

Description

fixes: #4470

This PR provides when turn_signal_on_swerving param is set to false; if EGO has to leave its lane because of to avoid an obstacle it'll turn the signal on

Tests performed

Before this PR:
Screenshot from 2023-08-07 01-17-48
Screenshot from 2023-08-07 01-17-14

After this PR:

Screenshot from 2023-08-07 00-52-52

Screenshot from 2023-08-07 00-54-13

Not applicable.

Effects on system behavior

Not applicable.

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.

  • 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.

Summary by CodeRabbit

Release Notes:

  • New Feature: Added support for calculating turn signal information based on the turn_signal_on_swerving parameter in the AvoidanceModule.
  • New Feature: Implemented intersection checks with lane bounds in the IntersectionChecker class.
  • New Feature: Created a vehicle footprint and updated the logic for creating it in the VehicleFootprint class.
  • Refactor: Introduced new type aliases for geometric types in the AvoidanceTypes namespace.
  • Refactor: Updated member variables and function declarations in the AvoidanceModule, IntersectionChecker, and VehicleFootprint classes.
  • Chore: Added a new source file to the CMakeLists.txt file.

"In paths we swerve, avoiding strife,
Geometric types guide our life.
Turn signals calculated true,
Intersection checks, we now do.
Vehicle footprint, clear and bright,
Code refactored, shining light.
With each change, we strive for more,
Building a module we adore."

@github-actions github-actions bot added the component:planning Route planning, decision-making, and navigation. (auto-assigned) label Jul 20, 2023
@beyzanurkaya beyzanurkaya self-assigned this Jul 24, 2023
@beyzanurkaya beyzanurkaya force-pushed the fix/turn-off-signal-when-avoiding branch 2 times, most recently from 2fbac43 to 90d20cb Compare July 25, 2023 16:13
@beyzanurkaya beyzanurkaya marked this pull request as ready for review July 25, 2023 16:13
@mehmetdogru mehmetdogru added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Jul 26, 2023
@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (14ef695) 14.78% compared to head (91279f5) 14.78%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4348   +/-   ##
=======================================
  Coverage   14.78%   14.78%           
=======================================
  Files        1636     1636           
  Lines      113602   113594    -8     
  Branches    34913    34913           
=======================================
  Hits        16793    16793           
+ Misses      77968    77959    -9     
- Partials    18841    18842    +1     
Flag Coverage Δ *Carryforward flag
differential 12.53% <0.00%> (?)
total 14.78% <ø> (+<0.01%) ⬆️ Carriedforward from 14ef695

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

Files Coverage Δ
...ehavior_velocity_intersection_module/src/debug.cpp 0.00% <ø> (ø)
...ity_intersection_module/src/scene_intersection.cpp 0.00% <ø> (ø)
...behavior_velocity_intersection_module/src/util.cpp 0.00% <ø> (ø)
...ior_velocity_intersection_module/src/util_type.hpp 0.00% <ø> (ø)
...em_monitor/src/process_monitor/process_monitor.cpp 0.00% <ø> (ø)
...er/src/scene_module/avoidance/avoidance_module.cpp 12.21% <0.00%> (-0.11%) ⬇️

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

@mehmetdogru
Copy link
Contributor

mehmetdogru commented Jul 27, 2023

@beyzanurkaya do we have issue related to this PR? If so please relate these two.

Can you also give more detail about the PR as well such as with videos, screenshots, text etc

Comment on lines 2974 to 2569
for (const auto & cl : *debug_data_.current_lanelets) {
const auto neigbor_lane = planner_data_->route_handler->getAllSharedLineStringLanelets(cl);
for (const auto & lane : neigbor_lane) {
LinearRing2d lane_points;
for (const auto & p : lane.polygon2d().basicPolygon()) {
lane_points.push_back(Point2d(p.x(), p.y()));
}
if (boost::geometry::intersects(path_points, lane_points)) {
if (segment_shift_length > 0.0) {
turn_signal_info.turn_signal.command = TurnIndicatorsCommand::ENABLE_LEFT;
} else {
turn_signal_info.turn_signal.command = TurnIndicatorsCommand::ENABLE_RIGHT;
}
}
}
}
Copy link
Contributor

@mehmetdogru mehmetdogru Jul 27, 2023

Choose a reason for hiding this comment

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

  1. I think it better to use the points of shifted_path where in the range of shift segment instead of iterating all the path points.( only the path points between front_shift_line.start_idx and front_shift_line.end_idx should be enough )

  1. Instead of using lanelet polygon you can use current lane's right and left bound. (If segment_shift_length > 0.0 then get left bound and if segment_shift_length < 0.0 get right bound ) ✔️

  1. It is not realistic to use path points for intersection because what matter in this scenario is that vehicle polygon is exceeding the current lane or not (means occupying any neighbor lane). So you should use vehicle dimensions and transform ego polygon to related path points
    Edit: If you don't transform ego vehicle polygon onto related path points (related path points explained in the first item) which is what the current implementation does ego vehicle will turn on the signal just only when is actually out of lane. What we want is to signal before the vehicle is actually out of lane bounds.

  1. Please add some comment lines which explain your purpose ✔️

Copy link
Contributor

@satoshi-ota satoshi-ota Aug 2, 2023

Choose a reason for hiding this comment

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

Instead of using lanelet polygon you can use current lane's right and left bound. (If segment_shift_length > 0.0 then get left bound and if segment_shift_length < 0.0 get right bound )

I think this is simple and good solution to achieve what you want to do, too.

Copy link
Contributor

@mehmetdogru mehmetdogru Aug 11, 2023

Choose a reason for hiding this comment

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

@beyzanurkaya please make sure that you actually applied the necessary changes before you resolve the conversation. 😄 Please take a look at unresolved conversion above.

@satoshi-ota
Copy link
Contributor

satoshi-ota commented Aug 2, 2023

Hi @beyzanurkaya @mehmetdogru ✋ Sorry for late response, and thanks for you fixing.
I'll run evaluator with this PR if this PR is ready. Before that, could you check some suggestions?

@mehmetdogru mehmetdogru marked this pull request as draft August 3, 2023 05:49
@mehmetdogru
Copy link
Contributor

@satoshi-ota I changed to PR status to draft now since @beyzanurkaya will work on it.

@beyzanurkaya beyzanurkaya changed the title fix(behavior_path_planner): if ego changes lanes turn the signal on fix(behavior_path_planner): if ego leaves the current lane turn the signal on Aug 6, 2023
@beyzanurkaya beyzanurkaya force-pushed the fix/turn-off-signal-when-avoiding branch from b7f15b7 to f86ceea Compare August 6, 2023 22:12
@github-actions github-actions bot removed component:system System design and integration. (auto-assigned) component:map Map creation, storage, and loading. (auto-assigned) type:ci Continuous Integration (CI) processes and testing. (auto-assigned) component:launch Launch files, scripts and initialization tools. (auto-assigned) component:common Common packages from the autoware-common repository. (auto-assigned) labels Aug 29, 2023
@beyzanurkaya beyzanurkaya force-pushed the fix/turn-off-signal-when-avoiding branch from 208aeec to f309b07 Compare August 31, 2023 07:56
@beyzanurkaya beyzanurkaya marked this pull request as ready for review August 31, 2023 07:57
@beyzanurkaya beyzanurkaya force-pushed the fix/turn-off-signal-when-avoiding branch from cb3e542 to 72b2791 Compare August 31, 2023 12:15
@mehmetdogru mehmetdogru force-pushed the fix/turn-off-signal-when-avoiding branch from 3009908 to 09df2cd Compare September 13, 2023 08:03
@mehmetdogru mehmetdogru 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 Sep 13, 2023
@beyzanurkaya beyzanurkaya force-pushed the fix/turn-off-signal-when-avoiding branch 2 times, most recently from 539a774 to f2e6ffa Compare September 26, 2023 07:54
@beyzanurkaya beyzanurkaya force-pushed the fix/turn-off-signal-when-avoiding branch from f2e6ffa to 57d1b2d Compare September 26, 2023 07:54
@mehmetdogru mehmetdogru 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 Sep 26, 2023
@beyzanurkaya
Copy link
Contributor Author

@satoshi-ota I made some changes could you review again?

@satoshi-ota
Copy link
Contributor

@satoshi-ota I made some changes could you review again?

Sure!

Copy link
Contributor

@satoshi-ota satoshi-ota left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@beyzanurkaya beyzanurkaya enabled auto-merge (squash) October 2, 2023 08:26
@beyzanurkaya beyzanurkaya merged commit c6b802f into autowarefoundation:main Oct 2, 2023
@beyzanurkaya beyzanurkaya deleted the fix/turn-off-signal-when-avoiding branch November 1, 2023 10:58
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) run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

if the ego is leaving its lane due avoiding an obstacle, the turn signal is not on
3 participants