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

feat(behavior_path_planner): keep original points in resampling of pull over #2478

Conversation

kosuke55
Copy link
Contributor

@kosuke55 kosuke55 commented Dec 8, 2022

Signed-off-by: kosuke55 kosuke.tnp@gmail.com

Description

  • keep original input poins in resampling of end of behavior_path_planner.
  • add flag for keeping oringal points to resamplePathWithSpline
    • set false defaultly, so in each scene module do not keep the orintal points unintentionally (not to increase calculation cost)

Related links

tier4 internal slack

Tests performed

psim, real vehicle

tier4 internal scenario test
821/825 -> 821/825

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.

@github-actions github-actions bot added the component:planning Route planning, decision-making, and navigation. (auto-assigned) label Dec 8, 2022
@@ -66,7 +67,7 @@ PathWithLaneId resamplePathWithSpline(const PathWithLaneId & path, double interv
transformed_path.at(i) = path.points.at(i).point;
}

constexpr double epsilon = 0.01;
constexpr double epsilon = 0.1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

0.01 seems too close and sometimes causes path distortion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
constexpr double epsilon = 0.1;
constexpr double epsilon = 0.2;

is better from @TakaHoribe

@codecov
Copy link

codecov bot commented Dec 8, 2022

Codecov Report

Base: 10.56% // Head: 10.48% // Decreases project coverage by -0.08% ⚠️

Coverage data is based on head (792c102) compared to base (f1a9a96).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2478      +/-   ##
==========================================
- Coverage   10.56%   10.48%   -0.09%     
==========================================
  Files        1264     1264              
  Lines       87260    88234     +974     
  Branches    20936    21637     +701     
==========================================
+ Hits         9222     9249      +27     
- Misses      68157    69031     +874     
- Partials     9881     9954      +73     
Flag Coverage Δ *Carryforward flag
differential 3.15% <0.00%> (?)
total 10.56% <0.00%> (ø) Carriedforward from 5691b07

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

Impacted Files Coverage Δ
...havior_path_planner/behavior_path_planner_node.hpp 0.00% <ø> (ø)
...or_path_planner/src/behavior_path_planner_node.cpp 0.28% <0.00%> (+0.09%) ⬆️
...nning/behavior_path_planner/src/path_utilities.cpp 0.00% <0.00%> (ø)
planning/static_centerline_optimizer/src/utils.cpp 0.00% <0.00%> (ø)
.../src/scene_module/side_shift/side_shift_module.cpp 0.00% <0.00%> (ø)
...nner/scene_module/side_shift/side_shift_module.hpp 0.00% <0.00%> (ø)
planning/behavior_path_planner/src/utilities.cpp 3.94% <0.00%> (+0.17%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@kosuke55 kosuke55 force-pushed the feat/keep_original_points_in_bpp_resample branch from ab2536b to fcd3f8c Compare December 8, 2022 10:22
@github-actions github-actions bot added the component:common Common packages from the autoware-common repository. (auto-assigned) label Dec 8, 2022
@satoshi-ota satoshi-ota changed the title feat(behavior_path_planner): keeep original points in resampling feat(behavior_path_planner): keep original points in resampling Dec 8, 2022
@kosuke55 kosuke55 force-pushed the feat/keep_original_points_in_bpp_resample branch from fcd3f8c to edfb2d5 Compare December 13, 2022 10:52
@github-actions github-actions bot removed the component:common Common packages from the autoware-common repository. (auto-assigned) label Dec 14, 2022
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
This reverts commit edfb2d5.

Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
@kosuke55 kosuke55 force-pushed the feat/keep_original_points_in_bpp_resample branch from 60db66c to 7cd0aef Compare December 16, 2022 05:11
@kosuke55
Copy link
Contributor Author

tier4 internal scenario test
821/825 -> 821/825

@kosuke55 kosuke55 changed the title feat(behavior_path_planner): keep original points in resampling feat(behavior_path_planner): keep original points in resampling of pull over Dec 16, 2022
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
@kosuke55 kosuke55 force-pushed the feat/keep_original_points_in_bpp_resample branch from 9c549ef to 5691b07 Compare December 16, 2022 07:53
@kosuke55
Copy link
Contributor Author

@TakaHoribe
Could you please approve this?

Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
@kosuke55 kosuke55 force-pushed the feat/keep_original_points_in_bpp_resample branch from 11c7625 to 792c102 Compare December 19, 2022 02:24
Copy link
Contributor

@TakaHoribe TakaHoribe left a comment

Choose a reason for hiding this comment

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

This is a temporal solution. Will be redesigned in the future.

@kosuke55 kosuke55 merged commit ee9a389 into autowarefoundation:main Dec 19, 2022
@kosuke55 kosuke55 deleted the feat/keep_original_points_in_bpp_resample branch December 19, 2022 15:39
kminoda pushed a commit to kminoda/autoware.universe that referenced this pull request Jan 6, 2023
…ll over (autowarefoundation#2478)

* feat(behavior_path_planner): keep original points in resampling

Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>

* Update planning/behavior_path_planner/src/path_utilities.cpp

Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>

* change motion_utils::overlap_threshold to 0.2 and use it

Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>

* Revert "change motion_utils::overlap_threshold to 0.2 and use it"

This reverts commit edfb2d5.

Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>

* keep input points only in pull over

Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>

* Add comment of temporary processe

Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>

Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
Signed-off-by: kminoda <koji.minoda@tier4.jp>
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants