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

refactor(autoware_pointcloud_preprocessor): rework approximate downsample filter parameters #8480

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

vividf
Copy link
Contributor

@vividf vividf commented Aug 15, 2024

Description

This PR includes the following changes

  1. Remove the default parameters for the approximate downsample filter node.
  2. Add parameter, launch file, and parameters schema for the node.
  3. Change the name nodelet to node.

A big thank you to @Ariiees for the PRs (#8297 and #7422), where we worked together on the entire pointcloud preprocessor parameters, launch file, and schema.

Related links

Parent Issue:

  • Link

How was this PR tested?

ros2 launch autoware_pointcloud_preprocessor approximate_downsample_filter.launch.xml

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

Signed-off-by: vividf <yihsiang.fang@tier4.jp>
@github-actions github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:sensing Data acquisition from sensors, drivers, preprocessing. (auto-assigned) labels Aug 15, 2024
Copy link

github-actions bot commented Aug 15, 2024

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@vividf vividf self-assigned this Aug 15, 2024
@vividf vividf added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Aug 15, 2024
@vividf vividf marked this pull request as ready for review August 15, 2024 03:33
@vividf vividf added the tag:deploy-docs Mark for deploy-docs action generation. (used-by-ci) label Aug 15, 2024
Copy link

codecov bot commented Aug 15, 2024

Codecov Report

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

Project coverage is 23.74%. Comparing base (0d55025) to head (9ee2a7f).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...mple_filter/approximate_downsample_filter_node.cpp 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8480      +/-   ##
==========================================
- Coverage   28.01%   23.74%   -4.27%     
==========================================
  Files        1319     1417      +98     
  Lines       98635   102019    +3384     
  Branches    39789    39031     -758     
==========================================
- Hits        27631    24224    -3407     
- Misses      70949    75487    +4538     
- Partials       55     2308    +2253     
Flag Coverage Δ *Carryforward flag
differential 21.57% <0.00%> (?)
total 23.92% <ø> (-4.09%) ⬇️ Carriedforward from e620d08

*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.

Copy link
Contributor

@knzo25 knzo25 left a comment

Choose a reason for hiding this comment

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

Left some nits and schema related comments

vividf and others added 2 commits August 20, 2024 17:27
Signed-off-by: vividf <yihsiang.fang@tier4.jp>
Signed-off-by: vividf <yihsiang.fang@tier4.jp>
@vividf vividf requested a review from knzo25 August 20, 2024 10:08
Signed-off-by: vividf <yihsiang.fang@tier4.jp>
Signed-off-by: vividf <yihsiang.fang@tier4.jp>
Signed-off-by: vividf <yihsiang.fang@tier4.jp>
@vividf vividf requested a review from mkuri as a code owner August 26, 2024 05:47
@vividf
Copy link
Contributor Author

vividf commented Sep 3, 2024

@knzo25 kindly ping

vividf and others added 2 commits September 5, 2024 23:30
@vividf vividf requested a review from knzo25 September 17, 2024 14:26
"type": "number",
"description": "voxel size along the x-axis [m]",
"default": "0.3",
"minimum": 0
Copy link
Contributor

Choose a reason for hiding this comment

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

As I explained in previous PRs, please keep consistency with the floating representation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:sensing Data acquisition from sensors, drivers, preprocessing. (auto-assigned) tag:deploy-docs Mark for deploy-docs action generation. (used-by-ci) tag:require-cuda-build-and-test 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.

2 participants