Merge branch 'filnet/OP-682_fix_for_crash_on_firmware_uploader_gadget' into next

ground/openpilotgcs/src/plugins/uploader/devicewidget.cpp

@@ -35,12 +35,12 @@ deviceWidget::deviceWidget(QWidget *parent) :
     // Initialization of the Device icon display
     myDevice->verticalGroupBox_loaded->setVisible(false);
     myDevice->groupCustom->setVisible(false);
-    myDevice->youdont->setVisible(false);
+    myDevice->confirmCheckBox->setVisible(false);
     myDevice->gVDevice->setScene(new QGraphicsScene(this));
     connect(myDevice->retrieveButton, SIGNAL(clicked()), this, SLOT(downloadFirmware()));
     connect(myDevice->updateButton, SIGNAL(clicked()), this, SLOT(uploadFirmware()));
     connect(myDevice->pbLoad, SIGNAL(clicked()), this, SLOT(loadFirmware()));
-    connect(myDevice->youdont, SIGNAL(stateChanged(int)), this, SLOT(confirmCB(int)));
+    connect(myDevice->confirmCheckBox, SIGNAL(stateChanged(int)), this, SLOT(confirmCB(int)));
     QPixmap pix = QPixmap(QString(":uploader/images/view-refresh.svg"));
     myDevice->statusIcon->setPixmap(pix);
 
