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(euclidean_cluster): use point_cloud_msg_wrapper #1200

Closed
wants to merge 6 commits into from

Conversation

esteve
Copy link
Contributor

@esteve esteve commented Jun 30, 2022

Description

This PR replaces the standard point cloud iterators from the sensor_msgs package with Point Cloud Message Wrapper

Related links

#1199

Tests performed

Notes for reviewers

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.

  • The PR follows the pull request guidelines.
  • The PR has been properly tested.
  • The PR has been reviewed by the code owners.

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.
  • The PR is ready for merge.

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

@esteve esteve self-assigned this Jun 30, 2022
@esteve esteve changed the title Migrate cloud iterator to point_cloud_msg_wrapper feat(euclidean_cluster): use point_cloud_msg_wrapper Jun 30, 2022
@esteve esteve force-pushed the point-cloud-msg-wrapper branch 3 times, most recently from d6ac66d to e331001 Compare June 30, 2022 10:43
@codecov
Copy link

codecov bot commented Jun 30, 2022

Codecov Report

Base: 10.40% // Head: 10.40% // No change to project coverage 👍

Coverage data is based on head (a07358a) compared to base (a07358a).
Patch has no changes to coverable lines.

❗ Current head a07358a differs from pull request most recent head 134c396. Consider uploading reports for the commit 134c396 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1200   +/-   ##
=======================================
  Coverage   10.40%   10.40%           
=======================================
  Files        1169     1169           
  Lines       83489    83489           
  Branches    19549    19549           
=======================================
  Hits         8685     8685           
  Misses      65301    65301           
  Partials     9503     9503           
Flag Coverage Δ
total 10.38% <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.

@esteve esteve force-pushed the point-cloud-msg-wrapper branch from e331001 to 04a6229 Compare July 6, 2022 17:53
@esteve esteve marked this pull request as ready for review July 6, 2022 17:54
@esteve esteve requested review from kenji-miyake and xmfcx July 6, 2022 17:54
@kenji-miyake
Copy link
Contributor

@yukkysaito @aohsato @miursh Could you review this? 🙏

@esteve esteve marked this pull request as draft July 6, 2022 17:57
@esteve
Copy link
Contributor Author

esteve commented Jul 6, 2022

@kenji-miyake sorry, this is still not ready, there are some more places in euclidean_cluster where I need to migrate to point_cloud_msg_wrapper

@kenji-miyake
Copy link
Contributor

Okay, when you are ready, please let them know again!

@esteve esteve force-pushed the point-cloud-msg-wrapper branch from 35233af to 4feeb7e Compare July 7, 2022 10:54
@esteve esteve marked this pull request as ready for review July 7, 2022 12:55
@esteve
Copy link
Contributor Author

esteve commented Jul 8, 2022

@kenji-miyake @yukkysaito @aohsato @miursh this is now read for review, but I have a question about how to store RGB values (see #1200 (comment))

@esteve esteve force-pushed the point-cloud-msg-wrapper branch 2 times, most recently from 256e891 to 5999c51 Compare July 19, 2022 10:17
@yukkysaito
Copy link
Contributor

Note:
Waiting for documents for point types.
#1200 (comment)

@xmfcx xmfcx self-requested a review September 20, 2022 16:23
@xmfcx xmfcx requested review from kaancolak and removed request for kenji-miyake, yukkysaito, aohsato, miursh and xmfcx October 4, 2022 15:57
esteve and others added 6 commits October 6, 2022 12:47
Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
@esteve esteve force-pushed the point-cloud-msg-wrapper branch from 5999c51 to 134c396 Compare October 6, 2022 10:47
@esteve
Copy link
Contributor Author

esteve commented Oct 6, 2022

@yukkysaito @mitsudome-r @kaancolak @xmfcx I've rebased this PR and fixed the conflicts, is it still waiting on #1200 (comment) ?

@mitsudome-r
Copy link
Member

For the distortion correction, we reverted migration to point_cloud_msg_wrapper.
Should we wait for this to be fixed?

@stale
Copy link

stale bot commented Jan 21, 2023

This pull request has been automatically marked as stale because it has not had recent activity.

@stale stale bot added the status:stale Inactive or outdated issues. (auto-assigned) label Jan 21, 2023
@yukkysaito yukkysaito added the component:perception Advanced sensor data processing and environment understanding. (auto-assigned) label Sep 21, 2023
@miursh
Copy link
Contributor

miursh commented Sep 21, 2023

@mitsudome-r @esteve
Do we still have the motivation to use this point_cloud_msg_wrapper?
If not, would it be acceptable to close this PR for now?

@stale stale bot removed the status:stale Inactive or outdated issues. (auto-assigned) label Sep 21, 2023
@miursh
Copy link
Contributor

miursh commented Oct 26, 2023

We are closing this issue for now since there has been no response for a month. Please feel free to reopen it if the issue still persists.

@miursh miursh closed this Oct 26, 2023
@esteve
Copy link
Contributor Author

esteve commented Oct 26, 2023

@miursh thanks

badai-nguyen pushed a commit to badai-nguyen/autoware.universe that referenced this pull request Jun 4, 2024
…avoidance

feat(avoidance): wait and see ambiguous stopped vehicle (autowarefoundation#6631)
@mitsudome-r mitsudome-r deleted the point-cloud-msg-wrapper branch June 10, 2024 08:07
iwatake2222 pushed a commit to iwatake2222/autoware.universe that referenced this pull request Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:perception Advanced sensor data processing and environment understanding. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants