From 5414f7379d9ba0592f60a37a0c610a569787456c Mon Sep 17 00:00:00 2001 From: Vojtech Kral Date: Tue, 5 Jun 2018 11:55:23 +0200 Subject: [PATCH] FirmwareDialog: Fix progress display --- xs/src/avrdude/avrdude-slic3r.cpp | 23 ++++++++++++++++++----- xs/src/avrdude/avrdude-slic3r.hpp | 6 ++++++ xs/src/avrdude/ser_posix.c | 4 ++++ xs/src/slic3r/GUI/FirmwareDialog.cpp | 23 +++++++++++++++-------- 4 files changed, 43 insertions(+), 13 deletions(-) diff --git a/xs/src/avrdude/avrdude-slic3r.cpp b/xs/src/avrdude/avrdude-slic3r.cpp index a859200fb..cf4380fdb 100644 --- a/xs/src/avrdude/avrdude-slic3r.cpp +++ b/xs/src/avrdude/avrdude-slic3r.cpp @@ -34,6 +34,7 @@ struct AvrDude::priv { std::string sys_config; std::vector args; + RunFn run_fn; MessageFn message_fn; ProgressFn progress_fn; CompleteFn complete_fn; @@ -94,6 +95,12 @@ AvrDude& AvrDude::args(std::vector args) return *this; } +AvrDude& AvrDude::on_run(RunFn fn) +{ + if (p) { p->run_fn = std::move(fn); } + return *this; +} + AvrDude& AvrDude::on_message(MessageFn fn) { if (p) { p->message_fn = std::move(fn); } @@ -123,11 +130,17 @@ AvrDude::Ptr AvrDude::run() if (self->p) { auto avrdude_thread = std::thread([self]() { - auto res = self->p->run(); - if (self->p->complete_fn) { - self->p->complete_fn(res); - } - }); + if (self->p->run_fn) { + self->p->run_fn(); + } + + auto res = self->p->run(); + + if (self->p->complete_fn) { + self->p->complete_fn(res); + } + }); + self->p->avrdude_thread = std::move(avrdude_thread); } diff --git a/xs/src/avrdude/avrdude-slic3r.hpp b/xs/src/avrdude/avrdude-slic3r.hpp index 8d881b094..29d96f72d 100644 --- a/xs/src/avrdude/avrdude-slic3r.hpp +++ b/xs/src/avrdude/avrdude-slic3r.hpp @@ -12,6 +12,7 @@ class AvrDude { public: typedef std::shared_ptr Ptr; + typedef std::function RunFn; typedef std::function MessageFn; typedef std::function ProgressFn; typedef std::function CompleteFn; @@ -29,6 +30,11 @@ public: // Set avrdude cli arguments AvrDude& args(std::vector args); + // Set a callback to be called just after run() before avrdude is ran + // This can be used to perform any needed setup tasks from the background thread. + // This has no effect when using run_sync(). + AvrDude& on_run(RunFn fn); + // Set message output callback AvrDude& on_message(MessageFn fn); diff --git a/xs/src/avrdude/ser_posix.c b/xs/src/avrdude/ser_posix.c index 91b18e945..cb0fc0385 100644 --- a/xs/src/avrdude/ser_posix.c +++ b/xs/src/avrdude/ser_posix.c @@ -376,6 +376,10 @@ static int ser_recv(union filedescriptor *fd, unsigned char * buf, size_t buflen FD_SET(fd->ifd, &rfds); nfds = select(fd->ifd + 1, &rfds, NULL, NULL, &to2); + // FIXME: The timeout has different behaviour on Linux vs other Unices + // On Linux, the timeout is modified by subtracting the time spent, + // on OS X (for example), it is not modified. + // POSIX recommends re-initializing it before selecting. if (nfds == 0) { avrdude_message(MSG_NOTICE2, "%s: ser_recv(): programmer is not responding\n", progname); diff --git a/xs/src/slic3r/GUI/FirmwareDialog.cpp b/xs/src/slic3r/GUI/FirmwareDialog.cpp index e57ec6326..bbb00e445 100644 --- a/xs/src/slic3r/GUI/FirmwareDialog.cpp +++ b/xs/src/slic3r/GUI/FirmwareDialog.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -36,7 +37,7 @@ namespace Slic3r { enum AvrdudeEvent { AE_MESSAGE, - AE_PRORGESS, + AE_PROGRESS, AE_EXIT, }; @@ -62,7 +63,6 @@ struct FirmwareDialog::priv std::vector ports; wxFilePickerCtrl *hex_picker; wxStaticText *txt_status; - wxStaticText *txt_progress; wxGauge *progressbar; wxCollapsiblePane *spoiler; wxTextCtrl *txt_stdout; @@ -72,6 +72,8 @@ struct FirmwareDialog::priv wxString btn_flash_label_ready; wxString btn_flash_label_flashing; + wxTimer timer_pulse; + // 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; @@ -83,6 +85,7 @@ struct FirmwareDialog::priv q(q), btn_flash_label_ready(_(L("Flash!"))), btn_flash_label_flashing(_(L("Cancel"))), + timer_pulse(q), avrdude_config((fs::path(::Slic3r::resources_dir()) / "avrdude" / "avrdude.conf").string()), progress_tasks_done(0), cancelled(false) @@ -131,6 +134,7 @@ void FirmwareDialog::priv::flashing_status(bool value, AvrDudeComplete complete) progressbar->SetValue(0); progress_tasks_done = 0; cancelled = false; + timer_pulse.Start(50); } else { auto text_color = wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOWTEXT); port_picker->Enable(); @@ -186,6 +190,7 @@ void FirmwareDialog::priv::perform_upload() avrdude = AvrDude() .sys_config(avrdude_config) .args(args) + .on_run([]() { /* TODO: needed? */ }) .on_message(std::move([q](const char *msg, unsigned /* size */) { auto evt = new wxCommandEvent(EVT_AVRDUDE, q->GetId()); auto wxmsg = wxString::FromUTF8(msg); @@ -195,7 +200,7 @@ void FirmwareDialog::priv::perform_upload() })) .on_progress(std::move([q](const char * /* task */, unsigned progress) { auto evt = new wxCommandEvent(EVT_AVRDUDE, q->GetId()); - evt->SetExtraLong(AE_PRORGESS); + evt->SetExtraLong(AE_PROGRESS); evt->SetInt(progress); wxQueueEvent(q, evt); })) @@ -226,19 +231,19 @@ void FirmwareDialog::priv::on_avrdude(const wxCommandEvent &evt) txt_stdout->AppendText(evt.GetString()); break; - case AE_PRORGESS: + case AE_PROGRESS: // We try to track overall progress here. // When uploading the firmware, avrdude first reads a littlebit of status data, // then performs write, then reading (verification). - // We Pulse() during the first read and combine progress of the latter two tasks. + // We ignore the first task (which just let's the timer_pulse work) + // and then display overall progress during the latter two tasks. - if (progress_tasks_done == 0) { - progressbar->Pulse(); - } else { + if (progress_tasks_done > 0) { progressbar->SetValue(progress_tasks_done - 100 + evt.GetInt()); } if (evt.GetInt() == 100) { + timer_pulse.Stop(); progress_tasks_done += 100; } @@ -376,6 +381,8 @@ FirmwareDialog::FirmwareDialog(wxWindow *parent) : } }); + Bind(wxEVT_TIMER, [this](wxTimerEvent &evt) { this->p->progressbar->Pulse(); }); + Bind(EVT_AVRDUDE, [this](wxCommandEvent &evt) { this->p->on_avrdude(evt); }); Bind(wxEVT_CLOSE_WINDOW, [this](wxCloseEvent &evt) {