@@ -143,10 +143,29 @@ void deviceWidget::populate()
   */
 void deviceWidget::freeze()
 {
+    updateButtons(false);
+}
+
+void deviceWidget::updateButtons(bool enabled)
+{
+    if (!enabled) {
         myDevice->description->setEnabled(false);
+        myDevice->pbLoad->setEnabled(false);
+        myDevice->confirmCheckBox->setEnabled(false);
         myDevice->updateButton->setEnabled(false);
         myDevice->retrieveButton->setEnabled(false);
     }
+    else {
+        myDevice->description->setEnabled(true);
+        // Load button (i.e. choose file) is always enabled
+        myDevice->pbLoad->setEnabled(true);
+        myDevice->confirmCheckBox->setEnabled(true);
+        // Update/Upload button is enabled if the "I know what I'm doing!" check box is checked
+        myDevice->updateButton->setEnabled(myDevice->confirmCheckBox->checkState() == Qt::Checked);
+        // Retreive/Download button is always enabled
+        myDevice->retrieveButton->setEnabled(true);
+    }
+}
 
 /**
   Populates the widget field with the description in case
@@ -222,12 +241,7 @@ void deviceWidget::dfuStatus(QString str)
 
 void deviceWidget::confirmCB(int value)
 {
-    if(value==Qt::Checked)
+    updateButtons(true);
-    {
-        myDevice->updateButton->setEnabled(true);
-    }
-    else
-        myDevice->updateButton->setEnabled(false);
 }
 
 /**
@@ -261,6 +275,9 @@ void deviceWidget::loadFirmware()
 
     filename = setOpenFileName();
 
+    myDevice->confirmCheckBox->setVisible(false);
+    myDevice->confirmCheckBox->setChecked(false);
+
     if (filename.isEmpty()) {
         status("Empty filename", STATUSICON_FAIL);
         return;
@@ -273,58 +290,46 @@ void deviceWidget::loadFirmware()
     }
 
     loadedFW = file.readAll();
-    myDevice->youdont->setVisible(false);
+
-    myDevice->youdont->setChecked(false);
     QByteArray desc = loadedFW.right(100);
     QPixmap px;
-    if(loadedFW.length()>m_dfu->devices[deviceID].SizeOfCode)
+    if (loadedFW.length()>m_dfu->devices[deviceID].SizeOfCode) {
         myDevice->lblCRCL->setText(tr("Can't calculate, file too big for device"));
-    else
+    }
+    else {
         myDevice->lblCRCL->setText( QString::number(DFUObject::CRCFromQBArray(loadedFW,m_dfu->devices[deviceID].SizeOfCode)));
+    }
     //myDevice->lblFirmwareSizeL->setText(QString("Firmware size: ")+QVariant(loadedFW.length()).toString()+ QString(" bytes"));
     if (populateLoadedStructuredDescription(desc))
     {
-        myDevice->youdont->setChecked(true);
+        myDevice->confirmCheckBox->setChecked(true);
         myDevice->verticalGroupBox_loaded->setVisible(true);
         myDevice->groupCustom->setVisible(false);
-        if(myDevice->lblCRC->text()==myDevice->lblCRCL->text())
+        if (myDevice->lblCRC->text() == myDevice->lblCRCL->text()) {
-        {
+            myDevice->statusLabel->setText(tr("The board has the same firmware as loaded. No need to update."));
-            myDevice->statusLabel->setText(tr("The board has the same firmware as loaded. No need to update"));
             px.load(QString(":/uploader/images/warning.svg"));
-        }
+        } else if (myDevice->lblDevName->text() != myDevice->lblBrdNameL->text()) {
-        else if(myDevice->lblDevName->text()!=myDevice->lblBrdNameL->text())
-        {
             myDevice->statusLabel->setText(tr("WARNING: the loaded firmware is for different hardware. Do not update!"));
             px.load(QString(":/uploader/images/error.svg"));
-        }
+        } else if (QDateTime::fromString(onBoardDescription.gitDate) > QDateTime::fromString(LoadedDescription.gitDate)) {
-        else if(QDateTime::fromString(onBoardDescription.gitDate)>QDateTime::fromString(LoadedDescription.gitDate))
-        {
             myDevice->statusLabel->setText(tr("The board has newer firmware than loaded. Are you sure you want to update?"));
             px.load(QString(":/uploader/images/warning.svg"));
-        }
+        } else if (!LoadedDescription.gitTag.startsWith("RELEASE", Qt::CaseSensitive)) {
-        else if(!LoadedDescription.gitTag.startsWith("RELEASE",Qt::CaseSensitive))
+            myDevice->statusLabel->setText(tr("The loaded firmware is untagged or custom build. Update only if it was received from a trusted source (official website or your own build)."));
-        {
-            myDevice->statusLabel->setText(tr("The loaded firmware is untagged or custom build. Update only if it was received from a trusted source (official website or your own build)"));
             px.load(QString(":/uploader/images/warning.svg"));
-        }
+        } else {
-        else
+            myDevice->statusLabel->setText(tr("This is the tagged officially released OpenPilot firmware."));
-        {
-            myDevice->statusLabel->setText(tr("This is the tagged officially released OpenPilot firmware"));
             px.load(QString(":/uploader/images/gtk-info.svg"));
         }
-    }
+    } else {
-    else
+        myDevice->statusLabel->setText(tr("WARNING: the loaded firmware was not packaged with the OpenPilot format. Do not update unless you know what you are doing."));
-    {
-        myDevice->statusLabel->setText(tr("WARNING: the loaded firmware was not packaged with the OpenPilot format. Do not update unless you know what you are doing"));
         px.load(QString(":/uploader/images/error.svg"));
-        myDevice->youdont->setChecked(false);
+        myDevice->confirmCheckBox->setChecked(false);
-        myDevice->youdont->setVisible(true);
+        myDevice->confirmCheckBox->setVisible(true);
         myDevice->verticalGroupBox_loaded->setVisible(false);
         myDevice->groupCustom->setVisible(true);
     }
     myDevice->statusIcon->setPixmap(px);
-    //myDevice->updateButton->setEnabled(true);
-
 }
 
 /**
@@ -332,10 +337,14 @@ void deviceWidget::loadFirmware()
   */
 void deviceWidget::uploadFirmware()
 {
-    myDevice->updateButton->setEnabled(false);
+    // clear progress bar now
+    // this avoids displaying an error message and the progress at 100% at the same time
+    setProgress(0);
+    updateButtons(false);
+
     if (!m_dfu->devices[deviceID].Writable) {
         status("Device not writable!", STATUSICON_FAIL);
-        myDevice->updateButton->setEnabled(true);
+        updateButtons(true);
         return;
     }
 
@@ -359,7 +368,7 @@ void deviceWidget::uploadFirmware()
             // These firmwares are designed to be backwards compatible
         } else if (firmwareBoard != board) {
             status("Error: firmware does not match board", STATUSICON_FAIL);
-            myDevice->updateButton->setEnabled(true);
+            updateButtons(true);
             return;
         }
         // Check the firmware embedded in the file:
