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 angle limits to nearest point search #595

Conversation

tkimura4
Copy link
Contributor

@tkimura4 tkimura4 commented Mar 28, 2022

Signed-off-by: tomoya.kimura tomoya.kimura@tier4.jp

Description

When searching for closest point to the ego-vehicle in behavior_path_planner, there is no angle limit.
Therefore, the closest point could not be searched correctly for the intersecting paths.

In this PR, an angle limits is added to closest-point search to fix this bug.

With the PR
image

Without the PR
image

Related links

Tests performed

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.

Signed-off-by: tomoya.kimura <tomoya.kimura@tier4.jp>
@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #595 (f6e2fe8) into main (2f3e335) will decrease coverage by 10.78%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #595       +/-   ##
==========================================
- Coverage   10.78%   0.00%   -10.79%     
==========================================
  Files         710      34      -676     
  Lines       50461    4943    -45518     
  Branches     6579       0     -6579     
==========================================
- Hits         5443       0     -5443     
+ Misses      40611    4943    -35668     
+ Partials     4407       0     -4407     
Impacted Files Coverage Δ
...r/scene_module/avoidance/avoidance_module_data.hpp 0.00% <ø> (ø)
...or_path_planner/src/behavior_path_planner_node.cpp 0.00% <0.00%> (ø)
...nning/behavior_path_planner/src/path_utilities.cpp 0.00% <0.00%> (ø)
...er/src/scene_module/avoidance/avoidance_module.cpp 0.00% <0.00%> (ø)
.../src/scene_module/side_shift/side_shift_module.cpp 0.00% <0.00%> (ø)
...ontrol/trajectory_follower/test/test_mpc_utils.cpp
...e_auto_geometry/test/include/test_bounding_box.hpp
...erception/shape_estimation/lib/corrector/utils.cpp
...rocessor/passthrough_filter/passthrough_uint16.hpp
... and 672 more

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 982263a...f6e2fe8. Read the comment docs.

@yukkysaito
Copy link
Contributor

yukkysaito commented Mar 28, 2022

cc @taikitanaka3 @takayuki5168
@taikitanaka3 has created a function to search, and I would like to use it.

#405
I would like to move this one to common utils for motion planner.

TakaHoribe
TakaHoribe previously approved these changes Mar 29, 2022
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.

LGTM

oops. I missed the comment above.

@TakaHoribe TakaHoribe self-requested a review March 29, 2022 01:42
@TakaHoribe TakaHoribe dismissed their stale review March 29, 2022 01:43

I missed one comment

@tkimura4
Copy link
Contributor Author

@yukkysaito

I would like to move this one to common utils for motion planner.

What function is it for?

@yukkysaito
Copy link
Contributor

What function is it for?

Sorry, it is not enough.
When the path intersects, this function searches the neighborhood within the path before the intersection.

@tkimura4
Copy link
Contributor Author

tkimura4 commented Mar 29, 2022

@yukkysaito
The behavior_path_planner already uses tier4_autoware_utils::findNearestIndex function.

@yukkysaito
Copy link
Contributor

@tkimura4 findFirstNearSearchRangeIndex should be used. I will describe it in detail later.

@tkimura4 tkimura4 requested a review from yukkysaito April 4, 2022 06:02
@yukkysaito
Copy link
Contributor

After discussion, a formal fix will be made in a separate PR.

@tkimura4 tkimura4 merged commit 70bc967 into autowarefoundation:main Apr 4, 2022
@tkimura4 tkimura4 deleted the fix/nearest_index_search_in_path_planner branch April 4, 2022 06:08
boyali referenced this pull request in boyali/autoware.universe Sep 28, 2022
…ier4#595)

* fix(behavior_path_planner): add angle limits to nearest point search

Signed-off-by: tomoya.kimura <tomoya.kimura@tier4.jp>

* ci(pre-commit): autofix

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
boyali referenced this pull request in boyali/autoware.universe Oct 3, 2022
…ier4#595)

* fix(behavior_path_planner): add angle limits to nearest point search

Signed-off-by: tomoya.kimura <tomoya.kimura@tier4.jp>

* ci(pre-commit): autofix

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
boyali referenced this pull request in boyali/autoware.universe Oct 3, 2022
…ier4#595)

* fix(behavior_path_planner): add angle limits to nearest point search

Signed-off-by: tomoya.kimura <tomoya.kimura@tier4.jp>

* ci(pre-commit): autofix

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
boyali referenced this pull request in boyali/autoware.universe Oct 19, 2022
…ier4#595)

* fix(behavior_path_planner): add angle limits to nearest point search

Signed-off-by: tomoya.kimura <tomoya.kimura@tier4.jp>

* ci(pre-commit): autofix

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.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.

3 participants