CodexLabsLLC / Colosseum

Open source simulator for autonomous robotics built on Unreal Engine with support for Unity
https://codexlabsllc.github.io/Colosseum/
Other
373 stars 119 forks source link

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

Closed nikola-j closed 1 year ago

nikola-j commented 1 year ago

Fixes: https://github.com/microsoft/AirSim/issues/4428 Fixes: https://github.com/microsoft/AirSim/issues/4569#issuecomment-1165480040

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 commented 1 year ago

@alonfaraj

xxEoD2242 commented 1 year ago

I will review and approve today.

alonfaraj commented 1 year ago

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 commented 1 year ago

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.