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

Concatenate node algorithm didn't work the same as it described #6832

Closed
3 tasks done
vividf opened this issue Apr 17, 2024 · 6 comments
Closed
3 tasks done

Concatenate node algorithm didn't work the same as it described #6832

vividf opened this issue Apr 17, 2024 · 6 comments
Assignees
Labels
component:sensing Data acquisition from sensors, drivers, preprocessing. (auto-assigned) status:stale Inactive or outdated issues. (auto-assigned)

Comments

@vividf
Copy link
Contributor

vividf commented Apr 17, 2024

Checklist

  • I've read the contribution guidelines.
  • I've searched other issues and no duplicate issues were found.
  • I've agreed with the maintainers that I can plan this task.

Description

In the concatenate node description, it describes that if one sensor (pointcloud) fails, the node will still concatenate the remaining pointcloud. (see the last 5 pointcloud in the diagram shown below)
image

However, the algorithm implementation doesn't work as in the diagram. In the scenario when one of the Pointcloud C doesn't come, and the Pointcloud D, which already exists in the buffer, arrives at the node, the timer will reset (start) with the period timeout_sec. In this case, the timeout won't happen in t3 as the timer just starts again. Finally, Pointcloud F arrives and the current algorithm will concatenate Pointcloud D, E, F, and discard Pointcloud A, B.

In some scenarios (imagine we have top, left, and right lidars) we can control the publish time of each Lidar. We will set the most important pointcould (top) as the last-coming pointcloud. In this case, the current algorithm works fine because if the left and the right pointcloud (Pointcloud A, B) come and the top pointcloud (Pointcloud C) doesn't come. It is better to discard the left and the right pointcloud (Pointcloud A, B) as the concatenate pointcloud (without top pointcloud) might cause problems to the perception module. However, in this case, we probably need to add a parameter/algorithm that indicates which lidar is the main lidar, and when the main lidar doesn't arrive, we won't concatenate the pointcloud.

reference:
https://github.com/autowarefoundation/autoware.universe/blob/main/sensing/pointcloud_preprocessor/docs/concatenate-data.md

Purpose

Provide a more robust algorithm for concatenating pointlcloud.

Possible approaches

This is the current algorithm of the concatenate node.

concatenode drawio (1)

Possible approach:

  • add new parameters main_pointcloud_topic, which we don't concatenate the pointcloud if the pointcloud buffer lacks the main_pointcloud.
  • Instead of resetting the timer to timeout_sec when receiving Pointcloud D, we do the concatenate if the main_pointcloud is in the buffer

Example

  • If Pointcloud C is main_pointcloud, then do the same thing as the current algorithm does.
  • If Pointcloud C is not the main_pointcloud, we wait until the timeout then we publish. Or we receive Pointcloud D before the timeout, we also concatenate the pointcloud.

Definition of done

The concatenate node can handle scenarios that are required.

@vividf vividf added the component:sensing Data acquisition from sensors, drivers, preprocessing. (auto-assigned) label Apr 17, 2024
@vividf vividf self-assigned this Apr 17, 2024
@vividf
Copy link
Contributor Author

vividf commented May 9, 2024

Possible solution diagram
concatenate drawio (3)

@palas21
Copy link

palas21 commented Jun 25, 2024

@vividf according to possible solutions without specifying the main_pointcloud, what happens if there are multiple pointcloud drops and when does the timer start, is it after getting the third pointcloud into the buffer?

@vividf
Copy link
Contributor Author

vividf commented Jun 25, 2024

@palas21
Thanks for your question.
The solution won't be the one described above.
I will provide the decided solution later :)

@vividf
Copy link
Contributor Author

vividf commented Jul 12, 2024

New design for concatenate node.

image

Copy link

stale bot commented Sep 11, 2024

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 Sep 11, 2024
@vividf
Copy link
Contributor Author

vividf commented Nov 7, 2024

This PR #8300 solves the issue and will be merged soon.
Thus, I will close this issue.

@vividf vividf closed this as completed Nov 7, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Sensing & Perception Working Group Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:sensing Data acquisition from sensors, drivers, preprocessing. (auto-assigned) status:stale Inactive or outdated issues. (auto-assigned)
Projects
No open projects
Development

No branches or pull requests

2 participants