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

simulator: set sensor update rate according to HIL_SENSOR bitmask #14228

Merged
merged 6 commits into from Feb 28, 2020

Conversation

TSC21
Copy link
Member

@TSC21 TSC21 commented Feb 25, 2020

Describe problem solved by this pull request
Fixes #14220.

Describe your solution
A check is made to the bitmask field in HIL_SENSOR Mavlink message and according to how the bitmask is set, the respective sensors get updated. This allows to properly set the sensor rate for each sensor source.

Test data / coverage
In SITL, running uorb top sensor_accel sensor_gyro sensor_baro sensor_mag:

update: 1s, num topics: 69
TOPIC NAME            INST #SUB #MSG #LOST #QSIZE
sensor_gyro              0    1  249   248 4
sensor_accel             0    1  249   248 4
sensor_gyro_integrated   0    2  249     0 1
sensor_accel_integrated  0    2  249     1 1
sensor_baro              0    1   42    41 1
sensor_mag               0    1   84    83 1
sensor_gyro_status       0    0   10     9 1
sensor_accel_status      0    2   10    18 1

Note that baro is a bit lower than 50Hz and mag lower than 100Hz. The reason is that they are dependent of the Subscription callbacks in Gazebo, which have lower priority than the OnUpdate() function thread. A possible fix can be found by running specific Mavlink publishing threads for each of these sensor sources.

@TSC21 TSC21 added Sim: SITL software in the loop simulation Sensors All the sensors! labels Feb 25, 2020
@LorenzMeier
Copy link
Member

There are build failures

@TSC21
Copy link
Member Author

TSC21 commented Feb 25, 2020

@julianoes I might need your input here: it seems that this change have uncovered some corner case on the optical flow testing that leads the vehicle to toilet bowl after it takes off - this is even more visible while flying in offboard. The interesting thing is that this behavior is not visible with other models like GPS, which leads me to discard this is a problem on the implementation of the sensor rate definition per si. I am running the MAVSDK tests on my local system with the GUI set so to check the issue visually.

After speaking with @jkflying there are three different sources that might cause the problem:

  1. The optical flow implementation expects the gyro rate to be a multiple of it's own rate, which leads the integration to fail on EKF2 - one thing I saw is that if I increase the real_time_update_rate of the world to 500Hz, the problem since to be reduced (though we might be hiding the problem like that);
  2. The integration time is not synced with the gyro timestamp, which might lead to the toilet bowl we can check;
  3. The IMU updates on accel and gyro might be inconsistent - but that would lead also to a problem on other models, which I am not seeing.

@kamilritz if you have the opportunity to evaluate this as well your input is welcomed.

@TSC21
Copy link
Member Author

TSC21 commented Feb 25, 2020

Setting an high rate to optical flow updates seems to cause the toilet bowl issue - which I suppose it shouldn't happen. The update rate of the optical flow mock is currently set to 20Hz, which seems to avoid the issue. Though, it is visible on the MAVSDK tests that on takeoff, the vehicle starts shaking a lot when it reaches the takeoff altitude - and after a couple of seconds it stabilises. I am not sure what's causing it at this point.

@TSC21 TSC21 force-pushed the fix/simulator_sensor_rate branch 2 times, most recently from 2f8e17d to edc8a21 Compare February 26, 2020 09:34
@kamilritz
Copy link
Contributor

Sending data at a higher rate to the EKF than usual can cause problems. As it will cause overflow of the buffers. For some sensors we added protection against this by downsampling the incoming data, but this is only done for mag and baro. I don't think it is done for the optical flow.
While adding the MAVSDK optical flow tests, I noticed that there are issues if I want to speed up the simulation.

@TSC21
Copy link
Member Author

TSC21 commented Feb 26, 2020

I changed the approach and right now I send the gyro and accel data on all messages and only send baro and mag according to their update rate. This seems to have fixed the issues I was seeing before. Thanks @bkueng and @bresch for the tip.

@TSC21 TSC21 force-pushed the fix/simulator_sensor_rate branch 2 times, most recently from a0c9913 to d240235 Compare February 27, 2020 10:45
@TSC21 TSC21 requested a review from julianoes February 27, 2020 12:39
@TSC21 TSC21 force-pushed the fix/simulator_sensor_rate branch 2 times, most recently from c6bc184 to de36578 Compare February 27, 2020 15:40
@TSC21 TSC21 force-pushed the fix/simulator_sensor_rate branch 2 times, most recently from 9ee9ed7 to 334aa7c Compare February 28, 2020 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Sensors All the sensors! Sim: SITL software in the loop simulation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mag/baro rates in SITL unrealistic
4 participants