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(AEB): add height filter for avoiding the ghost brake by false positive point clouds #6637

Merged

Conversation

Shin-kyoto
Copy link
Contributor

@Shin-kyoto Shin-kyoto commented Mar 15, 2024

Description

  • In the case of inclines, false positive point clouds may persist even after ground segmentation. This can lead to ghost braking by the AEB.
  • To prevent ghost braking, point clouds will be filtered based on their height.

Visualization

I checked /control/autonomous_emergency_braking/debug/obstacle_pointcloud

  • Before this PR

image

  • After this PR

image

Related links

TIER IV INTERNAL LINK

Tests performed

  • I have confirmed that there are no compile errors during the build.
  • I have confirmed that Autoware runs without runtime errors using this code and the autoware_launch with added parameters.
  • I have confirmed that AEB correctly filters out point clouds with a small z-coordinate through visualization.

Notes for reviewers

@takayuki5168
Please review this PR keeping the following points in mind;

  • Please check if the name of parameter (detection_range_min_height) is easy to understand.

  • Please check the default value of parameter (detection_range_min_height).
    - Please review whether 5.0 is an appropriate value in the following code. My rationale for choosing 5.0 is that the maximum height of large buses is within 3.7 meters..

  • Please check if the name of parameter (detection_range_max_height_margin) is easy to understand.

  • Please check if the default value of parameter (detection_range_max_height_margin) 0 is appropriate.

  • Note: I sent PR to awf/autoware_launch to add these params to config; feat(AEB): add detection range params autoware_launch#934

Interface changes

Nothing.

Effects on system behavior

  • When obstacle point clouds appear below the detection_range_min_height, AEB may no longer be able to activate braking.

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.

…sitive point clouds

Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>
@Shin-kyoto Shin-kyoto self-assigned this Mar 15, 2024
@github-actions github-actions bot added the component:control Vehicle control algorithms and mechanisms. (auto-assigned) label Mar 15, 2024
Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>
@github-actions github-actions bot added the type:documentation Creating or refining documentation. (auto-assigned) label Mar 15, 2024
@Shin-kyoto Shin-kyoto marked this pull request as ready for review March 16, 2024 02:41
@Shin-kyoto Shin-kyoto added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Mar 16, 2024
Copy link

codecov bot commented Mar 16, 2024

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 14.77%. Comparing base (a993a06) to head (4fbfcd0).
Report is 33 commits behind head on main.

Files Patch % Lines
control/autonomous_emergency_braking/src/node.cpp 0.00% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6637      +/-   ##
==========================================
- Coverage   14.77%   14.77%   -0.01%     
==========================================
  Files        1923     1923              
  Lines      132533   132543      +10     
  Branches    39398    39398              
==========================================
  Hits        19586    19586              
- Misses      91070    91080      +10     
  Partials    21877    21877              
Flag Coverage Δ *Carryforward flag
differential 0.00% <0.00%> (?)
total 14.77% <ø> (+<0.01%) ⬆️ Carriedforward from 49311e4

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

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

pcl::PassThrough<pcl::PointXYZ> height_filter;
height_filter.setInputCloud(pointcloud_ptr);
height_filter.setFilterFieldName("z");
height_filter.setFilterLimits(detection_range_min_height_, 5.0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, 5.0 should depend on the vehicle's height. How about setting the vehicle's height + ros parameter to this value?
The ros parameter's name would be detection_range_height_margin or detection_range_additional_height.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@takayuki5168

Thank you for your comment!
I changed this code following your comment in 49311e4

  • Please check if the name of parameter (detection_range_max_height_margin) is easy to understand.
  • Please check if the default value of parameter (detection_range_max_height_margin) 0 is appropriate.

Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>
Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>
@Shin-kyoto
Copy link
Contributor Author

Shin-kyoto commented Mar 26, 2024

@soblin

Can you review this PR as a code owner?

Please check the following things;

  • Please check if the name of parameters (detection_range_min_height, detection_range_max_height_margin) is easy to understand.

  • Please check the default value of parameter 0 is appropriate.

    • Since there hasn't been any testing conducted yet, we've set the default value for the minimum height to 0, to ensure that the behavior remains (mostly) unchanged.
  • Please check if the risk of this PR is acceptable.

    • Risk: When obstacle point clouds appear below the detection_range_min_height, AEB may no longer be able to activate braking.
  • Please check if my checking procedure is enough.

  • Note: I sent PR to awf/autoware_launch to add these params to config; feat(AEB): add detection range params autoware_launch#934

@Shin-kyoto Shin-kyoto merged commit ce6de02 into autowarefoundation:main Mar 26, 2024
25 of 28 checks passed
danielsanchezaran pushed a commit to tier4/autoware.universe that referenced this pull request Apr 15, 2024
…sitive point clouds (autowarefoundation#6637)

* feat(AEB): add height filter for avoiding the ghost brake by false positive point clouds

Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>

* docs(AEB): add param for height filter

Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>

* feat(AEB): add max height range in AEB

Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>

* feat(AEB): update default params

Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>

---------

Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>
karishma1911 pushed a commit to Interplai/autoware.universe that referenced this pull request Jun 3, 2024
…sitive point clouds (autowarefoundation#6637)

* feat(AEB): add height filter for avoiding the ghost brake by false positive point clouds

Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>

* docs(AEB): add param for height filter

Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>

* feat(AEB): add max height range in AEB

Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>

* feat(AEB): update default params

Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>

---------

Signed-off-by: Shin-kyoto <aquashin0202@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:control Vehicle control algorithms and mechanisms. (auto-assigned) 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.

3 participants