Merge pull request #975 from DonLakeFlyer/UpgradeErrors

Better error reporting during Firmware Upgrade

libs/qextserialport/src/qextserialport.cpp

@@ -755,6 +755,8 @@ QString QextSerialPort::errorString()
         return tr("Permission denied");
     case E_AGAIN:
         return tr("Device is already locked");
+    case E_OS_SPECIFIC:
+        return tr("OS error: %1").arg(d->lastOSErrString);
     default:
         return tr("Unknown error: %1").arg(d->lastErr);
     }

libs/qextserialport/src/qextserialport.h

@@ -68,6 +68,7 @@
 #define E_FILE_NOT_FOUND            15
 #define E_PERMISSION_DENIED         16
 #define E_AGAIN                     17
+#define E_OS_SPECIFIC               18  // Error did not translate, os error code in lastOSErr
 
 enum BaudRateType
 {
@@ -201,7 +202,7 @@ public:
     bool canReadLine() const;
     QByteArray readAll();
 
-    ulong lastError() const;
+    ulong lastError() const;    ulong lastOpenOSError() const;
 
     ulong lineStatus();
     QString errorString();

libs/qextserialport/src/qextserialport_p.h

@@ -197,6 +197,8 @@ public:
     QextReadBuffer readBuffer;
     int settingsDirtyFlags;
     ulong lastErr;
+    ulong lastOSErr;
+    QString lastOSErrString;
     QextSerialPort::QueryMode queryMode;
 
     // platform specific members

libs/qextserialport/src/qextserialport_unix.cpp

@@ -155,6 +155,11 @@ void QextSerialPortPrivate::translateError(ulong error)
     case EAGAIN:
         lastErr = E_AGAIN;
         break;
+    default:
+        lastOSErr = error;
+        lastOSErrString = strerror(error);
+        lastErr = E_OS_SPECIFIC;
+        break;
     }
 }
 

libs/qextserialport/src/qextserialport_win.cpp

@@ -106,6 +106,7 @@ bool QextSerialPortPrivate::open_sys(QIODevice::OpenMode mode)
         if (queryMode == QextSerialPort::EventDriven) {
             if (!SetCommMask(handle, EV_TXEMPTY | EV_RXCHAR | EV_DSR)) {
                 QESP_WARNING()<<"failed to set Comm Mask. Error code:"<<GetLastError();
+                translateError(GetLastError());
                 return false;
             }
             winEventNotifier = new QWinEventNotifier(overlap.hEvent, q);
@@ -115,6 +116,7 @@ bool QextSerialPortPrivate::open_sys(QIODevice::OpenMode mode)
         }
         return true;
     }
+    translateError(GetLastError());
     return false;
 }
 
@@ -159,30 +161,18 @@ qint64 QextSerialPortPrivate::bytesAvailable_sys() const
 */
 void QextSerialPortPrivate::translateError(ulong error)
 {
-    if (error & CE_BREAK) {
-        lastErr = E_BREAK_CONDITION;
-    }
-    else if (error & CE_FRAME) {
-        lastErr = E_FRAMING_ERROR;
-    }
-    else if (error & CE_IOE) {
-        lastErr = E_IO_ERROR;
-    }
-    else if (error & CE_MODE) {
-        lastErr = E_INVALID_FD;
-    }
-    else if (error & CE_OVERRUN) {
-        lastErr = E_BUFFER_OVERRUN;
-    }
-    else if (error & CE_RXPARITY) {
-        lastErr = E_RECEIVE_PARITY_ERROR;
-    }
-    else if (error & CE_RXOVER) {
-        lastErr = E_RECEIVE_OVERFLOW;
-    }
-    else if (error & CE_TXFULL) {
-        lastErr = E_TRANSMIT_OVERFLOW;
-    }
+    lastOSErr = error;
+    lastErr = E_OS_SPECIFIC;
+    
+    char buf[256];
+    FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM |
+                  FORMAT_MESSAGE_IGNORE_INSERTS,
+                  NULL,
+                  error,
+                  MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+                  buf, sizeof(buf), 
+				  NULL);
+    lastOSErrString = buf;
 }
 
 /*
@@ -212,8 +202,8 @@ qint64 QextSerialPortPrivate::readData_sys(char *data, qint64 maxSize)
     if (!failed)
         return (qint64)bytesRead;
 
-    lastErr = E_READ_FAILED;
-    return -1;
+	translateError(GetLastError());
+	return -1;
 }
 
 /*
@@ -257,7 +247,7 @@ qint64 QextSerialPortPrivate::writeData_sys(const char *data, qint64 maxSize)
     if (!failed)
         return (qint64)bytesWritten;
 
-    lastErr = E_WRITE_FAILED;
+	translateError(GetLastError());
     return -1;
 }
 

src/ui/px4_configuration/PX4Bootloader.cc

@@ -107,7 +107,7 @@ bool PX4Bootloader::write(QextSerialPort* port, const uint8_t byte)
     return write(port, buf, 1);
 }
 
-bool PX4Bootloader::read(QextSerialPort* port, uint8_t* data, qint64 maxSize, bool warnOnError, int readTimeout)
+bool PX4Bootloader::read(QextSerialPort* port, uint8_t* data, qint64 maxSize, int readTimeout)
 {
     qint64 bytesAlreadyRead = 0;
     
@@ -117,9 +117,6 @@ bool PX4Bootloader::read(QextSerialPort* port, uint8_t* data, qint64 maxSize, bo
         while (port->bytesAvailable() < 1) {
             if (timeout.elapsed() > readTimeout) {
                 _errorString = tr("Timeout waiting for bytes to be available");
-                if (warnOnError) {
-                    qWarning() << _errorString;
-                }
                 return false;
             }
             QGC::SLEEP::usleep(100);
@@ -129,10 +126,7 @@ bool PX4Bootloader::read(QextSerialPort* port, uint8_t* data, qint64 maxSize, bo
         bytesRead = port->read((char*)&data[bytesAlreadyRead], maxSize);
         
         if (bytesRead == -1) {
-            _errorString = tr("Read failed: Could not read 1 byte, error: %1").arg(port->errorString());
-            if (warnOnError) {
-                qWarning() << _errorString;
-            }
+            _errorString = tr("Read failed: error: %1").arg(port->errorString());
             return false;
         } else {
             Q_ASSERT(bytesRead != 0);
@@ -143,20 +137,18 @@ bool PX4Bootloader::read(QextSerialPort* port, uint8_t* data, qint64 maxSize, bo
     return true;
 }
 
-bool PX4Bootloader::getCommandResponse(QextSerialPort* port, bool warnOnError, int responseTimeout)
+bool PX4Bootloader::getCommandResponse(QextSerialPort* port, int responseTimeout)
 {
     uint8_t response[2];
     
-    if (!read(port, response, 2, warnOnError, responseTimeout)) {
+    if (!read(port, response, 2, responseTimeout)) {
+        _errorString.prepend("Get Command Response: ");
         return false;
     }
     
     // Make sure we get a good sync response
     if (response[0] != PROTO_INSYNC) {
         _errorString = tr("Invalid sync response: 0x%1 0x%2").arg(response[0], 2, 16, QLatin1Char('0')).arg(response[1], 2, 16, QLatin1Char('0'));
-        if (warnOnError) {
-            qWarning() << _errorString;
-        }
         return false;
     } else if (response[1] != PROTO_OK) {
         QString responseCode = tr("Unknown response code");
@@ -166,9 +158,6 @@ bool PX4Bootloader::getCommandResponse(QextSerialPort* port, bool warnOnError, i
             responseCode = "PROTO_INVALID";
         }
         _errorString = tr("Command failed: 0x%1 (%2)").arg(response[1], 2, 16, QLatin1Char('0')).arg(responseCode);
-        if (warnOnError) {
-            qWarning() << _errorString;
-        }
         return false;
     }
     
@@ -180,32 +169,47 @@ bool PX4Bootloader::getBoardInfo(QextSerialPort* port, uint8_t param, uint32_t&
     uint8_t buf[3] = { PROTO_GET_DEVICE, param, PROTO_EOC };
     
     if (!write(port, buf, sizeof(buf))) {
-        return false;
+        goto Error;
     }
     port->flush();
-    if (!read(port, (uint8_t*)&value, sizeof(value), warnOnError)) {
-        return false;
+    if (!read(port, (uint8_t*)&value, sizeof(value))) {
+        goto Error;
     }
-    return getCommandResponse(port, warnOnError);
+    if (!getCommandResponse(port)) {
+        goto Error;
+    }
+    
+    return true;
+    
+Error:
+    _errorString.prepend("Get Board Info: ");
+    return false;
 }
 
-bool PX4Bootloader::sendCommand(QextSerialPort* port, const uint8_t cmd, bool warnOnError, int responseTimeout)
+bool PX4Bootloader::sendCommand(QextSerialPort* port, const uint8_t cmd, int responseTimeout)
 {
     uint8_t buf[2] = { cmd, PROTO_EOC };
     
     if (!write(port, buf, 2)) {
-        return false;
+        goto Error;
     }
     port->flush();
-    return getCommandResponse(port, warnOnError, responseTimeout);
+    if (!getCommandResponse(port, responseTimeout)) {
+        goto Error;
+    }
+    
+    return true;
+
+Error:
+    _errorString.prepend("Send Command: ");
+    return false;
 }
 
 bool PX4Bootloader::erase(QextSerialPort* port)
 {
     // Erase is slow, need larger timeout
-    if (!sendCommand(port, PROTO_CHIP_ERASE, warnOnError, _eraseTimeout)) {
+    if (!sendCommand(port, PROTO_CHIP_ERASE, _eraseTimeout)) {
         _errorString = tr("Board erase failed: %1").arg(_errorString);
-        qWarning() << _errorString;
         return false;
     }
     
@@ -217,7 +221,6 @@ bool PX4Bootloader::program(QextSerialPort* port, const QString& firmwareFilenam
     QFile firmwareFile(firmwareFilename);
     if (!firmwareFile.open(QIODevice::ReadOnly)) {
         _errorString = tr("Unable to open firmware file %1: %2").arg(firmwareFilename).arg(firmwareFile.errorString());
-        qWarning() << _errorString;
         return false;
     }
     uint32_t imageSize = (uint32_t)firmwareFile.size();
@@ -238,8 +241,7 @@ bool PX4Bootloader::program(QextSerialPort* port, const QString& firmwareFilenam
         
         int bytesRead = firmwareFile.read((char *)imageBuf, bytesToSend);
         if (bytesRead == -1 || bytesRead != bytesToSend) {
-            _errorString = tr("Read failed: %1").arg(firmwareFile.errorString());
-            qWarning() << _errorString;
+            _errorString = tr("Firmware file read failed: %1").arg(firmwareFile.errorString());
             return false;
         }
         
@@ -251,7 +253,7 @@ bool PX4Bootloader::program(QextSerialPort* port, const QString& firmwareFilenam
                 if (write(port, imageBuf, bytesToSend)) {
                     if (write(port, PROTO_EOC)) {
                         port->flush();
-                        if (getCommandResponse(port, warnOnError)) {
+                        if (getCommandResponse(port)) {
                             failed = false;
                         }
                     }
@@ -260,7 +262,6 @@ bool PX4Bootloader::program(QextSerialPort* port, const QString& firmwareFilenam
         }
         if (failed) {
             _errorString = tr("Flash failed: %1 at address 0x%2").arg(_errorString).arg(bytesSent, 8, 16, QLatin1Char('0'));
-            qWarning() << _errorString;
             return false;
         }
         
@@ -305,12 +306,11 @@ bool PX4Bootloader::_bootloaderVerifyRev2(QextSerialPort* port, const QString fi
     QFile firmwareFile(firmwareFilename);
     if (!firmwareFile.open(QIODevice::ReadOnly)) {
         _errorString = tr("Unable to open firmware file %1: %2").arg(firmwareFilename).arg(firmwareFile.errorString());
-        qWarning() << _errorString;
         return false;
     }
     uint32_t imageSize = (uint32_t)firmwareFile.size();
     
-    if (!sendCommand(port, PROTO_CHIP_VERIFY, warnOnError)) {
+    if (!sendCommand(port, PROTO_CHIP_VERIFY)) {
         return false;
     }
     
@@ -330,8 +330,7 @@ bool PX4Bootloader::_bootloaderVerifyRev2(QextSerialPort* port, const QString fi
         
         int bytesRead = firmwareFile.read((char *)fileBuf, bytesToRead);
         if (bytesRead == -1 || bytesRead != bytesToRead) {
-            _errorString = tr("Read failed: %1").arg(firmwareFile.errorString());
-            qWarning() << _errorString;
+            _errorString = tr("Firmware file read failed: %1").arg(firmwareFile.errorString());
             return false;
         }
         
@@ -342,8 +341,8 @@ bool PX4Bootloader::_bootloaderVerifyRev2(QextSerialPort* port, const QString fi
             if (write(port, (uint8_t)bytesToRead)) {
                 if (write(port, PROTO_EOC)) {
                     port->flush();
-                    if (read(port, flashBuf, sizeof(flashBuf), warnOnError)) {
-                        if (getCommandResponse(port, warnOnError)) {
+                    if (read(port, flashBuf, sizeof(flashBuf))) {
+                        if (getCommandResponse(port)) {
                             failed = false;
                         }
                     }
@@ -352,14 +351,12 @@ bool PX4Bootloader::_bootloaderVerifyRev2(QextSerialPort* port, const QString fi
         }
         if (failed) {
             _errorString = tr("Verify failed: %1 at address: 0x%2").arg(_errorString).arg(bytesVerified, 8, 16, QLatin1Char('0'));
-            qWarning() << _errorString;
             return false;
         }
 
         for (int i=0; i<bytesToRead; i++) {
             if (fileBuf[i] != flashBuf[i]) {
                 _errorString = tr("Compare failed at %1: file(0x%2) flash(0x%3) at address: 0x%4").arg(bytesVerified + i).arg(fileBuf[i], 2, 16, QLatin1Char('0')).arg(flashBuf[i], 2, 16, QLatin1Char('0')).arg(bytesVerified, 8, 16, QLatin1Char('0'));
-                qWarning() << _errorString;
                 return false;
             }
         }
@@ -380,8 +377,8 @@ bool PX4Bootloader::_bootloaderVerifyRev3(QextSerialPort* port)
     bool failed = true;
     if (write(port, buf, 2)) {
         port->flush();
-        if (read(port, (uint8_t*)&flashCRC, sizeof(flashCRC), warnOnError, _verifyTimeout)) {
-            if (getCommandResponse(port, warnOnError)) {
+        if (read(port, (uint8_t*)&flashCRC, sizeof(flashCRC), _verifyTimeout)) {
+            if (getCommandResponse(port)) {
                 failed = false;
             }
         }
@@ -411,7 +408,6 @@ bool PX4Bootloader::open(QextSerialPort* port, const QString portName)
     
     if (!port->open(QIODevice::ReadWrite | QIODevice::Unbuffered)) {
         _errorString = tr("Open failed on port %1: %2").arg(portName).arg(port->errorString());
-        qWarning() << _errorString;
         return false;
     }
     
@@ -421,7 +417,12 @@ bool PX4Bootloader::open(QextSerialPort* port, const QString portName)
 bool PX4Bootloader::sync(QextSerialPort* port)
 {
     // Send sync command
-    return sendCommand(port, PROTO_GET_SYNC, noWarnOnError);
+    if (sendCommand(port, PROTO_GET_SYNC)) {
+        return true;
+    } else {
+        _errorString.prepend("Sync: ");
+        return false;
+    }
 }
 
 bool PX4Bootloader::getBoardInfo(QextSerialPort* port, uint32_t& bootloaderVersion, uint32_t& boardID, uint32_t& flashSize)
@@ -432,7 +433,6 @@ bool PX4Bootloader::getBoardInfo(QextSerialPort* port, uint32_t& bootloaderVersi
     }
     if (_bootloaderVersion < BL_REV_MIN || _bootloaderVersion > BL_REV_MAX) {
         _errorString = tr("Found unsupported bootloader version: %1").arg(_bootloaderVersion);
-        qWarning() << _errorString;
         goto Error;
     }
     
@@ -441,7 +441,6 @@ bool PX4Bootloader::getBoardInfo(QextSerialPort* port, uint32_t& bootloaderVersi
     }
     if (_boardID != _boardIDPX4Flow && _boardID != _boardIDPX4FMUV1 && _boardID != _boardIDPX4FMUV2) {
         _errorString = tr("Unsupported board: %1").arg(_boardID);
-        qWarning() << _errorString;
         goto Error;
     }
     
@@ -457,6 +456,7 @@ bool PX4Bootloader::getBoardInfo(QextSerialPort* port, uint32_t& bootloaderVersi
     return true;
     
 Error:
+    _errorString.prepend("Get Board Info: ");
     return false;
 }
 

src/ui/px4_configuration/PX4Bootloader.h

@@ -43,9 +43,6 @@ public:
     /// @brief Returns the error message associated with the last failed call to one of the bootloader
     ///         utility routine below.
     QString errorString(void) { return _errorString; }
-
-    static const bool warnOnError = true;       ///< call qWarning to log error message on error
-    static const bool noWarnOnError = false;    ///< Don't call qWarning on error
     
     /// @brief Write a byte to the port
     ///     @param port Port to write to
@@ -58,13 +55,12 @@ public:
     /// @brief Read a set of bytes from the port
     ///     @param data Read bytes into this buffer
     ///     @param maxSize Number of bytes to read
-    ///     @param warnOnError true: Log error using qWarning
     ///     @param readTimeout Msecs to wait for bytes to become available on port
-    bool read(QextSerialPort* port, uint8_t* data, qint64 maxSize, bool warnOnError, int readTimeout = _readTimout);
+    bool read(QextSerialPort* port, uint8_t* data, qint64 maxSize, int readTimeout = _readTimout);
     
     /// @brief Read a PROTO_SYNC command response from the bootloader
     ///     @param responseTimeout Msecs to wait for response bytes to become available on port
-    bool getCommandResponse(QextSerialPort* port, bool warnOnError, const int responseTimeout = _responseTimeout);
+    bool getCommandResponse(QextSerialPort* port, const int responseTimeout = _responseTimeout);
     
     /// @brief Send a PROTO_GET_DEVICE command to retrieve a value from the bootloader
     ///     @param param Value to retrieve using INFO_BOARD_* enums
@@ -74,7 +70,7 @@ public:
     /// @brief Send a command to the bootloader
     ///     @param cmd Command to send using PROTO_* enums
     /// @return true: Command sent and valid sync response returned
-    bool sendCommand(QextSerialPort* port, uint8_t cmd, bool warnOnError, int responseTimeout = _responseTimeout);
+    bool sendCommand(QextSerialPort* port, uint8_t cmd, int responseTimeout = _responseTimeout);
     
     /// @brief Program the board with the specified firmware
     bool program(QextSerialPort* port, const QString& firmwareFilename);

src/ui/px4_configuration/PX4FirmwareUpgradeThread.cc

@@ -102,11 +102,6 @@ void PX4FirmwareUpgradeThreadWorker::_findBoardOnce(void)
             portName = info.systemLocation();
             portDescription = info.description();
             
-#ifdef Q_OS_WIN
-            // Stupid windows fixes
-            portName.prepend("\\\\.\\");
-#endif
-            
             _closeFind();
             emit foundBoard(_findBoardFirstAttempt, portName, portDescription);
             return;
@@ -121,10 +116,10 @@ void PX4FirmwareUpgradeThreadWorker::_findBoardOnce(void)
 
 void PX4FirmwareUpgradeThreadWorker::findBootloader(const QString portName, int msecTimeout)
 {
-    connect(_timerRetry, &QTimer::timeout, this, &PX4FirmwareUpgradeThreadWorker::_findBootloaderOnce);
+    Q_UNUSED(msecTimeout);
+    
+    // Once the port shows up, we only try to connect to the bootloader a single time
     _portName = portName;
-    _timerTimeout->start(msecTimeout);
-    _elapsed.start();
     _findBootloaderOnce();
 }
 
@@ -144,28 +139,16 @@ void PX4FirmwareUpgradeThreadWorker::_findBootloaderOnce(void)
                 qDebug() << "Found bootloader";
                 emit foundBootloader(bootloaderVersion, boardID, flashSize);
                 return;
-            } else {
-                _closeFind();
-                _bootloaderPort->close();
-                _bootloaderPort->deleteLater();
-                _bootloaderPort = NULL;
-                qDebug() << "Bootloader Get Board Info error:" << _bootloader->errorString();
-                emit error(commandBootloader, _bootloader->errorString());
-                return;
             }
-        } else {
-            _closeFind();
-            _bootloaderPort->close();
-            _bootloaderPort->deleteLater();
-            _bootloaderPort = NULL;
-            qDebug() << "Bootloader sync failed";
-            emit bootloaderSyncFailed();
-            return;
         }
     }
     
-    emit updateProgress(_elapsed.elapsed(), _timerTimeout->interval());
-    _timerRetry->start();
+    _closeFind();
+    _bootloaderPort->close();
+    _bootloaderPort->deleteLater();
+    _bootloaderPort = NULL;
+    qDebug() << "Bootloader error:" << _bootloader->errorString();
+    emit error(commandBootloader, _bootloader->errorString());
 }
 
 void PX4FirmwareUpgradeThreadWorker::_closeFind(void)