From 212b34835531a7a9b75f1e9bde6900cb355f22cf Mon Sep 17 00:00:00 2001 From: Holden Ramsey Date: Sat, 9 May 2026 11:38:37 -0400 Subject: [PATCH 1/2] fix(ParameterEditor): accept whitespace and CSV parameter files in diff loader buildDiffFromFile only split lines on \t, so files with whitespace-aligned columns (Mission Planner / ArduPilot tooling exports like `1 1 SERVO_BLH_AUTO 1 2`) and simple `name,value` CSVs produced zero parsed entries. The diff dialog then reported "There are no differences between the file loaded and the current settings on the Vehicle" even when values clearly differed. Split on `[\s,]+` after trimming, accept both the existing 5-token `vehicleId componentId name value mavType` format and a new 2-token `name value` form. For 2-token rows the component is resolved by walking the vehicle's component ids and the type is derived from the live Fact, so we can still build a typed diff without the extra columns. Fixes mavlink/qgroundcontrol#14346 --- src/QmlControls/ParameterEditorController.cc | 157 ++++++++++++------- 1 file changed, 100 insertions(+), 57 deletions(-) diff --git a/src/QmlControls/ParameterEditorController.cc b/src/QmlControls/ParameterEditorController.cc index 2c618f37c061..acf028dd55a8 100644 --- a/src/QmlControls/ParameterEditorController.cc +++ b/src/QmlControls/ParameterEditorController.cc @@ -397,72 +397,115 @@ bool ParameterEditorController::buildDiffFromFile(const QString& filename) QTextStream stream(&file); + // Accept tab/whitespace/comma separators so we handle QGC's tab-separated + // export, Mission Planner whitespace-aligned dumps, and simple `name,value` + // CSVs from ArduPilot tooling. + static const QRegularExpression rxSeparator(QStringLiteral("[\\s,]+")); + int firstComponentId = -1; while (!stream.atEnd()) { - QString line = stream.readLine(); - if (!line.startsWith("#")) { - QStringList wpParams = line.split("\t"); - if (wpParams.size() == 5) { - int vehicleId = wpParams.at(0).toInt(); - int componentId = wpParams.at(1).toInt(); - QString paramName = wpParams.at(2); - QString fileValueStr = wpParams.at(3); - int mavParamType = wpParams.at(4).toInt(); - QString vehicleValueStr; - QString units; - QVariant fileValueVar = fileValueStr; - bool noVehicleValue = false; - bool readOnly = false; - - if (_vehicle->id() != vehicleId) { - _diffOtherVehicle = true; - } - if (firstComponentId == -1) { - firstComponentId = componentId; - } else if (firstComponentId != componentId) { - _diffMultipleComponents = true; - } + const QString line = stream.readLine().trimmed(); + if (line.isEmpty() || line.startsWith("#")) { + continue; + } - if (_parameterMgr->parameterExists(componentId, paramName)) { - Fact* vehicleFact = _parameterMgr->getParameter(componentId, paramName); - FactMetaData* vehicleFactMetaData = vehicleFact->metaData(); - Fact* fileFact = new Fact(vehicleFact->componentId(), vehicleFact->name(), vehicleFact->type(), this); - - // Turn off reboot messaging before setting value in fileFact - bool vehicleRebootRequired = vehicleFactMetaData->vehicleRebootRequired(); - vehicleFactMetaData->setVehicleRebootRequired(false); - fileFact->setMetaData(vehicleFact->metaData()); - fileFact->setRawValue(fileValueStr); - vehicleFactMetaData->setVehicleRebootRequired(vehicleRebootRequired); - readOnly = vehicleFact->readOnly(); - - if (vehicleFact->rawValue() == fileFact->rawValue()) { - continue; - } - fileValueStr = fileFact->enumOrValueString(); - fileValueVar = fileFact->rawValue(); - vehicleValueStr = vehicleFact->enumOrValueString(); - units = vehicleFact->cookedUnits(); - } else { - noVehicleValue = true; - } + const QStringList tokens = line.split(rxSeparator, Qt::SkipEmptyParts); + + int vehicleId = _vehicle->id(); + int componentId = MAV_COMP_ID_AUTOPILOT1; + QString paramName; + QString fileValueStr; + int mavParamType = -1; + + // Supported per-line formats: + // 5 tokens: vehicleId componentId name value mavType + // 2 tokens: name value (component/type resolved against the vehicle) + if (tokens.size() == 5) { + vehicleId = tokens.at(0).toInt(); + componentId = tokens.at(1).toInt(); + paramName = tokens.at(2); + fileValueStr = tokens.at(3); + mavParamType = tokens.at(4).toInt(); + } else if (tokens.size() == 2) { + paramName = tokens.at(0); + fileValueStr = tokens.at(1); + } else { + continue; + } - if (!readOnly) { - ParameterEditorDiff* paramDiff = new ParameterEditorDiff(this); + QString vehicleValueStr; + QString units; + QVariant fileValueVar = fileValueStr; + bool noVehicleValue = false; + bool readOnly = false; - paramDiff->componentId = componentId; - paramDiff->name = paramName; - paramDiff->valueType = ParameterManager::mavTypeToFactType(static_cast(mavParamType)); - paramDiff->fileValue = fileValueStr; - paramDiff->fileValueVar = fileValueVar; - paramDiff->vehicleValue = vehicleValueStr; - paramDiff->noVehicleValue = noVehicleValue; - paramDiff->units = units; + if (_vehicle->id() != vehicleId) { + _diffOtherVehicle = true; + } - _diffList.append(paramDiff); + // 2-column files don't carry a component id; locate the parameter on + // whichever component owns it before recording multi-component state. + if (tokens.size() == 2 && !_parameterMgr->parameterExists(componentId, paramName)) { + for (int compId : _parameterMgr->componentIds()) { + if (_parameterMgr->parameterExists(compId, paramName)) { + componentId = compId; + break; } } } + + if (firstComponentId == -1) { + firstComponentId = componentId; + } else if (firstComponentId != componentId) { + _diffMultipleComponents = true; + } + + if (_parameterMgr->parameterExists(componentId, paramName)) { + Fact* vehicleFact = _parameterMgr->getParameter(componentId, paramName); + FactMetaData* vehicleFactMetaData = vehicleFact->metaData(); + Fact* fileFact = new Fact(vehicleFact->componentId(), vehicleFact->name(), vehicleFact->type(), this); + + if (mavParamType < 0) { + mavParamType = ParameterManager::factTypeToMavType(vehicleFact->type()); + } + + // Turn off reboot messaging before setting value in fileFact + bool vehicleRebootRequired = vehicleFactMetaData->vehicleRebootRequired(); + vehicleFactMetaData->setVehicleRebootRequired(false); + fileFact->setMetaData(vehicleFact->metaData()); + fileFact->setRawValue(fileValueStr); + vehicleFactMetaData->setVehicleRebootRequired(vehicleRebootRequired); + readOnly = vehicleFact->readOnly(); + + if (vehicleFact->rawValue() == fileFact->rawValue()) { + continue; + } + fileValueStr = fileFact->enumOrValueString(); + fileValueVar = fileFact->rawValue(); + vehicleValueStr = vehicleFact->enumOrValueString(); + units = vehicleFact->cookedUnits(); + } else { + noVehicleValue = true; + if (mavParamType < 0) { + // No vehicle parameter and no type column: can't infer what to send. + continue; + } + } + + if (!readOnly) { + ParameterEditorDiff* paramDiff = new ParameterEditorDiff(this); + + paramDiff->componentId = componentId; + paramDiff->name = paramName; + paramDiff->valueType = ParameterManager::mavTypeToFactType(static_cast(mavParamType)); + paramDiff->fileValue = fileValueStr; + paramDiff->fileValueVar = fileValueVar; + paramDiff->vehicleValue = vehicleValueStr; + paramDiff->noVehicleValue = noVehicleValue; + paramDiff->units = units; + + _diffList.append(paramDiff); + } } file.close(); From 1ddcabd962ef609219144d8b3a3cf9f37421fba3 Mon Sep 17 00:00:00 2001 From: Holden Ramsey Date: Sat, 9 May 2026 11:38:49 -0400 Subject: [PATCH 2/2] fix(Sensors): suppress per-parameter read-failed popups on post-cal refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a compass calibration stops (especially on cancel) the FC is often slow to answer the bulk `COMPASS_*` / `INS_*` (or `CAL_*` / `SENS_*` on PX4) refresh that `_refreshParams` issues. Each timeout went through `_mavlinkParamRequestRead` with `notifyFailure=true`, firing a modal "Parameter read failed: …" via `QGC::showAppMessage` for every parameter. Dozens of modals stack on the QML root and freeze the UI. Add a `notifyFailure` parameter (defaults to true) to `ParameterManager::refreshParameter` and `refreshParametersPrefix` and pass `false` from the post-calibration refresh paths in both the APM and PX4 sensors controllers. The state machine still logs failures and emits `_paramRequestReadFailure`; only the user-facing modal is suppressed for the batch refresh. --- .../APM/APMSensorsComponentController.cc | 9 ++++++--- src/AutoPilotPlugins/PX4/SensorsComponentController.cc | 10 ++++++---- src/FactSystem/ParameterManager.cc | 8 ++++---- src/FactSystem/ParameterManager.h | 9 ++++++--- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/AutoPilotPlugins/APM/APMSensorsComponentController.cc b/src/AutoPilotPlugins/APM/APMSensorsComponentController.cc index 367aa4a51a3b..bd0a7e1385fb 100644 --- a/src/AutoPilotPlugins/APM/APMSensorsComponentController.cc +++ b/src/AutoPilotPlugins/APM/APMSensorsComponentController.cc @@ -381,13 +381,16 @@ void APMSensorsComponentController::_refreshParams() QStringLiteral("INS_ACCOFFS_X"), QStringLiteral("INS_ACCOFFS_Y"), QStringLiteral("INS_ACCOFFS_Z") }; + // After a calibration finishes (success or cancel) the FC can be slow to answer the + // bulk refresh, so suppress the per-parameter "read failed" popups — otherwise dozens + // of modals stack up and freeze the UI (e.g. cancelling compass cal on ArduPilot). for (const QString ¶mName : fastRefreshList) { - _vehicle->parameterManager()->refreshParameter(ParameterManager::defaultComponentId, paramName); + _vehicle->parameterManager()->refreshParameter(ParameterManager::defaultComponentId, paramName, false /* notifyFailure */); } // Now ask for all to refresh - _vehicle->parameterManager()->refreshParametersPrefix(ParameterManager::defaultComponentId, QStringLiteral("COMPASS_")); - _vehicle->parameterManager()->refreshParametersPrefix(ParameterManager::defaultComponentId, QStringLiteral("INS_")); + _vehicle->parameterManager()->refreshParametersPrefix(ParameterManager::defaultComponentId, QStringLiteral("COMPASS_"), false /* notifyFailure */); + _vehicle->parameterManager()->refreshParametersPrefix(ParameterManager::defaultComponentId, QStringLiteral("INS_"), false /* notifyFailure */); } void APMSensorsComponentController::_updateAndEmitShowOrientationCalArea(bool show) diff --git a/src/AutoPilotPlugins/PX4/SensorsComponentController.cc b/src/AutoPilotPlugins/PX4/SensorsComponentController.cc index be34615ceea9..fdf441eec35d 100644 --- a/src/AutoPilotPlugins/PX4/SensorsComponentController.cc +++ b/src/AutoPilotPlugins/PX4/SensorsComponentController.cc @@ -434,15 +434,17 @@ void SensorsComponentController::_refreshParams(void) { QStringList fastRefreshList; - // We ask for a refresh on these first so that the rotation combo show up as fast as possible + // We ask for a refresh on these first so that the rotation combo show up as fast as possible. + // Suppress per-parameter failure popups: a slow/unresponsive FC otherwise stacks dozens of + // modals during the bulk refresh and freezes the UI. fastRefreshList << "CAL_MAG0_ID" << "CAL_MAG1_ID" << "CAL_MAG2_ID" << "CAL_MAG0_ROT" << "CAL_MAG1_ROT" << "CAL_MAG2_ROT"; for (const QString ¶mName : std::as_const(fastRefreshList)) { - _vehicle->parameterManager()->refreshParameter(ParameterManager::defaultComponentId, paramName); + _vehicle->parameterManager()->refreshParameter(ParameterManager::defaultComponentId, paramName, false /* notifyFailure */); } // Now ask for all to refresh - _vehicle->parameterManager()->refreshParametersPrefix(ParameterManager::defaultComponentId, "CAL_"); - _vehicle->parameterManager()->refreshParametersPrefix(ParameterManager::defaultComponentId, "SENS_"); + _vehicle->parameterManager()->refreshParametersPrefix(ParameterManager::defaultComponentId, "CAL_", false /* notifyFailure */); + _vehicle->parameterManager()->refreshParametersPrefix(ParameterManager::defaultComponentId, "SENS_", false /* notifyFailure */); } void SensorsComponentController::_updateAndEmitShowOrientationCalArea(bool show) diff --git a/src/FactSystem/ParameterManager.cc b/src/FactSystem/ParameterManager.cc index 5380c4ffe661..7094061978c1 100644 --- a/src/FactSystem/ParameterManager.cc +++ b/src/FactSystem/ParameterManager.cc @@ -704,23 +704,23 @@ int ParameterManager::_actualComponentId(int componentId) const return componentId; } -void ParameterManager::refreshParameter(int componentId, const QString ¶mName) +void ParameterManager::refreshParameter(int componentId, const QString ¶mName, bool notifyFailure) { componentId = _actualComponentId(componentId); qCDebug(ParameterManagerLog) << _logVehiclePrefix(componentId) << "refreshParameter - name:" << paramName << ")"; - _mavlinkParamRequestRead(componentId, paramName, -1, true /* notifyFailure */); + _mavlinkParamRequestRead(componentId, paramName, -1, notifyFailure); } -void ParameterManager::refreshParametersPrefix(int componentId, const QString &namePrefix) +void ParameterManager::refreshParametersPrefix(int componentId, const QString &namePrefix, bool notifyFailure) { componentId = _actualComponentId(componentId); qCDebug(ParameterManagerLog) << _logVehiclePrefix(componentId) << "refreshParametersPrefix - name:" << namePrefix << ")"; for (const QString ¶mName: _mapCompId2FactMap[componentId].keys()) { if (paramName.startsWith(namePrefix)) { - refreshParameter(componentId, paramName); + refreshParameter(componentId, paramName, notifyFailure); } } } diff --git a/src/FactSystem/ParameterManager.h b/src/FactSystem/ParameterManager.h index 0dc70e69f15e..08f88d201ea0 100644 --- a/src/FactSystem/ParameterManager.h +++ b/src/FactSystem/ParameterManager.h @@ -55,11 +55,14 @@ class ParameterManager : public QObject /// vehicle is not PX4, cacheCheckOnlyFailed() is emitted. void tryHashCheckCacheLoad(); - /// Request a refresh on the specific parameter - void refreshParameter(int componentId, const QString ¶mName); + /// Request a refresh on the specific parameter. + /// @param notifyFailure: when true (default) a user-visible popup is shown if the + /// vehicle never responds. Pass false for batch/background refreshes that would + /// otherwise spawn one modal per parameter and freeze the UI. + void refreshParameter(int componentId, const QString ¶mName, bool notifyFailure = true); /// Request a refresh on all parameters that begin with the specified prefix - void refreshParametersPrefix(int componentId, const QString &namePrefix); + void refreshParametersPrefix(int componentId, const QString &namePrefix, bool notifyFailure = true); void resetAllParametersToDefaults(); void resetAllToVehicleConfiguration();