Merge pull request #238 from JF002/fix-operation-running-in-dfuservice

Fix error management in DfuService (for PineTime/InfiniTime)
piggz · Sep 9, 2022 · b7023fd · b7023fd
2 parents 4a9d5a2 + 6615a90
commit b7023fd
Show file tree

Hide file tree

Showing 7 changed files with 29 additions and 21 deletions.
diff --git a/daemon/src/devices/pinetimejfdevice.cpp b/daemon/src/devices/pinetimejfdevice.cpp
@@ -249,7 +249,7 @@ void PinetimeJFDevice::prepareFirmwareDownload(const AbstractFirmwareInfo *info)
 {
     DfuService *fw = qobject_cast<DfuService*>(service(DfuService::UUID_SERVICE_DFU));
     if (fw){
-        fw->prepareFirmwareDownload(info, new DfuOperation(info, fw));
+        fw->prepareFirmwareDownload(info);
     }
 }
 

diff --git a/daemon/src/operations/dfuoperation.cpp b/daemon/src/operations/dfuoperation.cpp
@@ -138,7 +138,7 @@ void DfuOperation::start()
         m_service->writeValue(DfuService::UUID_CHARACTERISTIC_DFU_CONTROL, UCHAR_TO_BYTEARRAY(DfuService::COMMAND_PACKET_RECEIPT_NOTIFICATION_REQUEST) + QByteArray(1, m_notificationPackets));
 
     } else {
-        m_service->message(QObject::tr("File does not seem to be supported"));
+        emit transferError("File does not seem to be supported");
     }
 }
 

diff --git a/daemon/src/operations/dfuoperation.h b/daemon/src/operations/dfuoperation.h
@@ -2,9 +2,10 @@
 #define DFUOPERATION_H
 
 #include "abstractoperation.h"
-#include "bipfirmwareinfo.h"
 #include "dfuworker.h"
 
+class DfuService;
+class AbstractFirmwareInfo;
 class DfuOperation : public QObject, public AbstractOperation
 {
     Q_OBJECT
@@ -15,8 +16,8 @@ class DfuOperation : public QObject, public AbstractOperation
     void handleData(const QByteArray &data) override;
     void start() override;
 
+    Q_SIGNAL void transferError(const QString error);
 protected:
-
     const AbstractFirmwareInfo *m_info = nullptr;
     QByteArray m_uncompressedFwBytes;
 
@@ -32,6 +33,7 @@ class DfuOperation : public QObject, public AbstractOperation
     uint16_t m_crc16;
 
     Q_SIGNAL void sendFirmware(DfuService* service, const QByteArray &data, int notificationPackets);
+
     Q_SLOT void packetNotification();
     bool probeArchive();
 };

diff --git a/daemon/src/operations/dfuworker.cpp b/daemon/src/operations/dfuworker.cpp
@@ -1,4 +1,8 @@
 #include "dfuworker.h"
+#include "qdebug.h"
+
+#include <QThread>
+#include "dfuservice.h"
 
 DfuWorker::DfuWorker(QObject *parent) : QObject(parent)
 {

diff --git a/daemon/src/operations/dfuworker.h b/daemon/src/operations/dfuworker.h
@@ -2,8 +2,8 @@
 #define DFUWORKER_H
 
 #include <QObject>
-#include "dfuservice.h"
 
+class DfuService;
 class DfuWorker : public QObject
 {
     Q_OBJECT

diff --git a/daemon/src/services/dfuservice.cpp b/daemon/src/services/dfuservice.cpp
@@ -31,25 +31,19 @@ void DfuService::characteristicChanged(const QString &characteristic, const QByt
         qDebug() << "...got metadata";
         if (m_operationRunning == 1 && m_updateFirmware) {
             if (m_updateFirmware->handleMetaData(value)) {
-                delete m_updateFirmware;
-                m_updateFirmware = nullptr;
+                m_updateFirmware.release();
                 m_operationRunning = 0;
             }
         }
     }
 }
 
-void DfuService::prepareFirmwareDownload(const AbstractFirmwareInfo *info, DfuOperation* operation)
+void DfuService::prepareFirmwareDownload(const AbstractFirmwareInfo *info)
 {
-    if (!m_updateFirmware) {
-        m_updateFirmware = operation;
+    if (m_operationRunning == 1) {
+        emit message(tr("An operation is currently running, please try later"));
     } else {
-        if (m_operationRunning == 1) {
-            emit message(tr("An operation is currently running, please try later"));
-        } else {
-            delete m_updateFirmware;
-            m_updateFirmware = new DfuOperation(info, this);
-        }
+        m_updateFirmware.reset(new DfuOperation(info, this));
     }
 }
 
@@ -58,6 +52,7 @@ void DfuService::startDownload()
     qDebug() << Q_FUNC_INFO;
     if (m_updateFirmware && m_operationRunning == 0) {
         m_operationRunning = 1;
+        connect(m_updateFirmware.get(), &DfuOperation::transferError, this, &DfuService::onTransferError);
         m_updateFirmware->start();
     } else {
         emit message(tr("No file selected"));
@@ -73,8 +68,7 @@ bool DfuService::operationRunning()
 void DfuService::abortOperations()
 {
     if (m_updateFirmware) {
-        delete m_updateFirmware;
-        m_updateFirmware = nullptr;
+        m_updateFirmware.release();
     }
     m_operationRunning = 0;
     emit operationRunningChanged();
@@ -89,3 +83,9 @@ bool DfuService::waitForWatch()
 {
     return m_waitForWatch.load();
 }
+
+void DfuService::onTransferError(const QString error) {
+    m_operationRunning = 0;
+    qDebug() << "Transfer error : " << error;
+    emit message(error);
+}
diff --git a/daemon/src/services/dfuservice.h b/daemon/src/services/dfuservice.h
@@ -1,9 +1,10 @@
 #ifndef DFUSERVICE_H
 #define DFUSERVICE_H
 
+#include <memory>
 #include "qble/qbleservice.h"
 #include "abstractfirmwareinfo.h"
-
+#include "dfuoperation.h"
 /*
 {00001530-1212-EFDE-1523-785FEABCD123} DFU Service
 --00001531-1212-EFDE-1523-785FEABCD123 //Control Point
@@ -47,7 +48,7 @@ class DfuService : public QBLEService
     static constexpr uint8_t ERROR_CRC_ERROR = 0x05;
     static constexpr uint8_t ERROR_OPERATION_FAILED = 0x06;
 
-    void prepareFirmwareDownload(const AbstractFirmwareInfo *info, DfuOperation* operation);
+    void prepareFirmwareDownload(const AbstractFirmwareInfo *info);
     void startDownload();
     Q_SIGNAL void downloadProgress(int percent);
 
@@ -59,9 +60,10 @@ class DfuService : public QBLEService
 
 private:
     Q_SLOT void characteristicChanged(const QString &characteristic, const QByteArray &value);
+    Q_SLOT void onTransferError(const QString error);
 
     int m_operationRunning = 0;
-    DfuOperation *m_updateFirmware = nullptr;
+    std::unique_ptr<DfuOperation> m_updateFirmware = nullptr;
     std::atomic<bool> m_waitForWatch;
 };