-
Notifications
You must be signed in to change notification settings - Fork 35
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(ndt_omp): apply static analysis #57
refactor(ndt_omp): apply static analysis #57
Conversation
Signed-off-by: a-maumau <[email protected]>
6bc4d3b
to
ca90f0f
Compare
Signed-off-by: a-maumau <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will review this after Ando-san's points are corrected.
Signed-off-by: a-maumau <[email protected]>
Signed-off-by: a-maumau <[email protected]>
Signed-off-by: a-maumau <[email protected]>
Signed-off-by: a-maumau <[email protected]>
Signed-off-by: a-maumau <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LTGM!
(I haven't reviewed the include/pclomp
because it is scheduled to be deleted.)
@SakodaShintaro Could you please start your review? |
Signed-off-by: a-maumau <[email protected]>
To avoid the build error until merging the autowarefoundation/autoware.universe#8124 , I added |
Signed-off-by: a-maumau <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much!
Description
This PR applies some suggestions from the linter to code.
Checked with clang-tidy and cppcheck.
needs to be merged with autowarefoundation/autoware.universe#8124
Fixed:
++
operatorNot Fixed:
applyFilter
ndt_omp/include/pclomp/voxel_grid_covariance_omp_impl.hpp
Line 420 in ef654bd
thread_num_
inndt_omp/include/multigrid_pclomp/multi_voxel_grid_covariance_omp_impl.hpp
Line 105 in ef654bd
ndt_omp/include/multigrid_pclomp/multigrid_ndt_omp_impl.hpp
Line 70 in ef654bd
testGradient
, not scoped for this PR)Interface Changes
This PR will change the estimate_xy_covariance_by_Laplace_approximation to
estimate_xy_covariance_by_laplace_approximation
(lowercaselaplace
) which effect the ndt_scan_matcher.Checks
Checked with clang-tidy (v14.0.0) and cppcheck (v2.14.1).
Use
.cppcheck_suppressions
for suppressing some notation from cppcheck.For ignoring some notes using the autoware, autoware.universe repo's suppression files , you need to add followings:
CPPLINT.cfg
:filter=-legal/copyright
.cppcheck_supressions
:missingMemberCopy
check_linter.sh
Before fixing the code:
Check with check_linter.sh
Check with cppcheck:
After this PR:
Check with check_linter.sh
Check with cppcheck
Test
ndt_scan_matcher
.ndt_scan_matcher