Printhost: Cancelation, bugfixes

src/libslic3r/Channel.hpp

@@ -77,8 +77,7 @@ public:
         }
     }
 
-    // Unlocked observers/hints
+    // Unlocked observer/hint. Thread unsafe! Keep in mind you need to re-verify the result after locking.
-    // Thread unsafe! Keep in mind you need to re-verify the result after locking!
     size_t size_hint() const noexcept { return m_queue.size(); }
 
     LockedConstPtr lock_read() const

src/slic3r/GUI/PrintHostDialogs.cpp

@@ -14,6 +14,7 @@
 #include <wx/debug.h>
 
 #include "GUI.hpp"
+#include "GUI_App.hpp"
 #include "MsgDialog.hpp"
 #include "I18N.hpp"
 #include "../Utils/PrintHost.hpp"
@@ -60,6 +61,7 @@ bool PrintHostSendDialog::start_print() const
 
 wxDEFINE_EVENT(EVT_PRINTHOST_PROGRESS, PrintHostQueueDialog::Event);
 wxDEFINE_EVENT(EVT_PRINTHOST_ERROR,    PrintHostQueueDialog::Event);
+wxDEFINE_EVENT(EVT_PRINTHOST_CANCEL,   PrintHostQueueDialog::Event);
 
 PrintHostQueueDialog::Event::Event(wxEventType eventType, int winid, size_t job_id)
     : wxEvent(winid, eventType)
@@ -87,6 +89,7 @@ PrintHostQueueDialog::PrintHostQueueDialog(wxWindow *parent)
     : wxDialog(parent, wxID_ANY, _(L("Print host upload queue")), wxDefaultPosition, wxDefaultSize, wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER)
     , on_progress_evt(this, EVT_PRINTHOST_PROGRESS, &PrintHostQueueDialog::on_progress, this)
     , on_error_evt(this, EVT_PRINTHOST_ERROR, &PrintHostQueueDialog::on_error, this)
+    , on_cancel_evt(this, EVT_PRINTHOST_CANCEL, &PrintHostQueueDialog::on_cancel, this)
 {
     enum { HEIGHT = 800, WIDTH = 400, SPACING = 5 };
 
@@ -127,6 +130,7 @@ PrintHostQueueDialog::PrintHostQueueDialog(wxWindow *parent)
         const JobState state = get_state(selected);
         if (state < ST_ERROR) {
             // TODO: cancel
+            GUI::wxGetApp().printhost_job_queue().cancel(selected);
         }
     });
 
@@ -161,6 +165,15 @@ void PrintHostQueueDialog::set_state(int idx, JobState state)
 {
     wxCHECK_RET(idx >= 0 && idx < job_list->GetItemCount(), "Out of bounds access to job list");
     job_list->SetItemData(job_list->RowToItem(idx), static_cast<wxUIntPtr>(state));
+
+    switch (state) {
+        case ST_NEW:        job_list->SetValue(_(L("Enqueued")), idx, COL_STATUS); break;
+        case ST_PROGRESS:   job_list->SetValue(_(L("Uploading")), idx, COL_STATUS); break;
+        case ST_ERROR:      job_list->SetValue(_(L("Error")), idx, COL_STATUS); break;
+        case ST_CANCELLING: job_list->SetValue(_(L("Cancelling")), idx, COL_STATUS); break;
+        case ST_CANCELLED:  job_list->SetValue(_(L("Cancelled")), idx, COL_STATUS); break;
+        case ST_COMPLETED:  job_list->SetValue(_(L("Completed")), idx, COL_STATUS); break;
+    }
 }
 
 void PrintHostQueueDialog::on_list_select()
@@ -183,11 +196,9 @@ void PrintHostQueueDialog::on_progress(Event &evt)
     if (evt.progress < 100) {
         set_state(evt.job_id, ST_PROGRESS);
         job_list->SetValue(wxVariant(evt.progress), evt.job_id, COL_PROGRESS);
-        job_list->SetValue(_(L("Uploading")), evt.job_id, COL_STATUS);
     } else {
         set_state(evt.job_id, ST_COMPLETED);
         job_list->SetValue(wxVariant(100), evt.job_id, COL_PROGRESS);
-        job_list->SetValue(_(L("Complete")), evt.job_id, COL_STATUS);
     }
 
     on_list_select();
@@ -201,7 +212,6 @@ void PrintHostQueueDialog::on_error(Event &evt)
 
     auto errormsg = wxString::Format("%s\n%s", _(L("Error uploading to print host:")), evt.error);
     job_list->SetValue(wxVariant(0), evt.job_id, COL_PROGRESS);
-    job_list->SetValue(wxVariant(_(L("Error"))), evt.job_id, COL_STATUS);
     job_list->SetValue(wxVariant(errormsg), evt.job_id, COL_ERRORMSG);    // Stashes the error message into a hidden column for later
 
     on_list_select();
@@ -209,5 +219,15 @@ void PrintHostQueueDialog::on_error(Event &evt)
     GUI::show_error(nullptr, std::move(errormsg));
 }
 
+void PrintHostQueueDialog::on_cancel(Event &evt)
+{
+    wxCHECK_RET(evt.job_id < job_list->GetItemCount(), "Out of bounds access to job list");
+
+    set_state(evt.job_id, ST_CANCELLED);
+    job_list->SetValue(wxVariant(0), evt.job_id, COL_PROGRESS);
+
+    on_list_select();
+}
+
 
 }}

src/slic3r/GUI/PrintHostDialogs.hpp

@@ -86,16 +86,19 @@ private:
     // Note: EventGuard prevents delivery of progress evts to a freed PrintHostQueueDialog
     EventGuard on_progress_evt;
     EventGuard on_error_evt;
+    EventGuard on_cancel_evt;
 
     JobState get_state(int idx);
     void set_state(int idx, JobState);
     void on_list_select();
     void on_progress(Event&);
     void on_error(Event&);
+    void on_cancel(Event&);
 };
 
 wxDECLARE_EVENT(EVT_PRINTHOST_PROGRESS, PrintHostQueueDialog::Event);
 wxDECLARE_EVENT(EVT_PRINTHOST_ERROR, PrintHostQueueDialog::Event);
+wxDECLARE_EVENT(EVT_PRINTHOST_CANCEL, PrintHostQueueDialog::Event);
 
 
 }}

src/slic3r/Utils/Http.cpp

@@ -149,7 +149,9 @@ int Http::priv::xfercb(void *userp, curl_off_t dltotal, curl_off_t dlnow, curl_o
 		self->progressfn(progress, cb_cancel);
 	}
 
-	return self->cancel || cb_cancel;
+	if (cb_cancel) { self->cancel = true; }
+
+	return self->cancel;
 }
 
 int Http::priv::xfercb_legacy(void *userp, double dltotal, double dlnow, double ultotal, double ulnow)

src/slic3r/Utils/PrintHost.cpp

@@ -49,6 +49,7 @@ struct PrintHostJobQueue::priv
     Channel<size_t> channel_cancels;
     size_t job_id = 0;
     int prev_progress = -1;
+    fs::path source_to_remove;
 
     std::thread bg_thread;
     bool bg_exit = false;
@@ -57,9 +58,14 @@ struct PrintHostJobQueue::priv
 
     priv(PrintHostJobQueue *q) : q(q) {}
 
+    void emit_progress(int progress);
+    void emit_error(wxString error);
+    void emit_cancel(size_t id);
     void start_bg_thread();
     void bg_thread_main();
     void progress_fn(Http::Progress progress, bool &cancel);
+    void remove_source(const fs::path &path);
+    void remove_source();
     void perform_job(PrintHostJob the_job);
 };
 
@@ -78,6 +84,24 @@ PrintHostJobQueue::~PrintHostJobQueue()
     }
 }
 
+void PrintHostJobQueue::priv::emit_progress(int progress)
+{
+    auto evt = new PrintHostQueueDialog::Event(GUI::EVT_PRINTHOST_PROGRESS, queue_dialog->GetId(), job_id, progress);
+    wxQueueEvent(queue_dialog, evt);
+}
+
+void PrintHostJobQueue::priv::emit_error(wxString error)
+{
+    auto evt = new PrintHostQueueDialog::Event(GUI::EVT_PRINTHOST_ERROR, queue_dialog->GetId(), job_id, std::move(error));
+    wxQueueEvent(queue_dialog, evt);
+}
+
+void PrintHostJobQueue::priv::emit_cancel(size_t id)
+{
+    auto evt = new PrintHostQueueDialog::Event(GUI::EVT_PRINTHOST_CANCEL, queue_dialog->GetId(), id);
+    wxQueueEvent(queue_dialog, evt);
+}
+
 void PrintHostJobQueue::priv::start_bg_thread()
 {
     if (bg_thread.joinable()) { return; }
@@ -96,21 +120,43 @@ void PrintHostJobQueue::priv::bg_thread_main()
         // Pick up jobs from the job channel:
         while (! bg_exit) {
             auto job = channel_jobs.pop();   // Sleeps in a cond var if there are no jobs
+            source_to_remove = job.upload_data.source_path;
+
+            BOOST_LOG_TRIVIAL(debug) << boost::format("PrintHostJobQueue/bg_thread: Received job: [%1%]: `%2%` -> `%3%`, cancelled: %4%")
+                % job_id
+                % job.upload_data.upload_path
+                % job.printhost->get_host()
+                % job.cancelled;
+
             if (! job.cancelled) {
                 perform_job(std::move(job));
             }
+
+            remove_source();
             job_id++;
         }
     } catch (const std::exception &e) {
-        auto evt = new PrintHostQueueDialog::Event(GUI::EVT_PRINTHOST_ERROR, queue_dialog->GetId(), job_id, e.what());
+        emit_error(e.what());
-        wxQueueEvent(queue_dialog, evt);
     } catch (...) {
         wxTheApp->OnUnhandledException();
     }
