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(obstacle_stop_planner): fix the issues when use_predicted_objects true #3556

Merged
merged 1 commit into from
May 9, 2023

Conversation

brkay54
Copy link
Member

@brkay54 brkay54 commented Apr 26, 2023

Description

In obstacle_stop_planner, use_predicted_object option was added but it does not work as we wanted. Firstly, there are some bugs while checking the obstacle. For example, it uses the last collision footprint to set collision point.

Screenshot from 2023-04-26 23-54-30

Also, it checks only intersected points while checking collision, however, corner points also need to be checked.

Screenshot from 2023-04-15 18-01-36

Also I needed make some changes in adaptive_cruise_planner while using predicted objects, because perception side is estimating obstacle velocity, adaptive_cruise_control does not need to estimate the velocity.

Related links

Tests performed

For Z-Axis-Filtering:

Screenshot from 2023-04-26 09-38-13

Stop test:

obstacle-stop-.2023-04-26-23-40-53.mp4

Vehicle following:

follow-vehicle-.2023-04-26-23-41-28.mp4

Slow-down:

slw-.2023-04-26-23-46-25.mp4

Notes for reviewers

Interface changes

Effects on system behavior

There is no effect for system behavior, all things working as we wanted.

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.

@brkay54 brkay54 requested a review from mehmetdogru April 26, 2023 21:13
@github-actions github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) labels Apr 26, 2023
@brkay54
Copy link
Member Author

brkay54 commented Apr 26, 2023

@satoshi-ota @shmpwk @taikitanaka3 @tkimura4 also if it is not problem, I want to add myself as maintainer. I want to maintain this package.

@brkay54 brkay54 force-pushed the osp-fix-use-predicted branch 2 times, most recently from 907966a to 9d9e998 Compare April 26, 2023 21:23
@codecov
Copy link

codecov bot commented Apr 26, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (9693e36) 13.82% compared to head (1f9cb59) 13.81%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3556      +/-   ##
==========================================
- Coverage   13.82%   13.81%   -0.01%     
==========================================
  Files        1395     1395              
  Lines       98036    98016      -20     
  Branches    29147    29125      -22     
==========================================
- Hits        13553    13542      -11     
+ Misses      69826    69820       -6     
+ Partials    14657    14654       -3     
Flag Coverage Δ *Carryforward flag
differential 8.80% <0.00%> (?)
total 13.84% <ø> (+0.02%) ⬆️ Carriedforward from 9693e36

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

Impacted Files Coverage Δ
...ner/include/behavior_path_planner/data_manager.hpp 23.07% <ø> (-14.43%) ⬇️
...lude/behavior_path_planner/turn_signal_decider.hpp 75.00% <ø> (ø)
...or_path_planner/src/behavior_path_planner_node.cpp 19.39% <ø> (-0.23%) ⬇️
.../behavior_path_planner/src/turn_signal_decider.cpp 50.00% <ø> (-0.74%) ⬇️
...top_planner/include/obstacle_stop_planner/node.hpp 0.00% <0.00%> (ø)
...tacle_stop_planner/src/adaptive_cruise_control.cpp 1.52% <0.00%> (-0.13%) ⬇️
planning/obstacle_stop_planner/src/node.cpp 9.26% <0.00%> (+0.44%) ⬆️
...anning/obstacle_stop_planner/src/planner_utils.cpp 13.42% <0.00%> (-0.55%) ⬇️

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

@brkay54 brkay54 self-assigned this Apr 26, 2023
@satoshi-ota
Copy link
Contributor

@satoshi-ota @shmpwk @taikitanaka3 @tkimura4 also if it is not problem, I want to add myself as maintainer. I want to maintain this package.

@brkay54 OK. We TIER IV are glad you are maintaining this package. Then, please add your name in package.xml like that. I think it is better to create other PR for adding maintainer. I'll approve quickly if you done.

<maintainer email="satoshi.ota@tier4.jp">Satoshi Ota</maintainer>

@brkay54
Copy link
Member Author

brkay54 commented Apr 27, 2023

Thank you @satoshi-ota . I created PR for this: #3559

@satoshi-ota
Copy link
Contributor

@brkay54 I ran action and updated code owner in #3557.

@brkay54 brkay54 force-pushed the osp-fix-use-predicted branch from 9d9e998 to ef45d5d Compare April 27, 2023 22:42
@satoshi-ota
Copy link
Contributor

satoshi-ota commented May 8, 2023

@brkay54 Sorry for late response. I would like to check this PR by scenario tests. Is this PR ready for review? (and, #3558, #3560 are also ready?)

@brkay54
Copy link
Member Author

brkay54 commented May 8, 2023

… true

Signed-off-by: Berkay Karaman <brkay54@gmail.com>
@brkay54 brkay54 force-pushed the osp-fix-use-predicted branch from ef45d5d to 1f9cb59 Compare May 8, 2023 23:35
@satoshi-ota
Copy link
Contributor

Scenarios test is running now...
Please wait a while.

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. Please rebase the other PRs.

@brkay54 brkay54 merged commit f85c90b into autowarefoundation:main May 9, 2023
@brkay54 brkay54 deleted the osp-fix-use-predicted branch May 9, 2023 09:59
@brkay54
Copy link
Member Author

brkay54 commented May 9, 2023

@satoshi-ota Thank you for your review 🙏🏼 . Other PRs are rebased.

@takayuki5168
Copy link
Contributor

@brkay54 cc @mehmetdogru

We have a new obstacle_cruise_planner package to stop/cruise/slow down against predicted objects.
https://autowarefoundation.github.io/autoware.universe/main/planning/obstacle_cruise_planner/

We TIER IV will replace the obstacle_stop_planner with the obstacle_cruise_planner for the stop/cruise/slow down planner.
I heard that Ota-kun introduced this package to you, but do you have any reason not to use the obstacle_cruise_planner?

@brkay54
Copy link
Member Author

brkay54 commented May 16, 2023

Hi, @takayuki5168 we are also planning to switch to obstacle_cruise_planner but currently using obstacle_stop_planner because the last time I asked in Discord, @mitsudome-r said it is only used in R&D vehicles to be tested.

While testing the obstacle_stop_planner, we saw some bugs and fixed those. After testing the latest obstacle_stop_planner enough, we will also switch to obstacle_cruise_planner. Thank you for your recommendation! 🙏🏼

@takayuki5168
Copy link
Contributor

@brkay54
Thank you for your comment. I understand.

Just for your information, currently, the obstacle_cruise_planner does not support the pointcloud input.
If you wanna use the pointcloud input, just adding the implementation of converting the input pointcloud to the internal abstract obstacle struct will work well.
Let me know any time if you have a question!

@satoshi-ota
Copy link
Contributor

Hi @brkay54
I found that you are NOT registered as CODEOWNER correctly.

Github says:

Unknown owner on line 51: make sure a user with the email address berkay@leodrive.ai exists and has write access to the repository

in this file.

Have you registered your company email address with github? If you haven't, could you register it? (Or fix address in package.xml.)

@brkay54
Copy link
Member Author

brkay54 commented May 19, 2023

@satoshi-ota Thank you for informing me! Sorry for that. I added the mail address as a secondary address. Can you check it again, please?

@satoshi-ota
Copy link
Contributor

@brkay54 The error was removed. I think it is ok 👍

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) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants