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

Multithreaded rtabmap node #1214

Merged
merged 8 commits into from
Oct 16, 2024
Merged

Multithreaded rtabmap node #1214

merged 8 commits into from
Oct 16, 2024

Conversation

matlabbe
Copy link
Member

@matlabbe matlabbe commented Sep 29, 2024

Addressing rtabmap node message_filters callback delay. This PR implements the suggested approach for ros2 from this comment: #1054 (comment)

CommonDataSubscriber virtual functions:

  • CommonDataSubscriber::commonMultiCameraCallback() thread-safe
  • CommonDataSubscriber::commonLaserScanCallback() thread-safe
  • CommonDataSubscriber::commonOdomCallback() thread-safe
  • CommonDataSubscriber::commonSensorDataCallback() thread-safe

Possible issues (I think this also applies to old approach):

  • sync messages are dropped if rtabmap is currently processing data, so it means rtabmap may not detect when we reset odometry or and may not detect large covariances. The logic to keep largest covariance and to detect odometry resets could be done on message_filters thread, unless we buffer all odometry messages and send them all to processing thread (<--I think this would require less changes).
  • a variance of the previous point: we could assume that the virtual functions of CommonDataSubscriber are called from synchronization thread. For examples, CoreWrapper::commonMultiCameraCallback() does independent pre-processing of the data before sending a SensorData to CoreWrapper::process(), there could be where we call the timer callback to change thread (note that we can skip all messages conversions if we know rtabmap is currently processing previous data, and just do the odometry update doing stuff of the previous point).

Update: Related to above issues, even if now we process all sync topics for odometry checks, we can miss some of them if odometry topic cannot be exactly synchronized with images/lidar data. Ideally, we should remove odometry topic from message_filters and process them independently. I'll check if this could be done easily, otherwise I'll create an issue about that and merge this PR. Added issue here: #1222

@matlabbe matlabbe added the ros2 label Sep 29, 2024
@matlabbe matlabbe changed the base branch from master to ros2 September 29, 2024 05:48
@matlabbe matlabbe marked this pull request as draft September 29, 2024 05:48
@matlabbe matlabbe marked this pull request as ready for review September 29, 2024 20:52
@matlabbe matlabbe merged commit ad97f56 into ros2 Oct 16, 2024
2 checks passed
@matlabbe matlabbe deleted the multithreaded_rtabmap_node branch October 16, 2024 04:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant