refactor(ndt_scan_matcher): delete default parameters from ndt_scan_matcher_core.cpp #5155

TaikiYamada4 · 2023-09-27T07:51:27Z

Description

Since there were still default values in the cpp code, which may occur unexpected results, I deleted them so that ndt_scan_matcher can get those value only from ndt_scan_matcher.param.yaml.
On this operation, parameter map_frame is extracted to ndt_scan_matcher.param.yaml.
Plus, I updated README.md so that it matches ndt_scan_matcher.param.yaml.

Related: autowarefoundation/autoware_launch#596

Tests performed

It is able to build ndt_scan_matcher locally.
It is also confirmed that the logging_simulator on the tutorial works fine.

Effects on system behavior

This won't directly do something to the system.
It is expected to be less prone to unexpected behavior when parameter definitions are incomplete.

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

I've confirmed the contribution guidelines.
The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

The PR follows the pull request guidelines.

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.

….cpp Extracted paramter map_frame to ndt_scan_matcher.param.yaml Signed-off-by: TaikiYamada4 <[email protected]>

Fixed minor grammar mistakes. Signed-off-by: TaikiYamada4 <[email protected]>

codecov · 2023-09-27T09:30:08Z

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (32c2e67) 14.85% compared to head (8c59aec) 14.88%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5155      +/-   ##
==========================================
+ Coverage   14.85%   14.88%   +0.02%     
==========================================
  Files        1627     1627              
  Lines      112673   112481     -192     
  Branches    34806    34723      -83     
==========================================
+ Hits        16740    16741       +1     
+ Misses      77173    76996     -177     
+ Partials    18760    18744      -16

Flag	Coverage Δ		*Carryforward flag
differential	`0.00% <0.00%> (?)`
total	`14.88% <ø> (+0.02%)`	⬆️	Carriedforward from facba81

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

Files	Coverage Δ
...avior_velocity_intersection_module/src/manager.cpp	`17.56% <ø> (ø)`
...ity_intersection_module/src/scene_intersection.cpp	`0.00% <ø> (ø)`
...ity_intersection_module/src/scene_intersection.hpp	`0.00% <ø> (ø)`
...behavior_velocity_intersection_module/src/util.cpp	`0.00% <ø> (ø)`
...ion/ndt_scan_matcher/src/ndt_scan_matcher_core.cpp	`0.00% <0.00%> (ø)`

... and 31 files with indirect coverage changes

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

Signed-off-by: TaikiYamada4 <[email protected]>

localization/ndt_scan_matcher/src/ndt_scan_matcher_core.cpp

…e defined in ndt_scan_matcher.param.yaml Signed-off-by: TaikiYamada4 <[email protected]>

KYabuuchi

LGTM! Thank you for creating the refactoring PR

…atcher_core.cpp (autowarefoundation#5155) * Removed default paramters in the constructor in ndt_scan_matcher_core.cpp Extracted paramter map_frame to ndt_scan_matcher.param.yaml Signed-off-by: TaikiYamada4 <[email protected]> * Updated README.md to match ndt_scan_matcher.param.yaml Fixed minor grammar mistakes. Signed-off-by: TaikiYamada4 <[email protected]> * style(pre-commit): autofix * style(pre-commit): autofix * Fixed minor grammar mistakes Signed-off-by: TaikiYamada4 <[email protected]> * Removed unnecessary value initialization for parameters that should be defined in ndt_scan_matcher.param.yaml Signed-off-by: TaikiYamada4 <[email protected]> --------- Signed-off-by: TaikiYamada4 <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

TaikiYamada4 added 2 commits September 27, 2023 16:04

Removed default paramters in the constructor in ndt_scan_matcher_core…

3686630

….cpp Extracted paramter map_frame to ndt_scan_matcher.param.yaml Signed-off-by: TaikiYamada4 <[email protected]>

Updated README.md to match ndt_scan_matcher.param.yaml

2c2bf34

Fixed minor grammar mistakes. Signed-off-by: TaikiYamada4 <[email protected]>

TaikiYamada4 self-assigned this Sep 27, 2023

github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:localization Vehicle's position determination in its environment. (auto-assigned) labels Sep 27, 2023

TaikiYamada4 changed the title ~~refactor(ndt_scan_matcher): Delete default paramters from ndt_scan_matcher_core.cpp~~ refactor(ndt_scan_matcher): delete default paramters from ndt_scan_matcher_core.cpp Sep 27, 2023

TaikiYamada4 changed the title ~~refactor(ndt_scan_matcher): delete default paramters from ndt_scan_matcher_core.cpp~~ refactor(ndt_scan_matcher): delete default parameters from ndt_scan_matcher_core.cpp Sep 27, 2023

style(pre-commit): autofix

b05b57f

TaikiYamada4 added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Sep 27, 2023

TaikiYamada4 mentioned this pull request Sep 27, 2023

refactor(ndt_scan_matcher): modified ndt_scan_matcher.param.yaml to match with the one in universe autowarefoundation/autoware_launch#596

Merged

4 tasks

TaikiYamada4 and others added 2 commits September 27, 2023 17:20

Merge branch 'main' into refactor/ndt_scan_matcher_delete_default_values

40ed520

style(pre-commit): autofix

f085b70

Fixed minor grammar mistakes

8a23293

Signed-off-by: TaikiYamada4 <[email protected]>

TaikiYamada4 marked this pull request as ready for review September 28, 2023 06:31

TaikiYamada4 requested review from KYabuuchi, kminoda, Motsu-san and YamatoAndo as code owners September 28, 2023 06:31

KYabuuchi reviewed Sep 28, 2023

View reviewed changes

localization/ndt_scan_matcher/src/ndt_scan_matcher_core.cpp Outdated Show resolved Hide resolved

Removed unnecessary value initialization for parameters that should b…

facba81

…e defined in ndt_scan_matcher.param.yaml Signed-off-by: TaikiYamada4 <[email protected]>

KYabuuchi approved these changes Sep 28, 2023

View reviewed changes

Merge branch 'main' into refactor/ndt_scan_matcher_delete_default_values

8c59aec

TaikiYamada4 enabled auto-merge (squash) September 28, 2023 07:36

TaikiYamada4 merged commit c56423d into autowarefoundation:main Sep 28, 2023
24 of 25 checks passed

TaikiYamada4 deleted the refactor/ndt_scan_matcher_delete_default_values branch September 28, 2023 07:42

This was referenced Oct 11, 2023

chore: sync upstream tier4/autoware.universe#927

Merged

chore: sync upstream tier4/autoware.universe#928

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(ndt_scan_matcher): delete default parameters from ndt_scan_matcher_core.cpp #5155

refactor(ndt_scan_matcher): delete default parameters from ndt_scan_matcher_core.cpp #5155

TaikiYamada4 commented Sep 27, 2023 •

edited

Loading

codecov bot commented Sep 27, 2023 •

edited

Loading

KYabuuchi left a comment

refactor(ndt_scan_matcher): delete default parameters from ndt_scan_matcher_core.cpp #5155

refactor(ndt_scan_matcher): delete default parameters from ndt_scan_matcher_core.cpp #5155

Conversation

TaikiYamada4 commented Sep 27, 2023 • edited Loading

Description

Tests performed

Effects on system behavior

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

codecov bot commented Sep 27, 2023 • edited Loading

Codecov Report

KYabuuchi left a comment

Choose a reason for hiding this comment

TaikiYamada4 commented Sep 27, 2023 •

edited

Loading

codecov bot commented Sep 27, 2023 •

edited

Loading