Merge pull request #3232 from DonLakeFlyer/APMFirmwareVersioning

Fix APM parameter versioning

qgcresources.qrc

@@ -241,7 +241,11 @@
         <file alias="APMAirframeFactMetaData.xml">src/AutoPilotPlugins/APM/APMAirframeFactMetaData.xml</file>
     </qresource>
     <qresource prefix="/FirmwarePlugin/APM">
-        <file alias="APMParameterFactMetaData.xml">src/FirmwarePlugin/APM/APMParameterFactMetaData.xml</file>
+        <file alias="APMParameterFactMetaData.Plane.3.3.xml">src/FirmwarePlugin/APM/APMParameterFactMetaData.Plane.3.3.xml</file>
+        <file alias="APMParameterFactMetaData.Plane.3.5.xml">src/FirmwarePlugin/APM/APMParameterFactMetaData.Plane.3.5.xml</file>
+        <file alias="APMParameterFactMetaData.Copter.3.3.xml">src/FirmwarePlugin/APM/APMParameterFactMetaData.Copter.3.3.xml</file>
+        <file alias="APMParameterFactMetaData.Copter.3.4.xml">src/FirmwarePlugin/APM/APMParameterFactMetaData.Copter.3.4.xml</file>
+        <file alias="APMParameterFactMetaData.Rover.3.0.xml">src/FirmwarePlugin/APM/APMParameterFactMetaData.Rover.3.0.xml</file>
     </qresource>
     <qresource prefix="/FirmwarePlugin/PX4">
         <file alias="PX4ParameterFactMetaData.xml">src/FirmwarePlugin/PX4/PX4ParameterFactMetaData.xml</file>

src/FactSystem/FactMetaData.cc