@@ -367,7 +376,7 @@ void deviceWidget::uploadFirmware()
         QByteArray fileHash = QCryptographicHash::hash(loadedFW.left(loadedFW.length()-100), QCryptographicHash::Sha1);
         if (firmwareHash != fileHash) {
             status("Error: firmware file corrupt", STATUSICON_FAIL);
-            myDevice->updateButton->setEnabled(true);
+            updateButtons(true);
             return;
         }
     } else {
@@ -376,16 +385,16 @@ void deviceWidget::uploadFirmware()
         descriptionArray.clear();
     }
 
-
     status("Starting firmware upload", STATUSICON_RUNNING);
+    emit uploadStarted();
+
     // We don't know which device was used previously, so we
     // are cautious and reenter DFU for this deviceID:
-    emit uploadStarted();
     if(!m_dfu->enterDFU(deviceID))
     {
-        status("Error:Could not enter DFU mode", STATUSICON_FAIL);
-        myDevice->updateButton->setEnabled(true);
         emit uploadEnded(false);
+        status("Error:Could not enter DFU mode", STATUSICON_FAIL);
+        updateButtons(true);
         return;
     }
     OP_DFU::Status ret=m_dfu->StatusRequest();
@@ -397,11 +406,12 @@ void deviceWidget::uploadFirmware()
     connect(m_dfu, SIGNAL(uploadFinished(OP_DFU::Status)), this, SLOT(uploadFinished(OP_DFU::Status)));
     bool retstatus = m_dfu->UploadFirmware(filename, verify, deviceID);
     if (!retstatus) {
-        status("Could not start upload", STATUSICON_FAIL);
-        myDevice->updateButton->setEnabled(true);
         emit uploadEnded(false);
+        status("Could not start upload!", STATUSICON_FAIL);
+        updateButtons(true);
         return;
     }
+
     status("Uploading, please wait...", STATUSICON_RUNNING);
 }
 
