Enumerate drives in seperate thread

Ref #87

CMakeLists.txt

@@ -19,7 +19,7 @@ set(CMAKE_AUTOMOC ON)
 set(CMAKE_AUTORCC ON)
 
 # Adding headers explicity so they are displayed in Qt Creator
-set(HEADERS config.h imagewriter.h networkaccessmanagerfactory.h nan.h drivelistitem.h drivelistmodel.h driveformatthread.h powersaveblocker.h
+set(HEADERS config.h imagewriter.h networkaccessmanagerfactory.h nan.h drivelistitem.h drivelistmodel.h drivelistmodelpollthread.h driveformatthread.h powersaveblocker.h
     downloadthread.h downloadextractthread.h localfileextractthread.h dependencies/mountutils/src/mountutils.hpp)
 
 # Add dependencies
@@ -67,7 +67,7 @@ if (NOT atomicbuiltin)
 endif()
 
 set(SOURCES "main.cpp" "imagewriter.cpp" "networkaccessmanagerfactory.cpp"
-    "drivelistitem.cpp" "drivelistmodel.cpp" "downloadthread.cpp" "downloadextractthread.cpp"
+    "drivelistitem.cpp" "drivelistmodel.cpp" "drivelistmodelpollthread.cpp" "downloadthread.cpp" "downloadextractthread.cpp"
      "driveformatthread.cpp" "localfileextractthread.cpp" "powersaveblocker.cpp" "qml.qrc")
 
 find_package(Qt5 COMPONENTS Core Quick LinguistTools Svg OPTIONAL_COMPONENTS Widgets)

drivelistmodel.cpp

@@ -20,6 +20,9 @@ DriveListModel::DriveListModel(QObject *parent)
         {isScsiRole, "isScsi"},
         {mountpointsRole, "mountpoints"}
     };
+
+    // Enumerate drives in seperate thread, but process results in UI thread
+    connect(&_thread, SIGNAL(newDriveList(std::vector<Drivelist::DeviceDescriptor>)), SLOT(processDriveList(std::vector<Drivelist::DeviceDescriptor>)));
 }
 
 int DriveListModel::rowCount(const QModelIndex &) const
@@ -45,11 +48,10 @@ QVariant DriveListModel::data(const QModelIndex &index, int role) const
         return _drivelist.values().at(row)->property(propertyName);
 }
 
-void DriveListModel::refreshDriveList()
+void DriveListModel::processDriveList(std::vector<Drivelist::DeviceDescriptor> l)
 {
     bool changes = false;
     bool filterSystemDrives = DRIVELIST_FILTER_SYSTEM_DRIVES;
-    auto l = Drivelist::ListStorageDevices();
     QSet<QString> drivesInNewList;
 
     for (auto &i: l)
@@ -117,3 +119,13 @@ void DriveListModel::refreshDriveList()
     if (changes)
         endResetModel();
 }
+
+void DriveListModel::startPolling()
+{
+    _thread.start();
+}
+
+void DriveListModel::stopPolling()
+{
+    _thread.stop();
+}

drivelistmodel.h

@@ -10,25 +10,30 @@
 #include <QMap>
 #include <QHash>
 #include "drivelistitem.h"
+#include "drivelistmodelpollthread.h"
 
 class DriveListModel : public QAbstractListModel
 {
+    Q_OBJECT
 public:
     DriveListModel(QObject *parent = nullptr);
     virtual int rowCount(const QModelIndex &) const;
     virtual QHash<int, QByteArray> roleNames() const;
     virtual QVariant data(const QModelIndex &index, int role) const;
+    void startPolling();
+    void stopPolling();
 
     enum driveListRoles {
         deviceRole = Qt::UserRole + 1, descriptionRole, sizeRole, isUsbRole, isScsiRole, mountpointsRole
     };
 
 public slots:
-    void refreshDriveList();
+    void processDriveList(std::vector<Drivelist::DeviceDescriptor> l);
 
 protected:
     QMap<QString,DriveListItem *> _drivelist;
     QHash<int, QByteArray> _rolenames;
+    DriveListModelPollThread _thread;
 };
 
 #endif // DRIVELISTMODEL_H

drivelistmodelpollthread.cpp

@@ -0,0 +1,47 @@
+#include "drivelistmodelpollthread.h"
+#include <QElapsedTimer>
+#include <QDebug>
+
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright (C) 2020 Raspberry Pi (Trading) Limited
+ */
+
+DriveListModelPollThread::DriveListModelPollThread(QObject *parent)
+    : QThread(parent), _terminate(false)
+{
+    qRegisterMetaType< std::vector<Drivelist::DeviceDescriptor> >( "std::vector<Drivelist::DeviceDescriptor>" );
+}
+
+DriveListModelPollThread::~DriveListModelPollThread()
+{
+    _terminate = true;
+    if (!wait(2000)) {
+        terminate();
+    }
+}
+
+void DriveListModelPollThread::stop()
+{
+    _terminate = true;
+}
+
+void DriveListModelPollThread::start()
+{
+    _terminate = false;
+    QThread::start();
+}
+
+void DriveListModelPollThread::run()
+{
+    QElapsedTimer t1;
+
+    while (!_terminate)
+    {
+        t1.start();
+        emit newDriveList( Drivelist::ListStorageDevices() );
+        if (t1.elapsed() > 1000)
+            qDebug() << "Enumerating drives took a long time:" << t1.elapsed()/1000.0 << "seconds";
+        QThread::sleep(1);
+    }
+}

drivelistmodelpollthread.h

@@ -0,0 +1,29 @@
+#ifndef DRIVELISTMODELPOLLTHREAD_H
+#define DRIVELISTMODELPOLLTHREAD_H
+
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright (C) 2020 Raspberry Pi (Trading) Limited
+ */
+
+#include <QThread>
+#include "dependencies/drivelist/src/drivelist.hpp"
+
+class DriveListModelPollThread : public QThread
+{
+    Q_OBJECT
+public:
+    DriveListModelPollThread(QObject *parent = nullptr);
+    ~DriveListModelPollThread();
+    void start();
+    void stop();
+
+protected:
+    bool _terminate;
+    virtual void run() override;
+
+signals:
+    void newDriveList(std::vector<Drivelist::DeviceDescriptor> list);
+};
+
+#endif // DRIVELISTMODELPOLLTHREAD_H

imagewriter.cpp

@@ -323,10 +323,16 @@ void ImageWriter::setCustomOsListUrl(const QUrl &url)
     _repo = url;
 }
 
-/* Refresh the list of available drives */
-void ImageWriter::refreshDriveList()
+/* Start polling the list of available drives */
+void ImageWriter::startDriveListPolling()
 {
-    _drivelist.refreshDriveList();
+    _drivelist.startPolling();
+}
+
+/* Stop polling the list of available drives */
+void ImageWriter::stopDriveListPolling()
+{
+    _drivelist.stopPolling();
 }
 
 DriveListModel *ImageWriter::getDriveList()

imagewriter.h

@@ -48,10 +48,13 @@ class ImageWriter : public QObject
     /* Return true if url is in our local disk cache */
     Q_INVOKABLE bool isCached(const QUrl &url, const QByteArray &sha256);
 
-    /* Refresh the list of available drives */
-    Q_INVOKABLE void refreshDriveList();
+    /* Start polling the list of available drives */
+    Q_INVOKABLE void startDriveListPolling();
 
-    /* Return list of available drives. Call refreshDriveList() first */
+    /* Stop polling the list of available drives */
+    Q_INVOKABLE void stopDriveListPolling();
+
+    /* Return list of available drives. Call startDriveListPolling() first */
     DriveListModel *getDriveList();
 
     /* Utility function to return filename part from URL */

main.qml

@@ -149,8 +149,7 @@ ApplicationWindow {
                         Layout.preferredWidth: 100
                         Layout.fillWidth: true
                         onClicked: {
-                            imageWriter.refreshDriveList()
-                            drivePollTimer.start()
+                            imageWriter.startDriveListPolling()
                             dstpopup.open()
                             dstlist.forceActiveFocus()
                         }
@@ -546,6 +545,7 @@ ApplicationWindow {
         height: parent.height-50
         padding: 0
         closePolicy: Popup.CloseOnEscape | Popup.CloseOnPressOutside
+        onClosed: imageWriter.stopDriveListPolling()
 
         // background of title
         Rectangle {
@@ -616,7 +616,6 @@ ApplicationWindow {
                         if (currentIndex == -1)
                             return
 
-                        drivePollTimer.stop()
                         dstpopup.close()
                         imageWriter.setDst(currentItem.device, currentItem.size)
                         dstbutton.text = currentItem.description
@@ -628,7 +627,6 @@ ApplicationWindow {
                         if (currentIndex == -1)
                             return
 
-                        drivePollTimer.stop()
                         dstpopup.close()
                         imageWriter.setDst(currentItem.device, currentItem.size)
                         dstbutton.text = currentItem.description
@@ -720,7 +718,6 @@ ApplicationWindow {
                 }
 
                 onClicked: {
-                    drivePollTimer.stop()
                     dstpopup.close()
                     imageWriter.setDst(device, size)
                     dstbutton.text = description
@@ -784,13 +781,6 @@ ApplicationWindow {
         property alias y: window.y
     }
 
-    /* Timer for polling drivelist changes */
-    Timer {
-        id: drivePollTimer
-        repeat: true
-        onTriggered: imageWriter.refreshDriveList()
-    }
-
     /* Utility functions */
     function httpRequest(url, callback) {
         var xhr = new XMLHttpRequest();