From c583afdbff9ce40048dc1557f25e063c4449a1ba Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Sun, 31 May 2026 12:21:25 -0700 Subject: [PATCH] ParameterEditor: support Mission Planner .param file import in diff dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parameter diff dialog previously only parsed tab-delimited QGC .params files. Mission Planner exports space-delimited .param files using the same 5-column format (VehicleId ComponentId Name Value Type), so valid files were silently rejected — parsedLineCount stayed 0 and the function returned false with no user-visible message. Changes: - buildDiffFromFile: split on /[\t ]+/ instead of '\t' so both formats are accepted. Trim each line before splitting to avoid empty leading fields. Add parsedLineCount guard: show an explicit error and return false when no 5-column lines are found (bad file format). - Remove dead ParameterManager::readParametersFromStream (declaration and implementation) — had no callers since the UI moved to buildDiffFromFile. - ParameterEditor.qml: add 'Mission Planner Files (*.param)' filter to the load file dialog alongside the existing QGC filter. - Add ParameterEditorControllerTest covering QGC format, MP format, no-diff (both formats), bad format, and unknown parameter. Fixes #14347 --- .../getting_started/whats_new.md | 5 + src/FactSystem/ParameterManager.cc | 56 ------- src/FactSystem/ParameterManager.h | 3 - src/QmlControls/ParameterEditor.qml | 2 +- src/QmlControls/ParameterEditorController.cc | 11 +- test/CMakeLists.txt | 1 + test/FactSystem/CMakeLists.txt | 2 + .../ParameterEditorControllerTest.cc | 146 ++++++++++++++++++ .../ParameterEditorControllerTest.h | 16 ++ 9 files changed, 180 insertions(+), 62 deletions(-) create mode 100644 test/FactSystem/ParameterEditorControllerTest.cc create mode 100644 test/FactSystem/ParameterEditorControllerTest.h diff --git a/docs/en/qgc-user-guide/getting_started/whats_new.md b/docs/en/qgc-user-guide/getting_started/whats_new.md index 9e18ba421e73..a0cea8ecec30 100644 --- a/docs/en/qgc-user-guide/getting_started/whats_new.md +++ b/docs/en/qgc-user-guide/getting_started/whats_new.md @@ -59,6 +59,11 @@ Multiple fixed wing landing sequences can now be planned for landings at differe Vehicle Configuration has been renamed from Vehicle Setup and is now intended mainly for initial vehicle design and configuration, not changes between flights. +### Parameter Editor — Mission Planner File Import + +The parameter diff dialog can now load Mission Planner `.param` files (whitespace-separated 5-column format) in addition to the existing QGC `.params` format. +Select a `.param` file using the new **Mission Planner Files (*.param)** filter in the load dialog; differences from the vehicle's current values are shown as usual before any changes are applied. + ### ArduPilot — Servo Outputs A new **Servo Outputs** page provides real-time visualization and configuration of servo outputs for ArduPilot vehicles: diff --git a/src/FactSystem/ParameterManager.cc b/src/FactSystem/ParameterManager.cc index 75001fc3fe9b..75f60f1e706d 100644 --- a/src/FactSystem/ParameterManager.cc +++ b/src/FactSystem/ParameterManager.cc @@ -1200,62 +1200,6 @@ void ParameterManager::_tryCacheHashLoad(int vehicleId, int componentId, const Q } } -QString ParameterManager::readParametersFromStream(QTextStream &stream) -{ - QString missingErrors; - QString typeErrors; - - while (!stream.atEnd()) { - const QString line = stream.readLine(); - if (!line.startsWith("#")) { - const QStringList wpParams = line.split("\t"); - const int lineMavId = wpParams.at(0).toInt(); - if (wpParams.size() == 5) { - if (_vehicle->id() != lineMavId) { - return QStringLiteral("The parameters in the stream have been saved from System Id %1, but the current vehicle has the System Id %2.").arg(lineMavId).arg(_vehicle->id()); - } - - const int componentId = wpParams.at(1).toInt(); - const QString paramName = wpParams.at(2); - const QString valStr = wpParams.at(3); - const uint mavType = wpParams.at(4).toUInt(); - - if (!parameterExists(componentId, paramName)) { - QString error; - error += QStringLiteral("%1:%2").arg(componentId).arg(paramName); - missingErrors += error; - qCDebug(ParameterManagerLog) << "Skipped due to missing:" << error; - continue; - } - - Fact *const fact = getParameter(componentId, paramName); - if (fact->type() != mavTypeToFactType(static_cast(mavType))) { - QString error; - error = QStringLiteral("%1:%2 ").arg(componentId).arg(paramName); - typeErrors += error; - qCDebug(ParameterManagerLog) << "Skipped due to type mismatch: %1" << error; - continue; - } - - qCDebug(ParameterManagerLog) << "Updating parameter" << componentId << paramName << valStr; - fact->setRawValue(valStr); - } - } - } - - QString errors; - - if (!missingErrors.isEmpty()) { - errors = tr("Parameters not loaded since they are not currently on the vehicle: %1\n").arg(missingErrors); - } - - if (!typeErrors.isEmpty()) { - errors += tr("Parameters not loaded due to type mismatch: %1").arg(typeErrors); - } - - return errors; -} - void ParameterManager::writeParametersToStream(QTextStream &stream) const { stream << "# Onboard parameters for Vehicle " << _vehicle->id() << "\n"; diff --git a/src/FactSystem/ParameterManager.h b/src/FactSystem/ParameterManager.h index 3626be65daa8..472840c1ea90 100644 --- a/src/FactSystem/ParameterManager.h +++ b/src/FactSystem/ParameterManager.h @@ -86,9 +86,6 @@ class ParameterManager : public QObject /// @param name: Parameter name Fact *getParameter(int componentId, const QString ¶mName); - /// Returns error messages from loading - QString readParametersFromStream(QTextStream &stream); - void writeParametersToStream(QTextStream &stream) const; bool pendingWrites() const; diff --git a/src/QmlControls/ParameterEditor.qml b/src/QmlControls/ParameterEditor.qml index ff1ec441f7e6..04d8ae3d7896 100644 --- a/src/QmlControls/ParameterEditor.qml +++ b/src/QmlControls/ParameterEditor.qml @@ -98,7 +98,7 @@ Item { QGCFileDialog { id: fileDialog folder: _appSettings.parameterSavePath - nameFilters: [ qsTr("Parameter Files (*.%1)").arg(_appSettings.parameterFileExtension) , qsTr("All Files (*)") ] + nameFilters: [ qsTr("Parameter Files (*.%1)").arg(_appSettings.parameterFileExtension), qsTr("Mission Planner Files (*.param)"), qsTr("All Files (*)") ] onAcceptedForSave: (file) => { controller.saveToFile(file) diff --git a/src/QmlControls/ParameterEditorController.cc b/src/QmlControls/ParameterEditorController.cc index 2c618f37c061..e44196b4a418 100644 --- a/src/QmlControls/ParameterEditorController.cc +++ b/src/QmlControls/ParameterEditorController.cc @@ -397,12 +397,14 @@ bool ParameterEditorController::buildDiffFromFile(const QString& filename) QTextStream stream(&file); + int parsedLineCount = 0; int firstComponentId = -1; while (!stream.atEnd()) { QString line = stream.readLine(); - if (!line.startsWith("#")) { - QStringList wpParams = line.split("\t"); + if (!line.startsWith("#") && !line.trimmed().isEmpty()) { + QStringList wpParams = line.trimmed().split(QRegularExpression("[\\t ]+")); if (wpParams.size() == 5) { + parsedLineCount++; int vehicleId = wpParams.at(0).toInt(); int componentId = wpParams.at(1).toInt(); QString paramName = wpParams.at(2); @@ -467,6 +469,11 @@ bool ParameterEditorController::buildDiffFromFile(const QString& filename) file.close(); + if (parsedLineCount == 0) { + QGC::showAppMessage(tr("No valid parameters found in file. Check that the file is in QGC or Mission Planner format.")); + return false; + } + emit diffOtherVehicleChanged(_diffOtherVehicle); emit diffMultipleComponentsChanged(_diffMultipleComponents); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 0ef189a18117..9a30475184cd 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -129,6 +129,7 @@ add_qgc_test(FactSystemTestPX4 LABELS Integration Vehicle RESOURCE_LOCK MockLink add_qgc_test(FactTest LABELS Unit) add_qgc_test(FactValueSliderListModelTest LABELS Unit) add_qgc_test(HashCheckTest LABELS Integration Vehicle SERIAL TIMEOUT ${QGC_TEST_TIMEOUT_EXTENDED}) +add_qgc_test(ParameterEditorControllerTest LABELS Integration Vehicle RESOURCE_LOCK MockLink) add_qgc_test(ParameterManagerTest LABELS Integration Vehicle SERIAL TIMEOUT ${QGC_TEST_TIMEOUT_EXTENDED}) # ---------------------------------------------------------------------------- diff --git a/test/FactSystem/CMakeLists.txt b/test/FactSystem/CMakeLists.txt index 1aa9723689a3..2b3668e40a0d 100644 --- a/test/FactSystem/CMakeLists.txt +++ b/test/FactSystem/CMakeLists.txt @@ -23,6 +23,8 @@ target_sources(${CMAKE_PROJECT_NAME} FactValueSliderListModelTest.h HashCheckTest.cc HashCheckTest.h + ParameterEditorControllerTest.cc + ParameterEditorControllerTest.h ParameterManagerTest.cc ParameterManagerTest.h ParameterMetaDataTestHelper.h diff --git a/test/FactSystem/ParameterEditorControllerTest.cc b/test/FactSystem/ParameterEditorControllerTest.cc new file mode 100644 index 000000000000..9742342c72f4 --- /dev/null +++ b/test/FactSystem/ParameterEditorControllerTest.cc @@ -0,0 +1,146 @@ +#include "ParameterEditorControllerTest.h" + +#include +#include +#include + +#include "ParameterEditorController.h" +#include "QmlObjectListModel.h" +#include "Fixtures/RAIIFixtures.h" +#include "UnitTest.h" + +UT_REGISTER_TEST(ParameterEditorControllerTest, TestLabel::Integration, TestLabel::Vehicle) + +// QGC tab-delimited file with SYS_AUTOSTART changed from 4001 → 4002 +static const char* kQGCParamsWithDiff = + "# Onboard parameters for Vehicle 1\n" + "#\n" + "# Vehicle-Id Component-Id Name Value Type\n" + "1\t1\tSYS_AUTOSTART\t4002\t6\n"; + +// QGC tab-delimited file with SYS_AUTOSTART matching vehicle default (4001) +static const char* kQGCParamsNoDiff = + "# Onboard parameters for Vehicle 1\n" + "#\n" + "# Vehicle-Id Component-Id Name Value Type\n" + "1\t1\tSYS_AUTOSTART\t4001\t6\n"; + +// Mission Planner space-delimited file with same diff +static const char* kMPParamsWithDiff = + "# Mission Planner parameter file\n" + "1 1 SYS_AUTOSTART 4002 6\n"; + +// Mission Planner space-delimited file matching vehicle default +static const char* kMPParamsNoDiff = + "# Mission Planner parameter file\n" + "1 1 SYS_AUTOSTART 4001 6\n"; + +// Garbage — no parseable lines +static const char* kBadFormatParams = + "This is not a parameter file\n" + "random text here\n"; + +// QGC file with a parameter that does not exist on the (PX4 mock) vehicle +static const char* kQGCParamsUnknownParam = + "# Onboard parameters for Vehicle 1\n" + "#\n" + "# Vehicle-Id Component-Id Name Value Type\n" + "1\t1\tNONEXISTENT_PARAM_XYZ\t1\t6\n"; + +void ParameterEditorControllerTest::_buildDiffQGCFormat() +{ + TestFixtures::TempFileFixture tempFile(QStringLiteral("test_XXXXXX.params")); + QVERIFY(tempFile.isValid()); + QVERIFY(tempFile.write(QByteArray(kQGCParamsWithDiff))); + QVERIFY(tempFile.file()->flush()); + + ParameterEditorController controller; + const bool result = controller.buildDiffFromFile(tempFile.path()); + + QVERIFY(result); + QCOMPARE(controller.diffList()->count(), 1); + const auto* diff = controller.diffList()->value(0); + QVERIFY(diff); + QCOMPARE(diff->name, QStringLiteral("SYS_AUTOSTART")); +} + +void ParameterEditorControllerTest::_buildDiffMPFormat() +{ + TestFixtures::TempFileFixture tempFile(QStringLiteral("test_XXXXXX.param")); + QVERIFY(tempFile.isValid()); + QVERIFY(tempFile.write(QByteArray(kMPParamsWithDiff))); + QVERIFY(tempFile.file()->flush()); + + ParameterEditorController controller; + const bool result = controller.buildDiffFromFile(tempFile.path()); + + QVERIFY(result); + QCOMPARE(controller.diffList()->count(), 1); + const auto* diff = controller.diffList()->value(0); + QVERIFY(diff); + QCOMPARE(diff->name, QStringLiteral("SYS_AUTOSTART")); +} + +void ParameterEditorControllerTest::_buildDiffNoDifferencesQGC() +{ + TestFixtures::TempFileFixture tempFile(QStringLiteral("test_XXXXXX.params")); + QVERIFY(tempFile.isValid()); + QVERIFY(tempFile.write(QByteArray(kQGCParamsNoDiff))); + QVERIFY(tempFile.file()->flush()); + + ParameterEditorController controller; + const bool result = controller.buildDiffFromFile(tempFile.path()); + + // parsedLineCount > 0 so returns true, but no diffs found + QVERIFY(result); + QCOMPARE(controller.diffList()->count(), 0); +} + +void ParameterEditorControllerTest::_buildDiffNoDifferencesMP() +{ + TestFixtures::TempFileFixture tempFile(QStringLiteral("test_XXXXXX.param")); + QVERIFY(tempFile.isValid()); + QVERIFY(tempFile.write(QByteArray(kMPParamsNoDiff))); + QVERIFY(tempFile.file()->flush()); + + ParameterEditorController controller; + const bool result = controller.buildDiffFromFile(tempFile.path()); + + QVERIFY(result); + QCOMPARE(controller.diffList()->count(), 0); +} + +void ParameterEditorControllerTest::_buildDiffBadFormat() +{ + TestFixtures::TempFileFixture tempFile(QStringLiteral("test_XXXXXX.txt")); + QVERIFY(tempFile.isValid()); + QVERIFY(tempFile.write(QByteArray(kBadFormatParams))); + QVERIFY(tempFile.file()->flush()); + + ParameterEditorController controller; + expectAppMessage(QRegularExpression("No valid parameters found")); + const bool result = controller.buildDiffFromFile(tempFile.path()); + + // parsedLineCount == 0 → returns false + QVERIFY(!result); + QCOMPARE(controller.diffList()->count(), 0); +} + +void ParameterEditorControllerTest::_buildDiffMissingOnVehicle() +{ + TestFixtures::TempFileFixture tempFile(QStringLiteral("test_XXXXXX.params")); + QVERIFY(tempFile.isValid()); + QVERIFY(tempFile.write(QByteArray(kQGCParamsUnknownParam))); + QVERIFY(tempFile.file()->flush()); + + ParameterEditorController controller; + const bool result = controller.buildDiffFromFile(tempFile.path()); + + // Line was parsed (parsedLineCount > 0) → returns true + QVERIFY(result); + QCOMPARE(controller.diffList()->count(), 1); + const auto* diff = controller.diffList()->value(0); + QVERIFY(diff); + QVERIFY(diff->noVehicleValue); + QCOMPARE(diff->name, QStringLiteral("NONEXISTENT_PARAM_XYZ")); +} diff --git a/test/FactSystem/ParameterEditorControllerTest.h b/test/FactSystem/ParameterEditorControllerTest.h new file mode 100644 index 000000000000..7a497204ff46 --- /dev/null +++ b/test/FactSystem/ParameterEditorControllerTest.h @@ -0,0 +1,16 @@ +#pragma once + +#include "BaseClasses/ParameterTest.h" + +class ParameterEditorControllerTest : public ParameterTest +{ + Q_OBJECT + +private slots: + void _buildDiffQGCFormat(); + void _buildDiffMPFormat(); + void _buildDiffNoDifferencesQGC(); + void _buildDiffNoDifferencesMP(); + void _buildDiffBadFormat(); + void _buildDiffMissingOnVehicle(); +};