+
+    // Cleanup leftover files, if any
+    remove_source();
+    auto jobs = channel_jobs.lock_rw();
+    for (const PrintHostJob &job : *jobs) {
+        remove_source(job.upload_data.source_path);
+    }
 }
 
 void PrintHostJobQueue::priv::progress_fn(Http::Progress progress, bool &cancel)
 {
+    if (cancel) {
+        // When cancel is true from the start, Http indicates request has been cancelled
+        emit_cancel(job_id);
+        return;
+    }
+
     if (bg_exit) {
         cancel = true;
         return;
@@ -125,49 +171,57 @@ void PrintHostJobQueue::priv::progress_fn(Http::Progress progress, bool &cancel)
             if (cancel_id == job_id) {
                 cancel = true;
             } else if (cancel_id > job_id) {
-                jobs->at(cancel_id - job_id).cancelled = true;
+                const size_t idx = cancel_id - job_id - 1;
+                if (idx < jobs->size()) {
+                    jobs->at(idx).cancelled = true;
+                    BOOST_LOG_TRIVIAL(debug) << boost::format("PrintHostJobQueue: Job id %1% cancelled") % cancel_id;
+                    emit_cancel(cancel_id);
+                }
             }
-            // TODO: emit cancelled
         }
 
         cancels->clear();
     }
 
+    if (! cancel) {
         int gui_progress = progress.ultotal > 0 ? 100*progress.ulnow / progress.ultotal : 0;
         if (gui_progress != prev_progress) {
-        auto evt = new PrintHostQueueDialog::Event(GUI::EVT_PRINTHOST_PROGRESS, queue_dialog->GetId(), job_id, gui_progress);
+            emit_progress(gui_progress);
-        wxQueueEvent(queue_dialog, evt);
             prev_progress = gui_progress;
         }
     }
+}
+
+void PrintHostJobQueue::priv::remove_source(const fs::path &path)
+{
+    if (! path.empty()) {
+        boost::system::error_code ec;
+        fs::remove(path, ec);
+        if (ec) {
+            BOOST_LOG_TRIVIAL(error) << boost::format("PrintHostJobQueue: Error removing file `%1%`: %2%") % path % ec;
+        }
+    }
+}
+
+void PrintHostJobQueue::priv::remove_source()
+{
+    remove_source(source_to_remove);
+    source_to_remove.clear();
+}
 
 void PrintHostJobQueue::priv::perform_job(PrintHostJob the_job)
 {
     if (bg_exit || the_job.empty()) { return; }
 
-    BOOST_LOG_TRIVIAL(debug) << boost::format("PrintHostJobQueue/bg_thread: Got job: `%1%` -> `%2%`")
-        % the_job.upload_data.upload_path
-        % the_job.printhost->get_host();
-
-    const fs::path gcode_path = the_job.upload_data.source_path;
-
     bool success = the_job.printhost->upload(std::move(the_job.upload_data),
         [this](Http::Progress progress, bool &cancel) { this->progress_fn(std::move(progress), cancel); },
         [this](wxString error) {
-            auto evt = new PrintHostQueueDialog::Event(GUI::EVT_PRINTHOST_ERROR, queue_dialog->GetId(), job_id, std::move(error));
+            emit_error(std::move(error));
-            wxQueueEvent(queue_dialog, evt);
         }
     );
 
     if (success) {
-        auto evt = new PrintHostQueueDialog::Event(GUI::EVT_PRINTHOST_PROGRESS, queue_dialog->GetId(), job_id, 100);
+        emit_progress(100);
-        wxQueueEvent(queue_dialog, evt);
-    }
-
-    boost::system::error_code ec;
-    fs::remove(gcode_path, ec);
-    if (ec) {
-        BOOST_LOG_TRIVIAL(error) << boost::format("PrintHostJobQueue: Error removing file `%1%`: %2%") % gcode_path % ec;
     }
 }
 
@@ -178,5 +232,10 @@ void PrintHostJobQueue::enqueue(PrintHostJob job)
     p->channel_jobs.push(std::move(job));
 }
 
+void PrintHostJobQueue::cancel(size_t id)
+{
+    p->channel_cancels.push(id);
+}
+
 
 }

src/slic3r/Utils/PrintHost.hpp

@@ -55,6 +55,7 @@ struct PrintHostJob
     PrintHostJob(PrintHostJob &&other)
         : upload_data(std::move(other.upload_data))
         , printhost(std::move(other.printhost))
+        , cancelled(other.cancelled)
     {}
 
     PrintHostJob(DynamicPrintConfig *config)
@@ -66,6 +67,7 @@ struct PrintHostJob
     {
         upload_data = std::move(other.upload_data);
         printhost = std::move(other.printhost);
+        cancelled = other.cancelled;
         return *this;
     }