LP-517 gcs: fix uavtalk logging timer threading issues

make sure QTimer stop and start are always called from owner thread

ground/gcs/src/libs/utils/logfile.cpp

@@ -29,6 +29,7 @@
 
 LogFile::LogFile(QObject *parent) :
     QIODevice(parent),
+    m_timer(this),
     m_lastTimeStamp(0),
     m_lastPlayed(0),
     m_timeOffset(0),
@@ -55,6 +56,7 @@ bool LogFile::open(OpenMode mode)
         // connection manager call...
         return true;
     }
+    qDebug() << "LogFile - open" << fileName();
 
     if (m_file.open(mode) == false) {
         qDebug() << "Unable to open " << m_file.fileName() << " for logging";
@@ -74,11 +76,8 @@ bool LogFile::open(OpenMode mode)
 
 void LogFile::close()
 {
+    qDebug() << "LogFile - close" << fileName();
     emit aboutToClose();
-
-    if (m_timer.isActive()) {
-        m_timer.stop();
-    }
     m_file.close();
     QIODevice::close();
 }
@@ -183,6 +182,7 @@ void LogFile::timerFired()
 
 bool LogFile::startReplay()
 {
+    qDebug() << "LogFile - startReplay";
     m_dataBuffer.clear();
     m_myTime.restart();
     m_timeOffset = 0;
@@ -196,7 +196,9 @@ bool LogFile::startReplay()
 
 bool LogFile::stopReplay()
 {
-    close();
+    qDebug() << "LogFile - stopReplay";
+    m_timer.stop();
+
     emit replayFinished();
     return true;
 }

ground/gcs/src/libs/utils/logfile.h

@@ -45,6 +45,10 @@ public:
         return m_file.bytesToWrite();
     };
     bool open(OpenMode mode);
+    QString fileName()
+    {
+        return m_file.fileName();
+    };
     void setFileName(QString name)
     {
         m_file.setFileName(name);
@@ -53,8 +57,6 @@ public:
     qint64 writeData(const char *data, qint64 dataSize);
     qint64 readData(char *data, qint64 maxlen);
 
-    bool startReplay();
-    bool stopReplay();
     void useProvidedTimeStamp(bool useProvidedTimeStamp)
     {
         m_useProvidedTimeStamp = useProvidedTimeStamp;
@@ -71,6 +73,8 @@ public slots:
         m_playbackSpeed = val;
         qDebug() << "Playback speed is now" << m_playbackSpeed;
     };
+    bool startReplay();
+    bool stopReplay();
     void pauseReplay();
     void resumeReplay();
 

ground/gcs/src/plugins/logging/loggingplugin.cpp

@@ -30,6 +30,8 @@
 
 #include "loggingplugin.h"
 #include "logginggadgetfactory.h"
+
+#include <QApplication>
 #include <QDebug>
 #include <QtPlugin>
 #include <QThread>
@@ -45,17 +47,14 @@
 #include "uavobjectmanager.h"
 
 
-LoggingConnection::LoggingConnection(LoggingPlugin *loggingPlugin) :
-    loggingPlugin(loggingPlugin),
-    m_deviceOpened(false)
+LoggingConnection::LoggingConnection() :
+    m_deviceOpened(false), logFile()
 {}
 
 LoggingConnection::~LoggingConnection()
-{}
-
-void LoggingConnection::onEnumerationChanged()
 {
-    emit availableDevChanged(this);
+    // make sure to close device to kill timers appropriately
+    closeDevice("");
 }
 
 QList <Core::IConnection::device> LoggingConnection::availableDevices()
@@ -71,34 +70,37 @@ QList <Core::IConnection::device> LoggingConnection::availableDevices()
 
 QIODevice *LoggingConnection::openDevice(const QString &deviceName)
 {
-    loggingPlugin->stopLogging();
     closeDevice(deviceName);
 
     QString fileName = QFileDialog::getOpenFileName(NULL, tr("Open file"), QString(""), tr("OpenPilot Log (*.opl)"));
     if (!fileName.isNull()) {
-        startReplay(fileName);
+        logFile.setFileName(fileName);
+        if (logFile.open(QIODevice::ReadOnly)) {
+            // call startReplay on correct thread to avoid error from LogFile's replay QTimer
+            // you can't start or stop the timer from a thread other than the QTimer owner thread.
+            // note that the LogFile IO device (and thus its owned QTimer) is moved to a dedicated thread by the TelemetryManager
+            Qt::ConnectionType ct = (QApplication::instance()->thread() == logFile.thread()) ? Qt::DirectConnection : Qt::BlockingQueuedConnection;
+            QMetaObject::invokeMethod(&logFile, "startReplay", ct);
+            m_deviceOpened = true;
+        }
         return &logFile;
     }
 
     return NULL;
 }
 
-void LoggingConnection::startReplay(QString file)
-{
-    logFile.setFileName(file);
-    if (logFile.open(QIODevice::ReadOnly)) {
-        qDebug() << "LoggingConnection - replaying " << file;
-        logFile.startReplay();
-    }
-}
-
 void LoggingConnection::closeDevice(const QString &deviceName)
 {
     Q_UNUSED(deviceName);
-    // we have to delete the serial connection we created
+
     if (logFile.isOpen()) {
-        logFile.close();
         m_deviceOpened = false;
+        // call stoptReplay on correct thread to avoid error from LogFile's replay QTimer
+        // you can't start or stop the timer from a thread other than the QTimer owner thread.
+        // note that the LogFile IO device (and thus its owned QTimer) is moved to a dedicated thread by the TelemetryManager
+        Qt::ConnectionType ct = (QApplication::instance()->thread() == logFile.thread()) ? Qt::DirectConnection : Qt::BlockingQueuedConnection;
+        QMetaObject::invokeMethod(&logFile, "stopReplay", ct);
+        logFile.close();
     }
 }
 
@@ -294,7 +296,7 @@ void LoggingThread::transactionCompleted(UAVObject *obj, bool success)
 LoggingPlugin::LoggingPlugin() :
     state(IDLE),
     loggingThread(NULL),
-    logConnection(new LoggingConnection(this)),
+    logConnection(new LoggingConnection()),
     mf(NULL),
     cmd(NULL)
 {}
@@ -302,6 +304,7 @@ LoggingPlugin::LoggingPlugin() :
 LoggingPlugin::~LoggingPlugin()
 {
     stopLogging();
+    // logConnection will be auto released
 }
 
 /**

ground/gcs/src/plugins/logging/loggingplugin.h

@@ -50,14 +50,14 @@ class LoggingGadgetFactory;
  *   Plugin will add a instance of this class to the pool,
  *   so the connection manager can use it.
  */
-class LoggingConnection
-    : public Core::IConnection {
+class LoggingConnection : public Core::IConnection {
     Q_OBJECT
 public:
-    LoggingConnection(LoggingPlugin *loggingPlugin);
+    LoggingConnection();
     virtual ~LoggingConnection();
 
     virtual QList <Core::IConnection::device> availableDevices();
+
     virtual QIODevice *openDevice(const QString &deviceName);
     virtual void closeDevice(const QString &deviceName);
 
@@ -74,15 +74,8 @@ public:
     }
 
 private:
-    LogFile logFile;
-    LoggingPlugin *loggingPlugin;
-
-protected slots:
-    void onEnumerationChanged();
-    void startReplay(QString file);
-
-protected:
     bool m_deviceOpened;
+    LogFile logFile;
 };
 
 class LoggingThread : public QThread {
@@ -103,9 +96,11 @@ public slots:
 
 protected:
     void run();
+
     QReadWriteLock lock;
     LogFile logFile;
     UAVTalk *uavTalk;
+private slots:
 
 private:
     QQueue<UAVDataObject *> queue;