FirmwareDialog: Fix progress display

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<std::string> args;
+	RunFn run_fn;
 	MessageFn message_fn;
 	ProgressFn progress_fn;
 	CompleteFn complete_fn;
@@ -94,6 +95,12 @@ AvrDude& AvrDude::args(std::vector<std::string> 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<AvrDude> Ptr;
+	typedef std::function<void()> RunFn;
 	typedef std::function<void(const char * /* msg */, unsigned /* size */)> MessageFn;
 	typedef std::function<void(const char * /* task */, unsigned /* progress */)> ProgressFn;
 	typedef std::function<void(int /* exit status */)> CompleteFn;
@@ -29,6 +30,11 @@ public:
 	// Set avrdude cli arguments
 	AvrDude& args(std::vector<std::string> 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 <wx/event.h>
 #include <wx/sizer.h>
 #include <wx/settings.h>
+#include <wx/timer.h>
 #include <wx/panel.h>
 #include <wx/button.h>
 #include <wx/filepicker.h>
@@ -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<Utils::SerialPortInfo> 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) {