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

refactor(avoidance): use common safety checker #4388

Conversation

satoshi-ota
Copy link
Contributor

@satoshi-ota satoshi-ota commented Jul 25, 2023

Description

🤖 Generated by Copilot at 7d5d1f6

This pull request improves the safety check feature of the behavior path planner, which aims to avoid collisions with other objects by shifting lanes or decelerating. It updates the configuration file, the documentation, and the data structures for the new parameters and logic. It also refactors and simplifies the code of the avoidance module and the avoidance manager, and adds some utility functions for the safety check calculations.

Please approve ⬇️ before this PR.
autowarefoundation/autoware_launch#477

Tests performed

Effects on system behavior

Nothing.

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.

@github-actions github-actions bot added the component:planning Route planning, decision-making, and navigation. (auto-assigned) label Jul 25, 2023
@satoshi-ota satoshi-ota force-pushed the refactor/use-common-safety-checker branch 3 times, most recently from 44cd3c0 to 7d5d1f6 Compare July 26, 2023 05:06
@github-actions github-actions bot added the type:documentation Creating or refining documentation. (auto-assigned) label Jul 26, 2023
@satoshi-ota satoshi-ota added the tag:deploy-docs Mark for deploy-docs action generation. (used-by-ci) label Jul 26, 2023
@satoshi-ota satoshi-ota marked this pull request as ready for review July 26, 2023 05:39
@satoshi-ota satoshi-ota added the tag: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

Patch coverage: 11.21% and project coverage change: -0.01% ⚠️

Comparison is base (e181dce) 14.93% compared to head (e72a556) 14.93%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4388      +/-   ##
==========================================
- Coverage   14.93%   14.93%   -0.01%     
==========================================
  Files        1515     1515              
  Lines      104547   104509      -38     
  Branches    31779    31770       -9     
==========================================
- Hits        15615    15607       -8     
+ Misses      71896    71883      -13     
+ Partials    17036    17019      -17     
Flag Coverage Δ *Carryforward flag
differential 13.32% <11.21%> (?)
total 14.93% <ø> (-0.01%) ⬇️ Carriedforward from e181dce

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

Files Changed Coverage Δ
...lanner/scene_module/avoidance/avoidance_module.hpp 0.00% <ø> (ø)
..._planner/utils/avoidance/avoidance_module_data.hpp 0.00% <ø> (ø)
...ath_planner/src/scene_module/avoidance/manager.cpp 6.72% <0.00%> (-0.07%) ⬇️
...ehavior_path_planner/src/utils/avoidance/utils.cpp 6.56% <7.79%> (+0.16%) ⬆️
...er/src/scene_module/avoidance/avoidance_module.cpp 11.78% <28.57%> (-0.15%) ⬇️

... and 1 file with indirect coverage changes

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

Copy link
Contributor

@kyoichi-sugahara kyoichi-sugahara left a comment

Choose a reason for hiding this comment

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

LGTM
Thank you so much for using common function. It helps when i make safety check class.

const auto & p1 = path.points.at(i - 1);
const auto & p2 = path.points.at(i);
const auto ego_idx = planner_data_->findEgoIndex(shifted_path.path.points);
const auto is_right_shift = [&]() -> std::optional<bool> {
Copy link
Contributor

Choose a reason for hiding this comment

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

@satoshi-ota
Like always some small question happenst to me.
This is_right_shift is the flag to judge ego-vehicle is shifting and judge if safety check should be excuted based on the information i guess.
If so, only shifting to right direction is only left side running country like Japan?
So is_shifting or is_avoiding is more suitable I thought.

Copy link
Contributor Author

@satoshi-ota satoshi-ota Jul 26, 2023

Choose a reason for hiding this comment

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

@kyoichi-sugahara Thanks for your question. 👍

This is_right_shift is the flag to judge ego-vehicle is shifting and judge if safety check should be excuted based on the information i guess.

Yes. If ego doesn't need to shift, the value will be std::nullopt.

If so, only shifting to right direction is only left side running country like Japan?

No. I think there is a possiblity to meet the situation we have to shift right side on right-side traffic country. Additionaly, we sometimes met left shift situation in Odaiba.

Therefore, I think the variable is_right_shift needs to show following three case:

  1. is_right_shift: true -> RIGHT side shift
  2. is_right_shift: false -> LEFT side shift
  3. is_right_shift: std::nullopt: NO shift lines exist

In conclusion, I suppose that it is better to use is_right_shift. What do you think?

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
@satoshi-ota satoshi-ota force-pushed the refactor/use-common-safety-checker branch from e8485f1 to e72a556 Compare July 26, 2023 23:33
@satoshi-ota satoshi-ota enabled auto-merge (squash) July 27, 2023 02:10
@satoshi-ota satoshi-ota merged commit a567bb2 into autowarefoundation:main Jul 27, 2023
23 of 29 checks passed
@satoshi-ota satoshi-ota deleted the refactor/use-common-safety-checker branch July 27, 2023 04:45
shmpwk pushed a commit to tier4/autoware.universe that referenced this pull request Aug 7, 2023
* refactor(avoidance): use common safety checker

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* docs(avoidance): rename params

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* chore(avoidance): rename variables

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

---------

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
PhoebeWu21 pushed a commit to PhoebeWu21/autoware.universe that referenced this pull request Aug 18, 2023
* refactor(avoidance): use common safety checker

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* docs(avoidance): rename params

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

* chore(avoidance): rename variables

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>

---------

Signed-off-by: satoshi-ota <satoshi.ota928@gmail.com>
Signed-off-by: PhoebeWu21 <wwcphoebe@gmail.com>
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) tag:deploy-docs Mark for deploy-docs action generation. (used-by-ci) tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants