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(ndt_scan_matcher): fixed as pointed out by clang-tidy #4270

Conversation

SakodaShintaro
Copy link
Contributor

Description

I modified

  • ndt_scan_matcher_core.hpp
  • ndt_scan_matcher_core.cpp

according to the warnings from clang-tidy.

Tests performed

I have checked the operation of the following three

  • logging_simulator
  • driving_log_replayer
  • awsim

Effects on system behavior

There are no changes about behavior.

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.

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.

After all checkboxes are checked, anyone who has write access can merge the PR.

Signed-off-by: Shintaro SAKODA <shintaro.sakoda@tier4.jp>
@github-actions github-actions bot added the component:localization Vehicle's position determination in its environment. (auto-assigned) label Jul 13, 2023
@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

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

Comparison is base (cb37d94) 15.21% compared to head (3e018bc) 14.21%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4270      +/-   ##
==========================================
- Coverage   15.21%   14.21%   -1.00%     
==========================================
  Files        1489     1597     +108     
  Lines      102642   109834    +7192     
  Branches    31523    31485      -38     
==========================================
  Hits        15613    15613              
- Misses      70080    77277    +7197     
+ Partials    16949    16944       -5     
Flag Coverage Δ *Carryforward flag
differential 0.00% <0.00%> (?)
total 14.22% <ø> (-1.00%) ⬇️ Carriedforward from d53d030

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

Impacted Files Coverage Δ
...ion/ndt_scan_matcher/src/ndt_scan_matcher_core.cpp 0.00% <0.00%> (ø)

... and 116 files with indirect coverage changes

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

@SakodaShintaro
Copy link
Contributor Author

The clang-tidy check is still in progress, but it seems that another lint-check is causing an error due to the effect of "NOLINT" that was added to avoid clang-tidy warnings.
https://results.pre-commit.ci/run/github/429651616/1689221255.qABtU78hRfiNym2rIKcIUQ

I didn't realize there were two types of lint-check running. I'll fix that later.

@kminoda
Copy link
Contributor

kminoda commented Jul 13, 2023

Thank you! Let us know if the required CIs are fixed so that we can start reviewing 👍 (Maybe better to make it a draft until then)

@SakodaShintaro SakodaShintaro self-assigned this Jul 13, 2023
@SakodaShintaro SakodaShintaro marked this pull request as draft July 13, 2023 04:43
@SakodaShintaro SakodaShintaro marked this pull request as ready for review July 13, 2023 09:00
Copy link
Contributor

@kminoda kminoda left a 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 for the PR 🙏

void publish_marker(
const rclcpp::Time & sensor_ros_time, const std::vector<geometry_msgs::msg::Pose> & pose_array);
void publish_initial_to_result_distances(
const rclcpp::Time & sensor_ros_msg, const geometry_msgs::msg::Pose & result_pose_msg,
const rclcpp::Time & sensor_ros_time, const geometry_msgs::msg::Pose & result_pose_msg,
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you so much for finding this...!

@KYabuuchi
Copy link
Contributor

KYabuuchi commented Jul 14, 2023

I found that spell-check failed due to undistortion, so I have submitted a PR to add it into the dictionary. tier4/autoware-spell-check-dict#563

image

Now, it passes spell-check-differential completely.

Signed-off-by: Shintaro SAKODA <shintaro.sakoda@tier4.jp>
Copy link
Contributor

@kminoda kminoda left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for the PR 🙏

@SakodaShintaro SakodaShintaro merged commit 20c749e into autowarefoundation:main Jul 14, 2023
@SakodaShintaro SakodaShintaro deleted the refactor/lint_ndt_scan_matcher branch July 14, 2023 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:localization Vehicle's position determination in its environment. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants