Merged in filnet/librepilot/LP-517_uavtalk_logging_qt5_8_0 (pull request #423)

LP-517 uavtalk logging qt5 8 0

Approved-by: Lalanne Laurent <f5soh@free.fr>
Approved-by: Alessio Morale <alessiomorale@gmail.com>
Approved-by: Philippe Renon <philippe_renon@yahoo.fr>
Approved-by: Brian Webb <webbbn@gmail.com>

ground/gcs/src/app/main.cpp

@@ -403,14 +403,14 @@ void loadFactoryDefaults(QSettings &settings, AppOptionValues &appOptionValues)
     qDebug() << "Configuration file" << fileName << "was loaded.";
 }
 
-void overrideSettings(QSettings &settings, int argc, char * *argv)
+void overrideSettings(QSettings &settings, const QStringList &arguments)
 {
     // Options like -D My/setting=test
     QRegExp rx("([^=]+)=(.*)");
 
-    for (int i = 0; i < argc; ++i) {
+    for (int i = 0; i < arguments.size(); ++i) {
-        if (CONFIG_OPTION == QString(argv[i])) {
+        if (CONFIG_OPTION == arguments[i]) {
-            if (rx.indexIn(argv[++i]) > -1) {
+            if (rx.indexIn(arguments[++i]) > -1) {
                 QString key   = rx.cap(1);
                 QString value = rx.cap(2);
                 qDebug() << "User setting" << key << "set to value" << value;
@@ -443,15 +443,12 @@ void loadTranslators(QString language, QTranslator &translator, QTranslator &qtT
     }
 }
 
-int main(int argc, char * *argv)
+int runApplication(int argc, char * *argv)
 {
     QElapsedTimer timer;
 
     timer.start();
 
-    // low level init
-    systemInit();
-
     // create application
     SharedTools::QtSingleApplication app(APP_NAME, argc, argv);
 
@@ -508,7 +505,7 @@ int main(int argc, char * *argv)
     // override settings with command line provided values
     // take notice that the overridden values will be saved in the user settings and will continue to be effective
     // in subsequent GCS runs
-    overrideSettings(settings, argc, argv);
+    overrideSettings(settings, app.arguments());
 
     // initialize GCS locale
     // use the value defined by the General/Locale setting or default to system Locale.
@@ -625,12 +622,21 @@ int main(int argc, char * *argv)
         delete splash;
     }
 
-    qDebug() << "main - main took" << timer.elapsed() << "ms";
+    qDebug() << "main - starting GCS took" << timer.elapsed() << "ms";
-
     int ret = app.exec();
-
     qDebug() << "main - GCS ran for" << timer.elapsed() << "ms";
 
+    return ret;
+}
+
+int main(int argc, char * *argv)
+{
+    // low level init
+    systemInit();
+
+    int ret = runApplication(argc, argv);
+
+    // close log file if needed
     logDeinit();
 
     return ret;

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

@@ -1,17 +1,50 @@
+/**
+ ******************************************************************************
+ *
+ * @file       logfile.cpp
+ * @author     The LibrePilot Project, http://www.librepilot.org Copyright (C) 2017.
+ *             The OpenPilot Team, http://www.openpilot.org Copyright (C) 2010.
+ * @see        The GNU Public License (GPL) Version 3
+ *
+ *****************************************************************************/
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
 #include "logfile.h"
+
 #include <QDebug>
 #include <QtGlobal>
 
-LogFile::LogFile(QObject *parent) :
+LogFile::LogFile(QObject *parent) : QIODevice(parent),
-    QIODevice(parent),
+    m_timer(this),
-    m_lastTimeStamp(0),
+    m_previousTimeStamp(0),
+    m_nextTimeStamp(0),
     m_lastPlayed(0),
     m_timeOffset(0),
     m_playbackSpeed(1.0),
-    m_nextTimeStamp(0),
+    paused(false),
-    m_useProvidedTimeStamp(false)
+    m_useProvidedTimeStamp(false),
+    m_providedTimeStamp(0)
 {
-    connect(&m_timer, SIGNAL(timeout()), this, SLOT(timerFired()));
+    connect(&m_timer, &QTimer::timeout, this, &LogFile::timerFired);
+}
+
+bool LogFile::isSequential() const
+{
+    // returning true fixes "UAVTalk - error : bad type" errors when replaying a log file
+    return true;
 }
 
 /**
@@ -30,9 +63,10 @@ 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";
+        qWarning() << "Unable to open " << m_file.fileName() << " for logging";
         return false;
     }
 
@@ -49,11 +83,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();
 }
@@ -64,14 +95,18 @@ qint64 LogFile::writeData(const char *data, qint64 dataSize)
         return dataSize;
     }
 
-    // If m_nextTimeStamp != -1 then use this timestamp instead of the timer
+    // If needed, use provided timestamp instead of the GCS timer
     // This is used when saving logs from on-board logging
-    quint32 timeStamp = m_useProvidedTimeStamp ? m_nextTimeStamp : m_myTime.elapsed();
+    quint32 timeStamp = m_useProvidedTimeStamp ? m_providedTimeStamp : m_myTime.elapsed();
 
     m_file.write((char *)&timeStamp, sizeof(timeStamp));
     m_file.write((char *)&dataSize, sizeof(dataSize));
 
     qint64 written = m_file.write(data, dataSize);
+
+    // flush (needed to avoid UAVTalk device full errors)
+    m_file.flush();
+
     if (written != -1) {
         emit bytesWritten(written);
     }
@@ -79,67 +114,83 @@ qint64 LogFile::writeData(const char *data, qint64 dataSize)
     return dataSize;
 }
 
-qint64 LogFile::readData(char *data, qint64 maxSize)
+qint64 LogFile::readData(char *data, qint64 maxlen)
 {
     QMutexLocker locker(&m_mutex);
-    qint64 toRead = qMin(maxSize, (qint64)m_dataBuffer.size());
 
-    memcpy(data, m_dataBuffer.data(), toRead);
+    qint64 len = qMin(maxlen, (qint64)m_dataBuffer.size());
-    m_dataBuffer.remove(0, toRead);
+
-    return toRead;
+    if (len) {
+        memcpy(data, m_dataBuffer.data(), len);
+        m_dataBuffer.remove(0, len);
+    }
+
+    return len;
 }
 
 qint64 LogFile::bytesAvailable() const
 {
-    return m_dataBuffer.size();
+    QMutexLocker locker(&m_mutex);
+
+    qint64 len = m_dataBuffer.size();
+
+    return len;
 }
 
 void LogFile::timerFired()
 {
-    qint64 dataSize;
-
     if (m_file.bytesAvailable() > 4) {
         int time;
         time = m_myTime.elapsed();
 
         // TODO: going back in time will be a problem
-        while ((m_lastPlayed + ((double)(time - m_timeOffset) * m_playbackSpeed) > m_lastTimeStamp)) {
+        while ((m_lastPlayed + ((double)(time - m_timeOffset) * m_playbackSpeed) > m_nextTimeStamp)) {
             m_lastPlayed += ((double)(time - m_timeOffset) * m_playbackSpeed);
+
+            // read data size
+            qint64 dataSize;
             if (m_file.bytesAvailable() < (qint64)sizeof(dataSize)) {
+                qDebug() << "LogFile - end of log file reached";
                 stopReplay();
                 return;
             }
-
             m_file.read((char *)&dataSize, sizeof(dataSize));
 
+            // check size consistency
             if (dataSize < 1 || dataSize > (1024 * 1024)) {
-                qDebug() << "Error: Logfile corrupted! Unlikely packet size: " << dataSize << "\n";
+                qWarning() << "LogFile - corrupted log file! Unlikely packet size:" << dataSize;
                 stopReplay();
                 return;
             }
 
+            // read data
             if (m_file.bytesAvailable() < dataSize) {
+                qDebug() << "LogFile - end of log file reached";
                 stopReplay();
                 return;
             }
+            QByteArray data = m_file.read(dataSize);
 
+            // make data available
             m_mutex.lock();
-            m_dataBuffer.append(m_file.read(dataSize));
+            m_dataBuffer.append(data);
             m_mutex.unlock();
 
             emit readyRead();
 
-            if (m_file.bytesAvailable() < (qint64)sizeof(m_lastTimeStamp)) {
+            // read next timestamp
+            if (m_file.bytesAvailable() < (qint64)sizeof(m_nextTimeStamp)) {
+                qDebug() << "LogFile - end of log file reached";
                 stopReplay();
                 return;
             }
+            m_previousTimeStamp = m_nextTimeStamp;
+            m_file.read((char *)&m_nextTimeStamp, sizeof(m_nextTimeStamp));
 
-            int save = m_lastTimeStamp;
-            m_file.read((char *)&m_lastTimeStamp, sizeof(m_lastTimeStamp));
             // some validity checks
-            if (m_lastTimeStamp < save // logfile goes back in time
+            if ((m_nextTimeStamp < m_previousTimeStamp) // logfile goes back in time
-                || (m_lastTimeStamp - save) > (60 * 60 * 1000)) { // gap of more than 60 minutes)
+                || ((m_nextTimeStamp - m_previousTimeStamp) > 60 * 60 * 1000)) { // gap of more than 60 minutes
-                qDebug() << "Error: Logfile corrupted! Unlikely timestamp " << m_lastTimeStamp << " after " << save << "\n";
+                qWarning() << "LogFile - corrupted log file! Unlikely timestamp:" << m_nextTimeStamp << "after" << m_previousTimeStamp;
                 stopReplay();
                 return;
             }
@@ -148,37 +199,83 @@ void LogFile::timerFired()
             time = m_myTime.elapsed();
         }
     } else {
+        qDebug() << "LogFile - end of log file reached";
         stopReplay();
     }
 }
 
+bool LogFile::isPlaying() const
+{
+    return m_file.isOpen() && m_timer.isActive();
+}
+
 bool LogFile::startReplay()
 {
-    m_dataBuffer.clear();
+    if (!m_file.isOpen() || m_timer.isActive()) {
+        return false;
+    }
+    qDebug() << "LogFile - startReplay";
+
     m_myTime.restart();
-    m_timeOffset = 0;
+    m_timeOffset        = 0;
-    m_lastPlayed = 0;
+    m_lastPlayed        = 0;
-    m_file.read((char *)&m_lastTimeStamp, sizeof(m_lastTimeStamp));
+    m_previousTimeStamp = 0;
+    m_nextTimeStamp     = 0;
+    m_dataBuffer.clear();
+
+    // read next timestamp
+    if (m_file.bytesAvailable() < (qint64)sizeof(m_nextTimeStamp)) {
+        qWarning() << "LogFile - invalid log file!";
+        return false;
+    }
+    m_file.read((char *)&m_nextTimeStamp, sizeof(m_nextTimeStamp));
+
     m_timer.setInterval(10);
     m_timer.start();
+    paused = false;
+
     emit replayStarted();
     return true;
 }
 
 bool LogFile::stopReplay()
 {
-    close();
+    if (!m_file.isOpen() || !(m_timer.isActive() || paused)) {
+        return false;
+    }
+    qDebug() << "LogFile - stopReplay";
+    m_timer.stop();
+    paused = false;
+
     emit replayFinished();
     return true;
 }
 
-void LogFile::pauseReplay()
+bool LogFile::pauseReplay()
 {
+    if (!m_timer.isActive()) {
+        return false;
+    }
+    qDebug() << "LogFile - pauseReplay";
     m_timer.stop();
+    paused = true;
+
+    // hack to notify UI that replay paused
+    emit replayStarted();
+    return true;
 }
 
-void LogFile::resumeReplay()
+bool LogFile::resumeReplay()
 {
+    if (m_timer.isActive()) {
+        return false;
+    }
+    qDebug() << "LogFile - resumeReplay";
     m_timeOffset = m_myTime.elapsed();
     m_timer.start();
+    paused = false;
+
+    // hack to notify UI that replay resumed
+    emit replayStarted();
+    return true;
 }

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

@@ -1,6 +1,32 @@
+/**
+ ******************************************************************************
+ *
+ * @file       logfile.h
+ * @author     The LibrePilot Project, http://www.librepilot.org Copyright (C) 2017.
+ *             The OpenPilot Team, http://www.openpilot.org Copyright (C) 2010.
+ * @see        The GNU Public License (GPL) Version 3
+ *
+ *****************************************************************************/
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
 #ifndef LOGFILE_H
 #define LOGFILE_H
 
+#include "utils_global.h"
+
 #include <QIODevice>
 #include <QTime>
 #include <QTimer>
@@ -8,36 +34,45 @@
 #include <QDebug>
 #include <QBuffer>
 #include <QFile>
-#include "utils_global.h"
 
 class QTCREATOR_UTILS_EXPORT LogFile : public QIODevice {
     Q_OBJECT
 public:
     explicit LogFile(QObject *parent = 0);
+
+    QString fileName() const
+    {
+        return m_file.fileName();
+    };
+    void setFileName(QString name)
+    {
+        m_file.setFileName(name);
+    };
+
+    bool isSequential() const;
+
+    bool isPlaying() const;
+
+    bool open(OpenMode mode);
+    void close();
+
     qint64 bytesAvailable() const;
     qint64 bytesToWrite() const
     {
         return m_file.bytesToWrite();
     };
-    bool open(OpenMode mode);
+
-    void setFileName(QString name)
-    {
-        m_file.setFileName(name);
-    };
-    void close();
     qint64 writeData(const char *data, qint64 dataSize);
     qint64 readData(char *data, qint64 maxlen);
 
-    bool startReplay();
-    bool stopReplay();
     void useProvidedTimeStamp(bool useProvidedTimeStamp)
     {
         m_useProvidedTimeStamp = useProvidedTimeStamp;
     }
 
-    void setNextTimeStamp(quint32 nextTimestamp)
+    void setNextTimeStamp(quint32 providedTimestamp)
     {
-        m_nextTimeStamp = nextTimestamp;
+        m_providedTimeStamp = providedTimestamp;
     }
 
 public slots:
@@ -46,8 +81,10 @@ public slots:
         m_playbackSpeed = val;
         qDebug() << "Playback speed is now" << m_playbackSpeed;
     };
-    void pauseReplay();
+    bool startReplay();
-    void resumeReplay();
+    bool stopReplay();
+    bool pauseReplay();
+    bool resumeReplay();
 
 protected slots:
     void timerFired();
@@ -62,17 +99,20 @@ protected:
     QTimer m_timer;
     QTime m_myTime;
     QFile m_file;
-    qint32 m_lastTimeStamp;
+    qint32 m_previousTimeStamp;
+    qint32 m_nextTimeStamp;
     double m_lastPlayed;
-    QMutex m_mutex;
+    // QMutex wants to be mutable
-
+    // http://stackoverflow.com/questions/25521570/can-mutex-locking-function-be-marked-as-const
+    mutable QMutex m_mutex;
 
     int m_timeOffset;
     double m_playbackSpeed;
+    bool paused;
 
 private:
-    quint32 m_nextTimeStamp;
     bool m_useProvidedTimeStamp;
+    qint32 m_providedTimeStamp;
 };
 
 #endif // LOGFILE_H

ground/gcs/src/plugins/logging/logging.ui

@@ -6,7 +6,7 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>536</width>
+    <width>439</width>
     <height>122</height>
    </rect>
   </property>
@@ -29,15 +29,15 @@
    <item>
     <layout class="QVBoxLayout" name="verticalLayout" stretch="0,0">
      <item>
-      <layout class="QHBoxLayout" name="horizontalLayout" stretch="2,2,0,0">
+      <layout class="QHBoxLayout" name="horizontalLayout" stretch="0,0,0,0,0">
        <property name="sizeConstraint">
         <enum>QLayout::SetNoConstraint</enum>
        </property>
        <item>
-        <widget class="QCommandLinkButton" name="playButton">
+        <widget class="QPushButton" name="playButton">
          <property name="sizePolicy">
-          <sizepolicy hsizetype="MinimumExpanding" vsizetype="Preferred">
+          <sizepolicy hsizetype="Preferred" vsizetype="Fixed">
-           <horstretch>30</horstretch>
+           <horstretch>0</horstretch>
            <verstretch>0</verstretch>
           </sizepolicy>
          </property>
@@ -57,10 +57,10 @@
         </widget>
        </item>
        <item>
-        <widget class="QCommandLinkButton" name="pauseButton">
+        <widget class="QPushButton" name="pauseButton">
          <property name="sizePolicy">
-          <sizepolicy hsizetype="MinimumExpanding" vsizetype="Preferred">
+          <sizepolicy hsizetype="Preferred" vsizetype="Fixed">
-           <horstretch>30</horstretch>
+           <horstretch>0</horstretch>
            <verstretch>0</verstretch>
           </sizepolicy>
          </property>
@@ -71,7 +71,7 @@
           </size>
          </property>
          <property name="text">
-          <string>Pause</string>
+          <string notr="true">Pause</string>
          </property>
          <property name="icon">
           <iconset resource="../notify/res.qrc">
@@ -79,6 +79,19 @@
          </property>
         </widget>
        </item>
+       <item>
+        <spacer name="horizontalSpacer_2">
+         <property name="orientation">
+          <enum>Qt::Horizontal</enum>
+         </property>
+         <property name="sizeHint" stdset="0">
+          <size>
+           <width>40</width>
+           <height>20</height>
+          </size>
+         </property>
+        </spacer>
+       </item>
        <item>
         <widget class="QLabel" name="label">
          <property name="text">
@@ -88,8 +101,14 @@
        </item>
        <item>
         <widget class="QLabel" name="statusLabel">
+         <property name="font">
+          <font>
+           <weight>75</weight>
+           <bold>true</bold>
+          </font>
+         </property>
          <property name="text">
-          <string>Idle</string>
+          <string notr="true">&lt;Status&gt;</string>
          </property>
         </widget>
        </item>

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

@@ -25,17 +25,15 @@
  * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */
 #include "logginggadgetwidget.h"
+
 #include "ui_logging.h"
 
-#include <QDebug>
-#include <QStringList>
-#include <QWidget>
-#include <QTextEdit>
-#include <QVBoxLayout>
-#include <QPushButton>
 #include <loggingplugin.h>
 
-LoggingGadgetWidget::LoggingGadgetWidget(QWidget *parent) : QLabel(parent)
+#include <QWidget>
+#include <QPushButton>
+
+LoggingGadgetWidget::LoggingGadgetWidget(QWidget *parent) : QWidget(parent), loggingPlugin(NULL)
 {
     m_logging = new Ui_Logging();
     m_logging->setupUi(this);
@@ -52,20 +50,42 @@ LoggingGadgetWidget::~LoggingGadgetWidget()
 void LoggingGadgetWidget::setPlugin(LoggingPlugin *p)
 {
     loggingPlugin = p;
-    connect(p, SIGNAL(stateChanged(QString)), this, SLOT(stateChanged(QString)));
+
-    connect(m_logging->playButton, SIGNAL(clicked()), p->getLogfile(), SLOT(resumeReplay()));
+    connect(m_logging->playButton, &QPushButton::clicked, scpPlugin, &ScopeGadgetFactory::startPlotting);
-    connect(m_logging->playButton, SIGNAL(clicked()), scpPlugin, SLOT(startPlotting()));
+    connect(m_logging->pauseButton, &QPushButton::clicked, scpPlugin, &ScopeGadgetFactory::stopPlotting);
-    connect(m_logging->pauseButton, SIGNAL(clicked()), p->getLogfile(), SLOT(pauseReplay()));
+
-    connect(m_logging->pauseButton, SIGNAL(clicked()), scpPlugin, SLOT(stopPlotting()));
+    LogFile *logFile = loggingPlugin->getLogfile();
-    connect(m_logging->playbackSpeed, SIGNAL(valueChanged(double)), p->getLogfile(), SLOT(setReplaySpeed(double)));
+    connect(m_logging->playButton, &QPushButton::clicked, logFile, &LogFile::resumeReplay);
-    void pauseReplay();
+    connect(m_logging->pauseButton, &QPushButton::clicked, logFile, &LogFile::pauseReplay);
-    void resumeReplay();
+    connect(m_logging->playbackSpeed, static_cast<void(QDoubleSpinBox::*) (double)>(&QDoubleSpinBox::valueChanged), logFile, &LogFile::setReplaySpeed);
+
+    connect(loggingPlugin, &LoggingPlugin::stateChanged, this, &LoggingGadgetWidget::stateChanged);
+
+    stateChanged(loggingPlugin->getState());
 }
 
-
+void LoggingGadgetWidget::stateChanged(LoggingPlugin::State state)
-void LoggingGadgetWidget::stateChanged(QString status)
 {
+    QString status;
+    bool enabled = false;
+
+    switch (state) {
+    case LoggingPlugin::IDLE:
+        status  = tr("Idle");
+        break;
+    case LoggingPlugin::LOGGING:
+        status  = tr("Logging");
+        break;
+    case LoggingPlugin::REPLAY:
+        status  = tr("Replaying");
+        enabled = true;
+        break;
+    }
     m_logging->statusLabel->setText(status);
+
+    bool playing = loggingPlugin->getLogfile()->isPlaying();
+    m_logging->playButton->setEnabled(enabled && !playing);
+    m_logging->pauseButton->setEnabled(enabled && playing);
 }
 
 /**

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

@@ -28,16 +28,17 @@
 #ifndef LoggingGADGETWIDGET_H_
 #define LoggingGADGETWIDGET_H_
 
-#include <QLabel>
+#include "loggingplugin.h"
+
 #include "extensionsystem/pluginmanager.h"
 #include "scope/scopeplugin.h"
 #include "scope/scopegadgetfactory.h"
 
+#include <QWidget>
 
 class Ui_Logging;
-class LoggingPlugin;
 
-class LoggingGadgetWidget : public QLabel {
+class LoggingGadgetWidget : public QWidget {
     Q_OBJECT
 
 public:
@@ -46,7 +47,7 @@ public:
     void setPlugin(LoggingPlugin *p);
 
 protected slots:
-    void stateChanged(QString status);
+    void stateChanged(LoggingPlugin::State state);
 
 signals:
     void pause();

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

@@ -2,7 +2,8 @@
  ******************************************************************************
  *
  * @file       logging.cpp
- * @author     The OpenPilot Team, http://www.openpilot.org Copyright (C) 2010.
+ * @author     The LibrePilot Project, http://www.librepilot.org Copyright (C) 2017.
+ *             The OpenPilot Team, http://www.openpilot.org Copyright (C) 2010.
  * @see        The GNU Public License (GPL) Version 3
  * @brief      Import/Export Plugin
  * @addtogroup GCSPlugins GCS Plugins
@@ -28,7 +29,16 @@
  */
 
 #include "loggingplugin.h"
+
+#include "gcstelemetrystats.h"
+
 #include "logginggadgetfactory.h"
+#include "uavobjectmanager.h"
+#include <uavtalk/uavtalk.h>
+#include <extensionsystem/pluginmanager.h>
+#include <coreplugin/actionmanager/actionmanager.h>
+
+#include <QApplication>
 #include <QDebug>
 #include <QtPlugin>
 #include <QThread>
@@ -38,23 +48,16 @@
 #include <QList>
 #include <QErrorMessage>
 #include <QWriteLocker>
-
-#include <extensionsystem/pluginmanager.h>
 #include <QKeySequence>
-#include "uavobjectmanager.h"
 
-
+LoggingConnection::LoggingConnection() :
-LoggingConnection::LoggingConnection(LoggingPlugin *loggingPlugin) :
+    m_deviceOpened(false), logFile()
-    loggingPlugin(loggingPlugin),
-    m_deviceOpened(false)
 {}
 
 LoggingConnection::~LoggingConnection()
-{}
-
-void LoggingConnection::onEnumerationChanged()
 {
-    emit availableDevChanged(this);
+    // make sure to close device to kill timers appropriately
+    closeDevice("");
 }
 
 QList <Core::IConnection::device> LoggingConnection::availableDevices()
@@ -70,39 +73,40 @@ 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() << "Replaying " << file;
-        // state = REPLAY;
-        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();
     }
 }
 
-
 QString LoggingConnection::connectionName()
 {
     return QString("Logfile replay");
@@ -113,10 +117,12 @@ QString LoggingConnection::shortName()
     return QString("Logfile");
 }
 
+LoggingThread::LoggingThread() : QThread(), uavTalk(0)
+{}
 
 LoggingThread::~LoggingThread()
 {
-    stopLogging();
+    delete uavTalk;
 }
 
 /**
@@ -125,7 +131,7 @@ LoggingThread::~LoggingThread()
  * @param[in] file File name to write to
  * @param[in] parent plugin
  */
-bool LoggingThread::openFile(QString file, LoggingPlugin *parent)
+bool LoggingThread::openFile(QString file)
 {
     logFile.setFileName(file);
     logFile.open(QIODevice::WriteOnly);
@@ -134,7 +140,6 @@ bool LoggingThread::openFile(QString file, LoggingPlugin *parent)
     UAVObjectManager *objManager = pm->getObject<UAVObjectManager>();
 
     uavTalk = new UAVTalk(&logFile, objManager);
-    connect(parent, SIGNAL(stopLoggingSignal()), this, SLOT(stopLogging()));
 
     return true;
 };
@@ -150,16 +155,24 @@ void LoggingThread::objectUpdated(UAVObject *obj)
     QWriteLocker locker(&lock);
 
     if (!uavTalk->sendObject(obj, false, false)) {
-        qDebug() << "Error logging " << obj->getName();
+        qDebug() << "LoggingThread - error logging" << obj->getName();
     }
 };
 
+
+void LoggingThread::run()
+{
+    startLogging();
+}
+
 /**
  * Connect signals from all the objects updates to the write routine then
  * run event loop
  */
-void LoggingThread::run()
+void LoggingThread::startLogging()
 {
+    qDebug() << "LoggingThread - start logging";
+
     ExtensionSystem::PluginManager *pm = ExtensionSystem::PluginManager::instance();
     UAVObjectManager *objManager = pm->getObject<UAVObjectManager>();
 
@@ -171,33 +184,33 @@ void LoggingThread::run()
 
     for (i = list.constBegin(); i != list.constEnd(); ++i) {
         for (j = (*i).constBegin(); j != (*i).constEnd(); ++j) {
-            connect(*j, SIGNAL(objectUpdated(UAVObject *)), (LoggingThread *)this, SLOT(objectUpdated(UAVObject *)));
+            connect(*j, &UAVObject::objectUpdated, this, &LoggingThread::objectUpdated);
             objects++;
-            // qDebug() << "Detected " << j[0];
         }
     }
 
     GCSTelemetryStats *gcsStatsObj = GCSTelemetryStats::GetInstance(objManager);
     GCSTelemetryStats::DataFields gcsStats = gcsStatsObj->getData();
     if (gcsStats.Status == GCSTelemetryStats::STATUS_CONNECTED) {
-        qDebug() << "Logging: connected already, ask for all settings";
+        qDebug() << "LoggingThread - connected, ask for all settings";
         retrieveSettings();
     } else {
-        qDebug() << "Logging: not connected, do no ask for settings";
+        qDebug() << "LoggingThread - not connected, do not ask for settings";
     }
 
-
     exec();
 }
 
-
 /**
- * Pass this command to the correct thread then close the file
+ * Disconnect signals from all the objects, close the log file and stop
+ * the event loop
  */
 void LoggingThread::stopLogging()
 {
     QWriteLocker locker(&lock);
 
+    qDebug() << "LoggingThread - stop logging";
+
     // Disconnect all objects we registered with:
     ExtensionSystem::PluginManager *pm = ExtensionSystem::PluginManager::instance();
     UAVObjectManager *objManager = pm->getObject<UAVObjectManager>();
@@ -209,13 +222,16 @@ void LoggingThread::stopLogging()
 
     for (i = list.constBegin(); i != list.constEnd(); ++i) {
         for (j = (*i).constBegin(); j != (*i).constEnd(); ++j) {
-            disconnect(*j, SIGNAL(objectUpdated(UAVObject *)), (LoggingThread *)this, SLOT(objectUpdated(UAVObject *)));
+            disconnect(*j, &UAVObject::objectUpdated, this, &LoggingThread::objectUpdated);
         }
     }
 
     logFile.close();
-    qDebug() << "File closed";
+
     quit();
+
+    // wait for thread to finish
+    wait();
 }
 
 /**
@@ -237,12 +253,10 @@ void LoggingThread::retrieveSettings()
         }
     }
     // Start retrieving
-    qDebug() << QString("Logging: retrieve settings objects from the autopilot (%1 objects)")
+    qDebug() << "LoggingThread - retrieving" << queue.length() << "objects";
-        .arg(queue.length());
     retrieveNextObject();
 }
 
-
 /**
  * Retrieve the next object in the queue
  */
@@ -250,13 +264,13 @@ void LoggingThread::retrieveNextObject()
 {
     // If queue is empty return
     if (queue.isEmpty()) {
-        qDebug() << "Logging: Object retrieval completed";
+        qDebug() << "LoggingThread - Object retrieval completed";
         return;
     }
     // Get next object from the queue
     UAVObject *obj = queue.dequeue();
     // Connect to object
-    connect(obj, SIGNAL(transactionCompleted(UAVObject *, bool)), this, SLOT(transactionCompleted(UAVObject *, bool)));
+    connect(obj, &UAVObject::transactionCompleted, this, &LoggingThread::transactionCompleted);
     // Request update
     obj->requestUpdate();
 }
@@ -278,31 +292,22 @@ void LoggingThread::transactionCompleted(UAVObject *obj, bool success)
     if (gcsStats.Status == GCSTelemetryStats::STATUS_CONNECTED) {
         retrieveNextObject();
     } else {
-        qDebug() << "Logging: Object retrieval has been cancelled";
+        qDebug() << "LoggingThread - object retrieval has been cancelled";
         queue.clear();
     }
 }
 
-
-/****************************************************************
-    Logging plugin
- ********************************/
-
-
 LoggingPlugin::LoggingPlugin() :
+    loggingCommand(NULL),
     state(IDLE),
     loggingThread(NULL),
-    logConnection(new LoggingConnection(this)),
+    logConnection(new LoggingConnection())
-    mf(NULL),
-    cmd(NULL)
 {}
 
 LoggingPlugin::~LoggingPlugin()
 {
-    delete loggingThread;
+    stopLogging();
-
+    // logConnection will be auto released
-    // Don't delete it, the plugin manager will do it:
-    // delete logConnection;
 }
 
 /**
@@ -313,33 +318,32 @@ bool LoggingPlugin::initialize(const QStringList & args, QString *errMsg)
     Q_UNUSED(args);
     Q_UNUSED(errMsg);
 
-    loggingThread = NULL;
-
     // Add Menu entry
     Core::ActionManager *am   = Core::ICore::instance()->actionManager();
     Core::ActionContainer *ac = am->actionContainer(Core::Constants::M_TOOLS);
 
-    // Command to start logging
+    // Command to start/stop logging
-    cmd = am->registerAction(new QAction(this),
+    loggingCommand = am->registerAction(new QAction(this),
-                             "LoggingPlugin.Logging",
+                                        "LoggingPlugin.Logging",
-                             QList<int>() <<
+                                        QList<int>() <<
-                             Core::Constants::C_GLOBAL_ID);
+                                        Core::Constants::C_GLOBAL_ID);
-    cmd->setDefaultKeySequence(QKeySequence("Ctrl+L"));
+    loggingCommand->setDefaultKeySequence(QKeySequence("Ctrl+L"));
-    cmd->action()->setText(tr("Start logging..."));
 
     ac->menu()->addSeparator();
     ac->appendGroup("Logging");
-    ac->addAction(cmd, "Logging");
+    ac->addAction(loggingCommand, "Logging");
 
-    connect(cmd->action(), SIGNAL(triggered(bool)), this, SLOT(toggleLogging()));
+    connect(loggingCommand->action(), &QAction::triggered, this, &LoggingPlugin::toggleLogging);
 
-
+    LoggingGadgetFactory *mf = new LoggingGadgetFactory(this);
-    mf = new LoggingGadgetFactory(this);
     addAutoReleasedObject(mf);
 
     // Map signal from end of replay to replay stopped
-    connect(getLogfile(), SIGNAL(replayFinished()), this, SLOT(replayStopped()));
+    connect(getLogfile(), &LogFile::replayFinished, this, &LoggingPlugin::replayStopped);
-    connect(getLogfile(), SIGNAL(replayStarted()), this, SLOT(replayStarted()));
+    connect(getLogfile(), &LogFile::replayStarted, this, &LoggingPlugin::replayStarted);
+
+    // update state and command
+    loggingStopped();
 
     return true;
 }
@@ -359,10 +363,8 @@ void LoggingPlugin::toggleLogging()
         }
 
         startLogging(fileName);
-        cmd->action()->setText(tr("Stop logging"));
     } else if (state == LOGGING) {
         stopLogging();
-        cmd->action()->setText(tr("Start logging..."));
     }
 }
 
@@ -372,20 +374,19 @@ void LoggingPlugin::toggleLogging()
  */
 void LoggingPlugin::startLogging(QString file)
 {
-    qDebug() << "Logging to " << file;
+    // needed ?
-    // We have to delete the previous logging thread if is was still there!
+    stopLogging();
-    if (loggingThread) {
+    // Start logging thread
-        delete loggingThread;
-    }
     loggingThread = new LoggingThread();
-    if (loggingThread->openFile(file, this)) {
+    if (loggingThread->openFile(file)) {
-        connect(loggingThread, SIGNAL(finished()), this, SLOT(loggingStopped()));
+        connect(loggingThread, &LoggingThread::finished, this, &LoggingPlugin::loggingStopped);
-        state = LOGGING;
         loggingThread->start();
-        emit stateChanged("LOGGING");
+        loggingStarted();
     } else {
+        delete loggingThread;
+        loggingThread = NULL;
         QErrorMessage err;
-        err.showMessage("Unable to open file for logging");
+        err.showMessage(tr("Unable to open file for logging"));
         err.exec();
     }
 }
@@ -395,11 +396,24 @@ void LoggingPlugin::startLogging(QString file)
  */
 void LoggingPlugin::stopLogging()
 {
-    emit stopLoggingSignal();
+    if (!loggingThread) {
+        return;
+    }
 
-    disconnect(this, SIGNAL(stopLoggingSignal()), 0, 0);
+    loggingThread->stopLogging();
+
+    delete loggingThread;
+    loggingThread = NULL;
 }
 
+void LoggingPlugin::loggingStarted()
+{
+    loggingCommand->action()->setText(tr("Stop logging"));
+    if (state == IDLE) {
+        state = LOGGING;
+        emit stateChanged(state);
+    }
+}
 
 /**
  * Receive the logging stopped signal from the LoggingThread
@@ -407,23 +421,11 @@ void LoggingPlugin::stopLogging()
  */
 void LoggingPlugin::loggingStopped()
 {
+    loggingCommand->action()->setText(tr("Start logging..."));
     if (state == LOGGING) {
         state = IDLE;
+        emit stateChanged(state);
     }
-
-    emit stateChanged("IDLE");
-
-    delete loggingThread;
-    loggingThread = NULL;
-}
-
-/**
- * Received the replay stopped signal from the LogFile
- */
-void LoggingPlugin::replayStopped()
-{
-    state = IDLE;
-    emit stateChanged("IDLE");
 }
 
 /**
@@ -431,10 +433,25 @@ void LoggingPlugin::replayStopped()
  */
 void LoggingPlugin::replayStarted()
 {
+    if (state == LOGGING) {
+        stopLogging();
+    }
+    loggingCommand->action()->setEnabled(false);
     state = REPLAY;
-    emit stateChanged("REPLAY");
+    emit stateChanged(state);
 }
 
+/**
+ * Received the replay stopped signal from the LogFile
+ */
+void LoggingPlugin::replayStopped()
+{
+    loggingCommand->action()->setEnabled(true);
+    if (state == REPLAY) {
+        state = IDLE;
+        emit stateChanged(state);
+    }
+}
 
 void LoggingPlugin::extensionsInitialized()
 {
@@ -445,6 +462,7 @@ void LoggingPlugin::shutdown()
 {
     // Do nothing
 }
+
 /**
  * @}
  * @}

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

@@ -1,7 +1,8 @@
 /**
  ******************************************************************************
  * @file       loggingplugin.h
- * @author     The OpenPilot Team, http://www.openpilot.org Copyright (C) 2010.
+ * @author     The LibrePilot Project, http://www.librepilot.org Copyright (C) 2017.
+ *             The OpenPilot Team, http://www.openpilot.org Copyright (C) 2010.
  * @see        The GNU Public License (GPL) Version 3
  * @addtogroup GCSPlugins GCS Plugins
  * @{
@@ -29,34 +30,37 @@
 
 #include <coreplugin/icore.h>
 #include <coreplugin/coreconstants.h>
-#include <coreplugin/actionmanager/actionmanager.h>
 #include <coreplugin/iconnection.h>
 #include <extensionsystem/iplugin.h>
-#include "uavobjectmanager.h"
-#include "gcstelemetrystats.h"
-#include <uavtalk/uavtalk.h>
 #include <utils/logfile.h>
 
 #include <QThread>
 #include <QQueue>
 #include <QReadWriteLock>
 
+class UAVObject;
+class UAVDataObject;
+class UAVTalk;
 class LoggingPlugin;
 class LoggingGadgetFactory;
 
+namespace Core {
+class Command;
+}
+
 /**
  *   Define a connection via the IConnection interface
  *   Plugin will add a instance of this class to the pool,
  *   so the connection manager can use it.
  */
-class LoggingConnection
+class LoggingConnection : public Core::IConnection {
-    : 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);
 
@@ -72,44 +76,36 @@ public:
         return &logFile;
     }
 
-
 private:
-    LogFile logFile;
-    LoggingPlugin *loggingPlugin;
-
-
-protected slots:
-    void onEnumerationChanged();
-    void startReplay(QString file);
-
-protected:
     bool m_deviceOpened;
+    LogFile logFile;
 };
 
-
 class LoggingThread : public QThread {
     Q_OBJECT
 public:
+    LoggingThread();
     virtual ~LoggingThread();
 
-    bool openFile(QString file, LoggingPlugin *parent);
+    bool openFile(QString file);
+
+public slots:
+    void startLogging();
+    void stopLogging();
+
+protected:
+    void run();
 
 private slots:
     void objectUpdated(UAVObject *obj);
     void transactionCompleted(UAVObject *obj, bool success);
 
-public slots:
+private:
-    void stopLogging();
-
-protected:
-    void run();
     QReadWriteLock lock;
+    QQueue<UAVDataObject *> queue;
     LogFile logFile;
     UAVTalk *uavTalk;
 
-private:
-    QQueue<UAVDataObject *> queue;
-
     void retrieveSettings();
     void retrieveNextObject();
 };
@@ -121,6 +117,8 @@ class LoggingPlugin : public ExtensionSystem::IPlugin {
     friend class LoggingConnection;
 
 public:
+    enum State { IDLE, LOGGING, REPLAY };
+
     LoggingPlugin();
     ~LoggingPlugin();
 
@@ -128,41 +126,34 @@ public:
     bool initialize(const QStringList & arguments, QString *errorString);
     void shutdown();
 
-    LoggingConnection *getLogConnection()
-    {
-        return logConnection;
-    };
     LogFile *getLogfile()
     {
         return logConnection->getLogfile();
     }
-    void setLogMenuTitle(QString str);
 
+    State getState()
+    {
+        return state;
+    }
 
 signals:
-    void stopLoggingSignal(void);
+    void stateChanged(State);
-    void stopReplaySignal(void);
-    void stateChanged(QString);
-
-
-protected:
-    enum { IDLE, LOGGING, REPLAY } state;
-    LoggingThread *loggingThread;
-
-    // These are used for replay, logging in its own thread
-    LoggingConnection *logConnection;
 
 private slots:
     void toggleLogging();
     void startLogging(QString file);
     void stopLogging();
+    void loggingStarted();
     void loggingStopped();
     void replayStarted();
     void replayStopped();
 
 private:
-    LoggingGadgetFactory *mf;
+    Core::Command *loggingCommand;
-    Core::Command *cmd;
+    State state;
+    // These are used for replay, logging in its own thread
+    LoggingThread *loggingThread;
+    LoggingConnection *logConnection;
 };
 #endif /* LoggingPLUGIN_H_ */
 /**