Print host bugfixes / refactoring

lib/Slic3r.pm

@@ -168,7 +168,6 @@ sub thread_cleanup {
     *Slic3r::GUI::TabIface::DESTROY         = sub {};
     *Slic3r::OctoPrint::DESTROY             = sub {};
     *Slic3r::Duet::DESTROY                  = sub {};
-    *Slic3r::PrintHostFactory::DESTROY      = sub {};
     *Slic3r::PresetUpdater::DESTROY         = sub {};
     return undef;  # this prevents a "Scalars leaked" warning
 }

lib/Slic3r/GUI/Plater.pm

@@ -1585,7 +1585,7 @@ sub on_export_completed {
 
     # Send $self->{send_gcode_file} to OctoPrint.
     if ($send_gcode) {
-        my $host = Slic3r::PrintHostFactory::get_print_host($self->{config});
+        my $host = Slic3r::PrintHost::get_print_host($self->{config});
 
         if ($host->send_gcode($self->{send_gcode_file})) {
             $self->statusbar->SetStatusText(L("Upload to host finished."));

xs/CMakeLists.txt

@@ -257,8 +257,8 @@ add_library(libslic3r_gui STATIC
     ${LIBDIR}/slic3r/Utils/OctoPrint.hpp
     ${LIBDIR}/slic3r/Utils/Duet.cpp
     ${LIBDIR}/slic3r/Utils/Duet.hpp
-    ${LIBDIR}/slic3r/Utils/PrintHostFactory.cpp
-    ${LIBDIR}/slic3r/Utils/PrintHostFactory.hpp
+    ${LIBDIR}/slic3r/Utils/PrintHost.cpp
+    ${LIBDIR}/slic3r/Utils/PrintHost.hpp
     ${LIBDIR}/slic3r/Utils/Bonjour.cpp
     ${LIBDIR}/slic3r/Utils/Bonjour.hpp
     ${LIBDIR}/slic3r/Utils/PresetUpdater.cpp
@@ -417,7 +417,6 @@ set(XS_XSP_FILES
     ${XSP_DIR}/Surface.xsp
     ${XSP_DIR}/SurfaceCollection.xsp
     ${XSP_DIR}/TriangleMesh.xsp
-    ${XSP_DIR}/Utils_PrintHostFactory.xsp
     ${XSP_DIR}/Utils_PrintHost.xsp
     ${XSP_DIR}/Utils_PresetUpdater.xsp
     ${XSP_DIR}/AppController.xsp

xs/src/libslic3r/PrintConfig.cpp

@@ -2144,9 +2144,6 @@ void PrintConfigDef::handle_legacy(t_config_option_key &opt_key, std::string &va
         "standby_temperature", "scale", "rotate", "duplicate", "duplicate_grid",
         "start_perimeters_at_concave_points", "start_perimeters_at_non_overhang", "randomize_start", 
         "seal_position", "vibration_limit", "bed_size", 
-        // Maybe one day we will rename octoprint_host to print_host as it has been done in the upstream Slic3r.
-        // Commenting this out fixes github issue #869 for now.
-        // "octoprint_host",
         "print_center", "g0", "threads", "pressure_advance", "wipe_tower_per_color_wipe"
     };
 
@@ -2156,7 +2153,6 @@ void PrintConfigDef::handle_legacy(t_config_option_key &opt_key, std::string &va
     }
     
     if (! print_config_def.has(opt_key)) {
-        //printf("Unknown option %s\n", opt_key.c_str());
         opt_key = "";
         return;
     }

xs/src/perlglue.cpp

@@ -67,7 +67,6 @@ REGISTER_CLASS(PresetUpdater, "PresetUpdater");
 REGISTER_CLASS(AppController, "AppController");
 REGISTER_CLASS(PrintController, "PrintController");
 REGISTER_CLASS(PrintHost, "PrintHost");
-REGISTER_CLASS(PrintHostFactory, "PrintHostFactory");
 
 SV* ConfigBase__as_hash(ConfigBase* THIS)
 {

xs/src/slic3r/GUI/Tab.cpp

@@ -5,7 +5,6 @@
 #include "../../libslic3r/Utils.hpp"
 
 #include "slic3r/Utils/Http.hpp"
-#include "slic3r/Utils/PrintHostFactory.hpp"
 #include "slic3r/Utils/PrintHost.hpp"
 #include "slic3r/Utils/Serial.hpp"
 #include "BonjourDialog.hpp"
@@ -1550,8 +1549,8 @@ void TabPrinter::build()
 			sizer->Add(btn);
 
 			btn->Bind(wxEVT_BUTTON, [this](wxCommandEvent e) {
-				PrintHost *host = PrintHostFactory::get_print_host(m_config);
-				if (host == NULL) {
+				std::unique_ptr<PrintHost> host(PrintHost::get_print_host(m_config));
+				if (! host) {
 					const auto text = wxString::Format("%s",
 						_(L("Could not get a valid Printer Host reference")));
 					show_error(this, text);
@@ -1563,8 +1562,6 @@ void TabPrinter::build()
 				} else {
 					show_error(this, host->get_test_failed_msg(msg));
 				}
-
-				delete (host);
 			});
 
 			return sizer;
@@ -1905,11 +1902,12 @@ void TabPrinter::update(){
 			m_serial_test_btn->Disable();
 	}
 
-	PrintHost *host = PrintHostFactory::get_print_host(m_config);
-	m_print_host_test_btn->Enable(!m_config->opt_string("print_host").empty() && host->can_test());
-	m_printhost_browse_btn->Enable(host->have_auto_discovery());
-	delete (host);
-	
+	{
+		std::unique_ptr<PrintHost> host(PrintHost::get_print_host(m_config));
+		m_print_host_test_btn->Enable(!m_config->opt_string("print_host").empty() && host->can_test());
+		m_printhost_browse_btn->Enable(host->has_auto_discovery());
+	}
+
 	bool have_multiple_extruders = m_extruders_count > 1;
 	get_field("toolchange_gcode")->toggle(have_multiple_extruders);
 	get_field("single_extruder_multi_material")->toggle(have_multiple_extruders);

xs/src/slic3r/Utils/Duet.cpp

@@ -2,6 +2,7 @@
 #include "PrintHostSendDialog.hpp"
 
 #include <algorithm>
+#include <ctime>
 #include <boost/filesystem/path.hpp>
 #include <boost/format.hpp>
 #include <boost/log/trivial.hpp>
@@ -31,6 +32,8 @@ Duet::Duet(DynamicPrintConfig *config) :
 	password(config->opt_string("printhost_apikey"))
 {}
 
+Duet::~Duet() {}
+
 bool Duet::test(wxString &msg) const
 {
 	bool connected = connect(msg);
@@ -48,8 +51,7 @@ wxString Duet::get_test_ok_msg () const
 
 wxString Duet::get_test_failed_msg (wxString &msg) const
 {
-	return wxString::Format("%s: %s",
-						_(L("Could not connect to Duet")), msg);
+	return wxString::Format("%s: %s", _(L("Could not connect to Duet")), msg);
 }
 
 bool Duet::send_gcode(const std::string &filename) const
@@ -91,23 +93,17 @@ bool Duet::send_gcode(const std::string &filename) const
 		% upload_cmd;
 
 	auto http = Http::post(std::move(upload_cmd));
-	http.postfield_add_file(filename)
+	http.set_post_body(filename)
 		.on_complete([&](std::string body, unsigned status) {
 			BOOST_LOG_TRIVIAL(debug) << boost::format("Duet: File uploaded: HTTP %1%: %2%") % status % body;
 			progress_dialog.Update(PROGRESS_RANGE);
 
 			int err_code = get_err_code_from_body(body);
-			switch (err_code) {
-				case 0:
-					break;
-				default:
-					auto msg = format_error(body, L("Unknown error occured"), 0);
-					GUI::show_error(&progress_dialog, std::move(msg));
-					res = false;
-					break;
-			}
-
-			if (err_code == 0 && print) {
+			if (err_code != 0) {
+				auto msg = format_error(body, L("Unknown error occured"), 0);
+				GUI::show_error(&progress_dialog, std::move(msg));
+				res = false;
+			} else if (print) {
 				wxString errormsg;
 				res = start_print(errormsg, upload_filepath.string());
 				if (!res) {
@@ -139,7 +135,7 @@ bool Duet::send_gcode(const std::string &filename) const
 	return res;
 }
 
-bool Duet::have_auto_discovery() const
+bool Duet::has_auto_discovery() const
 {
 	return false;
 }
@@ -228,12 +224,15 @@ std::string Duet::get_base_url() const
 
 std::string Duet::timestamp_str() const
 {
+	enum { BUFFER_SIZE = 32 };
+
 	auto t = std::time(nullptr);
 	auto tm = *std::localtime(&t);
-	std::stringstream ss;
-	ss << "time=" << std::put_time(&tm, "%Y-%d-%mT%H:%M:%S");
 
-	return ss.str();
+	char buffer[BUFFER_SIZE];
+	std::strftime(buffer, BUFFER_SIZE, "%Y-%d-%mT%H:%M:%S", &tm);
+
+	return std::string(buffer);
 }
 
 wxString Duet::format_error(const std::string &body, const std::string &error, unsigned status)

xs/src/slic3r/Utils/Duet.hpp

@@ -17,18 +17,19 @@ class Duet : public PrintHost
 {
 public:
 	Duet(DynamicPrintConfig *config);
+	virtual ~Duet();
 
 	bool test(wxString &curl_msg) const;
 	wxString get_test_ok_msg () const;
 	wxString get_test_failed_msg (wxString &msg) const;
 	// Send gcode file to duet, filename is expected to be in UTF-8
 	bool send_gcode(const std::string &filename) const;
-	bool have_auto_discovery() const;
+	bool has_auto_discovery() const;
 	bool can_test() const;
 private:
 	std::string host;
 	std::string password;
-	
+
 	std::string get_upload_url(const std::string &filename) const;
 	std::string get_connect_url() const;
 	std::string get_base_url() const;

xs/src/slic3r/Utils/Http.cpp

@@ -62,7 +62,7 @@ struct Http::priv
 	static size_t form_file_read_cb(char *buffer, size_t size, size_t nitems, void *userp);
 
 	void form_add_file(const char *name, const fs::path &path, const char* filename);
-	void postfield_add_file(const fs::path &path);
+	void set_post_body(const fs::path &path);
 
 	std::string curl_error(CURLcode curlcode);
 	std::string body_size_error();
@@ -190,14 +190,11 @@ void Http::priv::form_add_file(const char *name, const fs::path &path, const cha
 	}
 }
 
-void Http::priv::postfield_add_file(const fs::path &path)
+void Http::priv::set_post_body(const fs::path &path)
 {
-	std::ifstream f (path.string());
-	std::string file_content { std::istreambuf_iterator<char>(f), std::istreambuf_iterator<char>() };
-	if (!postfields.empty()) {
-		postfields += "&";
-	}
-	postfields += file_content;
+	std::ifstream file(path.string());
+	std::string file_content { std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>() };
+	postfields = file_content;
 }
 
 std::string Http::priv::curl_error(CURLcode curlcode)
@@ -356,9 +353,9 @@ Http& Http::form_add_file(const std::string &name, const fs::path &path, const s
 	return *this;
 }
 
-Http& Http::postfield_add_file(const fs::path &path)
+Http& Http::set_post_body(const fs::path &path)
 {
-	if (p) { p->postfield_add_file(path);}
+	if (p) { p->set_post_body(path);}
 	return *this;
 }
 

xs/src/slic3r/Utils/Http.hpp

@@ -73,8 +73,10 @@ public:
 	// Same as above except also override the file's filename with a custom one
 	Http& form_add_file(const std::string &name, const boost::filesystem::path &path, const std::string &filename);
 
-	// Add the file as POSTFIELD to the request, this can be used for hosts which do not support multipart requests
-	Http& postfield_add_file(const boost::filesystem::path &path);
+	// Set the file contents as a POST request body.
+	// The data is used verbatim, it is not additionally encoded in any way.
+	// This can be used for hosts which do not support multipart requests.
+	Http& set_post_body(const boost::filesystem::path &path);
 
 	// Callback called on HTTP request complete
 	Http& on_complete(CompleteFn fn);

xs/src/slic3r/Utils/OctoPrint.cpp

@@ -19,6 +19,8 @@ OctoPrint::OctoPrint(DynamicPrintConfig *config) :
 	cafile(config->opt_string("printhost_cafile"))
 {}
 
+OctoPrint::~OctoPrint() {}
+
 bool OctoPrint::test(wxString &msg) const
 {
 	// Since the request is performed synchronously here,
@@ -125,7 +127,7 @@ bool OctoPrint::send_gcode(const std::string &filename) const
 	return res;
 }
 
-bool OctoPrint::have_auto_discovery() const
+bool OctoPrint::has_auto_discovery() const
 {
 	return true;
 }

xs/src/slic3r/Utils/OctoPrint.hpp

@@ -17,13 +17,14 @@ class OctoPrint : public PrintHost
 {
 public:
 	OctoPrint(DynamicPrintConfig *config);
+	virtual ~OctoPrint();
 
 	bool test(wxString &curl_msg) const;
 	wxString get_test_ok_msg () const;
 	wxString get_test_failed_msg (wxString &msg) const;
 	// Send gcode file to octoprint, filename is expected to be in UTF-8
 	bool send_gcode(const std::string &filename) const;
-	bool have_auto_discovery() const;
+	bool has_auto_discovery() const;
 	bool can_test() const;
 private:
 	std::string host;

xs/src/slic3r/Utils/PrintHost.cpp

@@ -1,4 +1,3 @@
-#include "PrintHostFactory.hpp"
 #include "OctoPrint.hpp"
 #include "Duet.hpp"
 
@@ -7,7 +6,9 @@
 namespace Slic3r {
 
 
-PrintHost * PrintHostFactory::get_print_host(DynamicPrintConfig *config)
+PrintHost::~PrintHost() {}
+
+PrintHost* PrintHost::get_print_host(DynamicPrintConfig *config)
 {
 	PrintHostType kind = config->option<ConfigOptionEnum<PrintHostType>>("host_type")->value;
 	if (kind == htOctoPrint) {
@@ -15,7 +16,8 @@ PrintHost * PrintHostFactory::get_print_host(DynamicPrintConfig *config)
 	} else if (kind == htDuet) {
 		return new Duet(config);
 	}
-	return NULL;
+	return nullptr;
 }
 
+
 }

xs/src/slic3r/Utils/PrintHost.hpp

@@ -1,6 +1,7 @@
 #ifndef slic3r_PrintHost_hpp_
 #define slic3r_PrintHost_hpp_
 
+#include <memory>
 #include <string>
 #include <wx/string.h>
 
@@ -13,14 +14,17 @@ class DynamicPrintConfig;
 class PrintHost
 {
 public:
+	virtual ~PrintHost();
 
 	virtual bool test(wxString &curl_msg) const = 0;
 	virtual wxString get_test_ok_msg () const = 0;
 	virtual wxString get_test_failed_msg (wxString &msg) const = 0;
 	// Send gcode file to print host, filename is expected to be in UTF-8
 	virtual bool send_gcode(const std::string &filename) const = 0;
-	virtual bool have_auto_discovery() const = 0;
+	virtual bool has_auto_discovery() const = 0;
 	virtual bool can_test() const = 0;
+
+	static PrintHost* get_print_host(DynamicPrintConfig *config);
 };
 
 

xs/src/slic3r/Utils/PrintHostFactory.hpp

@@ -1,24 +0,0 @@
-#ifndef slic3r_PrintHostFactory_hpp_
-#define slic3r_PrintHostFactory_hpp_
-
-#include <string>
-#include <wx/string.h>
-
-
-namespace Slic3r {
-
-class DynamicPrintConfig;
-class PrintHost;
-
-class PrintHostFactory
-{
-public:
-	PrintHostFactory() {};
-	~PrintHostFactory() {};
-	static PrintHost * get_print_host(DynamicPrintConfig *config);
-};
-
-
-}
-
-#endif

xs/src/slic3r/Utils/PrintHostSendDialog.cpp

@@ -36,9 +36,7 @@ PrintHostSendDialog::PrintHostSendDialog(const fs::path &path, bool can_start_pr
 	wxString stem(path.stem().wstring());
 	txt_filename->SetSelection(0, stem.Length());
 
-	if (!can_start_print) {
-		box_print->Disable();
-	}
+	box_print->Enable(can_start_print);
 
 	Fit();
 }

xs/src/slic3r/Utils/PrintHostSendDialog.hpp

@@ -22,14 +22,12 @@ namespace Slic3r {
 
 class PrintHostSendDialog : public GUI::MsgDialog
 {
-
 private:
 	wxTextCtrl *txt_filename;
 	wxCheckBox *box_print;
 	bool can_start_print;
 
 public:
-
 	PrintHostSendDialog(const boost::filesystem::path &path, bool can_start_print);
 	boost::filesystem::path filename() const;
 	bool print() const;

xs/xsp/Utils_PrintHost.xsp

@@ -7,4 +7,6 @@
 
 %name{Slic3r::PrintHost} class PrintHost {
 	bool send_gcode(std::string filename) const;
+
+	static PrintHost* get_print_host(DynamicPrintConfig *config);
 };

xs/xsp/Utils_PrintHostFactory.xsp

@@ -1,13 +0,0 @@
-%module{Slic3r::XS};
-
-%{
-#include <xsinit.h>
-#include "slic3r/Utils/PrintHostFactory.hpp"
-%}
-
-%name{Slic3r::PrintHostFactory} class PrintHostFactory {
-    PrintHostFactory();
-    ~PrintHostFactory();
-
-   static PrintHost * get_print_host(DynamicPrintConfig *config);
-};

xs/xsp/my.map

@@ -239,11 +239,7 @@ Ref<TabIface> 				O_OBJECT_SLIC3R_T
 PresetUpdater*              O_OBJECT_SLIC3R
 Ref<PresetUpdater>          O_OBJECT_SLIC3R_T
 
-PrintHostFactory*            O_OBJECT_SLIC3R
-Ref<PrintHostFactory>        O_OBJECT_SLIC3R_T
-Clone<PrintHostFactory>      O_OBJECT_SLIC3R_T
-
-PrintHost*                   O_OBJECT_SLIC3R
+PrintHost*                  O_OBJECT_SLIC3R
 
 Axis                  T_UV
 ExtrusionLoopRole     T_UV