Add infrastructure to allow PRS1 loader to alert the user when it encounters unexpected data.

Only the tests currently react to this information.

oscar/SleepLib/loader_plugins/prs1_loader.cpp

@@ -197,12 +197,25 @@ static QString ts(qint64 msecs)
 }
 
 
-// TODO: have UNEXPECTED_VALUE set a flag in the importer/machine that this data set is unusual
+// TODO: See the LogUnexpectedMessage TODO about generalizing this for other loaders.
-#define UNEXPECTED_VALUE(SRC, VALS) { qWarning() << this->sessionid << QString("%1: %2 = %3 != %4").arg(__func__).arg(#SRC).arg(SRC).arg(VALS); }
+// Right now this macro assumes that it's called within a method that has a "loader" member
+// that points to the PRS1Loader* instance that's calling it.
+#define UNEXPECTED_VALUE(SRC, VALS) { \
+    QString message = QString("%1:%2: %3 = %4 != %5").arg(__func__).arg(__LINE__).arg(#SRC).arg(SRC).arg(VALS); \
+    qWarning() << this->sessionid << message; \
+    loader->LogUnexpectedMessage(message); \
+    }
 #define CHECK_VALUE(SRC, VAL) if ((SRC) != (VAL)) UNEXPECTED_VALUE(SRC, VAL)
 #define CHECK_VALUES(SRC, VAL1, VAL2) if ((SRC) != (VAL1) && (SRC) != (VAL2)) UNEXPECTED_VALUE(SRC, #VAL1 " or " #VAL2)
 // for more than 2 values, just write the test manually and use UNEXPECTED_VALUE if it fails
 
+void PRS1Loader::LogUnexpectedMessage(const QString & message)
+{
+    m_importMutex.lock();
+    m_unexpectedMessages += message;
+    m_importMutex.unlock();
+}
+
 
 enum FlexMode { FLEX_None, FLEX_CFlex, FLEX_CFlexPlus, FLEX_AFlex, FLEX_RiseTime, FLEX_BiFlex, FLEX_AVAPS, FLEX_PFlex, FLEX_Unknown  };
 
@@ -696,7 +709,6 @@ int PRS1Loader::OpenMachine(const QString & path)
     ScanFiles(paths, sessionid_base, m);
 
     int tasks = countTasks();
-    unknownCodes.clear();
 
     emit updateMessage(QObject::tr("Importing Sessions..."));
     QCoreApplication::processEvents();
@@ -708,15 +720,15 @@ int PRS1Loader::OpenMachine(const QString & path)
 
     finishAddingSessions();
 
-    if (unknownCodes.size() > 0) {
+    // TODO: pop up a warning if m_unexpectedMessages isn't empty, based on
-        for (auto it = unknownCodes.begin(), end=unknownCodes.end(); it != end; ++it) {
+    // p_profile->session->warnOnUnexpectedData pref.
-            qDebug() << QString("Unknown CPAP Codes '0x%1' was detected during import").arg((short)it.key(), 2, 16, QChar(0));
+    //
-            QStringList & strlist = it.value();
+    // TODO: compare this to the list of messages previously seen for this machine
-            for (int i=0;i<it.value().size(); ++i) {
+    // and only alert if there are new ones.
-                qDebug() << strlist.at(i);
+    //
-            }
+    // TODO: persist the list of previously seen messages along with the current
-        }
+    // OSCAR version number in the machine XML record. Reset the list when the
-    }
+    // OSCAR version changes.
     
     return m->unsupported() ? -1 : tasks;
 }
@@ -860,6 +872,7 @@ void PRS1Loader::ScanFiles(const QStringList & paths, int sessionid_base, Machin
 
     sesstasks.clear();
     new_sessions.clear(); // this hash is used by OpenFile
+    m_unexpectedMessages.clear();
 
 
     PRS1Import * task = nullptr;
@@ -7523,7 +7536,7 @@ QList<PRS1DataChunk *> PRS1Loader::ParseFile(const QString & path)
     int firstsession = 0;
 
     do {
-        chunk = PRS1DataChunk::ParseNext(f);
+        chunk = PRS1DataChunk::ParseNext(f, this);
         if (chunk == nullptr) {
             break;
         }
@@ -7576,7 +7589,7 @@ QList<PRS1DataChunk *> PRS1Loader::ParseFile(const QString & path)
 }
 
 
-PRS1DataChunk::PRS1DataChunk(QFile & f)
+PRS1DataChunk::PRS1DataChunk(QFile & f, PRS1Loader* in_loader) : loader(in_loader)
 {
     m_path = QFileInfo(f).canonicalFilePath();
 }
@@ -7590,10 +7603,10 @@ PRS1DataChunk::~PRS1DataChunk()
 }
 
 
-PRS1DataChunk* PRS1DataChunk::ParseNext(QFile & f)
+PRS1DataChunk* PRS1DataChunk::ParseNext(QFile & f, PRS1Loader* loader)
 {
     PRS1DataChunk* out_chunk = nullptr;
-    PRS1DataChunk* chunk = new PRS1DataChunk(f);
+    PRS1DataChunk* chunk = new PRS1DataChunk(f, loader);
 
     do {
         // Parse the header and calculate its checksum.

oscar/SleepLib/loader_plugins/prs1_loader.h

@@ -60,8 +60,8 @@ struct PRS1Waveform {
  *  \brief Representing a chunk of event/summary/waveform data after the header is parsed. */
 class PRS1DataChunk
 {
-    friend class PRS1DataGroup;
 public:
+    /*
     PRS1DataChunk() {
         fileVersion = 0;
         blockSize = 0;
@@ -77,7 +77,8 @@ public:
         m_filepos = -1;
         m_index = -1;
     }
-    PRS1DataChunk(class QFile & f);
+    */
+    PRS1DataChunk(class QFile & f, class PRS1Loader* loader);
     ~PRS1DataChunk();
     inline int size() const { return m_data.size(); }
 
@@ -123,7 +124,7 @@ public:
     inline quint64 hash(void) const { return ((((quint64) this->calcCrc) << 32) | this->timestamp); }
     
     //! \brief Parse and return the next chunk from a PRS1 file
-    static PRS1DataChunk* ParseNext(class QFile & f);
+    static PRS1DataChunk* ParseNext(class QFile & f, class PRS1Loader* loader);
 
     //! \brief Read and parse the next chunk header from a PRS1 file
     bool ReadHeader(class QFile & f);
@@ -213,6 +214,8 @@ public:
     bool ParseEventsF5V3(void);
 
 protected:
+    class PRS1Loader* loader;
+    
     //! \brief Add a parsed event to the chunk
     void AddEvent(class PRS1ParsedEvent* event);
 
@@ -422,7 +425,6 @@ class PRS1Loader : public CPAPLoader
 
 
     QHash<SessionID, PRS1Import*> sesstasks;
-    QMap<unsigned char, QStringList> unknownCodes;
 
   protected:
     QString last;
@@ -459,6 +461,16 @@ class PRS1Loader : public CPAPLoader
 
     //! \brief PRS1 Data files can store multiple sessions, so store them in this list for later processing.
     QHash<SessionID, Session *> new_sessions;
+
+    // TODO: This really belongs in a generic location that all loaders can use.
+    // But that will require retooling the overall call structure so that there's
+    // a top-level import job that's managing a specific import. Right now it's
+    // essentially managed by the importCPAP method rather than an object instance
+    // with state.
+    QMutex m_importMutex;
+    QSet<QString> m_unexpectedMessages;
+public:
+    void LogUnexpectedMessage(const QString & message);
 };
 
 

oscar/tests/prs1tests.cpp

@@ -79,9 +79,8 @@ void parseAndEmitSessionYaml(const QString & path)
     qDebug() << path;
 
     // This mirrors the functional bits of PRS1Loader::OpenMachine.
-    // Maybe there's a clever way to add parameters to OpenMachine that
+    // TODO: Refactor PRS1Loader so that the tests can use the same
-    // would make it more amenable to automated tests. But for now
+    // underlying logic as OpenMachine rather than duplicating it here.
-    // something is better than nothing.
 
     QStringList paths;
     QString propertyfile;
@@ -109,7 +108,10 @@ void parseAndEmitSessionYaml(const QString & path)
         
         delete session;
         delete task;
-   }
+    }
+    if (s_loader->m_unexpectedMessages.count() > 0) {
+        qWarning() << "*** Found unexpected data";
+    }
 }
 
 void PRS1Tests::testSessionsToYaml()