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): add function to check the shift points in avoidance module #794

Closed
wants to merge 2 commits into from
Closed

fix(behavior_path_planner): add function to check the shift points in avoidance module #794

wants to merge 2 commits into from

Conversation

shulanbushangshu
Copy link
Contributor

@shulanbushangshu shulanbushangshu commented Apr 25, 2022

Description

To solve #706

We can add function to detect points with offset distance to make sure they are within the drivable zone.
We can determine whether the offset points are all in the driveable lane in the generateAvoidancePath of avoidance module(Consider the width of the vehicle when detecting),otherwise we use reference_path as the obstacle avoidance path(No obstacle avoidance behavior)

Related links

Wrong path planned by avoidance module in a special case #706

Tests performed

Case1:
1.Run planning_simulator.
2.place ego position and goal
3. Set a obstacle near discontinuous adjacent lanes
4. result -- ego vehicle stops
2022-04-25 11-19-01
Case2:
1.Run planning_simulator.
2.place ego position and goal
3. Set a obstacle in continuous adjacent lanes
4. result-- ego vehicle bypasses obstacle
2022-04-25 11-25-21

Notes for reviewers

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.

@codecov
Copy link

codecov bot commented Apr 25, 2022

Codecov Report

Merging #794 (7355374) into main (a72a2f2) will decrease coverage by 0.11%.
The diff coverage is 0.00%.

@@           Coverage Diff            @@
##            main    #794      +/-   ##
========================================
- Coverage   9.55%   9.44%   -0.12%     
========================================
  Files        935     935              
  Lines      57795   58488     +693     
  Branches   10430   10430              
========================================
  Hits        5522    5522              
- Misses     47734   48427     +693     
  Partials    4539    4539              
Flag Coverage Δ *Carryforward flag
differential 0.00% <0.00%> (?)
total 9.55% <0.00%> (ø) Carriedforward from 0795c1b

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

Impacted Files Coverage Δ
...lanner/scene_module/avoidance/avoidance_module.hpp 0.00% <ø> (ø)
...r/scene_module/avoidance/avoidance_module_data.hpp 0.00% <ø> (ø)
...er/src/scene_module/avoidance/avoidance_module.cpp 0.00% <0.00%> (ø)
...ner/src/scene_module/avoidance/avoidance_utils.cpp 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a72a2f2...7355374. Read the comment docs.

@yukkysaito
Copy link
Contributor

@takayuki5168 @rej55 Can you review this PR?

@takayuki5168
Copy link
Contributor

I guess it's better to be reviewed by @TakaHoribe @zulfaqar-azmi-t4 🙏

@zulfaqar-azmi-t4
Copy link
Contributor

zulfaqar-azmi-t4 commented May 13, 2022

@shulanbushangshu
Sorry for the many comments.
Some additional comments include you need to pass DCO.
if you want to fix DOC
git rebase <branch> --signoff

@takayuki5168
Copy link
Contributor

git rebase --signoff

Or we just push "Set DCO to pass" button here.
image

@shulanbushangshu
Copy link
Contributor Author

Thank you.I will fix the DCO

@shulanbushangshu shulanbushangshu deleted the fix(avoidance-module)add-function-to-check-the-shift-points-in-avoidance-module-of-behavior_path_planner branch May 13, 2022 08:26
@shulanbushangshu shulanbushangshu restored the fix(avoidance-module)add-function-to-check-the-shift-points-in-avoidance-module-of-behavior_path_planner branch May 16, 2022 07:49
Signed-off-by: jack.song <jack.song@autocore.ai>
@shulanbushangshu shulanbushangshu requested review from zulfaqar-azmi-t4 and removed request for zulfaqar-azmi-t4 May 23, 2022 01:38
Signed-off-by: jack.song <jack.song@autocore.ai>
@zulfaqar-azmi-t4
Copy link
Contributor

@shulanbushangshu
I deeply apologize for the delay.
I've checked your solution using

  1. single obstacle
  2. dual obstacles

For single obstacle case, the solution works.
But when dual obstacle situation, the solution might be off.
Please refer to the following videos for example. (sorry again due to some technical issue, the video doesn't show full frame, but i think it should be sufficient)

review_after2.mp4

Let me illustrate the acceptable result first, as 1m36s: if furthest obstacle was ignored (path not shifted) but if we place an obstacle ahead, the path will still remain shifted if the condition is acceptable.

Then in your proposed solution, when the furthest obstacle near the discontinued path was ignore, when we place the obstacle ahead (0m24s), the new obstacle is also ignored.

@shulanbushangshu
Copy link
Contributor Author

shulanbushangshu commented May 25, 2022 via email

@shulanbushangshu shulanbushangshu marked this pull request as draft May 26, 2022 05:15
@takayuki5168 takayuki5168 removed their request for review June 8, 2022 01:29
@shulanbushangshu shulanbushangshu marked this pull request as ready for review June 14, 2022 07:52
@shulanbushangshu shulanbushangshu marked this pull request as draft June 14, 2022 07:53
@stale
Copy link

stale bot commented Aug 13, 2022

This pull request has been automatically marked as stale because it has not had recent activity.

@stale stale bot added the status:stale Inactive or outdated issues. (auto-assigned) label Aug 13, 2022
@shulanbushangshu
Copy link
Contributor Author

I close this pr and will test this issue in the newest code.
If this issue occurs in the newest code,I will work on another pr.

@stale stale bot removed the status:stale Inactive or outdated issues. (auto-assigned) label Mar 30, 2023
kyoichi-sugahara pushed a commit to kyoichi-sugahara/autoware.universe that referenced this pull request Sep 5, 2023
…0/update-start-goal-planner

Feat/beta/v0.10.0/update start goal planner
ryuichi-maeda pushed a commit to sensefield/autoware.universe that referenced this pull request Jan 10, 2025
iwatake2222 pushed a commit to iwatake2222/autoware.universe that referenced this pull request Jan 17, 2025
…ndation#794)

Signed-off-by: Takayuki Murooka <takayuki5168@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants