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(ndt_scan_matcher): check first old_pose_msg for initialization #2660

Conversation

meliketanrikulu
Copy link
Contributor

@meliketanrikulu meliketanrikulu commented Jan 16, 2023

melike tanrikulu [email protected]

Description

This PR related #2650 .

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.

@github-actions github-actions bot added the component:localization Vehicle's position determination in its environment. (auto-assigned) label Jan 16, 2023
@codecov
Copy link

codecov bot commented Jan 16, 2023

Codecov Report

Base: 11.38% // Head: 11.38% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (a14cd78) compared to base (2eafda6).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2660      +/-   ##
==========================================
- Coverage   11.38%   11.38%   -0.01%     
==========================================
  Files        1277     1277              
  Lines       89230    89272      +42     
  Branches    23619    23619              
==========================================
  Hits        10160    10160              
- Misses      68297    68339      +42     
  Partials    10773    10773              
Flag Coverage Δ *Carryforward flag
differential 0.00% <0.00%> (?)
total 11.38% <0.00%> (ø) Carriedforward from 2eafda6

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

Impacted Files Coverage Δ
localization/ndt_scan_matcher/src/util_func.cpp 0.00% <0.00%> (ø)
...or_path_planner/src/behavior_path_planner_node.cpp 0.16% <ø> (ø)
...anner/src/scene_module/pull_over/goal_searcher.cpp 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@YamatoAndo
Copy link
Contributor

@meliketanrikulu Thank you so much!
Looking at past code for this part, I see if (output_old_pose_cov_msg_ptr->header.stamp.toSec() == 0) {, and I certainly thought I had written that.
https://github.com/tier4/AutowareArchitectureProposal.iv/blob/v0.9.1/localization/pose_estimator/ndt_scan_matcher/ndt_scan_matcher/include/ndt_scan_matcher/util_func.h#L169

But apparently we made a mistake when we migrated from ROS1 to ROS2.
https://github.com/tier4/AutowareArchitectureProposal.iv/blob/v0.11.1/localization/pose_estimator/ndt_scan_matcher/ndt_scan_matcher/include/ndt_scan_matcher/util_func.hpp#L168

Since I was not involved in the migration process, I could not notice this mistake.
Thank you very much.

@meliketanrikulu meliketanrikulu force-pushed the fix(ndt_scan_matcher)fix_initialization_error branch 3 times, most recently from ced2019 to 8b81b70 Compare January 17, 2023 10:03
@meliketanrikulu meliketanrikulu force-pushed the fix(ndt_scan_matcher)fix_initialization_error branch from 8b81b70 to a14cd78 Compare January 17, 2023 10:11
@meliketanrikulu meliketanrikulu mentioned this pull request Jan 17, 2023
3 tasks
@meliketanrikulu meliketanrikulu self-assigned this Jan 17, 2023
@meliketanrikulu meliketanrikulu added the priority:high High urgency and importance. label Jan 17, 2023
Copy link
Contributor

@YamatoAndo YamatoAndo left a comment

Choose a reason for hiding this comment

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

LGTM!

@meliketanrikulu meliketanrikulu merged commit baefe27 into autowarefoundation:main Jan 17, 2023
maxime-clem pushed a commit to maxime-clem/autoware.universe that referenced this pull request Jan 30, 2023
…utowarefoundation#2660)

* fix(ndt_scan_matcher): check first old_pose_msg  for initialization

Signed-off-by: melike tanrikulu <[email protected]>

* fix(ndt_scan_matcher): check with old_pose_time_stamp

Signed-off-by: melike tanrikulu <[email protected]>

Signed-off-by: melike tanrikulu <[email protected]>
badai-nguyen pushed a commit to badai-nguyen/autoware.universe that referenced this pull request Mar 22, 2023
…utowarefoundation#2660)

* fix(ndt_scan_matcher): check first old_pose_msg  for initialization

Signed-off-by: melike tanrikulu <[email protected]>

* fix(ndt_scan_matcher): check with old_pose_time_stamp

Signed-off-by: melike tanrikulu <[email protected]>

Signed-off-by: melike tanrikulu <[email protected]>
badai-nguyen pushed a commit to badai-nguyen/autoware.universe that referenced this pull request Apr 4, 2023
…utowarefoundation#2660)

* fix(ndt_scan_matcher): check first old_pose_msg  for initialization

Signed-off-by: melike tanrikulu <[email protected]>

* fix(ndt_scan_matcher): check with old_pose_time_stamp

Signed-off-by: melike tanrikulu <[email protected]>

Signed-off-by: melike tanrikulu <[email protected]>
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) priority:high High urgency and importance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants