SIH-SITL integration tests

[mbjd]

Overview

This is an attempt at using SIH-as-SITL for integration tests. All mavsdk integration tests pass, but still not 100% reliably.

This has grown to quite a large collection of changes. Here I summarise the main points. Apologies for the messy nature! If you want help or have any questions, don't hesitate to approach me.

This is mostly a proof of concept. Doubts about the implementation are highlighted with 🤔. Possibilities to proceed:

• cherry pick small, self-contained changes from this to individual PRs
• re-do this entire PR (or parts of it) from the ground up, learning from this here to arrive at a cleaner implementation
• merge this PR directly, and improve the implementation subsequently
• discard it entirely in favour of a deeper overhaul of the integration tests. it would be very desirable to integrate them more tightly with PX4, avoiding the unreliable communication layer in between.

Entry point

Tests are launched with the command test/mavsdk_tests/mavsdk_test_runner.py test/mavsdk_tests/configs/sitl_sih.json. The file sets config['simulator'] = sih, which causes mavsdk_test_runner.py to do the rest. Commit: 41c9d1f

The tests are ran as a github workflow thanks to the workflow file. Commit: dd882c2

🤔 hacky implementation with setting env vars in mavsdk_test_runner.py
🤔 bloated yml file with gazebo-related stuff. Not sure if the build system even allows for building tests without gazebo, maybe deeper changes there are needed and it is not worth it to save a couple seconds per CI run.

Sensor Failures

Many tests are about sensor failures. For gazebo, these are handled in SimulatorMavlink::check_failure_injections(), with the sensors models being contained in gazebo. However, in SIH, the sensors are simulated separately by modules such as SensorGpsSim etc. Therefore, we implement the sensor failures within those modules. They are mostly a copy of the logic in SimulatorMavlink::check_failure_injections(). They are triggered from the test suite (example) but can also be manually triggered from the MAVLink console with the failure command.

Commits: 9a801c6, 4056a5d, 75dd0ce, 55a195c, c209d11

🤔 code repetition... both in terms of repeating logic from SimulatorMavlink, but also almost shared logic between the Sensor...Sim modules

For the airspeed sensor, there is also an entirely different way to simulate failure, by setting the parameter SIM_ARSPD_FAIL (see #21126). With the implementation of "proper" failures as used in the test suite this is not necessary anymore but is still possible.

Sensor Implementation

SIH itself also implements an airspeed sensor. Here we switch it off (a2a6c35) in favour of SensorAirspeedSim, where we implement the sensor failures. Notice that SensorAirspeedSim is only started if the airframe file contains param set-default SENS_EN_ARSPDSIM 1 (7576725, 96105ca) and the launch script considers that parameter (55a195c). Be sure to only apply those changes together to avoid conflicting or missing sensor data.

SensorAirspeedSim calculated the airspeed previously as dot([1, 0, 0], velocity), corresponding to the assumption that the pitot tube is mounted in body-forward direction. This is incorrect for tailsitters, in which case body-forward points forward in hovering/tail-sitting pose, but downwards in forward flight, where airspeed measurements are needed. As a simple fix, we now instead calculate airspeed as norm(velocity) (same as in SIH). Strictly speaking this is incorrect, as sideways components of the velocity now also contribute to the airspeed, which with an acutal pitot tube measurement would not happen. But in well-behaved fixed wing flight those should be small enough to ignore, and in line with the modelling accuracy standards for SIH generally. Commit: 7576725

Deterministic Simulation

To exclude the effect of sensor noise, we add a parameter SIH_NOISE_SCALE which when set to 0 disables sensor noise. The simulation is still not entirely deterministic due to timing. Commits: 4666a97, 1470a1c

🤔 This was mostly implemented as a debugging tool. If merging, we could probably drop it unless someone has a strong need.
💡 Very cool would also be to have a deterministic PRNG for reproducibility. Jax does this very nicely in the context of multiple threads but with some restrictions (pure functions i.e. no global state). But again, the advantage is probably not worth the effort.

Timing bugfixes

When running at large speed factors the tests were previously very unreliable. This was due to timing issues: When waiting for some condition or sleeping for a given amount of simulation time, this is done with real-time polling. However, if running with larger and larger speed factor, polling with the same speed (in real-time) becomes more and more inaccurate. In extreme cases this led to timeouts, with tests failing for example because the VTOL transition took too long, even though the log clearly showed that the transition occured faster than the prescribed timeout.

We alleviate this by increasing the polling frequency with the speed factor, so it stays equally fast in simulation time across all speed factors (if the simulation is slower than the set speed factor, polling becomes quicker w.r.t. simulation time. This should not be problematic unless we set extremely high, unrealistic speed factors like 10000000, in which case polling will hog the CPU). Commit: e16bf22

Also, some tests used real-time sleep statements, leading to obvious timing issues. Adapted to sim-time sleep here: 5a03cfb, PR here: #24266

🤔 Probably even sim-time sleep statements in tests are not a great idea and there should be better alternatives such as waiting explicitly until the condition we actually depend on is satisfied (or failing if not).
🤔 Fundamentally, is polling the right appproach to check conditions like these? Every relevant condition will only change once per simulation step. Can we couple the checks to that? Check not every x (real-time) milliseconds, but once at every lockstep simulation step? Or setup some interrupts for the relevant message directly?

Model & parameter changes

It is quite common for airframe files to be self-contradictory because some physical model parameters are redundant (e.g. SIH_T_MAX follows from CA_ROTOR*_CT, SIH_Q_MAX from CA_ROTOR*_KM, SIH_L_ROLL and SIH_L_PITCH from CA_ROTOR*_PX and CA_ROTOR*_PY). I corrected some of the contradictions here (350f160, 96105ca, 6779c70) but most likely I missed other ones. In the long term it would be very cool to have a minimal, non-redundant set of parameters that avoids this issue altogether and removes an opportunity for the user to mess up.

Standard VTOL: Magic number in sih.cpp is now explained as the factor by which the pusher motor is stronger than the MC motors: a2a6c35. Still hardcoded though.

Tailsitter: Channel reversal moved to SIH model to clean up config file: dd882c2

🤔 this will break all other SIH tailsitter airframes. in the PX4 repo atm there is only 1102, but externally people might have additional ones. If merging, think of some way to handle this smoothly.

Test changes

We adapt some test slightly to be more lenient w.r.t. model and tuning variations.

a2a6c35

• larger position tolerance for MC offboard control
• Higher takeoff altitude for VTOL Loiter with airspeed failure (standard VTOL)
  • For some configurations, the "wrong" (= too low) airspeed failure causes quite a significant pitch down (caused by TECS wanting to gain airspeed after reading the lower airspeed), before the failure being detected and corrected for. This pitch down is more severe with lower airspeed, and the SIH model flies at lower airspeed than the gazebo one. We also increase airspeed for this airframe in this commit.

🤔 Passing tests should also be possible to achieve by only changing the model towards the gazebo version which passes tests.... The gazebo models are sometimes very different than the SIH ones we are using here in terms of size/mass, so it's not entirely surprising that tests designed for the former sometimes fail with the latter.