From fd00ea0ca725eb4dd786769605b3c9792e1a52fe Mon Sep 17 00:00:00 2001 From: Vojtech Kral Date: Fri, 18 May 2018 18:37:16 +0200 Subject: [PATCH] Firmware updater: Add cancelation --- xs/src/avrdude/avr.c | 29 +++++++----- xs/src/avrdude/avrdude-slic3r.cpp | 8 ++-- xs/src/avrdude/avrdude-slic3r.hpp | 7 +-- xs/src/avrdude/avrdude.h | 6 ++- xs/src/avrdude/libavrdude.h | 4 +- xs/src/avrdude/main.c | 16 ++++--- xs/src/slic3r/GUI/FirmwareDialog.cpp | 70 +++++++++++++++++++++------- 7 files changed, 96 insertions(+), 44 deletions(-) diff --git a/xs/src/avrdude/avr.c b/xs/src/avrdude/avr.c index 47d97df71..b95cfe76a 100644 --- a/xs/src/avrdude/avr.c +++ b/xs/src/avrdude/avr.c @@ -358,7 +358,7 @@ int avr_read(PROGRAMMER * pgm, AVRPART * p, char * memtype, return -1; } } - report_progress(i, mem->size, NULL); + if (!report_progress(i, mem->size, NULL)) return -99; } return avr_mem_hiaddr(mem); } @@ -415,7 +415,7 @@ int avr_read(PROGRAMMER * pgm, AVRPART * p, char * memtype, progname, pageaddr / mem->page_size); } nread++; - report_progress(nread, npages, NULL); + if (!report_progress(nread, npages, NULL)) return -99; } if (!failure) { if (strcasecmp(mem->desc, "flash") == 0 || @@ -448,7 +448,7 @@ int avr_read(PROGRAMMER * pgm, AVRPART * p, char * memtype, return -2; } } - report_progress(i, mem->size, NULL); + if (!report_progress(i, mem->size, NULL)) return -99; } if (strcasecmp(mem->desc, "flash") == 0 || @@ -896,7 +896,7 @@ int avr_write(PROGRAMMER * pgm, AVRPART * p, char * memtype, int size, while (avr_tpi_poll_nvmbsy(pgm)); } - report_progress(i, wsize, NULL); + if (!report_progress(i, wsize, NULL)) return -99; } return i; } @@ -948,7 +948,7 @@ int avr_write(PROGRAMMER * pgm, AVRPART * p, char * memtype, int size, progname, pageaddr / m->page_size); } nwritten++; - report_progress(nwritten, npages, NULL); + if (!report_progress(nwritten, npages, NULL)) return -99; } if (!failure) return wsize; @@ -965,7 +965,7 @@ int avr_write(PROGRAMMER * pgm, AVRPART * p, char * memtype, int size, for (i=0; ibuf[i]; - report_progress(i, wsize, NULL); + if (!report_progress(i, wsize, NULL)) return -99; /* * Find out whether the write action must be invoked for this @@ -1050,14 +1050,14 @@ int avr_signature(PROGRAMMER * pgm, AVRPART * p) { int rc; - report_progress (0,1,"Reading"); + if (!report_progress(0,1,"Reading")) return -99; rc = avr_read(pgm, p, "signature", 0); if (rc < 0) { avrdude_message(MSG_INFO, "%s: error reading signature data for part \"%s\", rc=%d\n", progname, p->desc, rc); return -1; } - report_progress (1,1,NULL); + if (!report_progress(1,1,NULL)) return -99; return 0; } @@ -1214,16 +1214,19 @@ int avr_chip_erase(PROGRAMMER * pgm, AVRPART * p) * call for each of start, during and end cases. As things stand now, * that is not possible and makes maintenance a bit more work. */ -void report_progress (int completed, int total, char *hdr) +// Prusa version modification: report_progress() returns bool to faciliate cancelation +// the bool has "continue" semantics, ie. true = continue, false = interrupt +bool report_progress (int completed, int total, char *hdr) { static int last = 0; static double start_time; int percent = (total > 0) ? ((completed * 100) / total) : 100; struct timeval tv; double t; + bool res = true; if (update_progress == NULL) - return; + return true; gettimeofday(&tv, NULL); t = tv.tv_sec + ((double)tv.tv_usec)/1000000; @@ -1231,7 +1234,7 @@ void report_progress (int completed, int total, char *hdr) if (hdr) { last = 0; start_time = t; - update_progress (percent, t - start_time, hdr); + res = update_progress (percent, t - start_time, hdr); } if (percent > 100) @@ -1239,9 +1242,11 @@ void report_progress (int completed, int total, char *hdr) if (percent > last) { last = percent; - update_progress (percent, t - start_time, hdr); + res = update_progress (percent, t - start_time, hdr); } if (percent == 100) last = 0; /* Get ready for next time. */ + + return res; } diff --git a/xs/src/avrdude/avrdude-slic3r.cpp b/xs/src/avrdude/avrdude-slic3r.cpp index a581371ae..13c37e508 100644 --- a/xs/src/avrdude/avrdude-slic3r.cpp +++ b/xs/src/avrdude/avrdude-slic3r.cpp @@ -21,10 +21,10 @@ static void avrdude_message_handler_closure(const char *msg, unsigned size, void } // Used by our custom code in avrdude to report progress in the GUI -static void avrdude_progress_handler_closure(const char *task, unsigned progress, void *user_p) +static bool avrdude_progress_handler_closure(const char *task, unsigned progress, void *user_p) { auto *progress_fn = reinterpret_cast(user_p); - (*progress_fn)(task, progress); + return (*progress_fn)(task, progress); } @@ -73,6 +73,8 @@ int AvrDude::priv::run() { AvrDude::AvrDude() : p(new priv()) {} +AvrDude::AvrDude(AvrDude &&other) : p(std::move(other.p)) {} + AvrDude::~AvrDude() { if (p && p->avrdude_thread.joinable()) { @@ -98,7 +100,7 @@ AvrDude& AvrDude::on_message(MessageFn fn) return *this; } -AvrDude& AvrDude::on_progress(MessageFn fn) +AvrDude& AvrDude::on_progress(ProgressFn fn) { if (p) { p->progress_fn = std::move(fn); } return *this; diff --git a/xs/src/avrdude/avrdude-slic3r.hpp b/xs/src/avrdude/avrdude-slic3r.hpp index 097e46028..a9a3c8e5b 100644 --- a/xs/src/avrdude/avrdude-slic3r.hpp +++ b/xs/src/avrdude/avrdude-slic3r.hpp @@ -13,11 +13,11 @@ class AvrDude public: typedef std::shared_ptr Ptr; typedef std::function MessageFn; - typedef std::function ProgressFn; + typedef std::function ProgressFn; typedef std::function CompleteFn; AvrDude(); - AvrDude(AvrDude &&) = default; + AvrDude(AvrDude &&); AvrDude(const AvrDude &) = delete; AvrDude &operator=(AvrDude &&) = delete; AvrDude &operator=(const AvrDude &) = delete; @@ -34,7 +34,8 @@ public: // Set progress report callback // Progress is reported per each task (reading / writing), progress is reported in percents. - AvrDude& on_progress(MessageFn fn); + // The callback's return value indicates whether to continue flashing (true) or cancel (false). + AvrDude& on_progress(ProgressFn fn); // Called when avrdude's main function finishes AvrDude& on_complete(CompleteFn fn); diff --git a/xs/src/avrdude/avrdude.h b/xs/src/avrdude/avrdude.h index 794169921..501be642d 100644 --- a/xs/src/avrdude/avrdude.h +++ b/xs/src/avrdude/avrdude.h @@ -21,6 +21,8 @@ #ifndef avrdude_h #define avrdude_h +#include + extern char * progname; /* name of program, for messages */ extern char progbuf[]; /* spaces same length as progname */ @@ -34,9 +36,9 @@ int avrdude_message(const int msglvl, const char *format, ...); // Progress reporting callback // `progress` is in range 0 ~ 100 percent -typedef void (*avrdude_progress_handler_t)(const char *task, unsigned progress, void *user_p); +typedef bool (*avrdude_progress_handler_t)(const char *task, unsigned progress, void *user_p); void avrdude_progress_handler_set(avrdude_progress_handler_t newhandler, void *user_p); -void avrdude_progress_external(const char *task, unsigned progress); +bool avrdude_progress_external(const char *task, unsigned progress); #define MSG_INFO (0) /* no -v option, can be supressed with -qq */ #define MSG_NOTICE (1) /* displayed with -v */ diff --git a/xs/src/avrdude/libavrdude.h b/xs/src/avrdude/libavrdude.h index 42c413ed7..dc3ea2c10 100644 --- a/xs/src/avrdude/libavrdude.h +++ b/xs/src/avrdude/libavrdude.h @@ -727,7 +727,7 @@ void sort_programmers(LISTID programmers); /* formerly avr.h */ -typedef void (*FP_UpdateProgress)(int percent, double etime, char *hdr); +typedef bool (*FP_UpdateProgress)(int percent, double etime, char *hdr); extern struct avrpart parts[]; @@ -769,7 +769,7 @@ int avr_mem_hiaddr(AVRMEM * mem); int avr_chip_erase(PROGRAMMER * pgm, AVRPART * p); -void report_progress (int completed, int total, char *hdr); +bool report_progress (int completed, int total, char *hdr); #ifdef __cplusplus } diff --git a/xs/src/avrdude/main.c b/xs/src/avrdude/main.c index 727166ec1..3ac983e1e 100644 --- a/xs/src/avrdude/main.c +++ b/xs/src/avrdude/main.c @@ -115,12 +115,13 @@ int avrdude_message(const int msglvl, const char *format, ...) } -static void avrdude_progress_handler_null(const char *task, unsigned progress, void *user_p) +static bool avrdude_progress_handler_null(const char *task, unsigned progress, void *user_p) { // By default do nothing (void)task; (void)progress; (void)user_p; + return true; } static void *avrdude_progress_handler_user_p = NULL; @@ -137,9 +138,9 @@ void avrdude_progress_handler_set(avrdude_progress_handler_t newhandler, void *u } } -void avrdude_progress_external(const char *task, unsigned progress) +bool avrdude_progress_external(const char *task, unsigned progress) { - avrdude_progress_handler(task, progress, avrdude_progress_handler_user_p); + return avrdude_progress_handler(task, progress, avrdude_progress_handler_user_p); } @@ -244,12 +245,13 @@ static void usage(void) // setvbuf(stderr, (char*)NULL, _IOLBF, 0); // } -static void update_progress_no_tty (int percent, double etime, char *hdr) +static bool update_progress_no_tty (int percent, double etime, char *hdr) { static int done = 0; static int last = 0; static char *header = NULL; int cnt = (percent>>1)*2; + bool res = true; // setvbuf(stderr, (char*)NULL, _IONBF, 0); @@ -258,7 +260,7 @@ static void update_progress_no_tty (int percent, double etime, char *hdr) last = 0; done = 0; header = hdr; - avrdude_progress_external(header, 0); + res = avrdude_progress_external(header, 0); } else { while ((cnt > last) && (done == 0)) { @@ -267,7 +269,7 @@ static void update_progress_no_tty (int percent, double etime, char *hdr) } if (done == 0) { - avrdude_progress_external(header, percent > 99 ? 99 : percent); + res = avrdude_progress_external(header, percent > 99 ? 99 : percent); } } @@ -281,6 +283,8 @@ static void update_progress_no_tty (int percent, double etime, char *hdr) last = (percent>>1)*2; /* Make last a multiple of 2. */ // setvbuf(stderr, (char*)NULL, _IOLBF, 0); + + return res; } static void list_programmers_callback(const char *name, const char *desc, diff --git a/xs/src/slic3r/GUI/FirmwareDialog.cpp b/xs/src/slic3r/GUI/FirmwareDialog.cpp index 1cfc43dd5..bdc8df4dd 100644 --- a/xs/src/slic3r/GUI/FirmwareDialog.cpp +++ b/xs/src/slic3r/GUI/FirmwareDialog.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include "libslic3r/Utils.hpp" #include "avrdude/avrdude-slic3r.hpp" @@ -45,6 +46,14 @@ wxDEFINE_EVENT(EVT_AVRDUDE, wxCommandEvent); struct FirmwareDialog::priv { + enum AvrDudeComplete + { + AC_NONE, + AC_SUCCESS, + AC_FAILURE, + AC_CANCEL, + }; + FirmwareDialog *q; // PIMPL back pointer ("Q-Pointer") wxComboBox *port_picker; @@ -56,21 +65,27 @@ struct FirmwareDialog::priv wxButton *btn_rescan; wxButton *btn_close; wxButton *btn_flash; + wxString btn_flash_label_ready; + wxString btn_flash_label_flashing; // This is a shared pointer holding the background AvrDude task // also serves as a status indication (it is set _iff_ the background task is running, otherwise it is reset). AvrDude::Ptr avrdude; std::string avrdude_config; unsigned progress_tasks_done; + bool cancel; priv(FirmwareDialog *q) : q(q), + btn_flash_label_ready(_(L("Flash!"))), + btn_flash_label_flashing(_(L("Cancel"))), avrdude_config((fs::path(::Slic3r::resources_dir()) / "avrdude" / "avrdude.conf").string()), - progress_tasks_done(0) + progress_tasks_done(0), + cancel(false) {} void find_serial_ports(); - void flashing_status(bool flashing, int res = 0); + void flashing_status(bool flashing, AvrDudeComplete complete = AC_NONE); void perform_upload(); void on_avrdude(const wxCommandEvent &evt); }; @@ -87,7 +102,7 @@ void FirmwareDialog::priv::find_serial_ports() } } -void FirmwareDialog::priv::flashing_status(bool value, int res) +void FirmwareDialog::priv::flashing_status(bool value, AvrDudeComplete complete) { if (value) { txt_stdout->SetValue(wxEmptyString); @@ -97,22 +112,26 @@ void FirmwareDialog::priv::flashing_status(bool value, int res) btn_rescan->Disable(); hex_picker->Disable(); btn_close->Disable(); - btn_flash->Disable(); + btn_flash->SetLabel(btn_flash_label_flashing); progressbar->SetRange(200); // See progress callback below progressbar->SetValue(0); progress_tasks_done = 0; + cancel = false; } else { auto text_color = wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOWTEXT); port_picker->Enable(); btn_rescan->Enable(); hex_picker->Enable(); btn_close->Enable(); - btn_flash->Enable(); + btn_flash->SetLabel(btn_flash_label_ready); txt_status->SetForegroundColour(text_color); - txt_status->SetLabel( - res == 0 ? _(L("Flashing succeeded!")) : _(L("Flashing failed. Please see the avrdude log below.")) - ); progressbar->SetValue(200); + + switch (complete) { + case AC_SUCCESS: txt_status->SetLabel(_(L("Flashing succeeded!"))); break; + case AC_FAILURE: txt_status->SetLabel(_(L("Flashing failed. Please see the avrdude log below."))); break; + case AC_CANCEL: txt_status->SetLabel(_(L("Flashing cancelled."))); break; + } } } @@ -152,11 +171,12 @@ void FirmwareDialog::priv::perform_upload() evt->SetString(msg); wxQueueEvent(q, evt); })) - .on_progress(std::move([q](const char * /* task */, unsigned progress) { - auto evt = new wxCommandEvent(EVT_AVRDUDE, q->GetId()); + .on_progress(std::move([this](const char * /* task */, unsigned progress) { + auto evt = new wxCommandEvent(EVT_AVRDUDE, this->q->GetId()); evt->SetExtraLong(AE_PRORGESS); evt->SetInt(progress); - wxQueueEvent(q, evt); + wxQueueEvent(this->q, evt); + return !this->cancel; })) .on_complete(std::move([q](int status) { auto evt = new wxCommandEvent(EVT_AVRDUDE, q->GetId()); @@ -169,8 +189,9 @@ void FirmwareDialog::priv::perform_upload() void FirmwareDialog::priv::on_avrdude(const wxCommandEvent &evt) { - switch (evt.GetExtraLong()) - { + AvrDudeComplete complete_kind; + + switch (evt.GetExtraLong()) { case AE_MESSAGE: txt_stdout->AppendText(evt.GetString()); break; @@ -195,7 +216,9 @@ void FirmwareDialog::priv::on_avrdude(const wxCommandEvent &evt) case AE_EXIT: BOOST_LOG_TRIVIAL(info) << "avrdude exit code: " << evt.GetInt(); - flashing_status(false, evt.GetInt()); + + complete_kind = cancel ? AC_CANCEL : (evt.GetInt() == 0 ? AC_SUCCESS : AC_FAILURE); + flashing_status(false, complete_kind); // Make sure the background thread is collected and the AvrDude object reset if (avrdude) { avrdude->join(); } @@ -278,7 +301,7 @@ FirmwareDialog::FirmwareDialog(wxWindow *parent) : vsizer->Add(spoiler, 1, wxEXPAND | wxBOTTOM, SPACING); p->btn_close = new wxButton(panel, wxID_CLOSE); - p->btn_flash = new wxButton(panel, wxID_ANY, _(L("Flash!"))); + p->btn_flash = new wxButton(panel, wxID_ANY, p->btn_flash_label_ready); auto *bsizer = new wxBoxSizer(wxHORIZONTAL); bsizer->Add(p->btn_close); bsizer->AddStretchSpacer(); @@ -293,9 +316,24 @@ FirmwareDialog::FirmwareDialog(wxWindow *parent) : p->find_serial_ports(); p->btn_close->Bind(wxEVT_BUTTON, [this](wxCommandEvent &) { this->Close(); }); - p->btn_flash->Bind(wxEVT_BUTTON, [this](wxCommandEvent &) { this->p->perform_upload(); }); p->btn_rescan->Bind(wxEVT_BUTTON, [this](wxCommandEvent &) { this->p->find_serial_ports(); }); + p->btn_flash->Bind(wxEVT_BUTTON, [this](wxCommandEvent &) { + if (this->p->avrdude) { + // Flashing is in progress, ask the user if they're really sure about canceling it + wxMessageDialog dlg(this, + _(L("Are you sure you want to cancel firmware flashing?\nThis could leave your printer in an unusable state!")), + _(L("Confirmation")), + wxYES_NO | wxNO_DEFAULT | wxICON_QUESTION); + if (dlg.ShowModal() == wxID_YES) { + this->p->cancel = true; + } + } else { + // Start a flashing task + this->p->perform_upload(); + } + }); + Bind(EVT_AVRDUDE, [this](wxCommandEvent &evt) { this->p->on_avrdude(evt); }); Bind(wxEVT_CLOSE_WINDOW, [this](wxCloseEvent &evt) {