From d59155b27fc1bd7b7ce303d5c57bb5d3abed662c Mon Sep 17 00:00:00 2001 From: packerlschupfer <83344883+packerlschupfer@users.noreply.github.com> Date: Sat, 10 Oct 2026 14:27:05 +0200 Subject: [PATCH] =?UTF-8?q?Moonraker:=20pass=20print=3Dtrue=20in=20upload?= =?UTF-8?q?=20=E2=80=94=20fix=20Upload=20&=20Print=20race=20(fixes=20#1494?= =?UTF-8?q?5)=20(#15032)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Moonraker: pass print=true in upload — fixes Upload & Print race with power-on-upload Closes #14945. Upload & Print on the Moonraker (Klipper) host type failed with HTTP 503 "Klippy Host not connected" on any printer that Moonraker powers up in response to an upload (the [power] on_when_upload_queued feature). The file landed on disk, the print never started, and the user hit an error dialog. Root cause: after POST /server/files/upload succeeds we immediately fire POST /printer/print/start. On a cold printer that Moonraker just powered up, Klippy is still coming up when /printer/print/start arrives, so Moonraker returns 503. Fix: add `print=true` to the upload multipart form. Moonraker's own upload endpoint queues the print inside the upload response — and when a [power] device with on_when_upload_queued is configured, it powers the printer on and waits for Klippy READY before starting. That's the whole point of the power-on-upload feature; our second POST was defeating it. Also read `result.print_started` from the upload response — when true, skip our explicit /printer/print/start (Moonraker handled it); when false (older Moonraker or buddy-fork that ignores the print flag), fall back to the explicit call so the existing behaviour is preserved for those servers. Reporter and root-cause identification: @RubenOllesch. (cherry picked from commit bd442155ba8c4d9aa052db4658d443222c2a4f19) * Moonraker: treat print_queued as Moonraker owning the print Reading only result.print_started missed the exact case this PR set out to fix. Moonraker's upload response carries two flags: print_started : it began the print immediately print_queued : it accepted the job but has not started it yet The power-on path (`[power] on_when_upload_queued`) is the second one: Moonraker queues the job, powers the printer up and waits for Klippy to report READY, so it answers print_started=false, print_queued=true. With only print_started read, moonraker_started_print stayed false, the fallback fired, and our explicit /printer/print/start hit the same not-ready Klippy that produced the original 503 — i.e. the fix did not fix #14945 for the configuration that reported it. Verified the response schema against Moonraker v0.11.0 (API 1.5.0); an upload with print=true on a ready printer returns: {"action": "create_file", "item": {...}, "print_started": true, "print_queued": false} Both fields are present, so reading print_queued is safe on this version and the `false` default keeps older hosts on the existing fallback path. Caught by @raistlin7447 in review of #15032; the fix is their suggestion. (cherry picked from commit 43e8eff0035543ce0cb4708eb473545fc2dd6eeb) * Moonraker: read the upload reply's fields at top level Moonraker's FileUploadHandler writes the upload result straight to the response instead of wrapping it in {"result": ...} like the endpoints registered through register_endpoint. item.path, print_started and print_queued are therefore top-level keys. Reading them under result. silently fell back to the local filename and to "not started", so the explicit /printer/print/start still ran after every upload. Also correct the comments on when Moonraker queues a job instead of starting it, and on what it renames on upload. --- src/slic3r/Utils/Moonraker.cpp | 54 ++++++++++++++++++++++------------ src/slic3r/Utils/Moonraker.hpp | 6 ++-- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/slic3r/Utils/Moonraker.cpp b/src/slic3r/Utils/Moonraker.cpp index ee437595ef..faa245bb28 100644 --- a/src/slic3r/Utils/Moonraker.cpp +++ b/src/slic3r/Utils/Moonraker.cpp @@ -178,10 +178,10 @@ bool Moonraker::get_storage(wxArrayString &storage_path, wxArrayString &storage_ bool Moonraker::start_print(wxString &error_msg, const std::string &filename) const { //ORCA: POST /printer/print/start with JSON body { "filename": ".gcode" }. - // `filename` is what /server/files/upload returned as result.item.path (the storage-relative - // path inside `root`, no leading slash, with extension). Build the body via property_tree - // so that special characters in the filename (server-side collision-suffix could produce - // paths with quotes / backslashes on exotic file systems) are properly escaped. + // `filename` is the item.path /server/files/upload returned (the storage-relative + // path inside `root`, no leading slash, with extension), or the original filename + // when the reply had none. Build the body via property_tree so that special characters + // in the filename (quotes, backslashes) are properly escaped. const char *name = get_name(); bool res = true; auto url = make_url("printer/print/start"); @@ -216,13 +216,16 @@ bool Moonraker::start_print(wxString &error_msg, const std::string &filename) co bool Moonraker::upload(PrintHostUpload upload_data, ProgressFn progress_fn, ErrorFn error_fn, InfoFn info_fn) const { - //ORCA: POST /server/files/upload as multipart/form-data with: - // file = - // root = (Moonraker default: "gcodes") - // Successful response shape: - // { "result": { "item": { "path": ".gcode", "root": "" }, "print_started": } } - // We always start the print explicitly via /printer/print/start regardless of `print_started` - // so the user can rely on a single call site for state. + // POST /server/files/upload with `print=true` so Moonraker starts the print + // itself (issue #14945). It reports back in two fields: print_started when it + // began the print immediately, print_queued when it could not start it (Klippy + // not ready, or another print running) but put the job in its queue + // ([file_manager] queue_gcode_uploads). That covers the power-on case: a + // [power ] section with on_when_job_queued switches the printer on, and + // with [job_queue] load_on_startup the queue runs the job once Klippy is READY. + // Either flag means Moonraker owns the print and we must NOT issue our own + // /printer/print/start. When both are false (no job queue, a non-G-code upload, + // or a server that ignores `print`) we fall back to the explicit start below. wxString test_msg; if (!test(test_msg)) { error_fn(std::move(test_msg)); @@ -237,9 +240,12 @@ bool Moonraker::upload(PrintHostUpload upload_data, ProgressFn progress_fn, Erro // addition later (storage picker) needs no change to this method. const std::string root = upload_data.storage.empty() ? std::string("gcodes") : upload_data.storage; + const bool want_start = upload_data.post_action == PrintHostPostUploadAction::StartPrint; + std::string url = make_url("server/files/upload"); bool result = true; std::string uploaded_path; + bool moonraker_started_print = false; //ORCA: gcode inside a .gcode.3mf is index-coded (Metadata/plate_.gcode), so the upload names the // plate via a 1-based `plateindex` (set only in the .3mf path, see Plater::send_gcode_legacy); @@ -253,13 +259,14 @@ bool Moonraker::upload(PrintHostUpload upload_data, ProgressFn progress_fn, Erro % root % upload_filename.string() % (plateindex.empty() ? "-" : plateindex) - % (upload_data.post_action == PrintHostPostUploadAction::StartPrint ? "true" : "false"); + % (want_start ? "true" : "false"); auto http = Http::post(std::move(url)); set_auth(http); http.form_add("root", root); if (!plateindex.empty()) http.form_add("plateindex", plateindex); + http.form_add("print", want_start ? "true" : "false"); http.form_add_file("file", upload_data.source_path.string(), upload_filename.string()) .on_complete([&](std::string body, unsigned status) { BOOST_LOG_TRIVIAL(debug) << boost::format("%1%: upload HTTP %2%: %3%") % name % status % body; @@ -268,20 +275,27 @@ bool Moonraker::upload(PrintHostUpload upload_data, ProgressFn progress_fn, Erro pt::ptree ptree; pt::read_json(ss, ptree); - //ORCA: Moonraker confirms the storage-relative path in result.item.path. We pass exactly - // that string to /printer/print/start so any server-side renaming (collision suffix, - // etc.) is respected. - const auto stored_path = ptree.get_optional("result.item.path"); + //ORCA: unlike the other endpoints (compare result.klippy_state in test()), the upload + // answers with a bare object, not a {"result": ...} envelope: Moonraker's + // FileUploadHandler writes finalize_upload()'s dict straight to the response, so + // item, print_started and print_queued are top-level. Reading them under result. + // silently took the fallbacks below on every upload. + //ORCA: Moonraker confirms the storage-relative path in item.path. We pass exactly + // that string to /printer/print/start so any server-side renaming (.ufp stored as + // .gcode, surrounding whitespace and leading slashes stripped) is respected. + const auto stored_path = ptree.get_optional("item.path"); if (stored_path) { uploaded_path = *stored_path; } else { - //ORCA: fallback if the server response omits result.item.path (older Moonraker, or + //ORCA: fallback if the server response omits item.path (older Moonraker, or // a buddy-fork that returns a slimmer envelope). Use the original filename. uploaded_path = upload_filename.string(); BOOST_LOG_TRIVIAL(warning) << boost::format( - "%1%: upload response missing result.item.path, falling back to original filename `%2%`") + "%1%: upload response missing item.path, falling back to original filename `%2%`") % name % uploaded_path; } + moonraker_started_print = ptree.get("print_started", false) || + ptree.get("print_queued", false); } catch (const std::exception &ex) { BOOST_LOG_TRIVIAL(warning) << boost::format( "%1%: could not parse upload response (%2%); falling back to original filename") @@ -310,7 +324,9 @@ bool Moonraker::upload(PrintHostUpload upload_data, ProgressFn progress_fn, Erro if (!result) return false; - if (upload_data.post_action == PrintHostPostUploadAction::StartPrint && !uploaded_path.empty()) { + // Fallback only when Moonraker neither started nor queued the print. If the + // printer cannot start it, the error from this request is what the user sees. + if (want_start && !uploaded_path.empty() && !moonraker_started_print) { wxString start_msg; if (!start_print(start_msg, uploaded_path)) { error_fn(std::move(start_msg)); diff --git a/src/slic3r/Utils/Moonraker.hpp b/src/slic3r/Utils/Moonraker.hpp index e91e65b49d..a7f1d8dd68 100644 --- a/src/slic3r/Utils/Moonraker.hpp +++ b/src/slic3r/Utils/Moonraker.hpp @@ -18,11 +18,13 @@ class Http; // Moonraker is the JSON / WebSocket gateway that ships in front of Klipper // (and on Klipper-API-compatible firmwares like the Prusa-Firmware-Buddy // Buddy-Klipper fork). REST shape differs from OctoPrint: distinct paths, -// JSON body for print/start, {"result":...}/{"error":...} envelope. +// JSON body for print/start, {"result":...}/{"error":...} envelope (except the +// upload reply, which is a bare object). // // Endpoints used: // GET /server/info -- connection test, reads klippy_state -// POST /server/files/upload (multipart) -- upload gcode (form fields: file, root) +// POST /server/files/upload (multipart) -- upload gcode (form fields: file, root, print, +// plateindex) // POST /printer/print/start (json) -- {"filename":".gcode"} starts print // // Auth: X-Api-Key header if `printhost_apikey` is non-empty; Moonraker accepts