From de40161e8cc0c09ca3be45c2cd0134ed8f8c3691 Mon Sep 17 00:00:00 2001 From: sawinglogz <3787776-sawinglogz@users.noreply.gitlab.com> Date: Wed, 1 Sep 2021 15:31:06 -0400 Subject: [PATCH] Add ImportContext to begin reducing loader dependencies on Profile. For now only the PRS1Loader makes use of the ImportContext. --- oscar/SleepLib/importcontext.cpp | 96 +++++++++++++++++++ oscar/SleepLib/importcontext.h | 88 +++++++++++++++++ oscar/SleepLib/loader_plugins/prs1_loader.cpp | 54 +++++------ oscar/SleepLib/loader_plugins/prs1_loader.h | 17 ++-- oscar/oscar.pro | 2 + oscar/tests/prs1tests.cpp | 18 +++- 6 files changed, 236 insertions(+), 39 deletions(-) create mode 100644 oscar/SleepLib/importcontext.cpp create mode 100644 oscar/SleepLib/importcontext.h diff --git a/oscar/SleepLib/importcontext.cpp b/oscar/SleepLib/importcontext.cpp new file mode 100644 index 00000000..5070da07 --- /dev/null +++ b/oscar/SleepLib/importcontext.cpp @@ -0,0 +1,96 @@ +/* SleepLib Import Context Implementation +* +* Copyright (c) 2021 The OSCAR Team +* +* This file is subject to the terms and conditions of the GNU General Public +* License. See the file COPYING in the main directory of the source code +* for more details. */ + +#include +#include + +#include "SleepLib/importcontext.h" + + +ImportContext::~ImportContext() +{ + FlushUnexpectedMessages(); +} + +void ImportContext::LogUnexpectedMessage(const QString & message) +{ + m_mutex.lock(); + m_unexpectedMessages += message; + m_mutex.unlock(); +} + +void ImportContext::FlushUnexpectedMessages() +{ + if (m_unexpectedMessages.count() > 0 && m_machine) { + // Compare this to the list of messages previously seen for this machine + // and only alert if there are new ones. + QSet newMessages = m_unexpectedMessages - m_machine->previouslySeenUnexpectedData(); + if (newMessages.count() > 0) { + emit importEncounteredUnexpectedData(m_machine->getInfo(), newMessages); + m_machine->previouslySeenUnexpectedData() += newMessages; + } + } + m_unexpectedMessages.clear(); +} + + +ProfileImportContext::ProfileImportContext(Profile* profile) + : m_profile(profile) +{ + Q_ASSERT(m_profile); +} + +bool ProfileImportContext::ShouldIgnoreOldSessions() +{ + return m_profile->session->ignoreOlderSessions(); +} + +QDateTime ProfileImportContext::IgnoreSessionsOlderThan() +{ + return m_profile->session->ignoreOlderSessionsDate(); +} + +Machine* ProfileImportContext::CreateMachineFromInfo(const MachineInfo & info) +{ + m_machineInfo = info; + m_machine = m_profile->CreateMachine(m_machineInfo); + return m_machine; +} + + +// MARK: - + +ImportUI::ImportUI(Profile* profile) + : m_profile(profile) +{ + Q_ASSERT(m_profile); +} + +void ImportUI::onUnexpectedData(const MachineInfo & info, QSet & /*newMessages*/) +{ + QMessageBox::information(QApplication::activeWindow(), + QObject::tr("Untested Data"), + QObject::tr("Your %1 %2 (%3) generated data that OSCAR has never seen before.").arg(info.brand).arg(info.model).arg(info.modelnumber) +"\n\n"+ + QObject::tr("The imported data may not be entirely accurate, so the developers would like a .zip copy of this machine's SD card and matching official .pdf reports to make sure OSCAR is handling the data correctly.") + ,QMessageBox::Ok); +} + +void ImportUI::onDeviceReportsUsageOnly(const MachineInfo & /*info*/) +{ + // TODO +} + +void ImportUI::onDeviceIsUntested(const MachineInfo & /*info*/) +{ + // TODO +} + +void ImportUI::onDeviceIsUnsupported(const MachineInfo & /*info*/) +{ + // TODO +} diff --git a/oscar/SleepLib/importcontext.h b/oscar/SleepLib/importcontext.h new file mode 100644 index 00000000..43e3ba96 --- /dev/null +++ b/oscar/SleepLib/importcontext.h @@ -0,0 +1,88 @@ +/* SleepLib Import Context Header + * + * Copyright (c) 2021 The OSCAR Team + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file COPYING in the main directory of the source code + * for more details. */ + +#ifndef IMPORTCONTEXT_H +#define IMPORTCONTEXT_H + +#include "SleepLib/machine_loader.h" + + +class ImportContext : public QObject +{ + Q_OBJECT +public: + ImportContext() {} + virtual ~ImportContext(); + + // Loaders will call this directly. It manages the machine's stored set of previously seen messages + // and will emit an importEncounteredUnexpectedData signal in its dtor if any are new. + void LogUnexpectedMessage(const QString & message); + +signals: + void importEncounteredUnexpectedData(const MachineInfo & info, QSet & newMessages); + +public: + virtual bool ShouldIgnoreOldSessions() { return false; } + virtual QDateTime IgnoreSessionsOlderThan() { return QDateTime(); } + + // TODO: Isolate the Machine object from the loader rather than returning it. + virtual Machine* CreateMachineFromInfo(const MachineInfo & info) = 0; + +protected: + void FlushUnexpectedMessages(); + + QMutex m_mutex; + QSet m_unexpectedMessages; + MachineInfo m_machineInfo; + Machine* m_machine; +}; + + +// ProfileImportContext isolates the loader from Profile and its storage. +class Profile; +class ProfileImportContext : public ImportContext +{ + Q_OBJECT +public: + ProfileImportContext(Profile* profile); + virtual ~ProfileImportContext() {}; + + virtual bool ShouldIgnoreOldSessions(); + virtual QDateTime IgnoreSessionsOlderThan(); + + virtual Machine* CreateMachineFromInfo(const MachineInfo & info); + +protected: + Profile* m_profile; +}; + + +// TODO: Add a TestImportContext that writes the data to YAML for regression tests. + + +// TODO: Once all loaders support context and UI, move this into the main application +// and refactor its import UI logic into this class. +class ImportUI : public QObject +{ + Q_OBJECT +public: + ImportUI(Profile* profile); + virtual ~ImportUI() {} + +public slots: + void onUnexpectedData(const MachineInfo & machine, QSet & newMessages); + void onDeviceReportsUsageOnly(const MachineInfo & info); + void onDeviceIsUntested(const MachineInfo & info); + void onDeviceIsUnsupported(const MachineInfo & info); + +protected: + Profile* m_profile; +}; + + +#endif // IMPORTCONTEXT_H diff --git a/oscar/SleepLib/loader_plugins/prs1_loader.cpp b/oscar/SleepLib/loader_plugins/prs1_loader.cpp index 7ac638dd..2f484fc8 100644 --- a/oscar/SleepLib/loader_plugins/prs1_loader.cpp +++ b/oscar/SleepLib/loader_plugins/prs1_loader.cpp @@ -18,6 +18,7 @@ #include #include "SleepLib/schema.h" +#include "SleepLib/importcontext.h" #include "prs1_loader.h" #include "prs1_parser.h" #include "SleepLib/session.h" @@ -53,9 +54,7 @@ QString ts(qint64 msecs) // TODO: See the LogUnexpectedMessage TODO about generalizing this for other loaders. void PRS1Loader::LogUnexpectedMessage(const QString & message) { - m_importMutex.lock(); - m_unexpectedMessages += message; - m_importMutex.unlock(); + m_ctx->LogUnexpectedMessage(message); } @@ -459,6 +458,7 @@ PRS1Loader::PRS1Loader() #endif m_type = MT_CPAP; + m_ctx = nullptr; } PRS1Loader::~PRS1Loader() @@ -737,9 +737,25 @@ int PRS1Loader::Open(const QString & selectedPath) } // Import each machine, from oldest to newest. + // TODO: Loaders should return the set of machines during detection, so that Open() will + // open a unique device, instead of surprising the user. int c = 0; for (auto & machinePath : machines) { +#if 1 + // TODO: Move this to the main application once all loaders support contexts and UI signals. + if (p_profile == nullptr) { + qWarning() << "PRS1Loader::Open() called without a valid p_profile object present"; + return 0; + } + ImportUI ui(p_profile); + ImportContext* ctx = new ProfileImportContext(p_profile); + SetContext(ctx); + connect(ctx, &ImportContext::importEncounteredUnexpectedData, &ui, &ImportUI::onUnexpectedData); +#endif c += OpenMachine(machinePath); +#if 1 + delete ctx; +#endif } return c; } @@ -747,10 +763,7 @@ int PRS1Loader::Open(const QString & selectedPath) int PRS1Loader::OpenMachine(const QString & path) { - if (p_profile == nullptr) { - qWarning() << "PRS1Loader::OpenMachine() called without a valid p_profile object present"; - return 0; - } + Q_ASSERT(m_ctx); qDebug() << "Opening PRS1 " << path; QDir dir(path); @@ -802,22 +815,6 @@ int PRS1Loader::OpenMachine(const QString & path) finishAddingSessions(); - if (m_unexpectedMessages.count() > 0 && p_profile->session->warnOnUnexpectedData()) { - // Compare this to the list of messages previously seen for this machine - // and only alert if there are new ones. - QSet newMessages = m_unexpectedMessages - m->previouslySeenUnexpectedData(); - if (newMessages.count() > 0) { - // TODO: Rework the importer call structure so that this can become an - // emit statement to the appropriate import job. - QMessageBox::information(QApplication::activeWindow(), - QObject::tr("Untested Data"), - QObject::tr("Your Philips Respironics %1 (%2) generated data that OSCAR has never seen before.").arg(m->getInfo().model).arg(m->getInfo().modelnumber) +"\n\n"+ - QObject::tr("The imported data may not be entirely accurate, so the developers would like a .zip copy of this machine's SD card and matching Encore .pdf reports to make sure OSCAR is handling the data correctly.") - ,QMessageBox::Ok); - m->previouslySeenUnexpectedData() += newMessages; - } - } - return m->unsupported() ? -1 : tasks; } @@ -907,7 +904,7 @@ Machine* PRS1Loader::CreateMachineFromProperties(QString propertyfile) } // Which is needed to get the right machine record.. - Machine *m = p_profile->CreateMachine(info); + Machine *m = m_ctx->CreateMachineFromInfo(info); // This time supply the machine object so it can populate machine properties.. PeekProperties(m->info, propertyfile, m); @@ -975,7 +972,6 @@ 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; @@ -983,8 +979,8 @@ void PRS1Loader::ScanFiles(const QStringList & paths, int sessionid_base, Machin QDateTime datetime; - qint64 ignoreBefore = p_profile->session->ignoreOlderSessionsDate().toMSecsSinceEpoch()/1000; - bool ignoreOldSessions = p_profile->session->ignoreOlderSessions(); + qint64 ignoreBefore = m_ctx->IgnoreSessionsOlderThan().toMSecsSinceEpoch()/1000; + bool ignoreOldSessions = m_ctx->ShouldIgnoreOldSessions(); QSet skipped; // for each p0/p1/p2/etc... folder @@ -2333,8 +2329,8 @@ QList PRS1Import::CoalesceWaveformChunks(QList // This won't be perfect, since any coalesced chunks starting after midnight of the threshhold // date will also be imported, but those should be relatively few, and tolerable imprecision. QList coalescedAndFiltered; - qint64 ignoreBefore = p_profile->session->ignoreOlderSessionsDate().toMSecsSinceEpoch()/1000; - bool ignoreOldSessions = p_profile->session->ignoreOlderSessions(); + qint64 ignoreBefore = loader->context()->IgnoreSessionsOlderThan().toMSecsSinceEpoch()/1000; + bool ignoreOldSessions = loader->context()->ShouldIgnoreOldSessions(); for (auto & chunk : coalesced) { if (ignoreOldSessions && chunk->timestamp < ignoreBefore) { diff --git a/oscar/SleepLib/loader_plugins/prs1_loader.h b/oscar/SleepLib/loader_plugins/prs1_loader.h index b20b6fed..bda34f90 100644 --- a/oscar/SleepLib/loader_plugins/prs1_loader.h +++ b/oscar/SleepLib/loader_plugins/prs1_loader.h @@ -57,6 +57,7 @@ struct PRS1Waveform { quint8 sample_format; }; +class ImportContext; class PRS1DataChunk; class PRS1ParsedEvent; @@ -178,6 +179,9 @@ class PRS1Loader : public CPAPLoader public: PRS1Loader(); virtual ~PRS1Loader(); + + void SetContext(ImportContext* ctx) { m_ctx = ctx; } + inline ImportContext* context() { return m_ctx; } //! \brief Peek into PROP.TXT or properties.txt at given path, and return it as a normalized key/value hash bool PeekProperties(const QString & filename, QHash & props); @@ -231,7 +235,13 @@ class PRS1Loader : public CPAPLoader QHash sesstasks; + signals: + void deviceReportsUsageOnly(MachineInfo & info); + void deviceIsUntested(MachineInfo & info); + void deviceIsUnsupported(MachineInfo & info); + protected: + ImportContext* m_ctx; QString last; QHash PRS1List; @@ -269,13 +279,6 @@ 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/oscar.pro b/oscar/oscar.pro index 853ef392..d170ad51 100644 --- a/oscar/oscar.pro +++ b/oscar/oscar.pro @@ -288,6 +288,7 @@ SOURCES += \ SleepLib/event.cpp \ SleepLib/machine.cpp \ SleepLib/machine_loader.cpp \ + SleepLib/importcontext.cpp \ SleepLib/preferences.cpp \ SleepLib/profiles.cpp \ SleepLib/schema.cpp \ @@ -375,6 +376,7 @@ HEADERS += \ SleepLib/machine.h \ SleepLib/machine_common.h \ SleepLib/machine_loader.h \ + SleepLib/importcontext.h \ SleepLib/preferences.h \ SleepLib/profiles.h \ SleepLib/schema.h \ diff --git a/oscar/tests/prs1tests.cpp b/oscar/tests/prs1tests.cpp index d53a3b88..08160f22 100644 --- a/oscar/tests/prs1tests.cpp +++ b/oscar/tests/prs1tests.cpp @@ -11,6 +11,7 @@ #include "../SleepLib/loader_plugins/prs1_loader.h" #include "../SleepLib/loader_plugins/prs1_parser.h" +#include "../SleepLib/importcontext.h" #define TESTDATA_PATH "./testdata/" @@ -80,6 +81,13 @@ void parseAndEmitSessionYaml(const QString & path) { qDebug() << path; + ImportContext* ctx = new ProfileImportContext(p_profile); + s_loader->SetContext(ctx); + + ctx->connect(ctx, &ImportContext::importEncounteredUnexpectedData, [=]() { + qWarning() << "*** Found unexpected data"; + }); + // This mirrors the functional bits of PRS1Loader::OpenMachine. // TODO: Refactor PRS1Loader so that the tests can use the same // underlying logic as OpenMachine rather than duplicating it here. @@ -114,9 +122,8 @@ void parseAndEmitSessionYaml(const QString & path) delete session; delete task; } - if (s_loader->m_unexpectedMessages.count() > 0) { - qWarning() << "*** Found unexpected data"; - } + + delete ctx; } void PRS1Tests::testSessionsToYaml() @@ -263,6 +270,9 @@ void parseAndEmitChunkYaml(const QString & path) bool FV = false; // set this to true to emit family/familyVersion for this path qDebug() << path; + ImportContext* ctx = new ProfileImportContext(p_profile); + s_loader->SetContext(ctx); + QHash> written; QStringList paths; QString propertyfile; @@ -371,6 +381,8 @@ void parseAndEmitChunkYaml(const QString & path) } } + delete ctx; + p_profile->removeMachine(m); delete m; }