@@ -147,7 +147,7 @@ void FactMetaData::setRawDefaultValue(const QVariant& rawDefaultValue)
 
 void FactMetaData::setRawMin(const QVariant& rawMin)
 {
-    if (rawMin > _minForType()) {
+    if (rawMin >= _minForType()) {
         _rawMin = rawMin;
         _minIsDefaultForType = false;
     } else {

src/FactSystem/ParameterLoader.cc

@@ -30,6 +30,7 @@
 #include "QGCApplication.h"
 #include "UASMessageHandler.h"
 #include "FirmwarePlugin.h"
+#include "APMFirmwarePlugin.h"
 #include "UAS.h"
 
 #include <QEasingCurve>
@@ -904,11 +905,20 @@ void ParameterLoader::_addMetaDataToDefaultComponent(void)
          return;
      }
 
-    // Load best parameter meta data set
+     QString metaDataFile;
      int majorVersion, minorVersion;
-     QString metaDataFile = parameterMetaDataFile(_vehicle->firmwareType(), _parameterSetMajorVersion, majorVersion, minorVersion);
+     if (_vehicle->firmwareType() == MAV_AUTOPILOT_ARDUPILOTMEGA) {
+         // Parameter versioning is still not really figured out correctly. We need to handle ArduPilot specially based on vehicle version.
+         // The current three version are hardcoded in.
+         metaDataFile = ((APMFirmwarePlugin*)_vehicle->firmwarePlugin())->getParameterMetaDataFile(_vehicle);
+         qCDebug(ParameterLoaderLog) << "Adding meta data to Vehicle file:major:minor" << metaDataFile;
+     } else {
+         // Load best parameter meta data set
+         metaDataFile = parameterMetaDataFile(_vehicle->firmwareType(), _parameterSetMajorVersion, majorVersion, minorVersion);
+         qCDebug(ParameterLoaderLog) << "Adding meta data to Vehicle file:major:minor" << metaDataFile << majorVersion << minorVersion;
+     }
+
      _parameterMetaData = _vehicle->firmwarePlugin()->loadParameterMetaData(metaDataFile);
-     qCDebug(ParameterLoaderLog) << "Adding meta data to Vehicle file:major:minor" << metaDataFile << majorVersion << minorVersion;
 
     // Loop over all parameters in default component adding meta data
     QVariantMap& factMap = _mapParameterName2Variant[_defaultComponentId];

src/FirmwarePlugin/APM/APMFirmwarePlugin.cc

@@ -313,44 +313,44 @@ bool APMFirmwarePlugin::_handleStatusText(Vehicle* vehicle, mavlink_message_t* m
     mavlink_statustext_t statusText;
     mavlink_msg_statustext_decode(message, &statusText);
 
-    if (!_firmwareVersion.isValid() || statusText.severity < MAV_SEVERITY_NOTICE) {
+    if (vehicle->firmwareMajorVersion() == -1 || statusText.severity < MAV_SEVERITY_NOTICE) {
         messageText = _getMessageText(message);
         qCDebug(APMFirmwarePluginLog) << messageText;
 
-        if (!_firmwareVersion.isValid() && !messageText.contains(APM_SOLO_REXP)) {
+        if (!messageText.contains(APM_SOLO_REXP)) {
             // if don't know firmwareVersion yet, try and see if this message contains it
             if (messageText.contains(APM_COPTER_REXP) || messageText.contains(APM_PLANE_REXP) || messageText.contains(APM_ROVER_REXP)) {
                 // found version string
-                _firmwareVersion = APMFirmwareVersion(messageText);
-                _textSeverityAdjustmentNeeded = _isTextSeverityAdjustmentNeeded(_firmwareVersion);
-
-                if (!_firmwareVersion.isBeta() && !_firmwareVersion.isDev()) {
-                    int supportedMajorNumber = -1;
-                    int supportedMinorNumber = -1;
-
-                    switch (vehicle->vehicleType()) {
-                    case MAV_TYPE_FIXED_WING:
-                        supportedMajorNumber = 3;
-                        supportedMinorNumber = 2;
-                        break;
-                    case MAV_TYPE_QUADROTOR:
-                    case MAV_TYPE_COAXIAL:
-                    case MAV_TYPE_HELICOPTER:
-                    case MAV_TYPE_SUBMARINE:
-                    case MAV_TYPE_HEXAROTOR:
-                    case MAV_TYPE_OCTOROTOR:
-                    case MAV_TYPE_TRICOPTER:
-                        supportedMajorNumber = 3;
-                        supportedMinorNumber = 2;
-                        break;
-                    default:
-                        break;
-                    }
+                APMFirmwareVersion firmwareVersion(messageText);
+                _textSeverityAdjustmentNeeded = _isTextSeverityAdjustmentNeeded(firmwareVersion);
+
+                vehicle->setFirmwareVersion(firmwareVersion.majorNumber(), firmwareVersion.minorNumber(), firmwareVersion.patchNumber());
+
+                int supportedMajorNumber = -1;
+                int supportedMinorNumber = -1;
+
+                switch (vehicle->vehicleType()) {
+                case MAV_TYPE_FIXED_WING:
+                    supportedMajorNumber = 3;
+                    supportedMinorNumber = 2;
+                    break;
+                case MAV_TYPE_QUADROTOR:
+                case MAV_TYPE_COAXIAL:
+                case MAV_TYPE_HELICOPTER:
+                case MAV_TYPE_SUBMARINE:
+                case MAV_TYPE_HEXAROTOR:
+                case MAV_TYPE_OCTOROTOR:
+                case MAV_TYPE_TRICOPTER:
+                    supportedMajorNumber = 3;
+                    supportedMinorNumber = 2;
+                    break;
+                default:
+                    break;
+                }
 
-                    if (supportedMajorNumber != -1) {
-                        if (_firmwareVersion.majorNumber() < supportedMajorNumber || _firmwareVersion.minorNumber() < supportedMinorNumber) {
-                            qgcApp()->showMessage(QString("QGroundControl fully supports Version %1.%2 and above. You are using a version prior to that. This combination is untested, you may run into unpredictable results.").arg(supportedMajorNumber).arg(supportedMinorNumber));
-                        }
+                if (supportedMajorNumber != -1) {
+                    if (firmwareVersion.majorNumber() < supportedMajorNumber || firmwareVersion.minorNumber() < supportedMinorNumber) {
+                        qgcApp()->showMessage(QString("QGroundControl fully supports Version %1.%2 and above. You are using a version prior to that. This combination is untested, you may run into unpredictable results.").arg(supportedMajorNumber).arg(supportedMinorNumber));
                     }
                 }
             }
@@ -601,7 +601,8 @@ QObject* APMFirmwarePlugin::loadParameterMetaData(const QString& metaDataFile)
 {
     Q_UNUSED(metaDataFile);
 
-    APMParameterMetaData* metaData = new APMParameterMetaData;
+    APMParameterMetaData* metaData = new APMParameterMetaData();
+    metaData->loadParameterFactMetaDataFile(metaDataFile);
     return metaData;
 }
 
@@ -630,3 +631,32 @@ void APMFirmwarePlugin::_artooSocketError(QAbstractSocket::SocketError socketErr
 {
     qgcApp()->showMessage(tr("Error during Solo video link setup: %1").arg(socketError));
 }
+
+QString APMFirmwarePlugin::getParameterMetaDataFile(Vehicle* vehicle)
+{
+    switch (vehicle->vehicleType()) {
+    case MAV_TYPE_QUADROTOR:
+    case MAV_TYPE_HEXAROTOR:
+    case MAV_TYPE_OCTOROTOR:
+    case MAV_TYPE_TRICOPTER:
+    case MAV_TYPE_COAXIAL:
+    case MAV_TYPE_HELICOPTER:
+        if (vehicle->firmwareMajorVersion() < 3 || (vehicle->firmwareMajorVersion() == 3 && vehicle->firmwareMinorVersion() <= 3)) {
+            return QStringLiteral(":/FirmwarePlugin/APM/APMParameterFactMetaData.Copter.3.3.xml");
+        } else {
+            return QStringLiteral(":/FirmwarePlugin/APM/APMParameterFactMetaData.Copter.3.4.xml");
+        }
+    case MAV_TYPE_FIXED_WING:
+        if (vehicle->firmwareMajorVersion() < 3 || (vehicle->firmwareMajorVersion() == 3 && vehicle->firmwareMinorVersion() <= 3)) {
+            return QStringLiteral(":/FirmwarePlugin/APM/APMParameterFactMetaData.Plane.3.3.xml");
+        } else {
+            return QStringLiteral(":/FirmwarePlugin/APM/APMParameterFactMetaData.Plane.3.5.xml");
+        }
+    case MAV_TYPE_GROUND_ROVER:
+    case MAV_TYPE_SURFACE_BOAT:
+    case MAV_TYPE_SUBMARINE:
+        return QStringLiteral(":/FirmwarePlugin/APM/APMParameterFactMetaData.Rover.3.0.xml");
+    default:
+        return QString();
+    }
+}

src/FirmwarePlugin/APM/APMFirmwarePlugin.h

@@ -105,6 +105,8 @@ public:
     void        getParameterMetaDataVersionInfo (const QString& metaDataFile, int& majorVersion, int& minorVersion) final { APMParameterMetaData::getParameterMetaDataVersionInfo(metaDataFile, majorVersion, minorVersion); }
     QObject*    loadParameterMetaData           (const QString& metaDataFile);
 
+    QString     getParameterMetaDataFile(Vehicle* vehicle);
+
 protected:
     /// All access to singleton is through stack specific implementation
     APMFirmwarePlugin(void);
@@ -125,7 +127,6 @@ private:
     void _handleHeartbeat(Vehicle* vehicle, mavlink_message_t* message);
     void _soloVideoHandshake(Vehicle* vehicle);
 
-    APMFirmwareVersion      _firmwareVersion;
     bool                    _textSeverityAdjustmentNeeded;
     QList<APMCustomMode>    _supportedModes;
     QMap<QString, QTime>    _noisyPrearmMap;

src/FirmwarePlugin/APM/APMParameterMetaData.cc

@@ -37,8 +37,7 @@ QGC_LOGGING_CATEGORY(APMParameterMetaDataVerboseLog,    "APMParameterMetaDataVer
 APMParameterMetaData::APMParameterMetaData(void)
     : _parameterMetaDataLoaded(false)
 {
-    // APM meta data is not yet versioned
-    _loadParameterFactMetaData();
+
 }
 
 /// Converts a string to a typed QVariant
@@ -123,10 +122,7 @@ QString APMParameterMetaData::mavTypeToString(MAV_TYPE vehicleTypeEnum)
     return vehicleName;
 }
 
-/// Load Parameter Fact meta data
-///
-/// The meta data comes from firmware parameters.xml file.
-void APMParameterMetaData::_loadParameterFactMetaData()
+void APMParameterMetaData::loadParameterFactMetaDataFile(const QString& metaDataFile)
 {
     if (_parameterMetaDataLoaded) {
         return;
@@ -136,17 +132,9 @@ void APMParameterMetaData::_loadParameterFactMetaData()
     QRegExp parameterCategories = QRegExp("ArduCopter|ArduPlane|APMrover2|AntennaTracker");
     QString currentCategory;
 
-    QString parameterFilename;
-
-    // Fixme:: always picking up the bundled xml, we would like to update it from web
-    // just not sure right now as the xml is in bad shape.
-    if (parameterFilename.isEmpty() || !QFile(parameterFilename).exists()) {
-        parameterFilename = ":/FirmwarePlugin/APM/APMParameterFactMetaData.xml";
-    }
-
-    qCDebug(APMParameterMetaDataLog) << "Loading parameter meta data:" << parameterFilename;
+    qCDebug(APMParameterMetaDataLog) << "Loading parameter meta data:" << metaDataFile;
 
-    QFile xmlFile(parameterFilename);
+    QFile xmlFile(metaDataFile);
     Q_ASSERT(xmlFile.exists());
 
     bool success = xmlFile.open(QIODevice::ReadOnly);
@@ -591,15 +579,18 @@ void APMParameterMetaData::addMetaDataToFact(Fact* fact, MAV_TYPE vehicleType)
         }
     }
 
-    // FixMe:: not handling increment size as their is no place for it in FactMetaData and no ui
     fact->setMetaData(metaData);
 }
 
 void APMParameterMetaData::getParameterMetaDataVersionInfo(const QString& metaDataFile, int& majorVersion, int& minorVersion)
 {
-    Q_UNUSED(metaDataFile);
-
-    // Versioning not yet supported
     majorVersion = -1;
     minorVersion = -1;
+
+    // Meta data version is hacked in for now based on file name
+    QRegExp regExp(".*\\.(\\d)\\.(\\d)\\.xml$");
+    if (regExp.exactMatch(metaDataFile) && regExp.captureCount() == 2) {
+        majorVersion = regExp.cap(2).toInt();
+        minorVersion = 0;
+    }
 }

src/FirmwarePlugin/APM/APMParameterMetaData.h

@@ -70,6 +70,7 @@ public:
     APMParameterMetaData(void);
 
     void addMetaDataToFact(Fact* fact, MAV_TYPE vehicleType);
+    void loadParameterFactMetaDataFile(const QString& metaDataFile);
 
     static void getParameterMetaDataVersionInfo(const QString& metaDataFile, int& majorVersion, int& minorVersion);
 
@@ -86,7 +87,6 @@ private:
         XmlStateDone
     };    
 
-    void _loadParameterFactMetaData();
     QVariant _stringToTypedVariant(const QString& string, FactMetaData::ValueType_t type, bool* convertOk);
     bool skipXMLBlock(QXmlStreamReader& xml, const QString& blockName);
     bool parseParameterAttributes(QXmlStreamReader& xml, APMFactMetaDataRaw *rawMetaData);

src/Vehicle/Vehicle.cc

@@ -121,6 +121,9 @@ Vehicle::Vehicle(LinkInterface*             link,
     , _messageSeq(0)
     , _compID(0)
     , _heardFrom(false)
+    , _firmwareMajorVersion(-1)
+    , _firmwareMinorVersion(-1)
+    , _firmwarePatchVersion(-1)
     , _rollFact             (0, _rollFactName,              FactMetaData::valueTypeDouble)
     , _pitchFact            (0, _pitchFactName,             FactMetaData::valueTypeDouble)
     , _headingFact          (0, _headingFactName,           FactMetaData::valueTypeDouble)
@@ -288,6 +291,9 @@ Vehicle::Vehicle(QObject* parent)
     , _messageSeq(0)
     , _compID(0)
     , _heardFrom(false)
+    , _firmwareMajorVersion(-1)
+    , _firmwareMinorVersion(-1)
+    , _firmwarePatchVersion(-1)
     , _rollFact             (0, _rollFactName,              FactMetaData::valueTypeDouble)
     , _pitchFact            (0, _pitchFactName,             FactMetaData::valueTypeDouble)
     , _headingFact          (0, _headingFactName,           FactMetaData::valueTypeDouble)
@@ -1587,6 +1593,13 @@ void Vehicle::_prearmErrorTimeout(void)
     setPrearmError(QString());
 }
 
+void Vehicle::setFirmwareVersion(int majorVersion, int minorVersion, int patchVersion)
+{
+    _firmwareMajorVersion = majorVersion;
+    _firmwareMinorVersion = minorVersion;
+    _firmwarePatchVersion = patchVersion;
+}
+
 const char* VehicleGPSFactGroup::_hdopFactName =                "hdop";
 const char* VehicleGPSFactGroup::_vdopFactName =                "vdop";
 const char* VehicleGPSFactGroup::_courseOverGroundFactName =    "courseOverGround";

src/Vehicle/Vehicle.h

@@ -520,6 +520,11 @@ public:
     bool containsLink(LinkInterface* link) { return _links.contains(link); }
     void doCommandLong(int component, MAV_CMD command, float param1 = 0.0f, float param2 = 0.0f, float param3 = 0.0f, float param4 = 0.0f, float param5 = 0.0f, float param6 = 0.0f, float param7 = 0.0f);
 
+    int firmwareMajorVersion(void) const { return _firmwareMajorVersion; }
+    int firmwareMinorVersion(void) const { return _firmwareMinorVersion; }
+    int firmwarePatchVersion(void) const { return _firmwarePatchVersion; }
+    void setFirmwareVersion(int majorVersion, int minorVersion, int patchVersion);
+
 public slots:
     void setLatitude(double latitude);
     void setLongitude(double longitude);
@@ -731,6 +736,10 @@ private:
     uint8_t             _compID;
     bool                _heardFrom;
 
+    int _firmwareMajorVersion;
+    int _firmwareMinorVersion;
+    int _firmwarePatchVersion;
+
     static const int    _lowBatteryAnnounceRepeatMSecs; // Amount of time in between each low battery announcement
     QElapsedTimer       _lowBatteryAnnounceTimer;