(updated) SIH-SITL integration tests #25978
Draft
+1,363
−131
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+18
to
+153
| name: Testing PX4 ${{ matrix.config.model }} | ||
| runs-on: [runs-on,runner=16cpu-linux-x64,image=ubuntu22-full-x64,"run-id=${{ github.run_id }}",spot=false] | ||
| container: | ||
| image: px4io/px4-dev-simulation-focal:2021-09-08 | ||
| options: --privileged --ulimit core=-1 --security-opt seccomp=unconfined | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| config: | ||
| - {model: "quadx", latitude: "59.617693", longitude: "-151.145316", altitude: "48", build_type: "RelWithDebInfo" } # Alaska | ||
| - {model: "xvert" , latitude: "29.660316", longitude: "-82.316658", altitude: "30", build_type: "RelWithDebInfo" } # Florida | ||
| - {model: "standard_vtol", latitude: "47.397742", longitude: "8.545594", altitude: "488", build_type: "Coverage" } # Zurich | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Git Ownership Workaround | ||
| run: git config --system --add safe.directory '*' | ||
|
|
||
| - id: set-timestamp | ||
| name: Set timestamp for cache | ||
| run: echo "::set-output name=timestamp::$(date +"%Y%m%d%H%M%S")" | ||
|
|
||
| - name: Cache Key Config | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: ~/.ccache | ||
| key: sitl-ccache-${{ steps.set-timestamp.outputs.timestamp }} | ||
| restore-keys: sitl-ccache-${{ steps.set-timestamp.outputs.timestamp }} | ||
|
|
||
| - name: Cache Conf Config | ||
| run: | | ||
| mkdir -p ~/.ccache | ||
| echo "base_dir = ${GITHUB_WORKSPACE}" > ~/.ccache/ccache.conf | ||
| echo "compression = true" >> ~/.ccache/ccache.conf | ||
| echo "compression_level = 6" >> ~/.ccache/ccache.conf | ||
| echo "max_size = 120M" >> ~/.ccache/ccache.conf | ||
| echo "hash_dir = false" >> ~/.ccache/ccache.conf | ||
| ccache -s | ||
| ccache -z | ||
| - name: Build PX4 | ||
| env: | ||
| PX4_CMAKE_BUILD_TYPE: ${{matrix.config.build_type}} | ||
| run: make px4_sitl_default | ||
|
|
||
| - name: Cache Post-Run [px4_sitl_default] | ||
| run: ccache -s | ||
|
|
||
| - name: Build SITL Gazebo | ||
| env: | ||
| PX4_CMAKE_BUILD_TYPE: ${{matrix.config.build_type}} | ||
| run: make px4_sitl_default sitl_gazebo-classic | ||
|
|
||
| - name: Cache Post-Run [sitl_gazebo-classic] | ||
| run: ccache -s | ||
|
|
||
| - name: Download MAVSDK | ||
| run: wget "https://github.com/mavlink/MAVSDK/releases/download/v$(cat test/mavsdk_tests/MAVSDK_VERSION)/libmavsdk-dev_$(cat test/mavsdk_tests/MAVSDK_VERSION)_ubuntu20.04_amd64.deb" | ||
|
|
||
| - name: Install MAVSDK | ||
| run: dpkg -i "libmavsdk-dev_$(cat test/mavsdk_tests/MAVSDK_VERSION)_ubuntu20.04_amd64.deb" | ||
|
|
||
| - name: Check PX4 Environment Variables | ||
| env: | ||
| PX4_HOME_LAT: ${{matrix.config.latitude}} | ||
| PX4_HOME_LON: ${{matrix.config.longitude}} | ||
| PX4_HOME_ALT: ${{matrix.config.altitude}} | ||
| PX4_CMAKE_BUILD_TYPE: ${{matrix.config.build_type}} | ||
| run: | | ||
| export | ||
| ulimit -a | ||
| - name: Build PX4 / MAVSDK tests | ||
| env: | ||
| PX4_CMAKE_BUILD_TYPE: ${{matrix.config.build_type}} | ||
| DONT_RUN: 1 | ||
| run: make px4_sitl_default gazebo mavsdk_tests | ||
|
|
||
| - name: Cache Post-Run [px4_sitl_default sitl_gazebo-classic mavsdk_tests] | ||
| run: ccache -s | ||
|
|
||
| - name: Core Dump Settings | ||
| run: | | ||
| ulimit -c unlimited | ||
| echo "`pwd`/%e.core" > /proc/sys/kernel/core_pattern | ||
| - name: Run SITL / MAVSDK Tests | ||
| env: | ||
| PX4_HOME_LAT: ${{matrix.config.latitude}} | ||
| PX4_HOME_LON: ${{matrix.config.longitude}} | ||
| PX4_HOME_ALT: ${{matrix.config.altitude}} | ||
| PX4_CMAKE_BUILD_TYPE: ${{matrix.config.build_type}} | ||
| run: test/mavsdk_tests/mavsdk_test_runner.py --speed-factor 1 --model ${{matrix.config.model}} --upload test/mavsdk_tests/configs/sitl_sih.json --verbose | ||
| timeout-minutes: 45 | ||
|
|
||
| - name: Upload failed logs | ||
| if: failure() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: failed-${{matrix.config.model}}-logs.zip | ||
| path: | | ||
| logs/**/**/**/*.log | ||
| logs/**/**/**/*.ulg | ||
| build/px4_sitl_default/tmp_mavsdk_tests/rootfs/*.ulg | ||
| - name: Look at Core files | ||
| if: failure() && ${{ hashFiles('px4.core') != '' }} | ||
| run: gdb build/px4_sitl_default/bin/px4 px4.core -ex "thread apply all bt" -ex "quit" | ||
|
|
||
| - name: Upload PX4 coredump | ||
| if: failure() && ${{ hashFiles('px4.core') != '' }} | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: coredump | ||
| path: px4.core | ||
|
|
||
| - name: Setup & Generate Coverage Report | ||
| if: contains(matrix.config.build_type, 'Coverage') | ||
| run: | | ||
| git config --global credential.helper "" # disable the keychain credential helper | ||
| git config --global --add credential.helper store # enable the local store credential helper | ||
| echo "https://x-access-token:${{ secrets.ACCESS_TOKEN }}@github.com" >> ~/.git-credentials # add credential | ||
| git config --global url."https://github.com/".insteadof [email protected]: # credentials add credential | ||
| mkdir -p coverage | ||
| lcov --directory build/px4_sitl_default --base-directory build/px4_sitl_default --gcov-tool gcov --capture -o coverage/lcov.info | ||
| - name: Upload Coverage Information to Codecov | ||
| if: contains(matrix.config.build_type, 'Coverage') | ||
| uses: codecov/codecov-action@v4 | ||
| with: | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
| flags: mavsdk | ||
| file: coverage/lcov.info |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
The best way to fix the problem is to add an explicit permissions block specifying the minimum required scopes at the root of the workflow file (before or after the on: block, but before jobs:). For most CI workflows, contents: read is usually sufficient unless there are steps requiring more permissions, such as writing to pull requests or updating issues. In the present workflow, actions like artifact upload, caching, and using third-party actions like Codecov do not require broad write permissions, but Codecov uploads might need access to contents: read (and possibly write depending on configuration, but normally just read).
The change should be made near the top of .github/workflows/sitl_sih_tests.yml, adding a block:
permissions:
contents: readThis restricts the workflow's permissions to only reading repository contents. If further permissions are identified as needed for specific steps, those can be added in future modifications. Make this addition after the name key and before the on key, following usual workflow file conventions.
Suggested changeset
1
.github/workflows/sitl_sih_tests.yml
| @@ -5,6 +5,9 @@ | ||
|
|
||
| name: SITL Tests using SIH | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: | ||
| branches: |
Copilot is powered by AI and may make mistakes. Always verify output.
also, fix tailsitter model so actuator signs follow convention
airspeed, baro, and mag off. agp has no failures, and other failure types like stuck etc not implemented at all. doubting this implementation, lots of repetitions and almost-repetitions. maybe we should do it more similar to SimulatorMavlink.cpp? Where simulator_sih handles the sensor failures itself, with one big if mess, but the rest of it nice and lean. failing tests for quadx: - 'Continue on baro stuck during mission (baro height mode)': failed - 'Continue on baro stuck during mission (GPS height mode)': failed - 'Fly straight Multicopter Mission': failed - 'Offboard takeoff and land': failed - 'Offboard position control': failed
this makes SIH artificial sensor noise smaller or larger. useful for deterministic-ish sim. setting it to 0 does not work right now for mysterious reasons.
Previously, DataValidator would automatically reject sensor data as stale if (almost) constant. But if setting SIH_NOISE_SCALE = 0 for deterministic sim, constant sensor data are to be expected. This adds logic to not flag sensor values as stale if noise scale is very small. If the sensor has *actually* gone stale with very low noise scale, this means we cannot detect it anymore.
AFAIK this is not done anywhere else for SIH-as-SITL. Just below for gazebo it is already present. the corresponding check for SIH-on-FC happens at ROMFS/px4fmu_common/init.d/rcS:388 - also no airspeed is enabled anywhere. is that intended? also, handle airspeed failure correctly in SensorAirspeedSim.cpp
previously I removed this assuming that it would add the forward thrust as a resulting force. but it just considers its influence on the wing segment.
the original std::this_thread::sleep_for is with respect to host system time which is different from autopilot time if speed factor != 1, if some component runs slower than real time, or if debugging. tester.sleep_for (which already existed) correctly sleeps w.r.t. px4/simulation time.
maybe this is a band-aid fix, and we actually should make sure the transition can be achieved in the given 5 seconds, as it is very reliably in the corresponding gazebo test case. check: - does SIH_T_MAX make sense? - does SIH_MASS make sense? - and any other model parameters which are suspicious
- higher airspeed targets
- higher pusher thrust
- higher takeoff attitude (arsp failure test)
in the airspeed failure case, the lowered airspeed reading causes the
controller to want to speed up until the (wrong) airspeed is at the
setpoint, i.e. the real airspeed quite a bit faster. without these
fixes, tecs does a nosedive to regain airspeed, but never becomes fast
enough (with already maxed out pusher thrust). we pervent this with
the first two adaptations, while the last one gives space for the
remaining nosedive.
fix this permanently by:
- ensuring the model makes sense definitely (pusher thrust, mass, aero
properties)
- noticing the failure faster
- adapting tecs so it doesn't nosedive?
- stop sending airspeed in sih
src/modules/simulation/sensor_airspeed_sim/SensorAirspeedSim.cpp
is already doing that. in the sensor failure case (wrong), that implements
the failure, but the SIH one not, giving conflicting data. so switch
it off
- larger acceptance radius
quadx position mode is just not that accurate. is it a control/model
mismatch problem, or a simulation problem?
- wait longer for disarm in test_vtol_rtl
this would fail at large speed factors previously. timing bug or
acceptable small variation?
SensorAirspeedSim has hardcoded pitot tube direction in body front velocity. for tailsitter, this points DOWN in forward flight, thus the simulated sensor would read 0 in forward flight for tailsitters. fixed by using norm(velocity) instead like in sih.
If sim is running faster, we should also poll faster to keep the same timing resolution. This fixed quite a large number of test failures.
* gz: use server config file for loading world plugins * submodule * use server.config in tree * newlines * format * gzbridge: rename function * format * gzbridge: add magnetometer callback * change gz_find_package to find_package * fix up directory structure and cmake to allow multiple plugins * newlines * add comment block explaining gz_env.sh * remove dupe readme * remove SENS_EN_MAGSIM from all gz airframe files except spacecraft * update gz submodule
Co-authored-by: Crowdin Bot <[email protected]>
Co-authored-by: Crowdin Bot <[email protected]>
* sim: refactor px4-rc.simulator into sim specific startup scripts. Fix gz sim for standalone mode. * shellcheck disable=SC2154
* Add new INS driver sbgECom Implement sbgECom messages handling to provide IMU sensors, GNSS and EKF data to the autopilot Be able to parametrize the serial port, baudrate and the communicating mode Clone sbgECom library only if sbgecom support is enabled and apply a patch Be able to send SBG Systems INS settings in several ways when starting sbgecom driver * Fix sensor airspeed simulator units * Fix HIGHRES_IMU pressure unit * Allow HIGHRES_IMU to support 4 IMUs * sbgECom: Add documentation * Use submodule instead of fetching sbgECom using CMake * Remove patch strategy * Fix dates * Remove patch file * Update SBG dev type ID Co-authored-by: Jacob Dahl <[email protected]> --------- Co-authored-by: Samuel Toledano <[email protected]> Co-authored-by: Jacob Dahl <[email protected]>
mrpollo
force-pushed
the
mrpollo/mbjd-sih_sitl_testing
branch
from
4befc35 to
6e40154
Compare
julianoes
requested changes
Nov 24, 2025
| mavsdk::Mavsdk _mavsdk{Mavsdk::Configuration{ComponentType::GroundStation}}; | ||
| ======= | ||
| mavsdk::Mavsdk _mavsdk{}; | ||
| >>>>>>> 5a03cfb986e (mavsdk tests: use tester sleep function) |
Contributor
There was a problem hiding this comment.
This needs updating the v3 ctor. I think the top one is right.
See: https://mavsdk.mavlink.io/main/en/cpp/api_changes.html#mavsdk-configuration
| return result | ||
|
|
||
|
|
||
| class Tester: |
Contributor
There was a problem hiding this comment.
This looks like a copy of test/mavsdk_tests/integration_test_runner/test_runner.py. Probably needs consolidation or cleanup.
| @@ -0,0 +1,26 @@ | |||
| { | |||
| "mode": "sitl", | |||
| "simulator": "sih", | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Attempt to rebase #24237
Likely lots of rebase errors, please be careful reviewing as I might have mistakenly introduced regressions