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

feat(tier4_localization_msgs): add reliability for localization pose #143

Merged
merged 4 commits into from
Aug 7, 2024

Conversation

RyuYamamoto
Copy link
Contributor

@RyuYamamoto RyuYamamoto commented Jul 22, 2024

Related Links

autowarefoundation/autoware.universe#8275

Description

Added reliability variable to PoseWithCovarianceStamped.csv.

For the following PR, I would like to add reliability variable to check the reliability of the initial pose estimation result with localization score.
autowarefoundation/autoware.universe#8275

As a reason for that I want to separate bool variable for whether initial pose estimation has completed successfully and whether initial pose estimation results is reliable.

bool success # check estimation has completed successfully
bool reliability # check result is reliable  

Remarks

Pre-Review Checklist for the PR Author

PR Author should check the checkboxes below when creating the PR.

  • Code is properly formatted
  • Assign PR to reviewer

Checklist for the PR Reviewer

Reviewers should check the checkboxes below before approval.

  • Commits are properly organized and messages are according to the guideline
  • Code is properly formatted
  • PR title describes the changes

Post-Review Checklist for the PR Author

PR Author should check the checkboxes below before merging.

  • All open points are addressed and tracked via issues or tickets
  • Write release notes

CI Checks

  • Build and test for PR: Required to pass before the merge.
  • Check spelling: NOT required to pass before the merge. It is up to the reviewer(s). See here if you want to add some words to the spell check dictionary.

@RyuYamamoto RyuYamamoto marked this pull request as ready for review August 1, 2024 13:40
@yukkysaito
Copy link
Collaborator

As a reason for that, I want to separate the boolean variable for whether the initial pose estimation has completed successfully and whether the initial pose estimation result is reliable.

Could you please confirm the clear distinction between "success" and "reliable"? Does the "success" field become false when the initial pose estimation wasn't performed in the first place? When the initial pose estimation is clearly incorrect, does "success" become false and "reliable" also become false?

@RyuYamamoto
Copy link
Contributor Author

@yukkysaito

If failure to run initial pose estimation with some reason and could not get result, set success to false.
For example, following reasons.

  • ros service is not start
  • sensor points / map points is not set on the side ndt_scan_matcher
  • etc...

In that case, without checking reliable, transition UNINTIALIZED Status.

reliable is used to check whether result of initial pose estimation was output but reliability of pose was good or bad.

@yukkysaito
Copy link
Collaborator

@yukkysaito

If failure to run initial pose estimation with some reason and could not get result, set success to false. For example, following reasons.

  • ros service is not start
  • sensor points / map points is not set on the side ndt_scan_matcher
  • etc...

In that case, without checking reliable, transition UNINTIALIZED Status.

reliable is used to check whether result of initial pose estimation was output but reliability of pose was good or bad.

Thank you for your explanation. I understand that there are reasonable grounds. 👍

@RyuYamamoto
Copy link
Contributor Author

I will also include a comment in srv files.

Signed-off-by: RyuYamamoto <[email protected]>
@RyuYamamoto
Copy link
Contributor Author

@yukkysaito @mitsudome-r Could you please check the PR?

@yukkysaito
Copy link
Collaborator

Thank you. The changes themselves seem good.

Due to various circumstances, we need to carefully consider the impact of this PR within the company. Please wait a moment.

Copy link
Collaborator

@yukkysaito yukkysaito left a comment

Choose a reason for hiding this comment

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

LGTM

@yukkysaito yukkysaito merged commit 12504f6 into tier4:tier4/universe Aug 7, 2024
9 checks passed
@RyuYamamoto RyuYamamoto deleted the feat/add_reliability branch August 7, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants