From 26a844c72094349d12feda64bf3ad71fc5305857 Mon Sep 17 00:00:00 2001 From: sawinglogz <3787776-sawinglogz@users.noreply.gitlab.com> Date: Sat, 4 Jan 2020 20:47:28 -0500 Subject: [PATCH] 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 | 47 ++++++++++++------- oscar/SleepLib/loader_plugins/prs1_loader.h | 20 ++++++-- oscar/tests/prs1tests.cpp | 10 ++-- 3 files changed, 52 insertions(+), 25 deletions(-) diff --git a/oscar/SleepLib/loader_plugins/prs1_loader.cpp b/oscar/SleepLib/loader_plugins/prs1_loader.cpp index 363f62d4..d870e811 100644 --- a/oscar/SleepLib/loader_plugins/prs1_loader.cpp +++ b/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 -#define UNEXPECTED_VALUE(SRC, VALS) { qWarning() << this->sessionid << QString("%1: %2 = %3 != %4").arg(__func__).arg(#SRC).arg(SRC).arg(VALS); } +// TODO: See the LogUnexpectedMessage TODO about generalizing this for other loaders. +// 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,16 +720,16 @@ int PRS1Loader::OpenMachine(const QString & path) finishAddingSessions(); - if (unknownCodes.size() > 0) { - for (auto it = unknownCodes.begin(), end=unknownCodes.end(); it != end; ++it) { - qDebug() << QString("Unknown CPAP Codes '0x%1' was detected during import").arg((short)it.key(), 2, 16, QChar(0)); - QStringList & strlist = it.value(); - for (int i=0;isession->warnOnUnexpectedData pref. + // + // TODO: compare this to the list of messages previously seen for this machine + // and only alert if there are new ones. + // + // 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 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 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. diff --git a/oscar/SleepLib/loader_plugins/prs1_loader.h b/oscar/SleepLib/loader_plugins/prs1_loader.h index d6e32500..f5b036cd 100644 --- a/oscar/SleepLib/loader_plugins/prs1_loader.h +++ b/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 sesstasks; - QMap 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 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 m_unexpectedMessages; +public: + void LogUnexpectedMessage(const QString & message); }; diff --git a/oscar/tests/prs1tests.cpp b/oscar/tests/prs1tests.cpp index 28752eb6..95f7e7e4 100644 --- a/oscar/tests/prs1tests.cpp +++ b/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 - // would make it more amenable to automated tests. But for now - // something is better than nothing. + // TODO: Refactor PRS1Loader so that the tests can use the same + // underlying logic as OpenMachine rather than duplicating it here. 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()