Ref T129, improved flight plan remarks parsing

* unit tests
* improved parsing
* return value objects instead of just strings where applicable

src/blackmisc/aviation/flightplan.cpp

@@ -10,7 +10,6 @@
 #include "flightplan.h"
 #include "airlineicaocode.h"
 #include "flightplan.h"
-#include "selcal.h"
 #include "blackmisc/iconlist.h"
 #include "blackmisc/icons.h"
 #include <QStringBuilder>
@@ -28,35 +27,36 @@ namespace BlackMisc
         CFlightPlanRemarks::CFlightPlanRemarks(const QString &remarks, bool parse) : m_remarks(remarks)
         {
             if (parse) { this->parseFlightPlanRemarks(); }
-            m_isNull = false;
         }
 
         CFlightPlanRemarks::CFlightPlanRemarks(const QString &remarks, CVoiceCapabilities voiceCapabilities, bool parse) :
             m_remarks(remarks), m_voiceCapabilities(voiceCapabilities)
         {
             if (parse) { this->parseFlightPlanRemarks(); }
-            m_isNull = false;
         }
 
         bool CFlightPlanRemarks::hasAnyParsedRemarks() const
         {
-            return this->hasParsedAirlineRemarks() || !m_selcalCode.isEmpty() || !m_voiceCapabilities.isUnknown();
+            if (!this->m_isParsed) { return false; }
+            return this->hasParsedAirlineRemarks() || m_selcalCode.isValid() || !m_voiceCapabilities.isUnknown();
         }
 
         bool CFlightPlanRemarks::hasParsedAirlineRemarks() const
         {
-            return !m_radioTelephony.isEmpty() || !m_flightOperator.isEmpty() || !m_airlineIcao.isEmpty();
+            if (!this->m_isParsed) { return false; }
+            return !m_radioTelephony.isEmpty() || !m_flightOperator.isEmpty() || m_airlineIcao.hasValidDesignator();
         }
 
         QString CFlightPlanRemarks::convertToQString(bool i18n) const
         {
-            const QString s = m_callsign.toQString(i18n)
+            const QString s =
-                              % QLatin1Char(' ') % m_airlineIcao
+                (m_registration.isEmpty() ? QStringLiteral("") : QStringLiteral("reg.: ") % m_registration.toQString(i18n))
-                              % QLatin1Char(' ') % m_radioTelephony
+                % (!this->hasValidAirlineIcao() ? QStringLiteral("") : QStringLiteral(" airline: ") % m_airlineIcao.getDesignator())
-                              % QLatin1Char(' ') % m_flightOperator
+                % (m_radioTelephony.isEmpty() ?  QStringLiteral("") : QStringLiteral(" radio tel.:") % m_radioTelephony)
-                              % QLatin1Char(' ') % m_selcalCode
+                % (m_flightOperator.isEmpty() ? QStringLiteral("") : QStringLiteral(" operator: ") % m_flightOperator)
-                              % QLatin1Char(' ') % m_voiceCapabilities.toQString(i18n);
+                % (!m_selcalCode.isValid() ? QStringLiteral("") : QStringLiteral(" SELCAL: ") % m_selcalCode.getCode())
-            return s;
+                % QStringLiteral(" voice: ") % m_voiceCapabilities.toQString(i18n);
+            return s.simplified().trimmed();
         }
 
         void CFlightPlanRemarks::parseFlightPlanRemarks()
@@ -64,7 +64,7 @@ namespace BlackMisc
             // examples: VFPS = VATSIM Flightplan Prefile System
             // 1) RT/KESTREL OPR/MYTRAVEL REG/G-DAJC SEL/FP-ES PER/C NAV/RNP10
             // 2) OPR/UAL CALLSIGN/UNITED
-            // 3) /v/FPL-VIR9-IS-A346/DEP/S-EGLL/ARR/KJFK/REG/G-VGAS/TCAS RVR/200 OPR/VIRGI
+            // 3) /v/FPL-VIR9-IS-A346/DEP/S-EGLL/ARR/KJFK/REG/G-VGAS/TCAS RVR/200 OPR/VIRGIN AIRLINES
 
             if (m_isParsed) { return; }
             m_isParsed = true;
@@ -73,17 +73,16 @@ namespace BlackMisc
             const QString callsign = CCallsign::unifyCallsign(this->cut(remarks, "REG/")); // registration is a callsign
             if (CCallsign::isValidAircraftCallsign(callsign)) { m_registration = CCallsign(callsign, CCallsign::Aircraft); }
             m_voiceCapabilities = m_voiceCapabilities.isUnknown() ? CVoiceCapabilities(m_remarks) : m_voiceCapabilities;
-            m_flightOperator = this->cut(r, "OPR/"); // operator, e.g. British airways, sometimes people use ICAO code here
+            m_flightOperator = this->cut(remarks, "OPR/"); // operator, e.g. British airways, sometimes people use ICAO code here
-            const QString selcal = this->cut(r, "SEL/"); // SELCAL
+            m_selcalCode = CSelcal(this->cut(remarks, "SEL/"));
-            if (CSelcal::isValidCode(selcal)) m_selcalCode = selcal;
+            m_radioTelephony = cut(remarks, "CALLSIGN/"); // used similar to radio telephony
-            m_radioTelephony = cut(r, "CALLSIGN/"); // used similar to radio telephony
+            if (m_radioTelephony.isEmpty()) { m_radioTelephony = cut(remarks, "RT/"); }
-            if (m_radioTelephony.isEmpty()) { m_radioTelephony = cut(r, "RT/"); }
             if (!m_flightOperator.isEmpty() && CAirlineIcaoCode::isValidAirlineDesignator(m_flightOperator))
             {
-                // if people use ICAO as flight operator move to airline ICAO
+                // if people use ICAO code as flight operator swap with airline ICAO
-                qSwap(m_flightOperator, m_airlineIcao);
+                m_airlineIcao = CAirlineIcaoCode(m_flightOperator);
+                m_flightOperator.clear();
             }
-            m_isNull = false;
         }
 
         QString CFlightPlanRemarks::aircraftIcaoCodeFromEquipmentCode(const QString &equipmentCodeAndAircraft)
@@ -103,10 +102,19 @@ namespace BlackMisc
             if (f < 0) { return ""; }
             f += marker.length();
             if (maxIndex <= f) { return ""; }
-            int to = remarks.indexOf(' ', f + 1);
+
-            if (to < 0) { to = maxIndex; } // no more spaces
+            // the remarks are poorly formatted:
-            const QString cut = remarks.mid(f, to - f).replace('/', ' '); // variations like /OPR/EASYJET/
+            // 1) sometimes the values are enclosed in "/", like "/REG/D-AMBZ/"
-            // problem is that this cuts something like "Uzbekistan Airways"
+            // 2) sometimes the values are containing space, like "/OPR/DELTA AIRLINES"
+            // 3) in many cases the end delimiter is a new marker or the EOL (otherwise 1 applies)
+
+            thread_local const QRegularExpression nextMarker("\\s+\\w*/|$");
+            int to1 = remarks.indexOf(nextMarker, f + 1); // for case 2,3
+            if (to1 < 0) { to1 = maxIndex + 1; }
+            int to2 = remarks.indexOf('/', f + 1); // for case 1
+            if (to2 < 0) { to2 = maxIndex + 1; } // no more end markes, ends after last character
+            const int to = qMin(to1, to2);
+            const QString cut = remarks.mid(f, to - f).simplified();
             return cut;
         }
 

src/blackmisc/aviation/flightplan.h

@@ -15,7 +15,9 @@
 #include "airporticaocode.h"
 #include "aircrafticaocode.h"
 #include "altitude.h"
+#include "airlineicaocode.h"
 #include "callsign.h"
+#include "selcal.h"
 #include "blackmisc/network/voicecapabilities.h"
 #include "blackmisc/pq/speed.h"
 #include "blackmisc/pq/time.h"
@@ -37,6 +39,10 @@ namespace BlackMisc
     namespace Aviation
     {
         //! Flight plan remarks, parsed values
+        //! \remark Actually the term "remarks" is not accurate, as the FP remarks are normally only the /RMK part of a flight plan.
+        //!         But on flight sim. networks "remarks" is used to fill in all parts not fitting in other fields.
+        //!         The correct term would be ITEM18 ("OTHER INFORMATION") or ITEM19 ("SUPPLEMENTARY INFORMATION")
+        //!         according to https://www.skybrary.aero/index.php/Flight_Plan_Filling
         class BLACKMISC_EXPORT CFlightPlanRemarks : public CValueObject<CFlightPlanRemarks>
         {
         public:
@@ -59,13 +65,13 @@ namespace BlackMisc
             const QString &getFlightOperator() const { return m_flightOperator; }
 
             //! Airline ICAO if provided in flight plan
-            const QString &getAirlineIcao() const { return m_airlineIcao; }
+            const CAirlineIcaoCode &getAirlineIcao() const { return m_airlineIcao; }
 
             //! SELCAL code
-            const QString &getSelcalCode() const { return m_selcalCode; }
+            const CSelcal &getSelcalCode() const { return m_selcalCode; }
 
-            //! Get SELCAL
+            //! Get registration (a callsign, but normally not the flight callsign)
-            const CCallsign &getCallsign() const { return m_callsign; }
+            const CCallsign &getRegistration() const { return m_registration; }
 
             //! Voice capabilities
             const Network::CVoiceCapabilities &getVoiceCapabilities() const { return m_voiceCapabilities; }
@@ -81,10 +87,13 @@ namespace BlackMisc
 
             //! Valid airline ICAO?
             //! \remark valid here means valid syntax, no guarantee it really exists
-            bool hasValidAirlineIcao() const { return !m_airlineIcao.isEmpty(); }
+            bool hasValidAirlineIcao() const { return m_airlineIcao.hasValidDesignator(); }
 
-            //! Not initialized
+            //! Empty remarks?
-            bool isNull() const { return m_isNull; }
+            bool isEmpty() const { return m_remarks.isEmpty(); }
+
+            //! Already parsed?
+            bool isParsed() const { return m_isParsed; }
 
             //! \copydoc BlackMisc::Mixin::String::toQString()
             QString convertToQString(bool i18n = false) const;
@@ -97,11 +106,11 @@ namespace BlackMisc
             QString m_remarks;        //!< the unparsed string
             QString m_radioTelephony; //!< radio telephony designator
             QString m_flightOperator; //!< operator, i.e. normally the airline name
-            QString m_airlineIcao;    //!< airline ICAO if provided in flight plan
+            CCallsign m_registration; //!< callsign of other pilot
-            QString m_selcalCode;     //!< SELCAL code
+            CSelcal m_selcalCode;     //!< SELCAL code
-            CCallsign m_callsign;     //!< callsign of other pilot
+            CAirlineIcaoCode m_airlineIcao; //!< airline ICAO if provided in flight plan
-            bool m_isNull = true;     //!< marked as NULL
             Network::CVoiceCapabilities m_voiceCapabilities; //!< voice capabilities
+            bool m_isParsed = false;  //!< marked as parsed
 
             BLACK_METACLASS(
                 CFlightPlanRemarks,
@@ -109,8 +118,8 @@ namespace BlackMisc
                 BLACK_METAMEMBER(flightOperator),
                 BLACK_METAMEMBER(airlineIcao),
                 BLACK_METAMEMBER(selcalCode),
-                BLACK_METAMEMBER(callsign),
+                BLACK_METAMEMBER(registration),
-                BLACK_METAMEMBER(isNull),
+                BLACK_METAMEMBER(isParsed),
                 BLACK_METAMEMBER(voiceCapabilities)
             );
 

tests/blackmisc/testblackmiscmain.cpp

@@ -29,6 +29,7 @@
 #include "testvariantandmap.h"
 #include "testweather.h"
 #include "testdbus.h"
+#include "testflightplan.h"
 #include "blackmisc/test/test.h"
 
 #include <QStringList>
@@ -60,6 +61,10 @@ namespace BlackMiscTest
             CTestGeo geoTests;
             status |= test.exec(&geoTests, "blackmisc_geo");
         }
+        {
+            CTestFlightPlan fpTests;
+            status |= test.exec(&fpTests, "blackmisc_flightplan");
+        }
         {
             CTestContainers containerTests;
             status |= test.exec(&containerTests, "blackmisc_containers");

tests/blackmisc/testflightplan.cpp

@@ -0,0 +1,61 @@
+/* Copyright (C) 2017
+ * swift Project Community / Contributors
+ *
+ * This file is part of swift project. It is subject to the license terms in the LICENSE file found in the top-level
+ * directory of this distribution and at http://www.swift-project.org/license.html. No part of swift project,
+ * including this file, may be copied, modified, propagated, or distributed except according to the terms
+ * contained in the LICENSE file.
+ */
+
+//! \cond PRIVATE_TESTS
+
+/*!
+ * \file
+ * \ingroup testblackmisc
+ */
+
+#include "testflightplan.h"
+#include "blackmisc/aviation/flightplan.h"
+#include "blackmisc/aviation/selcal.h"
+#include "blackmisc/network/voicecapabilities.h"
+#include <QTest>
+
+using namespace BlackMisc::Aviation;
+using namespace BlackMisc::Network;
+
+namespace BlackMiscTest
+{
+    void CTestFlightPlan::flightPlanRemarks()
+    {
+        QString remarks;
+        CFlightPlanRemarks fpRemarks(remarks);
+        QVERIFY2(fpRemarks.isEmpty(), "Expect NULL flight plan remarks");
+
+        remarks = QStringLiteral("RT/KESTREL OPR/MYTRAVEL REG/G-DAJC SEL/FP-ES PER/C NAV/RNP10");
+        fpRemarks = CFlightPlanRemarks(remarks);
+        QVERIFY2(fpRemarks.getSelcalCode().getCode() == "FPES" , "Wrong SELCAL code");
+        QVERIFY2(fpRemarks.getFlightOperator() == "MYTRAVEL" , "Wrong flight operator");
+        QVERIFY2(fpRemarks.getRegistration().asString() == "GDAJC" , "Wrong registration");
+
+        remarks = QStringLiteral("OPR/UAL CALLSIGN/UNITED");
+        fpRemarks = CFlightPlanRemarks(remarks);
+        QVERIFY2(fpRemarks.getAirlineIcao().getDesignator() == "UAL" , "Wrong airline, expect UAL");
+        QVERIFY2(fpRemarks.getFlightOperator().isEmpty() , "Expect to operator, should be in airline");
+        QVERIFY2(fpRemarks.getRegistration().isEmpty(), "Expect no registration");
+        QVERIFY2(fpRemarks.getRadioTelephony() == "UNITED" , "Expect telephony");
+
+        remarks = QStringLiteral("/v/FPL-VIR9-IS-A346/DEP/S-EGLL/ARR/KJFK/REG/G-VGAS/TCAS RVR/200 OPR/VIRGIN AIRLINES");
+        fpRemarks = CFlightPlanRemarks(remarks);
+        QVERIFY2(fpRemarks.getRegistration().asString() == "GVGAS" , "Wrong registration");
+        QVERIFY2(fpRemarks.getFlightOperator() == "VIRGIN AIRLINES", "Wrong operator");
+        QVERIFY2(fpRemarks.getVoiceCapabilities().getCapabilities() == CVoiceCapabilities::Voice, "Wrong airline, expect UAL");
+
+        remarks = QStringLiteral("/v/FPL-VIR9-IS-A346/ OPR/VIRGIN AIRLINES DEP/S-EGLL/ARR/KJFK/REG/G-VGAS/TCAS RVR/200");
+        fpRemarks = CFlightPlanRemarks(remarks);
+        QVERIFY2(fpRemarks.getRegistration().asString() == "GVGAS" , "Wrong registration");
+        QVERIFY2(fpRemarks.getFlightOperator() == "VIRGIN AIRLINES", "Wrong operator");
+        QVERIFY2(fpRemarks.getVoiceCapabilities().getCapabilities() == CVoiceCapabilities::Voice, "Wrong airline, expect UAL");
+    }
+} // ns
+
+//! \endcond

tests/blackmisc/testflightplan.h

@@ -0,0 +1,38 @@
+/* Copyright (C) 2017
+ * swift project community / contributors
+ *
+ * This file is part of swift project. It is subject to the license terms in the LICENSE file found in the top-level
+ * directory of this distribution and at http://www.swift-project.org/license.html. No part of swift project,
+ * including this file, may be copied, modified, propagated, or distributed except according to the terms
+ * contained in the LICENSE file.
+ */
+
+#ifndef BLACKMISCTEST_TESTFLIGHTPLAN_H
+#define BLACKMISCTEST_TESTFLIGHTPLAN_H
+
+//! \cond PRIVATE_TESTS
+//! \file
+//! \ingroup testblackmisc
+
+#include <QObject>
+
+namespace BlackMiscTest
+{
+    //! Geo classes tests
+    class CTestFlightPlan : public QObject
+    {
+        Q_OBJECT
+
+    public:
+        //! Standard test case constructor
+        explicit CTestFlightPlan(QObject *parent = nullptr) : QObject(parent) {}
+
+    private slots:
+        //! Flight plan remarks (parsing)
+        void flightPlanRemarks();
+    };
+} // namespace
+
+//! \endcond
+
+#endif // guard