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

thread-caused malfunction #104

Closed
DrShIkIgAmy opened this issue Apr 17, 2023 · 5 comments
Closed

thread-caused malfunction #104

DrShIkIgAmy opened this issue Apr 17, 2023 · 5 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@DrShIkIgAmy
Copy link

Hi @Samahu , there're some critical issues in ros2 version (at least ros2 version) srcs.
Case: Since it is components, I'd like to run it under container. I have two lidars and I have to run two os_cloud_node and two os_sensor_node. To obtain resulting point cloud I have to run merger_component (custom component). Now here we got a problem: to provide effective intra-process comm I have to run all five nodes under one container, so it would be one process with several threads. At this point I face at least two problems. First one is: the sensor reinits continuously. The second one is: pointcloud messages could get corrupted.

static uint32_t last_init_id = current_init_id + 1;

static pcl::PCLPointCloud2 pcl_pc2;

The problem lays in the static vars. I can't even imaging what for u've used the static var in these cases, but it is big mistake.

@DrShIkIgAmy DrShIkIgAmy added the bug Something isn't working label Apr 17, 2023
@Samahu Samahu self-assigned this Apr 17, 2023
@Samahu
Copy link
Contributor

Samahu commented Apr 18, 2023 via email

@DrShIkIgAmy
Copy link
Author

I appreciate it. I thought that it's common to use at least two lidars on autonomous object, so the static vars look like a bug to me, so I tagged it like a bug. I don't know that it is not considered to run under one process. But I would be nice to implement that possibility, 'cause it could increase efficiency of node communication.

@Samahu
Copy link
Contributor

Samahu commented Apr 20, 2023

@DrShIkIgAmy Could you try any of the two pull requests and check if this completely resolves the issue for your use case or not?

Thanks
Ussama

@Samahu Samahu added the enhancement New feature or request label Apr 25, 2023
@Samahu
Copy link
Contributor

Samahu commented Apr 28, 2023

@DrShIkIgAmy Any luck verifying that the changes included in #108 and #109 fully addresses this issue? I am ready to merge it but I don't want to do so right away and then find out that I missed something that still prevents you from spawning multiple components of same type within the same process.

@Samahu
Copy link
Contributor

Samahu commented May 2, 2023

@DrShIkIgAmy I haven't heard back from you since last week, I did run my own assessment which includes setting up multiple sensors and cloud nodes within a single process. That being said I went ahead and merged the fix. Let me know if this didn't fully address your problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants