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(autoware_pointcloud_preprocessor): blockage diag node add runtime error when the parameter is wrong #8564

Conversation

vividf
Copy link
Contributor

@vividf vividf commented Aug 22, 2024

Description

TIER4_INTERNAL_LINK

When the user sets the parameter "vertical_bins" less than the number of LiDAR's channel, it will output a runtime error to let the user know they should set the vertical_bins correctly instead of only getting the malloc() error.

Related links

Parent Issue:

  • Link

How was this PR tested?

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 component:sensing Data acquisition from sensors, drivers, preprocessing. (auto-assigned) tag:require-cuda-build-and-test labels Aug 22, 2024
Copy link

github-actions bot commented Aug 22, 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 22, 2024
vividf and others added 2 commits August 23, 2024 12:12
Co-authored-by: badai nguyen  <94814556+badai-nguyen@users.noreply.github.com>
@vividf vividf added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Aug 23, 2024
@vividf
Copy link
Contributor Author

vividf commented Aug 23, 2024

@badai-nguyen Could you recheck this, thanks!

Copy link

codecov bot commented Aug 23, 2024

Codecov Report

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

Project coverage is 23.93%. Comparing base (a64566e) to head (caef352).
Report is 1 commits behind head on main.

Files Patch % Lines
...processor/src/blockage_diag/blockage_diag_node.cpp 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8564      +/-   ##
==========================================
- Coverage   24.11%   23.93%   -0.19%     
==========================================
  Files        1399     1400       +1     
  Lines      102423   101971     -452     
  Branches    38926    38765     -161     
==========================================
- Hits        24702    24402     -300     
+ Misses      75223    75118     -105     
+ Partials     2498     2451      -47     
Flag Coverage Δ *Carryforward flag
differential 18.79% <0.00%> (?)
total 23.98% <ø> (-0.14%) ⬇️ Carriedforward from 65b3ac7

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

vividf added 2 commits August 26, 2024 14:20
Signed-off-by: vividf <yihsiang.fang@tier4.jp>
Signed-off-by: vividf <yihsiang.fang@tier4.jp>
@vividf
Copy link
Contributor Author

vividf commented Aug 26, 2024

@badai-nguyen Thanks for your suggestion!! I added the logging message.
However, instead of letting the system break by itself because of the memory-allocating problem
I think it is better to interrupt by ourselves when the parameter setting is wrong. Therefore, I think keeping the runtime error is needed. What do you think?

Thanks!

@badai-nguyen
Copy link
Contributor

@badai-nguyen Thanks for your suggestion!! I added the logging message. However, instead of letting the system break by itself because of the memory-allocating problem I think it is better to interrupt by ourselves when the parameter setting is wrong. Therefore, I think keeping the runtime error is needed. What do you think?

Thanks!

@vividf Yes, I agree, actually I missed return; in my suggestion

Copy link
Contributor

@badai-nguyen badai-nguyen left a comment

Choose a reason for hiding this comment

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

LGTM

@vividf vividf enabled auto-merge (squash) August 27, 2024 07:39
@vividf vividf merged commit d08dcfe into autowarefoundation:main Aug 27, 2024
30 of 31 checks passed
a-maumau pushed a commit to a-maumau/autoware.universe that referenced this pull request Sep 2, 2024
… error when the parameter is wrong (autowarefoundation#8564)

* fix: add runtime error

Signed-off-by: vividf <yihsiang.fang@tier4.jp>

* Update blockage_diag_node.cpp

Co-authored-by: badai nguyen  <94814556+badai-nguyen@users.noreply.github.com>

* fix: add RCLCPP error logging

Signed-off-by: vividf <yihsiang.fang@tier4.jp>

* chore: remove unused variable

Signed-off-by: vividf <yihsiang.fang@tier4.jp>

---------

Signed-off-by: vividf <yihsiang.fang@tier4.jp>
Co-authored-by: badai nguyen <94814556+badai-nguyen@users.noreply.github.com>
batuhanbeytekin pushed a commit to batuhanbeytekin/autoware.universe that referenced this pull request Sep 2, 2024
… error when the parameter is wrong (autowarefoundation#8564)

* fix: add runtime error

Signed-off-by: vividf <yihsiang.fang@tier4.jp>

* Update blockage_diag_node.cpp

Co-authored-by: badai nguyen  <94814556+badai-nguyen@users.noreply.github.com>

* fix: add RCLCPP error logging

Signed-off-by: vividf <yihsiang.fang@tier4.jp>

* chore: remove unused variable

Signed-off-by: vividf <yihsiang.fang@tier4.jp>

---------

Signed-off-by: vividf <yihsiang.fang@tier4.jp>
Co-authored-by: badai nguyen <94814556+badai-nguyen@users.noreply.github.com>
Signed-off-by: Batuhan Beytekin <batuhanbeytekin@gmail.com>
ktro2828 pushed a commit to ktro2828/autoware.universe that referenced this pull request Sep 18, 2024
… error when the parameter is wrong (autowarefoundation#8564)

* fix: add runtime error

Signed-off-by: vividf <yihsiang.fang@tier4.jp>

* Update blockage_diag_node.cpp

Co-authored-by: badai nguyen  <94814556+badai-nguyen@users.noreply.github.com>

* fix: add RCLCPP error logging

Signed-off-by: vividf <yihsiang.fang@tier4.jp>

* chore: remove unused variable

Signed-off-by: vividf <yihsiang.fang@tier4.jp>

---------

Signed-off-by: vividf <yihsiang.fang@tier4.jp>
Co-authored-by: badai nguyen <94814556+badai-nguyen@users.noreply.github.com>
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) run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) tag:require-cuda-build-and-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants