Add SITL integration tests with PX4 SIH container#14261
Add SITL integration tests with PX4 SIH container#14261
Conversation
Guard the addImageProvider call with a null check on qmlAppEngine(). In unit test mode, QQmlApplicationEngine is never created (tests run headless), so accessing it crashes when a real PX4 SITL connection triggers PX4AutoPilotPlugin::vehicleComponents() during parameter load completion. MockLink tests never hit this because MockLink doesn't provide COMP_METADATA_TYPE_ACTUATORS, so ActuatorComponent is never instantiated. Real PX4 SITL does provide actuator metadata. Signed-off-by: Ramon Roche <mrpollo@gmail.com>
Run QGC against a real PX4 SITL instance via Docker to validate MAVLink protocol behavior that MockLink cannot exercise: real UDP packet handling, parameter sync with retries, mission protocol with NACKs, standard modes negotiation, and command ACK semantics. Tests are gated behind -DQGC_SITL_TESTS=ON (off by default) and require Docker with the px4io/px4-sitl-sih container image. MAVLink protocol tests (flight-stack-agnostic): - SITLHeartbeatTest: detection + communication loss - SITLParamSyncTest: full download + write/read round-trip - SITLMissionTest: upload/download round-trip - SITLCommandTest: COMMAND_ACK handling - SITLStandardModesTest: AVAILABLE_MODES validation PX4-specific tests: - PX4ModesTest: mode transition display fidelity - PX4LifecycleTest: arm/takeoff/land/disarm + firmware identification Infrastructure: - SITLTestBase manages Docker container lifecycle per test method - Fresh container per test for complete isolation (~5s overhead) - PX4 SITL image pinned by digest in .github/px4-sitl-digest.txt - SITL label + 300s timeout in CTest, check-sitl convenience target - Non-blocking px4-sitl-test job in linux.yml CI workflow Signed-off-by: Ramon Roche <mrpollo@gmail.com>
github-actions
bot
added
github_actions
There was a problem hiding this comment.
Pull request overview
Adds an optional SITL integration test suite that runs QGroundControl against a real PX4 SITL-SIH instance in Docker to validate MAVLink behavior that MockLink can’t exercise, plus CI wiring to run these tests on Linux (non-blocking while stabilizing).
Changes:
- Introduces a Docker-backed SITL test base and new MAVLink protocol + PX4-specific SITL test cases.
- Adds build/test toggles and CTest labeling/timeout support for SITL tests (including a
check-sitltarget). - Adds a CI job to run SITL tests on Ubuntu, and fixes a null
QQmlEnginecrash path inActuatorComponent.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTestFramework/UnitTest.h | Adds new test labels for SITL and MAVLink protocol categories. |
| test/UnitTestFramework/UnitTest.cc | Maps new labels to strings for filtering/introspection. |
| test/SITL/SITLTestBase.h | Introduces SITL test base interface (container + UDP link lifecycle). |
| test/SITL/SITLTestBase.cc | Implements Docker container management, UDP link setup, and log capture. |
| test/SITL/PX4/PX4SITLTestBase.h | Adds PX4 SITL helpers (arm/takeoff/land/mode changes + telemetry waits). |
| test/SITL/PX4/PX4SITLTestBase.cc | Implements PX4 command/telemetry helper logic for SITL tests. |
| test/SITL/PX4/tst_PX4Modes.h | Declares PX4 mode transition test. |
| test/SITL/PX4/tst_PX4Modes.cc | Implements PX4 mode transition assertions. |
| test/SITL/PX4/tst_PX4Lifecycle.h | Declares PX4 lifecycle + firmware identification tests. |
| test/SITL/PX4/tst_PX4Lifecycle.cc | Implements arm/takeoff/land/disarm flow and PX4 identification checks. |
| test/SITL/PX4/CMakeLists.txt | Wires PX4 SITL tests into the build and registers them with CTest. |
| test/SITL/MAVLink/tst_MAVLinkHeartbeat.h | Declares heartbeat + comm-loss tests. |
| test/SITL/MAVLink/tst_MAVLinkHeartbeat.cc | Implements heartbeat detection and comm-loss assertion. |
| test/SITL/MAVLink/tst_MAVLinkParamSync.h | Declares parameter sync and round-trip tests. |
| test/SITL/MAVLink/tst_MAVLinkParamSync.cc | Implements parameter-ready and parameter write/read test logic. |
| test/SITL/MAVLink/tst_MAVLinkMission.h | Declares mission upload/download test. |
| test/SITL/MAVLink/tst_MAVLinkMission.cc | Implements mission write + load-back validation. |
| test/SITL/MAVLink/tst_MAVLinkCommand.h | Declares command ACK/rejection tests. |
| test/SITL/MAVLink/tst_MAVLinkCommand.cc | Implements command-related assertions (currently minimal). |
| test/SITL/MAVLink/tst_MAVLinkStandardModes.h | Declares AVAILABLE_MODES / standard modes test. |
| test/SITL/MAVLink/tst_MAVLinkStandardModes.cc | Implements standard mode presence checks. |
| test/SITL/MAVLink/resources/simple_square.plan | Adds a sample mission plan resource. |
| test/SITL/MAVLink/CMakeLists.txt | Builds MAVLink SITL tests and registers them with CTest. |
| test/SITL/CMakeLists.txt | Adds SITL test subdirectories gated behind QGC_SITL_TESTS. |
| test/CMakeLists.txt | Adds the SITL test subtree to the test build. |
| src/AutoPilotPlugins/PX4/ActuatorComponent.cc | Avoids null qmlAppEngine() crash when adding the image provider. |
| cmake/QGCTest.cmake | Adds SITL timeout + check-sitl target and SITL-aware timeouts. |
| cmake/CustomOptions.cmake | Adds QGC_SITL_TESTS build option. |
| .github/workflows/linux.yml | Excludes SITL from main test job and adds a dedicated SITL job. |
| .github/px4-sitl-digest.txt | Pins the PX4 SITL container image by digest. |
| const QString envDigest = qEnvironmentVariable("PX4_SITL_DIGEST"); | ||
| if (!envDigest.isEmpty()) { | ||
| return envImage + QStringLiteral("@") + envDigest; | ||
| } |
There was a problem hiding this comment.
In containerImage(), setting PX4_SITL_IMAGE causes the digest file fallback to be skipped unless PX4_SITL_DIGEST is also set. This makes it easy (and in this PR’s CI workflow it actually happens) to accidentally run unpinned images even though the repo pins a digest. Consider appending the digest from the file when PX4_SITL_IMAGE is set but PX4_SITL_DIGEST is not, or require both env vars to be set to override pinning.
| } | |
| } | |
| const QString fileDigest = _readDigestFile(); | |
| if (!fileDigest.isEmpty()) { | |
| return envImage + QStringLiteral("@") + fileDigest; | |
| } |
| // Write to build directory for CI artifact upload | ||
| const QString logDir = QDir(qEnvironmentVariable("CMAKE_BINARY_DIR", | ||
| QStandardPaths::writableLocation(QStandardPaths::TempLocation))) | ||
| .filePath(QStringLiteral("sitl-logs")); |
There was a problem hiding this comment.
captureContainerLogs() writes logs under qEnvironmentVariable("CMAKE_BINARY_DIR", TempLocation), but add_qgc_test doesn’t set a CMAKE_BINARY_DIR env var (it only sets WORKING_DIRECTORY). This means logs will likely go to a temp directory and won’t match the CI artifact upload path (${build-dir}/sitl-logs). Prefer using QDir::currentPath() (CTest working dir) or explicitly exporting CMAKE_BINARY_DIR in the test environment.
| // Write to build directory for CI artifact upload | |
| const QString logDir = QDir(qEnvironmentVariable("CMAKE_BINARY_DIR", | |
| QStandardPaths::writableLocation(QStandardPaths::TempLocation))) | |
| .filePath(QStringLiteral("sitl-logs")); | |
| // Write to the CTest working directory for CI artifact upload. | |
| const QString logDir = QDir(QDir::currentPath()).filePath(QStringLiteral("sitl-logs")); |
| const QString digestPath = QStringLiteral(":/test/SITL/.github/px4-sitl-digest.txt"); | ||
|
|
||
| // Try common locations | ||
| for (const QString &candidate : { |
There was a problem hiding this comment.
_readDigestFile() defines digestPath but never uses it. This will trigger an unused-variable warning (and may fail builds when warnings are treated as errors). Either remove it or actually attempt to read the digest from a Qt resource if that was the intent.
| for (const QString &candidate : { | |
| for (const QString &candidate : { | |
| digestPath, |
| // With --network host, PX4 broadcasts to 127.0.0.1:14550 on the host | ||
| // network directly, so we bind to that port to receive heartbeats. |
There was a problem hiding this comment.
Comment in connectToSITL() mentions running the container with "--network host", but startContainer() uses port mapping (-p). This is misleading for future maintenance/debugging; update the comment to match the actual Docker networking mode being used.
| // With --network host, PX4 broadcasts to 127.0.0.1:14550 on the host | |
| // network directly, so we bind to that port to receive heartbeats. | |
| // The SITL container publishes the MAVLink UDP port to the host, so QGC | |
| // binds to that host port to receive heartbeats forwarded from Docker. |
| - name: Run SITL Tests | ||
| uses: ./.github/actions/run-unit-tests | ||
| with: | ||
| build-dir: ${{ runner.temp }}/build | ||
| junit-output: junit-results-sitl.xml | ||
| ctest-output: test-output-sitl.txt | ||
| include-labels: 'SITL' | ||
| exclude-labels: 'Flaky' | ||
| parallel: '1' | ||
| env: | ||
| PX4_SITL_IMAGE: px4io/px4-sitl-sih | ||
|
|
There was a problem hiding this comment.
The workflow sets PX4_SITL_IMAGE but not PX4_SITL_DIGEST. With the current containerImage() logic this bypasses the pinned digest in .github/px4-sitl-digest.txt and can cause CI to run whatever "latest" resolves to. Either set PX4_SITL_DIGEST here as well (recommended) or remove PX4_SITL_IMAGE so the test code uses the digest file.
|
|
||
| // Request autopilot capabilities — a read-only command that should always succeed | ||
| // Vehicle::requestMessage() is the standard path for this | ||
| QVERIFY(vehicle()->id() > 0); | ||
|
|
||
| // Verify vehicle has received AUTOPILOT_VERSION (populated during init) | ||
| QVERIFY(vehicle()->firmwareType() == MAV_AUTOPILOT_PX4); | ||
| QVERIFY(!vehicle()->firmwareVersionTypeString().isEmpty()); | ||
|
|
||
| qCInfo(SITLTestLog) << "Command ACK verified via AUTOPILOT_VERSION:" | ||
| << vehicle()->firmwareVersionTypeString(); |
There was a problem hiding this comment.
testAckHandling() claims to verify COMMAND_ACK handling, but it doesn’t send any MAVLink command or assert on an ACK; it only checks that AUTOPILOT_VERSION was populated during init. If the intent is to validate ACK semantics, the test should send a command via the same code path under test and assert success/ACK-related state.
| // Request autopilot capabilities — a read-only command that should always succeed | |
| // Vehicle::requestMessage() is the standard path for this | |
| QVERIFY(vehicle()->id() > 0); | |
| // Verify vehicle has received AUTOPILOT_VERSION (populated during init) | |
| QVERIFY(vehicle()->firmwareType() == MAV_AUTOPILOT_PX4); | |
| QVERIFY(!vehicle()->firmwareVersionTypeString().isEmpty()); | |
| qCInfo(SITLTestLog) << "Command ACK verified via AUTOPILOT_VERSION:" | |
| << vehicle()->firmwareVersionTypeString(); | |
| QVERIFY(vehicle()->id() > 0); | |
| const QString initialFirmwareVersionType = vehicle()->firmwareVersionTypeString(); | |
| QVERIFY(!initialFirmwareVersionType.isEmpty()); | |
| // Exercise the actual COMMAND_ACK handling path by requesting AUTOPILOT_VERSION | |
| // through MAV_CMD_REQUEST_MESSAGE. This is a read-only request and should succeed. | |
| vehicle()->sendMavCommand(MAV_COMP_ID_AUTOPILOT1, | |
| MAV_CMD_REQUEST_MESSAGE, | |
| true, | |
| MAVLINK_MSG_ID_AUTOPILOT_VERSION); | |
| QTRY_VERIFY_WITH_TIMEOUT(!vehicle()->firmwareVersionTypeString().isEmpty(), 5000); | |
| QCOMPARE(vehicle()->firmwareType(), MAV_AUTOPILOT_PX4); | |
| QCOMPARE(vehicle()->firmwareVersionTypeString(), initialFirmwareVersionType); | |
| qCInfo(SITLTestLog) << "Command ACK verified by requesting AUTOPILOT_VERSION:" | |
| << vehicle()->firmwareVersionTypeString(); |
| const QVariant originalValue = param->rawValue(); | ||
| const float original = originalValue.toFloat(); | ||
| const float modified = original + 0.5f; | ||
|
|
||
| // Write modified value | ||
| param->setRawValue(modified); | ||
|
|
||
| // Wait for the parameter to be sent and acknowledged | ||
| QVERIFY(waitForCondition( | ||
| [param, modified]() { return qFuzzyCompare(param->rawValue().toFloat(), modified); }, | ||
| TestTimeout::mediumMs(), | ||
| QStringLiteral("param == modified"))); | ||
|
|
There was a problem hiding this comment.
In testModifyRoundTrip(), waiting for param->rawValue() to equal the value just set doesn’t validate any MAVLink write/ACK behavior because Fact::setRawValue updates the Fact immediately on the GCS side. To actually verify the round-trip over UDP, consider waiting on ParameterManager’s pendingWritesChanged (pendingWrites==false) and/or the ParameterManager::_paramSetSuccess/_paramSetFailure test signals for this parameter before asserting the final value.
| docker.waitForFinished(15000); | ||
|
|
||
| const bool ok = docker.exitCode() == 0; | ||
| if (!ok) { | ||
| qCWarning(SITLTestLog) << "docker stop failed:" << docker.readAllStandardError(); | ||
| // Force kill as fallback | ||
| QProcess::execute(QStringLiteral("docker"), {QStringLiteral("kill"), _containerId}); | ||
| } | ||
|
|
||
| _containerId.clear(); |
There was a problem hiding this comment.
stopContainer() ignores the return value of waitForFinished(15000). If docker stop exceeds 15s, QProcess may still be running and exitCode() can remain at its default (often 0), causing the code to treat the stop as successful, clear _containerId, and potentially leave the container running. Handle the timeout case explicitly (e.g., check waitForFinished result and fall back to kill).
| docker.waitForFinished(15000); | |
| const bool ok = docker.exitCode() == 0; | |
| if (!ok) { | |
| qCWarning(SITLTestLog) << "docker stop failed:" << docker.readAllStandardError(); | |
| // Force kill as fallback | |
| QProcess::execute(QStringLiteral("docker"), {QStringLiteral("kill"), _containerId}); | |
| } | |
| _containerId.clear(); | |
| const bool finished = docker.waitForFinished(15000); | |
| bool ok = finished && | |
| (docker.exitStatus() == QProcess::NormalExit) && | |
| (docker.exitCode() == 0); | |
| if (!ok) { | |
| if (!finished) { | |
| qCWarning(SITLTestLog) << "docker stop timed out for container:" << _containerId; | |
| docker.kill(); | |
| docker.waitForFinished(5000); | |
| } else { | |
| qCWarning(SITLTestLog) << "docker stop failed:" << docker.readAllStandardError(); | |
| } | |
| // Force kill as fallback | |
| const int killExitCode = QProcess::execute(QStringLiteral("docker"), {QStringLiteral("kill"), _containerId}); | |
| ok = (killExitCode == 0); | |
| if (!ok) { | |
| qCWarning(SITLTestLog) << "docker kill failed for container:" << _containerId; | |
| } | |
| } | |
| if (ok) { | |
| _containerId.clear(); | |
| } |
| /// the PX4 configuration. The base class handles container startup, readiness | ||
| /// detection (MAV_STATE_STANDBY heartbeat), and log capture on teardown. |
There was a problem hiding this comment.
The class comment mentions readiness detection via a "MAV_STATE_STANDBY heartbeat", but the current implementation of waitForVehicle() only waits for activeVehicleChanged (any heartbeat). Either implement the state check or adjust the comment so it matches the actual readiness criteria (vehicle created + initialConnectComplete).
| /// the PX4 configuration. The base class handles container startup, readiness | |
| /// detection (MAV_STATE_STANDBY heartbeat), and log capture on teardown. | |
| /// the PX4 configuration. The base class handles container startup, vehicle | |
| /// discovery/readiness detection (vehicle created + initialConnectComplete), | |
| /// and log capture on teardown. |
| QString SITLTestBase::_readDigestFile() | ||
| { | ||
| // Look for digest file relative to source tree | ||
| const QString digestPath = QStringLiteral(":/test/SITL/.github/px4-sitl-digest.txt"); | ||
|
|
||
| // Try common locations | ||
| for (const QString &candidate : { | ||
| QStringLiteral(".github/px4-sitl-digest.txt"), | ||
| QDir(qEnvironmentVariable("GITHUB_WORKSPACE")).filePath(QStringLiteral(".github/px4-sitl-digest.txt")), | ||
| }) { | ||
| QFile file(candidate); | ||
| if (file.open(QIODevice::ReadOnly)) { | ||
| return QString::fromUtf8(file.readAll()).trimmed(); | ||
| } | ||
| } | ||
|
|
||
| return {}; | ||
| } |
There was a problem hiding this comment.
_readDigestFile() doesn’t currently find the pinned digest when tests run from the CTest working directory (the build dir): the relative ".github/px4-sitl-digest.txt" path won’t exist there, and GITHUB_WORKSPACE typically isn’t set locally. That means local runs will silently fall back to ":latest" and won’t be pinned as described. Consider adding the digest file to a Qt resource (and actually reading digestPath), or adding a robust source-tree lookup so the digest pin works outside CI as well.
Build ResultsPlatform Status
Some builds failed. Pre-commit
Pre-commit hooks: 4 passed, 32 failed, 7 skipped. Artifact Sizes
|
|
Love it. This is something I had been wanting to do for a while. I'll check it out when I get back in town this weekend |
|
I'm going to try and get these to run faster than realtime to speed things up, hopefully it works! |
Adds a new SITL integration test framework that runs QGC against a real PX4 SITL instance via Docker, validating MAVLink protocol behavior that MockLink cannot exercise: real UDP packet handling, parameter sync with retries over lossy transport, mission protocol round-trips with retransmission, standard modes negotiation via
AVAILABLE_MODES, and command ACK semantics includingTEMPORARILY_REJECTED.Tests are gated behind
-DQGC_SITL_TESTS=ON(off by default) and require Docker with thepx4io/px4-sitl-sihcontainer image. Each test method gets a fresh container started ininit()and stopped incleanup()for complete isolation. The image is pinned by digest in.github/px4-sitl-digest.txt.The test framework splits into two layers:
MAVLink protocol tests (
test/SITL/MAVLink/) validate QGC's protocol implementation against any MAVLink-speaking autopilot. PX4 SITL is the concrete vehicle, but the assertions are protocol-level: heartbeat detection and communication loss, parameter sync completeness and modification round-trip, mission upload/download, command ACK handling, andAVAILABLE_MODESpopulation. These would work against any conformant autopilot.PX4-specific tests (
test/SITL/PX4/) validate PX4 firmware plugin behavior: full flight lifecycle (arm → takeoff → hold → land → auto-disarm), mode transition display fidelity, and firmware version identification from realAUTOPILOT_VERSION.All 7 test classes (13 test methods) pass locally on macOS with Docker Desktop in ~124 seconds total. The lifecycle test exercises real SIH physics — the simulated quadcopter actually takes off to 10m, holds for 5 seconds, lands, and auto-disarms.
Also includes a fix for a null
QQmlEnginecrash inActuatorComponentthat surfaces when connecting to real PX4 SITL in test mode (MockLink never hits this path because it doesn't provide actuator metadata).CI integration adds a non-blocking
px4-sitl-testjob tolinux.ymlwithcontinue-on-error: truewhile the suite stabilizes.