@@ -410,29 +420,43 @@ void deviceWidget::uploadFirmware()
   */
 void deviceWidget::downloadFirmware()
 {
+    // clear progress bar now
+    // this avoids displaying an error message and the progress at 100% at the same time
+    setProgress(0);
+    updateButtons(false);
+
     if (!m_dfu->devices[deviceID].Readable) {
         myDevice->statusLabel->setText(QString("Device not readable!"));
+        status("Device not readable!", STATUSICON_FAIL);
+        updateButtons(true);
         return;
     }
 
-    myDevice->retrieveButton->setEnabled(false);
     filename = setSaveFileName();
-
     if (filename.isEmpty()) {
         status("Empty filename", STATUSICON_FAIL);
+        updateButtons(true);
         return;
     }
 
-    status("Downloading firmware from device", STATUSICON_RUNNING);
+    status("Starting firmware download", STATUSICON_RUNNING);
+    emit downloadStarted();
+
     connect(m_dfu, SIGNAL(progressUpdated(int)), this, SLOT(setProgress(int)));
     connect(m_dfu, SIGNAL(downloadFinished()), this, SLOT(downloadFinished()));
+
     downloadedFirmware.clear(); // Empty the byte array
     bool ret = m_dfu->DownloadFirmware(&downloadedFirmware,deviceID);
+
     if (!ret) {
+        emit downloadEnded(false);
         status("Could not start download!", STATUSICON_FAIL);
+        updateButtons(true);
         return;
     }
-    status("Download started, please wait", STATUSICON_RUNNING);
+
+    status("Downloading, please wait...", STATUSICON_RUNNING);
+    return;
 }
 
 /**
@@ -442,10 +466,13 @@ void deviceWidget::downloadFinished()
 {
     disconnect(m_dfu, SIGNAL(downloadFinished()), this, SLOT(downloadFinished()));
     disconnect(m_dfu, SIGNAL(progressUpdated(int)), this, SLOT(setProgress(int)));
-    status("Download successful", STATUSICON_OK);
+
     // Now save the result (use the utility function from OP_DFU)
     m_dfu->SaveByteArrayToFile(filename, downloadedFirmware);
-    myDevice->retrieveButton->setEnabled(true);
+
+    emit downloadEnded(true);
+    status("Download successful", STATUSICON_OK);
+    updateButtons(true);
 }
 
 /**
@@ -453,13 +480,14 @@ void deviceWidget::downloadFinished()
   */
 void deviceWidget::uploadFinished(OP_DFU::Status retstatus)
 {
-    myDevice->updateButton->setEnabled(true);
     disconnect(m_dfu, SIGNAL(uploadFinished(OP_DFU::Status)), this, SLOT(uploadFinished(OP_DFU::Status)));
     disconnect(m_dfu, SIGNAL(progressUpdated(int)), this, SLOT(setProgress(int)));
     disconnect(m_dfu, SIGNAL(operationProgress(QString)), this, SLOT(dfuStatus(QString)));
+
     if (retstatus != OP_DFU::Last_operation_Success) {
-        status(QString("Upload failed with code: ") + m_dfu->StatusToString(retstatus).toLatin1().data(), STATUSICON_FAIL);
         emit uploadEnded(false);
+        status(QString("Upload failed with code: ") + m_dfu->StatusToString(retstatus).toLatin1().data(), STATUSICON_FAIL);
+        updateButtons(true);
         return;
     } else
         if (!descriptionArray.isEmpty()) {
@@ -468,8 +496,9 @@ void deviceWidget::uploadFinished(OP_DFU::Status retstatus)
             repaint(); // Make sure the text above shows right away
             retstatus = m_dfu->UploadDescription(descriptionArray);
             if (retstatus != OP_DFU::Last_operation_Success) {
-                status(QString("Upload failed with code: ") + m_dfu->StatusToString(retstatus).toLatin1().data(), STATUSICON_FAIL);
                 emit uploadEnded(false);
+                status(QString("Upload failed with code: ") + m_dfu->StatusToString(retstatus).toLatin1().data(), STATUSICON_FAIL);
+                updateButtons(true);
                 return;
             }
 
@@ -479,15 +508,18 @@ void deviceWidget::uploadFinished(OP_DFU::Status retstatus)
             repaint(); // Make sure the text above shows right away
             retstatus = m_dfu->UploadDescription(myDevice->description->text());
             if (retstatus != OP_DFU::Last_operation_Success) {
-                status(QString("Upload failed with code: ") + m_dfu->StatusToString(retstatus).toLatin1().data(), STATUSICON_FAIL);
                 emit uploadEnded(false);
+                status(QString("Upload failed with code: ") + m_dfu->StatusToString(retstatus).toLatin1().data(), STATUSICON_FAIL);
+                updateButtons(true);
                 return;
             }
         }
+
     populate();
+
     emit uploadEnded(true);
     status("Upload successful", STATUSICON_OK);
-
+    updateButtons(true);
 }
 
 /**

ground/openpilotgcs/src/plugins/uploader/devicewidget.h

@@ -57,6 +57,7 @@ public:
     typedef enum { STATUSICON_OK, STATUSICON_RUNNING, STATUSICON_FAIL, STATUSICON_INFO} StatusIcon;
     QString setOpenFileName();
     QString setSaveFileName();
+
 private:
     deviceDescriptorStruct onBoardDescription;
     deviceDescriptorStruct LoadedDescription;
@@ -71,10 +72,14 @@ private:
     void status(QString str, StatusIcon ic);
     bool populateBoardStructuredDescription(QByteArray arr);
     bool populateLoadedStructuredDescription(QByteArray arr);
+    void updateButtons(bool enabled);
 
 signals:
     void uploadStarted();
     void uploadEnded(bool success);
+    void downloadStarted();
+    void downloadEnded(bool success);
+
 public slots:
     void uploadFirmware();
     void loadFirmware();

ground/openpilotgcs/src/plugins/uploader/devicewidget.ui

@@ -101,9 +101,9 @@
                </widget>
               </item>
               <item>
-               <widget class="QCheckBox" name="youdont">
+               <widget class="QCheckBox" name="confirmCheckBox">
                 <property name="text">
-                 <string>I know what I'm doing</string>
+                 <string>I know what I'm doing!</string>
                 </property>
                 <property name="checked">
                  <bool>true</bool>

ground/openpilotgcs/src/plugins/uploader/uploadergadgetwidget.cpp

@@ -120,6 +120,8 @@ void UploaderGadgetWidget::connectSignalSlot(QWidget *widget)
 {
     connect(qobject_cast<deviceWidget *>(widget),SIGNAL(uploadStarted()),this,SLOT(uploadStarted()));
     connect(qobject_cast<deviceWidget *>(widget),SIGNAL(uploadEnded(bool)),this,SLOT(uploadEnded(bool)));
+    connect(qobject_cast<deviceWidget *>(widget),SIGNAL(downloadStarted()),this,SLOT(downloadStarted()));
+    connect(qobject_cast<deviceWidget *>(widget),SIGNAL(downloadEnded(bool)),this,SLOT(downloadEnded(bool)));
 }
 
 FlightStatus *UploaderGadgetWidget::getFlightStatus()
@@ -398,7 +400,7 @@ void UploaderGadgetWidget::systemReset()
             delete dfu;
             dfu = NULL;
         }
-        m_config->textBrowser->clear();
+        clearLog();
         log("Board Reset initiated.");
         goToBootloader();
     }
@@ -470,15 +472,23 @@ void UploaderGadgetWidget::commonSystemBoot(bool safeboot)
         // Freeze the tabs, they are not useful anymore and their buttons
         // will cause segfaults or weird stuff if we use them.
         for (int i=0; i< m_config->systemElements->count(); i++) {
-             deviceWidget *qw = (deviceWidget*)m_config->systemElements->widget(i);
+            // OP-682 arriving here too "early" (before the devices are refreshed) was leading to a crash
+            // OP-682 the crash was due to an unchecked cast in the line below that would cast a RunningDeviceGadget to a DeviceGadget
+            deviceWidget *qw = dynamic_cast<deviceWidget*>(m_config->systemElements->widget(i));
+            if (qw) {
+                // OP-682 fixed a second crash by disabling *all* buttons in the device widget
+                // disabling the buttons is only half of the solution as even if the buttons are enabled
+                // the app should not crash
                 qw->freeze();
             }
         }
+    }
     currentStep = IAP_STATE_READY;
     log("You can now reconnect telemetry...");
     delete dfu; // Frees up the USB/Serial port too
     dfu = NULL;
 }
+
 bool UploaderGadgetWidget::autoUpdateCapable()
 {
     return QDir(":/build").exists();
@@ -750,15 +760,44 @@ void UploaderGadgetWidget::cancel()
 
 void UploaderGadgetWidget::uploadStarted()
 {
+    m_config->haltButton->setEnabled(false);
     m_config->bootButton->setEnabled(false);
     m_config->safeBootButton->setEnabled(false);
+    m_config->resetButton->setEnabled(false);
+    m_config->rescueButton->setEnabled(false);
 }
 
 void UploaderGadgetWidget::uploadEnded(bool succeed)
 {
     Q_UNUSED(succeed);
+    // device is halted so no halt
+    m_config->haltButton->setEnabled(false);
     m_config->bootButton->setEnabled(true);
     m_config->safeBootButton->setEnabled(true);
+    // device is halted so no reset
+    m_config->resetButton->setEnabled(false);
+    m_config->rescueButton->setEnabled(true);
+}
+
+void UploaderGadgetWidget::downloadStarted()
+{
+    m_config->haltButton->setEnabled(false);
+    m_config->bootButton->setEnabled(false);
+    m_config->safeBootButton->setEnabled(false);
+    m_config->resetButton->setEnabled(false);
+    m_config->rescueButton->setEnabled(false);
+}
+
+void UploaderGadgetWidget::downloadEnded(bool succeed)
+{
+    Q_UNUSED(succeed);
+    // device is halted so no halt
+    m_config->haltButton->setEnabled(false);
+    m_config->bootButton->setEnabled(true);
+    m_config->safeBootButton->setEnabled(true);
+    // device is halted so no reset
+    m_config->resetButton->setEnabled(false);
+    m_config->rescueButton->setEnabled(true);
 }
 
 /**
@@ -766,9 +805,9 @@ void UploaderGadgetWidget::uploadEnded(bool succeed)
   */
 void UploaderGadgetWidget::log(QString str)
 {
+   qDebug() << str;
    m_config->textBrowser->append(str);
    m_config->textBrowser->repaint();
-
 }
 
 void UploaderGadgetWidget::clearLog()
@@ -815,6 +854,7 @@ void UploaderGadgetWidget::error(QString errorString, int errorNumber)
     msgBox.exec();
     m_config->boardStatus->setText(errorString);
 }
+
 /**
 Shows a message box with an information string.
 

ground/openpilotgcs/src/plugins/uploader/uploadergadgetwidget.h

@@ -115,7 +115,8 @@ private slots:
     void cancel();
     void uploadStarted();
     void uploadEnded(bool succeed);
-
+    void downloadStarted();
+    void downloadEnded(bool succeed);
 };
 
 #endif // UPLOADERGADGETWIDGET_H