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.

Sign up for GitHub

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

Jump to bottom

feat(map_loader): warn if some pcds from the metadata file are missing #7406

Merged
merged 13 commits into from
Jun 15, 2024
Merged

feat(map_loader): warn if some pcds from the metadata file are missing #7406

merged 13 commits into from
Jun 15, 2024

Conversation

anhnv3991
Copy link
Contributor

@anhnv3991 anhnv3991 commented Jun 10, 2024 •
edited by KYabuuchi
Loading

Description

  • In map_loader, file a warning if some pcds from the metadata file were not found in the input pcd paths.

Tests performed

  • Odaiba map (any version)
  • Use pointcloud_divider to divide the input PCD map into non-overlapped segments and generate a metadata file
  • Delete some segment PCDs
  • Launch the logging simulator and the startup terminal will shows the list of missing segment PCDs.

Effects on system behavior

The node exits under the following conditions (previously, the node remained but couldn't use the map):

  • The point cloud is divided but metadata is missing
  • The pcd file cannot be loaded
  • The pcd files listed in the metadata do not exist

Interface changes

None

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:map Map creation, storage, and loading. (auto-assigned) label Jun 10, 2024
@KYabuuchi KYabuuchi changed the title feat(map_loader): Warn if some pcds from the metadata file are missing feat(map_loader): warn if some pcds from the metadata file are missing Jun 12, 2024
@KYabuuchi KYabuuchi added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Jun 12, 2024
@KYabuuchi
Copy link
Contributor

@anhnv3991 The pre-commit CI is failing. Please check the details and make the necessary corrections. 🙏

Also, pcds is judged as spell-error, please change it to something like pcd_paths. 🙏

And, the first letter of the PR title cannot be capitalized. I have corrected it this time. 👌

  • before: feat(map_loader): Warn if some pcds from the metadata file are missing
  • after: feat(map_loader): warn if some pcds from the metadata file are missing

KYabuuchi
KYabuuchi requested changes Jun 12, 2024
View reviewed changes
Copy link
Contributor

@KYabuuchi KYabuuchi left a comment

Choose a reason for hiding this comment

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

Please change it so that the node does not continue if some PCD files are missing. 🙏

map/map_loader/src/pointcloud_map_loader/pointcloud_map_loader_node.cpp Outdated Show resolved Hide resolved
anhnv3991 added 4 commits June 13, 2024 10:39
@anhnv3991
Fixing CI
a2f488d 
Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>
@anhnv3991
Fixing CI
51a57e2 
Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>
@anhnv3991
Merge branch 'main' into feature/warn_missing_pcd
a557315
@anhnv3991
Fixing CI
d3a61f4 
Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>
KYabuuchi
KYabuuchi reviewed Jun 13, 2024
View reviewed changes
Copy link
Contributor

@KYabuuchi KYabuuchi 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 for making some corrections.
The pre-commit is still issuing a warning, suggesting that you add #include <set> , so please fix that.

@anhnv3991
Copy link
Contributor Author

anhnv3991 commented Jun 13, 2024 •
edited
Loading

@KYabuuchi

suggesting that you add #include

Just curious but why did the compilation of this test file not raise any warning or error when building locally on my PC?

@anhnv3991
Fix CI related to map_loader
75553d6 
Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>
@KYabuuchi
Copy link
Contributor

@anhnv3991 Strictly speaking, that does not result in a compilation error. The compilation succeeds because they are included from other header files. This pre-commit forces us to directly include the standard libraries we actually use in the source files.

@anhnv3991 anhnv3991 requested a review from KYabuuchi June 14, 2024 03:09
KYabuuchi
KYabuuchi approved these changes Jun 14, 2024
View reviewed changes
Copy link
Contributor

@KYabuuchi KYabuuchi left a comment

Choose a reason for hiding this comment

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

Looks Good To Me!
Thank you for patiently responding to my reviews. 🙏

I made a few edits to the PR description.

image

@anhnv3991 anhnv3991 merged commit ea41a5c into autowarefoundation:main Jun 15, 2024
25 of 26 checks passed
simon-eisenmann-driveblocks pushed a commit to simon-eisenmann-driveblocks/autoware.universe that referenced this pull request Jun 26, 2024
@anhnv3991 @pre-commit-ci @simon-eisenmann-driveblocks
feat(map_loader): warn if some pcds from the metadata file are missing (
1127aca 
autowarefoundation#7406)

* Examine if there are PCD segments found in the metadata file but are missing from the input pcd paths

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>

* style(pre-commit): autofix

* Fixing CI

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>

* Fixing CI

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>

* Fixing CI

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>

* Fix CI related to map_loader

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>

* Removed try{} block from getPCDMetadata and redundant std::endl at the end of error messages

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>

---------

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: Simon Eisenmann <simon.eisenmann@driveblocks.ai>
KhalilSelyan pushed a commit that referenced this pull request Jul 22, 2024
@anhnv3991 @pre-commit-ci @KhalilSelyan
feat(map_loader): warn if some pcds from the metadata file are missing (
b38dfd4 
#7406)

* Examine if there are PCD segments found in the metadata file but are missing from the input pcd paths

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>

* style(pre-commit): autofix

* Fixing CI

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>

* Fixing CI

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>

* Fixing CI

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>

* Fix CI related to map_loader

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>

* Removed try{} block from getPCDMetadata and redundant std::endl at the end of error messages

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>

---------

Signed-off-by: Anh Nguyen <anh.nguyen.2@tier4.jp>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KYabuuchi KYabuuchi KYabuuchi approved these changes

@kminoda kminoda Awaiting requested review from kminoda

@Motsu-san Motsu-san Awaiting requested review from Motsu-san

@mitsudome-r mitsudome-r Awaiting requested review from mitsudome-r

@RyuYamamoto RyuYamamoto Awaiting requested review from RyuYamamoto

@SakodaShintaro SakodaShintaro Awaiting requested review from SakodaShintaro

@TaikiYamada4 TaikiYamada4 Awaiting requested review from TaikiYamada4

@YamatoAndo YamatoAndo Awaiting requested review from YamatoAndo

Assignees
No one assigned
Labels
component:map Map creation, storage, and loading. (auto-assigned) run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@anhnv3991 @KYabuuchi @YamatoAndo