From 59da382613280b14b34df0921d308b910a36ce32 Mon Sep 17 00:00:00 2001 From: Danilo Cesar Lemes de Paula Date: Sun, 25 Aug 2013 19:02:30 -0300 Subject: improve DownloadDialog UI control * Removes the InterfaceThread which is basically an unecessary proxy between the MainThread and the DownloadThread. * Use a state machine to control the DownloadWidget UI logic. Signed-off-by: Danilo Cesar Lemes de Paula --- qt-ui/downloadfromdivecomputer.cpp | 141 ++++++++++++++++++++++++------------- 1 file changed, 92 insertions(+), 49 deletions(-) (limited to 'qt-ui/downloadfromdivecomputer.cpp') diff --git a/qt-ui/downloadfromdivecomputer.cpp b/qt-ui/downloadfromdivecomputer.cpp index 7514c9482..f5c6fd51f 100644 --- a/qt-ui/downloadfromdivecomputer.cpp +++ b/qt-ui/downloadfromdivecomputer.cpp @@ -10,6 +10,7 @@ #include #include #include +#include struct product { const char *product; @@ -42,7 +43,8 @@ DownloadFromDCWidget *DownloadFromDCWidget::instance() } DownloadFromDCWidget::DownloadFromDCWidget(QWidget* parent, Qt::WindowFlags f) : - QDialog(parent, f), ui(new Ui::DownloadFromDiveComputer), thread(0), downloading(false) + QDialog(parent, f), ui(new Ui::DownloadFromDiveComputer), thread(0), timer(new QTimer(this)), + currentState(INITIAL) { ui->setupUi(this); ui->progressBar->hide(); @@ -61,21 +63,77 @@ DownloadFromDCWidget::DownloadFromDCWidget(QWidget* parent, Qt::WindowFlags f) : } if (default_dive_computer_device) ui->device->setText(default_dive_computer_device); + + timer->setInterval(200); + connect(timer, SIGNAL(timeout()), this, SLOT(updateProgressBar())); + + updateState(INITIAL); } void DownloadFromDCWidget::runDialog() { - // Since the DownloadDialog is only - // created once, we need to do put some starting code here - ui->progressBar->hide(); - markChildrenAsEnabled(); + updateState(INITIAL); exec(); } -void DownloadFromDCWidget::stoppedDownloading() +void DownloadFromDCWidget::updateProgressBar() { - downloading = false; + ui->progressBar->setValue(progress_bar_fraction *100); +} + +void DownloadFromDCWidget::updateState(states state) +{ + if (state == currentState) + return; + + if (state == INITIAL) { + ui->progressBar->hide(); + markChildrenAsEnabled(); + timer->stop(); + } + + // tries to cancel an on going download + else if (currentState == DOWNLOADING && state == CANCELLING) { + import_thread_cancelled = true; + ui->cancel->setEnabled(false); + } + + // user pressed cancel but the application isn't doing anything. + // means close the window + else if ((currentState == INITIAL || currentState == CANCELLED || currentState == DONE) + && state == CANCELLING) { + timer->stop(); + reject(); + } + + // A cancel is finished + else if (currentState == CANCELLING && (state == DONE || state == CANCELLED)) { + timer->stop(); + state = CANCELLED; + ui->progressBar->setValue(0); + ui->progressbar->hide(); + markChildrenAsEnabled(); + } + + // DOWNLOAD is finally done, close the dialog and go back to the main window + else if (currentState == DOWNLOADING && state == DONE) { + timer->stop(); + ui->progressBar->setValue(100); + markChildrenAsEnabled(); + accept(); + } + + // DOWNLOAD is started. + else if (state == DOWNLOADING) { + timer->start(); + ui->progressBar->setValue(0); + ui->progressBar->show(); + markChildrenAsDisabled(); + } + + // properly updating the widget state + currentState = state; } void DownloadFromDCWidget::on_vendor_currentIndexChanged(const QString& vendor) @@ -136,28 +194,15 @@ void DownloadFromDCWidget::fill_computer_list() void DownloadFromDCWidget::on_cancel_clicked() { - import_thread_cancelled = true; - if (thread) { - thread->wait(); - thread->deleteLater(); - thread = 0; - } - - // Confusing, but if the user press cancel during a download - // he probably want to cancel the download, not to close the window. - if (!downloading) - close(); + updateState(CANCELLING); } void DownloadFromDCWidget::on_ok_clicked() { - if (downloading) - return; - - markChildrenAsDisabled(); - ui->progressBar->setValue(0); - ui->progressBar->show(); + updateState(DOWNLOADING); + // I don't really think that create/destroy the thread + // is really necessary. if (thread) { thread->deleteLater(); } @@ -172,14 +217,15 @@ void DownloadFromDCWidget::on_ok_clicked() set_default_dive_computer(data.vendor, data.product); set_default_dive_computer_device(data.devname); - thread = new InterfaceThread(this, &data); - connect(thread, SIGNAL(updateInterface(int)), - ui->progressBar, SLOT(setValue(int)), Qt::QueuedConnection); // Qt::QueuedConnection == threadsafe. + thread = new DownloadThread(this, &data); - connect(thread, SIGNAL(finished()), this, SLOT(close())); + connect(thread, SIGNAL(finished()), + this, SLOT(onDownloadThreadFinished()), Qt::QueuedConnection); + + MainWindow *w = mainWindow(); + connect(thread, SIGNAL(finished()), w, SLOT(refreshDisplay())); thread->start(); - downloading = true; } bool DownloadFromDCWidget::preferDownloaded() @@ -191,10 +237,18 @@ void DownloadFromDCWidget::reject() { // we don't want the download window being able to close // while we're still downloading. - if (!downloading) + if (currentState != DOWNLOADING && currentState != CANCELLING) QDialog::reject(); } +void DownloadFromDCWidget::onDownloadThreadFinished() +{ + if (currentState == DOWNLOADING) + updateState(DONE); + else + updateState(CANCELLED); +} + void DownloadFromDCWidget::markChildrenAsDisabled() { ui->device->setDisabled(true); @@ -214,37 +268,26 @@ void DownloadFromDCWidget::markChildrenAsEnabled() ui->forceDownload->setDisabled(false); ui->preferDownloaded->setDisabled(false); ui->ok->setDisabled(false); + ui->cancel->setDisabled(false); ui->search->setDisabled(false); } -DownloadThread::DownloadThread(device_data_t* data): data(data) +DownloadThread::DownloadThread(QObject* parent, device_data_t* data): QThread(parent), + data(data) { } void DownloadThread::run() { DownloadFromDCWidget *dfdcw = DownloadFromDCWidget::instance(); + if (!strcmp(data->vendor, "Uemis")) do_uemis_import(data->devname, data->force_download); else + // TODO: implement error handling do_libdivecomputer_import(data); - process_dives(TRUE, dfdcw->preferDownloaded()); - dfdcw->stoppedDownloading(); -} - -InterfaceThread::InterfaceThread(QObject* parent, device_data_t* data): QThread(parent), data(data) -{ -} -void InterfaceThread::run() -{ - DownloadThread *download = new DownloadThread(data); - MainWindow *w = mainWindow(); - connect(download, SIGNAL(finished()), w, SLOT(refreshDisplay())); - download->start(); - while (download->isRunning()) { - msleep(200); - updateInterface(progress_bar_fraction *100); - } - updateInterface(100); + // I'm not sure if we should really process_dives even + // if there's an error or a cancelation + process_dives(TRUE, dfdcw->preferDownloaded()); } -- cgit v1.2.3-70-g09d2 From 3f8e183008530fb8e110b77d04cf794c46ba4720 Mon Sep 17 00:00:00 2001 From: Danilo Cesar Lemes de Paula Date: Sun, 1 Sep 2013 11:14:54 -0300 Subject: improve DownloadDialog's error handling shows an error message when libdivecomputer returns an error. Signed-off-by: Danilo Cesar Lemes de Paula --- qt-ui/downloadfromdivecomputer.cpp | 47 ++++++++++++++++++++++++++++++-------- qt-ui/downloadfromdivecomputer.h | 5 +++- 2 files changed, 41 insertions(+), 11 deletions(-) (limited to 'qt-ui/downloadfromdivecomputer.cpp') diff --git a/qt-ui/downloadfromdivecomputer.cpp b/qt-ui/downloadfromdivecomputer.cpp index f5c6fd51f..acdca89d3 100644 --- a/qt-ui/downloadfromdivecomputer.cpp +++ b/qt-ui/downloadfromdivecomputer.cpp @@ -11,6 +11,7 @@ #include #include #include +#include struct product { const char *product; @@ -101,18 +102,18 @@ void DownloadFromDCWidget::updateState(states state) // user pressed cancel but the application isn't doing anything. // means close the window - else if ((currentState == INITIAL || currentState == CANCELLED || currentState == DONE) + else if ((currentState == INITIAL || currentState == CANCELLED || currentState == DONE || currentState == ERROR) && state == CANCELLING) { timer->stop(); reject(); } - // A cancel is finished + // the cancelation process is finished else if (currentState == CANCELLING && (state == DONE || state == CANCELLED)) { timer->stop(); state = CANCELLED; ui->progressBar->setValue(0); - ui->progressbar->hide(); + ui->progressBar->hide(); markChildrenAsEnabled(); } @@ -132,6 +133,15 @@ void DownloadFromDCWidget::updateState(states state) markChildrenAsDisabled(); } + // got an error + else if (state == ERROR) { + QMessageBox::critical(this, tr("Error"), this->thread->error, QMessageBox::Ok); + + markChildrenAsEnabled(); + ui->progressBar->hide(); + ui->ok->setText(tr("retry")); + } + // properly updating the widget state currentState = state; } @@ -243,9 +253,12 @@ void DownloadFromDCWidget::reject() void DownloadFromDCWidget::onDownloadThreadFinished() { - if (currentState == DOWNLOADING) - updateState(DONE); - else + if (currentState == DOWNLOADING) { + if (thread->error.isEmpty()) + updateState(DONE); + else + updateState(ERROR); + } else updateState(CANCELLED); } @@ -277,17 +290,31 @@ DownloadThread::DownloadThread(QObject* parent, device_data_t* data): QThread(pa { } +static QString str_error(const char *fmt, ...) +{ + va_list args; + va_start(args, fmt); + const QString str = QString().vsprintf( fmt, args ); + va_end(args); + + return str; +} + void DownloadThread::run() { DownloadFromDCWidget *dfdcw = DownloadFromDCWidget::instance(); + const char *error; if (!strcmp(data->vendor, "Uemis")) - do_uemis_import(data->devname, data->force_download); + error = do_uemis_import(data->devname, data->force_download); else - // TODO: implement error handling - do_libdivecomputer_import(data); + error = do_libdivecomputer_import(data); + + if (error) { + this->error = str_error(error, data->devname, data->vendor, data->product); + } - // I'm not sure if we should really process_dives even + // I'm not sure if we should really call process_dives even // if there's an error or a cancelation process_dives(TRUE, dfdcw->preferDownloaded()); } diff --git a/qt-ui/downloadfromdivecomputer.h b/qt-ui/downloadfromdivecomputer.h index f092314c0..e10d61b38 100644 --- a/qt-ui/downloadfromdivecomputer.h +++ b/qt-ui/downloadfromdivecomputer.h @@ -15,8 +15,10 @@ struct device_data_t; class DownloadThread : public QThread{ Q_OBJECT public: - explicit DownloadThread(QObject* parent, device_data_t* data); + DownloadThread(QObject* parent, device_data_t* data); virtual void run(); + + QString error; private: device_data_t *data; }; @@ -34,6 +36,7 @@ public: DOWNLOADING, CANCELLING, CANCELLED, + ERROR, DONE, }; -- cgit v1.2.3-70-g09d2