PX4 / PX4-SITL_gazebo-classic

Set of plugins, models and worlds to use with OSRF Gazebo Simulator in SITL and HITL.
http://dev.px4.io/simulation-gazebo.html
376 stars 791 forks source link

Running Multiple Drones Breaks Custom Sim Speed #879

Open SpBills opened 2 years ago

SpBills commented 2 years ago

Whenever setting the PX4_SIM_SPEED_FACTOR with multiple aircraft in Gazebo, it accumulates real_time_update_rate and thus the correctness math checking is incorrect, where it checks if the reciprocal of real_time_update_rate is equal to max_step_size.

Here is where the value is "accumulated" for every aircraft: https://github.com/PX4/PX4-SITL_gazebo/blob/5610c3fb441a2f3babc8ad7a63c8c4ce3e40abfa/src/gazebo_mavlink_interface.cpp#L408

Here is where the math is done for correctness: https://github.com/PX4/PX4-SITL_gazebo/blob/5610c3fb441a2f3babc8ad7a63c8c4ce3e40abfa/src/gazebo_mavlink_interface.cpp#L395

Note that this is not a problem with a default sim speed factor of 1, since 1 will multiply to be itself and thus not accumulate or change.

https://discuss.px4.io/t/px4-sim-speed-factor-on-multiple-drones-in-gazebo/28237

I have a solution that's working for me. I'll submit a PR with the changes.

SpBills commented 2 years ago

Note that you can recreate this by running PX4_SIM_SPEED_FACTOR=4 Tools/gazebo_sitl_multiple_run.sh -n2 from the PX4 parent directory.

SpBills commented 2 years ago

https://github.com/PX4/PX4-SITL_gazebo/pull/880 Here's the PR :)

Edit: It's a pretty naive solution, and I'd love to hear if there's a way to do this by setting the config for each individual aircraft instead of globally.

mnumanuyar commented 1 year ago

Hi @Jaeyoung-Lim

I am interested in this issue. However I am a little confused.

What is the purpose of the sanity check 1.0 / real_time_update_rate != max_step_size ?

I am asking this because I think real_time_update_rate *= speed_factor_; defeats the purpose of it.

depending on your answer my suggestion is either:

  1. change max_step_size so sanity check holds
  2. change sanity check to 1.0 / (real_time_update_rate/speed_factor_) != max_step_size or something equavalent and do it after the change.

of course these do not solve the accumulation problem. But in second case we can use the suggested method in the merge request (checking and setting) or simply always set real_time_update_rate to speed_factor_/max_step_size without any consideration to previous value and without any check.

Note that I am not familiar with this project

mnumanuyar commented 1 year ago

in adition i noticed real_time_update_rate_int is ignored in merge request.Assuming we set real_time_update_rate = speed_factor_/max_step_size; what should it be?

1.0 / max_step_size *250 or real_time_update_rate_int / 250.

mnumanuyar commented 1 year ago

I have tried, simulation did run, and I also checked value via console messages; but "real time factor" in gazebo GUI did not change according to speedfactor so I am not sure if it is worked or something is wrong.

Any suggestions?