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

Create a new MutuallyExclusive callback group for every timer, use a multithreaded executor #40

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

nikola-j
Copy link

Fixes: microsoft#4428
Fixes: microsoft#4569 (comment)

About

This will fix the issue by using a multithreaded executor and adding a MutuallyExclusive callback group to every lidar, camera, and airsim control timer. That way they can all execute in parallel and not block each other. Also removed unreachable code in airsim_node.cpp.
This pr also fixes the issue of timestamps being mixed up, the previous iteration used reentrant callbacks which allowed out-of-order execution of timers.

How Has This Been Tested?

Added a settings file with multiple cameras and a lidar, did a /airsim_node/local_position_goal service call.

Screenshots (if appropriate):

The following are two videos depicting ros2 performance with and without the fixes:
https://drive.google.com/file/d/1uIuTfWGhaD56h-wfzxXaOMoVkZL451wI/view?usp=sharing
https://drive.google.com/file/d/1M9JUGOquZmnV6HBqtH7uRjbOCeHgpJfi/view?usp=share_link

@nikola-j
Copy link
Author

@alonfaraj

@xxEoD2242
Copy link

I will review and approve today.

@alonfaraj
Copy link

Hi @nikola-j!

Can please share a benchmark for a camera topic with and without that fix?
Just ros2 topic hz <topic> will be great.

@nikola-j
Copy link
Author

I get 20hz with and without the fix. It seems that by default the camera processing has a higher priority than drone control and that's why the drone is stuttering.

Copy link

@xxEoD2242 xxEoD2242 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! I'll merge

@xxEoD2242 xxEoD2242 merged commit d0bade6 into CodexLabsLLC:main Jul 18, 2023
@nikola-j nikola-j deleted the multi_threaded branch November 22, 2023 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants