From e8b459721bc504e8b853bc7696364cd388d996e4 Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Fri, 14 Nov 2014 11:36:50 -0800 Subject: [PATCH] Better error reporting --- libs/qextserialport/src/qextserialport.cpp | 2 + libs/qextserialport/src/qextserialport.h | 3 +- libs/qextserialport/src/qextserialport_p.h | 2 + .../src/qextserialport_unix.cpp | 5 ++ .../qextserialport/src/qextserialport_win.cpp | 40 ++++----- src/ui/px4_configuration/PX4Bootloader.cc | 88 +++++++++---------- src/ui/px4_configuration/PX4Bootloader.h | 10 +-- .../PX4FirmwareUpgradeThread.cc | 30 ++----- 8 files changed, 83 insertions(+), 97 deletions(-) diff --git a/libs/qextserialport/src/qextserialport.cpp b/libs/qextserialport/src/qextserialport.cpp index 119bef868..edbde76c8 100755 --- a/libs/qextserialport/src/qextserialport.cpp +++ b/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); } diff --git a/libs/qextserialport/src/qextserialport.h b/libs/qextserialport/src/qextserialport.h index 5334e94b2..f35184cff 100755 --- a/libs/qextserialport/src/qextserialport.h +++ b/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(); diff --git a/libs/qextserialport/src/qextserialport_p.h b/libs/qextserialport/src/qextserialport_p.h index d278a17e9..0a92cb7a0 100755 --- a/libs/qextserialport/src/qextserialport_p.h +++ b/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 diff --git a/libs/qextserialport/src/qextserialport_unix.cpp b/libs/qextserialport/src/qextserialport_unix.cpp index 4c02a206d..97d7ac1a3 100755 --- a/libs/qextserialport/src/qextserialport_unix.cpp +++ b/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; } } diff --git a/libs/qextserialport/src/qextserialport_win.cpp b/libs/qextserialport/src/qextserialport_win.cpp index 4dfd97ac3..caa68988d 100755 --- a/libs/qextserialport/src/qextserialport_win.cpp +++ b/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:"<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; iflush(); - 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; } diff --git a/src/ui/px4_configuration/PX4Bootloader.h b/src/ui/px4_configuration/PX4Bootloader.h index aec3c78e6..2587c6ae7 100644 --- a/src/ui/px4_configuration/PX4Bootloader.h +++ b/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); diff --git a/src/ui/px4_configuration/PX4FirmwareUpgradeThread.cc b/src/ui/px4_configuration/PX4FirmwareUpgradeThread.cc index 168f2ef22..f06b303f0 100644 --- a/src/ui/px4_configuration/PX4FirmwareUpgradeThread.cc +++ b/src/ui/px4_configuration/PX4FirmwareUpgradeThread.cc @@ -121,10 +121,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 +144,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) -- 2.22.0