diff --git a/examples/printer_polymorphism.rs b/examples/printer_polymorphism.rs index b3215c4..feb4a98 100644 --- a/examples/printer_polymorphism.rs +++ b/examples/printer_polymorphism.rs @@ -10,7 +10,7 @@ use continuum_proxy::printer::{ BambuPrinter, BambuProtocolPreference, BambuTls, GenericPrinter, KlipperPrinter, PrinterHandle, - PrusaLinkPrinter, PrusaSerialPrinter, + OctoPrintFlavour, OctoPrintPrinter, PrusaSerialPrinter, }; #[tokio::main] @@ -36,7 +36,13 @@ async fn main() { BambuTls::BundledCa, BambuProtocolPreference::Auto, )), - PrinterHandle::PrusaLink(PrusaLinkPrinter::new("p3", "MK4", "192.168.1.51", "prusa-api-key")), + PrinterHandle::OctoPrint(OctoPrintPrinter::new( + "p3", + "MK4", + "192.168.1.51", + "prusa-api-key", + OctoPrintFlavour::PrusaLink, + )), PrinterHandle::PrusaSerial(PrusaSerialPrinter::new("p4", "MK3S+", "/dev/ttyACM0", 115_200)), PrinterHandle::Klipper(KlipperPrinter::new("p5", "Voron 2.4", "192.168.1.52")), // No host configured - this one will hit the Err path. diff --git a/printers.example.toml b/printers.example.toml index c4c9fde..855f63d 100644 --- a/printers.example.toml +++ b/printers.example.toml @@ -17,6 +17,12 @@ # - so every block it prints still needs that one field before this file will # load. Note that only `bambu` has a working client today; the others are # found and reported, but their drivers in src/printer/ are still stubs. +# +# One thing discovery cannot answer for you: `octoprint` vs `prusa_link`. +# PrusaLink implements OctoPrint's API, so from the network the two are +# identical - discovery suggests `octoprint` (the protocol it actually saw) +# and leaves a comment offering the other. Change it if you know it is a +# Prusa serving its own API. [[printer]] id = "p1p" diff --git a/src/discovery/mod.rs b/src/discovery/mod.rs index 75f5379..39b93d5 100644 --- a/src/discovery/mod.rs +++ b/src/discovery/mod.rs @@ -98,7 +98,15 @@ pub const SSDP_PORTS: [u16; 2] = [2021, 1990]; pub enum Vendor { Bambu, Klipper, + /// Prusa's own implementation of the OctoPrint API, on an MK4/XL/MINI. PrusaLink, + /// OctoPrint itself, or anything else serving its API. + /// + /// This is the honest default for a printer found by probing, because + /// what a probe observes is "something here speaks the OctoPrint API" - + /// never which implementation. Reporting `prusa_link` would assert a + /// product nothing on the wire names. + Octoprint, /// Marlin-family firmware over a raw serial port. Named for Prusa /// because that is what `fleet.rs` calls it, but /// `printer/marlin_serial.rs` drives any Marlin board the same way - @@ -117,6 +125,7 @@ impl Vendor { Vendor::Bambu => "bambu", Vendor::Klipper => "klipper", Vendor::PrusaLink => "prusa_link", + Vendor::Octoprint => "octoprint", Vendor::PrusaSerial => "prusa_serial", } } @@ -131,7 +140,7 @@ impl Vendor { Vendor::Bambu => true, // `printer/klipper.rs` and `printer/prusa.rs` are stubs whose // methods return without doing anything. - Vendor::Klipper | Vendor::PrusaLink => false, + Vendor::Klipper | Vendor::PrusaLink | Vendor::Octoprint => false, // `marlin_serial.rs` is a real transport, but // `PrusaSerialPrinter`'s command methods on top of it are not. Vendor::PrusaSerial => false, @@ -177,6 +186,14 @@ pub struct Candidate { /// the machine by. Not rendered into the TOML - nothing in `fleet.rs` /// takes a version, and a stale one in a config file would be a lie. pub detail: Option, + /// Another vendor this could equally be, when the probe genuinely cannot + /// tell - which today means only OctoPrint versus PrusaLink. + /// + /// Carried rather than resolved by guessing, because the operator knows + /// what they own and the network does not. It renders as a comment above + /// `vendor`, so changing it is a one-word edit in the file they are + /// already pasting into. + pub ambiguous_with: Option, pub source: Source, } @@ -190,6 +207,7 @@ impl Candidate { name: None, bind_state: None, detail: None, + ambiguous_with: None, source, } } @@ -222,6 +240,13 @@ impl Candidate { out.push_str("[[printer]]\n"); out.push_str(&format!("id = {}\n", toml_string(id))); out.push_str(&format!("name = {}\n", toml_string(&name))); + if let Some(other) = self.ambiguous_with { + out.push_str(&format!( + "# vendor: change to {:?} if that is what this is - nothing on the network\n\ + # distinguishes them, so this is a guess only you can settle.\n", + other.toml_value() + )); + } out.push_str(&format!("vendor = {}\n", toml_string(self.vendor.toml_value()))); match self.vendor { @@ -238,9 +263,10 @@ impl Candidate { // The only field `fleet.rs` wants, and Moonraker needs no // credential from us - it authorizes by client address. Vendor::Klipper => out.push_str(&format!("host = {}\n", toml_string(&self.host))), - Vendor::PrusaLink => { + // One client, two flavours - same required fields. + Vendor::PrusaLink | Vendor::Octoprint => { out.push_str(&format!("host = {}\n", toml_string(&self.host))); - out.push_str("api_key = \"\" # REQUIRED: PrusaLink web UI > Settings > API key\n"); + out.push_str("api_key = \"\" # REQUIRED: the printer's or OctoPrint's web UI > API key\n"); } Vendor::PrusaSerial => { out.push_str(&format!("port = {}\n", toml_string(&self.host))); @@ -621,6 +647,7 @@ mod tests { (Vendor::Bambu, "192.168.0.21", vec!["host", "sn", "access_code"]), (Vendor::Klipper, "192.168.0.50", vec!["host"]), (Vendor::PrusaLink, "192.168.0.51", vec!["host", "api_key"]), + (Vendor::Octoprint, "192.168.0.60", vec!["host", "api_key"]), (Vendor::PrusaSerial, "/dev/ttyACM0", vec!["port", "baud_rate"]), ]; for (vendor, host, expected) in cases { @@ -645,7 +672,7 @@ mod tests { #[test] fn a_vendor_with_a_stub_client_says_so_in_its_entry() { - for vendor in [Vendor::Klipper, Vendor::PrusaLink, Vendor::PrusaSerial] { + for vendor in [Vendor::Klipper, Vendor::PrusaLink, Vendor::Octoprint, Vendor::PrusaSerial] { let entry = Candidate::new("192.168.0.50", vendor, Source::HttpApi).to_toml_entry("x"); assert!(entry.contains("WARNING"), "{vendor:?} is a stub and must say so:\n{entry}"); assert!(entry.contains("stub"), "{vendor:?}:\n{entry}"); diff --git a/src/discovery/net.rs b/src/discovery/net.rs index 05ff153..09d5390 100644 --- a/src/discovery/net.rs +++ b/src/discovery/net.rs @@ -188,20 +188,26 @@ pub fn parse_moonraker_info(host: &str, body: &[u8]) -> anyhow::Result anyhow::Result { let url = format!("http://{host}:{PRUSALINK_PORT}/api/version"); let (status, body, challenge) = http_get(&url, timeout).await?; - classify_prusalink(host, status, &body, challenge) + classify_octoprint_api(host, status, &body, challenge) } /// The decision `probe_prusalink` makes once the response is in hand, split /// out so the 401 path can be tested without a printer. -pub fn classify_prusalink( +pub fn classify_octoprint_api( host: &str, status: u16, body: &[u8], @@ -209,7 +215,7 @@ pub fn classify_prusalink( ) -> anyhow::Result { match status { // Some builds answer unauthenticated. Take the detail if so. - 200 => parse_prusalink_version(host, body), + 200 => parse_octoprint_version(host, body), 401 => { // A bare 401 is *not* enough. Plenty of things on a LAN password // protect port 80 - routers, NASes, cameras - and accepting any @@ -225,10 +231,15 @@ pub fn classify_prusalink( "{host} wants {challenge:?} on /api/version, not Digest - some other password-protected web server, not PrusaLink" ); } - let mut candidate = Candidate::new(host, Vendor::PrusaLink, Source::HttpApi); + // Reported as the generic flavour: a digest challenge proves the + // OctoPrint API is here and proves nothing about which + // implementation serves it. + let mut candidate = Candidate::new(host, Vendor::Octoprint, Source::HttpApi); + candidate.ambiguous_with = Some(Vendor::PrusaLink); // The challenge is worth quoting back: `Digest realm="Printer API"` // is what an operator can check against what they expect there. - candidate.detail = Some(format!("PrusaLink or OctoPrint - needs an API key ({challenge})")); + candidate.detail = + Some(format!("an OctoPrint-compatible API needing a key - OctoPrint or PrusaLink ({challenge})")); Ok(candidate) } other => anyhow::bail!("{host} answered /api/version with HTTP {other}, which is not PrusaLink"), @@ -236,7 +247,10 @@ pub fn classify_prusalink( } /// Reads an `/api/version` body, for the builds that answer without auth. -pub fn parse_prusalink_version(host: &str, body: &[u8]) -> anyhow::Result { +/// +/// This is the one path that can name the implementation rather than the +/// protocol, because `text` says which it is. +pub fn parse_octoprint_version(host: &str, body: &[u8]) -> anyhow::Result { let parsed: serde_json::Value = serde_json::from_slice(body) .map_err(|err| anyhow::anyhow!("{host} answered /api/version with something that is not JSON: {err}"))?; // `api` is the field OctoPrint's contract guarantees; without it this is @@ -244,12 +258,25 @@ pub fn parse_prusalink_version(host: &str, body: &[u8]) -> anyhow::Result format!("{text}, firmware {firmware}"), - None => text.to_string(), + Some(firmware) => format!("{described}, firmware {firmware}"), + None => described.to_string(), }); candidate.name = parsed.get("hostname").and_then(|v| v.as_str()).map(str::to_string); Ok(candidate) @@ -409,20 +436,54 @@ mod tests { assert!(parse_moonraker_info("192.168.0.50", b"hello").is_err()); } + /// `text` names the implementation, so this is the one case where the + /// product is known rather than inferred - and it must be taken, not + /// flattened into the generic answer. #[test] - fn a_prusalink_reply_names_the_firmware() { - let found = parse_prusalink_version("192.168.0.51", PRUSALINK_VERSION).expect("PrusaLink"); + fn a_body_that_names_itself_prusalink_is_reported_as_prusalink() { + let found = parse_octoprint_version("192.168.0.51", PRUSALINK_VERSION).expect("PrusaLink"); assert_eq!(found.vendor, Vendor::PrusaLink); + assert_eq!(found.ambiguous_with, None, "`text` settled it - nothing to offer the operator"); assert_eq!(found.detail.as_deref(), Some("PrusaLink 0.7.0, firmware 3.10.1-4697")); // The API key is the field no probe can supply, so it must be // rendered empty and labelled rather than omitted. let parsed: toml::Value = toml::from_str(&found.to_toml_entry("mk4")).expect("valid TOML"); assert_eq!(parsed["printer"][0]["api_key"].as_str(), Some("")); + assert_eq!(parsed["printer"][0]["vendor"].as_str(), Some("prusa_link")); + } + + #[test] + fn a_body_that_names_itself_octoprint_is_reported_as_octoprint() { + let body = br#"{"api":"0.1","server":"1.9.3","text":"OctoPrint 1.9.3"}"#; + let found = parse_octoprint_version("192.168.0.60", body).expect("OctoPrint"); + assert_eq!(found.vendor, Vendor::Octoprint); + assert_eq!(found.ambiguous_with, None); + } + + /// The common case: the API answers but names no product, so the vendor + /// reported is the protocol that was observed and the alternative is + /// offered rather than guessed away. + #[test] + fn an_unnamed_octoprint_api_reports_the_protocol_and_offers_the_choice() { + let body = br#"{"api":"2.0.0","version":"2.1.2"}"#; + let found = parse_octoprint_version("192.168.0.51", body).expect("the API"); + assert_eq!(found.vendor, Vendor::Octoprint); + assert_eq!(found.ambiguous_with, Some(Vendor::PrusaLink)); + + let entry = found.to_toml_entry("mk4"); + assert_eq!( + toml::from_str::(&entry).expect("valid TOML")["printer"][0]["vendor"].as_str(), + Some("octoprint") + ); + // The other choice has to be a one-word edit in the file the + // operator is already pasting into. + assert!(entry.contains("prusa_link"), "the alternative must be offered:\n{entry}"); + assert!(entry.contains("only you can settle"), "{entry}"); } #[test] fn a_web_server_that_is_not_octoprint_shaped_is_refused() { - let err = parse_prusalink_version("192.168.0.51", br#"{"version":"1.0"}"#).expect_err("must refuse"); + let err = parse_octoprint_version("192.168.0.51", br#"{"version":"1.0"}"#).expect_err("must refuse"); assert!(err.to_string().contains("not PrusaLink"), "{err}"); } @@ -448,7 +509,7 @@ var g_Lan = 394;"#, b"", ]; for body in real_bodies { - let err = classify_prusalink("192.168.0.1", 200, body, None).expect_err("not a printer"); + let err = classify_octoprint_api("192.168.0.1", 200, body, None).expect_err("not a printer"); assert!(err.to_string().contains("not JSON") || err.to_string().contains("not PrusaLink"), "{err}"); } } @@ -457,17 +518,58 @@ var g_Lan = 394;"#, fn a_password_protected_router_is_not_a_printer() { // The false positive that a bare "401 means PrusaLink" would create. // Routers, NASes and cameras all password protect port 80. - let basic = classify_prusalink("192.168.0.1", 401, b"", Some(r#"Basic realm="RouterOS""#.into())); + let basic = classify_octoprint_api("192.168.0.1", 401, b"", Some(r#"Basic realm="RouterOS""#.into())); let err = basic.expect_err("Basic auth is not PrusaLink"); assert!(err.to_string().contains("not Digest"), "{err}"); - let silent = classify_prusalink("192.168.0.1", 401, b"", None); + let silent = classify_octoprint_api("192.168.0.1", 401, b"", None); assert!(silent.is_err(), "a 401 with no challenge names nothing"); // What a real PrusaLink answers, which must still be found. - let found = classify_prusalink("192.168.0.51", 401, b"", Some(r#"Digest realm="Printer API""#.into())) - .expect("a digest challenge on /api/version is PrusaLink"); - assert_eq!(found.vendor, Vendor::PrusaLink); + let found = classify_octoprint_api("192.168.0.51", 401, b"", Some(r#"Digest realm="Printer API""#.into())) + .expect("a digest challenge on /api/version is the OctoPrint API"); + // Generic, because a challenge proves the API is here and proves + // nothing about which implementation serves it. + assert_eq!(found.vendor, Vendor::Octoprint); + assert_eq!(found.ambiguous_with, Some(Vendor::PrusaLink)); + assert!(found.detail.unwrap().contains("Printer API")); + } + + /// The one link between a real socket and `classify_octoprint_api` that + /// unit tests on the classifier cannot reach: whether the challenge + /// header actually survives the trip. PrusaLink is identified *by* that + /// header, so if `http_get` dropped it every PrusaLink would be reported + /// as "no challenge - not PrusaLink". Port 80 cannot be bound without + /// root here, so this stands in for the real probe's plumbing. + #[tokio::test] + async fn a_digest_challenge_survives_the_round_trip_and_identifies_the_api() { + let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("a port"); + let port = listener.local_addr().expect("an address").port(); + std::thread::spawn(move || { + use std::io::{Read, Write}; + let (mut socket, _) = listener.accept().expect("one connection"); + let mut scratch = [0u8; 1024]; + let _ = socket.read(&mut scratch); + // Exactly what PrusaLink answers an unauthenticated GET. + let _ = socket.write_all( + b"HTTP/1.1 401 Unauthorized\r\n\ + WWW-Authenticate: Digest realm=\"Printer API\", nonce=\"abc\", qop=\"auth\"\r\n\ + Content-Length: 0\r\n\r\n", + ); + }); + + let (status, body, challenge) = + http_get(&format!("http://127.0.0.1:{port}/api/version"), Duration::from_secs(5)) + .await + .expect("the stand-in answers"); + assert_eq!(status, 401); + assert!(challenge.as_deref().unwrap_or_default().contains("Digest"), "{challenge:?}"); + + // And the classifier turns that into the generic flavour with the + // alternative offered - the whole path, end to end. + let found = classify_octoprint_api("127.0.0.1", status, &body, challenge).expect("the OctoPrint API"); + assert_eq!(found.vendor, Vendor::Octoprint); + assert_eq!(found.ambiguous_with, Some(Vendor::PrusaLink)); assert!(found.detail.unwrap().contains("Printer API")); } diff --git a/src/fleet.rs b/src/fleet.rs index fd6224e..36852da 100644 --- a/src/fleet.rs +++ b/src/fleet.rs @@ -8,8 +8,8 @@ use std::path::Path; use serde::Deserialize; use crate::printer::{ - BambuPrinter, BambuProtocolPreference, BambuTls, KlipperPrinter, PrinterHandle, PrusaLinkPrinter, - PrusaSerialPrinter, + BambuPrinter, BambuProtocolPreference, BambuTls, KlipperPrinter, OctoPrintFlavour, OctoPrintPrinter, + PrinterHandle, PrusaSerialPrinter, }; #[derive(Debug, Deserialize)] @@ -25,7 +25,12 @@ struct PrinterEntry { /// One of: "bambu" (auto-detects MQTT v5-vs-v3.1.1/BambuMQTT v2-vs-v1 - /// the default, since a firmware update can move the same physical /// printer between them), "bambu_v1"/"bambu_v2" (force a specific one), - /// "prusa_link", "prusa_serial", "klipper". + /// "prusa_link", "octoprint", "prusa_serial", "klipper". + /// + /// `prusa_link` and `octoprint` are the same REST client with a different + /// flavour - see `OctoPrintFlavour`. Pick `octoprint` if it is OctoPrint + /// on a Pi and `prusa_link` if it is an MK4/XL/MINI serving its own API; + /// discovery cannot tell, so it suggests `octoprint` and says so. vendor: String, host: Option, access_code: Option, @@ -71,11 +76,15 @@ fn build_printer(entry: PrinterEntry) -> anyhow::Result { "bambu" => build_bambu(entry, BambuProtocolPreference::Auto), "bambu_v1" => build_bambu(entry, BambuProtocolPreference::ForceV1), "bambu_v2" => build_bambu(entry, BambuProtocolPreference::ForceV2), - "prusa_link" => { - let host = require_field(&entry.id, "host", entry.host)?; - let api_key = require_field(&entry.id, "api_key", entry.api_key)?; - Ok(PrinterHandle::PrusaLink(PrusaLinkPrinter::new(&entry.id, &entry.name, &host, &api_key))) - } + // Two names for one client, because PrusaLink implements OctoPrint's + // API. The distinction is kept rather than collapsed: the two differ + // in which endpoints they offer beyond `/api/version`, and - the + // reason it has to come from config - nothing on the network can tell + // them apart, since every PrusaLink identity endpoint needs a + // credential. So this is the operator's answer, and `octoprint` is + // the honest default for a printer found by discovery. + "prusa_link" => build_octoprint(entry, OctoPrintFlavour::PrusaLink), + "octoprint" => build_octoprint(entry, OctoPrintFlavour::Generic), "prusa_serial" => { let port = require_field(&entry.id, "port", entry.port)?; Ok(PrinterHandle::PrusaSerial(PrusaSerialPrinter::new(&entry.id, &entry.name, &port, entry.baud_rate))) @@ -88,6 +97,12 @@ fn build_printer(entry: PrinterEntry) -> anyhow::Result { } } +fn build_octoprint(entry: PrinterEntry, flavour: OctoPrintFlavour) -> anyhow::Result { + let host = require_field(&entry.id, "host", entry.host)?; + let api_key = require_field(&entry.id, "api_key", entry.api_key)?; + Ok(PrinterHandle::OctoPrint(OctoPrintPrinter::new(&entry.id, &entry.name, &host, &api_key, flavour))) +} + fn build_bambu(entry: PrinterEntry, preference: BambuProtocolPreference) -> anyhow::Result { let host = require_field(&entry.id, "host", entry.host)?; let access_code = require_field(&entry.id, "access_code", entry.access_code)?; @@ -96,6 +111,31 @@ fn build_bambu(entry: PrinterEntry, preference: BambuProtocolPreference) -> anyh Ok(PrinterHandle::Bambu(BambuPrinter::new(&entry.id, &entry.name, &host, &access_code, &sn, tls, preference))) } +/// A required field, where **empty counts as missing**. +/// +/// The emptiness check is the load-bearing half. `Option` only catches an +/// absent key, so `access_code = ""` arrived here as `Some("")` and loaded +/// happily - and an empty credential is not a usable one, it just moves the +/// failure somewhere worse. A Bambu printer configured that way reaches MQTT +/// and is refused, a PrusaLink one gets a 401 on every poll, and nothing in +/// either chain says "that field is blank in printers.toml". +/// +/// It is also what makes a half-finished entry safe. `discovery` prints +/// blocks with `access_code = ""`/`api_key = ""` for an operator to +/// complete, and pasting one in and forgetting to finish it has to fail at +/// startup naming the field - which is only true because of this check. +/// `tests/discovery_to_fleet.rs` asserts exactly that, and is how the gap +/// was found. +/// +/// This is the same trap as `??`-instead-of-`||` on an environment variable +/// that ships empty, which `continuum-backend`'s `auth/edge_secret.ts` and +/// `s3/index.ts` both document. It bites identically on a TOML key. fn require_field(printer_id: &str, field: &str, value: Option) -> anyhow::Result { - value.ok_or_else(|| anyhow::anyhow!("{printer_id}: missing required field '{field}'")) + match value { + Some(value) if !value.trim().is_empty() => Ok(value), + Some(_) => Err(anyhow::anyhow!( + "{printer_id}: required field '{field}' is empty - fill it in (discovery leaves it blank on purpose; an empty value is not a usable one)" + )), + None => Err(anyhow::anyhow!("{printer_id}: missing required field '{field}'")), + } } diff --git a/src/printer/mod.rs b/src/printer/mod.rs index 8efd543..1790922 100644 --- a/src/printer/mod.rs +++ b/src/printer/mod.rs @@ -53,7 +53,7 @@ pub use bambu_commands::{ XcamSettings, }; pub use klipper::KlipperPrinter; -pub use prusa::{PrusaLinkPrinter, PrusaSerialPrinter}; +pub use prusa::{OctoPrintFlavour, OctoPrintPrinter, PrusaSerialPrinter}; /// A tiny error type: just a message. `String` would work too - this exists /// mainly so `?` has something concrete to convert into everywhere. @@ -188,7 +188,7 @@ impl PrintControl { /// The closed set of printer kinds this fleet can talk to. pub enum PrinterHandle { Bambu(BambuPrinter), - PrusaLink(PrusaLinkPrinter), + OctoPrint(OctoPrintPrinter), PrusaSerial(PrusaSerialPrinter), Klipper(KlipperPrinter), } @@ -205,7 +205,7 @@ impl PrinterHandle { pub async fn print_status(&self) -> Option { match self { PrinterHandle::Bambu(p) => p.print_status().await, - PrinterHandle::PrusaLink(_) | PrinterHandle::PrusaSerial(_) | PrinterHandle::Klipper(_) => None, + PrinterHandle::OctoPrint(_) | PrinterHandle::PrusaSerial(_) | PrinterHandle::Klipper(_) => None, } } @@ -234,7 +234,7 @@ impl PrinterHandle { pub async fn refresh_status(&self) -> Result<(), PrinterError> { match self { PrinterHandle::Bambu(p) => p.pushall().await, - PrinterHandle::PrusaLink(_) | PrinterHandle::PrusaSerial(_) | PrinterHandle::Klipper(_) => Ok(()), + PrinterHandle::OctoPrint(_) | PrinterHandle::PrusaSerial(_) | PrinterHandle::Klipper(_) => Ok(()), } } } @@ -247,7 +247,7 @@ impl GenericPrinter for PrinterHandle { async fn control_print(&mut self, action: PrintControl) -> Result<(), PrinterError> { match self { PrinterHandle::Bambu(p) => p.control_print(action).await, - PrinterHandle::PrusaLink(p) => p.control_print(action).await, + PrinterHandle::OctoPrint(p) => p.control_print(action).await, PrinterHandle::PrusaSerial(p) => p.control_print(action).await, PrinterHandle::Klipper(p) => p.control_print(action).await, } @@ -256,7 +256,7 @@ impl GenericPrinter for PrinterHandle { fn base(&self) -> &PrinterBase { match self { PrinterHandle::Bambu(p) => p.base(), - PrinterHandle::PrusaLink(p) => p.base(), + PrinterHandle::OctoPrint(p) => p.base(), PrinterHandle::PrusaSerial(p) => p.base(), PrinterHandle::Klipper(p) => p.base(), } @@ -265,7 +265,7 @@ impl GenericPrinter for PrinterHandle { async fn connect(&mut self) -> Result<(), PrinterError> { match self { PrinterHandle::Bambu(p) => p.connect().await, - PrinterHandle::PrusaLink(p) => p.connect().await, + PrinterHandle::OctoPrint(p) => p.connect().await, PrinterHandle::PrusaSerial(p) => p.connect().await, PrinterHandle::Klipper(p) => p.connect().await, } @@ -274,7 +274,7 @@ impl GenericPrinter for PrinterHandle { async fn publish(&mut self, payload: String) -> Result<(), PrinterError> { match self { PrinterHandle::Bambu(p) => p.publish(payload).await, - PrinterHandle::PrusaLink(p) => p.publish(payload).await, + PrinterHandle::OctoPrint(p) => p.publish(payload).await, PrinterHandle::PrusaSerial(p) => p.publish(payload).await, PrinterHandle::Klipper(p) => p.publish(payload).await, } @@ -283,7 +283,7 @@ impl GenericPrinter for PrinterHandle { async fn set_fan_speed(&mut self, percent: u8) -> Result<(), PrinterError> { match self { PrinterHandle::Bambu(p) => p.set_fan_speed(percent).await, - PrinterHandle::PrusaLink(p) => p.set_fan_speed(percent).await, + PrinterHandle::OctoPrint(p) => p.set_fan_speed(percent).await, PrinterHandle::PrusaSerial(p) => p.set_fan_speed(percent).await, PrinterHandle::Klipper(p) => p.set_fan_speed(percent).await, } @@ -292,7 +292,7 @@ impl GenericPrinter for PrinterHandle { async fn upload_project_file(&mut self, file_name: String, data: Vec) -> Result<(), PrinterError> { match self { PrinterHandle::Bambu(p) => p.upload_project_file(file_name, data).await, - PrinterHandle::PrusaLink(p) => p.upload_project_file(file_name, data).await, + PrinterHandle::OctoPrint(p) => p.upload_project_file(file_name, data).await, PrinterHandle::PrusaSerial(p) => p.upload_project_file(file_name, data).await, PrinterHandle::Klipper(p) => p.upload_project_file(file_name, data).await, } @@ -301,7 +301,7 @@ impl GenericPrinter for PrinterHandle { async fn send_gcode_line(&mut self, gcode_line: String) -> Result<(), PrinterError> { match self { PrinterHandle::Bambu(p) => p.send_gcode_line(gcode_line).await, - PrinterHandle::PrusaLink(p) => p.send_gcode_line(gcode_line).await, + PrinterHandle::OctoPrint(p) => p.send_gcode_line(gcode_line).await, PrinterHandle::PrusaSerial(p) => p.send_gcode_line(gcode_line).await, PrinterHandle::Klipper(p) => p.send_gcode_line(gcode_line).await, } diff --git a/src/printer/prusa.rs b/src/printer/prusa.rs index 283b009..7c73090 100644 --- a/src/printer/prusa.rs +++ b/src/printer/prusa.rs @@ -1,28 +1,62 @@ use super::{GenericPrinter, PrinterBase, PrinterError}; -/****************** PrusaLink ******************/ +/****************** OctoPrint API family ******************/ -pub struct PrusaLinkPrinter { - base: PrinterBase, - pub api_key: String, +/// Which implementation of the OctoPrint REST API is on the other end. +/// +/// PrusaLink deliberately implements OctoPrint's API, so one client serves +/// both - but not identically. PrusaLink adds its own `/api/v1/*` (`info`, +/// `status`, `job`) which is where the useful detail lives, while OctoPrint +/// proper has the older `/api/printer` and `/api/job` and a plugin surface +/// PrusaLink does not have. +/// +/// This is recorded from config now, while the client is still a stub, +/// because **no probe can settle it**: both answer `/api/version`, and every +/// PrusaLink identity endpoint needs a credential, so from outside the two +/// are indistinguishable (see `discovery::net::classify_octoprint_api`). It +/// is the operator's answer to a question only they can answer, so it needs +/// somewhere to live the moment they are asked - otherwise the real client +/// would have to ask them a second time. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum OctoPrintFlavour { + /// Prusa's own firmware-hosted implementation, on an MK4/XL/MINI. + PrusaLink, + /// OctoPrint itself, usually on a Raspberry Pi in front of any printer. + Generic, } -impl PrusaLinkPrinter { - pub fn new(id: &str, name: &str, host: &str, api_key: &str) -> Self { - Self { - base: PrinterBase { id: id.into(), name: name.into(), host: host.into() }, - api_key: api_key.into(), +impl OctoPrintFlavour { + pub fn as_str(self) -> &'static str { + match self { + OctoPrintFlavour::PrusaLink => "PrusaLink", + OctoPrintFlavour::Generic => "OctoPrint", } } } -impl GenericPrinter for PrusaLinkPrinter { +pub struct OctoPrintPrinter { + base: PrinterBase, + pub api_key: String, + pub flavour: OctoPrintFlavour, +} + +impl OctoPrintPrinter { + pub fn new(id: &str, name: &str, host: &str, api_key: &str, flavour: OctoPrintFlavour) -> Self { + Self { + base: PrinterBase { id: id.into(), name: name.into(), host: host.into() }, + api_key: api_key.into(), + flavour, + } + } +} + +impl GenericPrinter for OctoPrintPrinter { /// OVERRIDE: real version is `PUT /api/v1/job/{id}/pause`, `/resume`, or /// `DELETE /api/v1/job/{id}` to cancel - PrusaLink addresses the running /// job by id rather than the printer, so a real implementation has to /// fetch the current job first. async fn control_print(&mut self, action: super::PrintControl) -> Result<(), PrinterError> { - println!("[{}] PrusaLink job control: {}", self.base.name, action.as_str()); + println!("[{}] {} job control: {}", self.base.name, self.flavour.as_str(), action.as_str()); Ok(()) } @@ -35,7 +69,7 @@ impl GenericPrinter for PrusaLinkPrinter { /// plain REST API (real version: `GET /api/v1/status` with the API /// key, see `adapters::prusalink`), no persistent session to hold. async fn connect(&mut self) -> Result<(), PrinterError> { - println!("[{}] connecting over HTTP (PrusaLink)", self.base.name); + println!("[{}] connecting over HTTP ({})", self.base.name, self.flavour.as_str()); Ok(()) } diff --git a/tests/discovery_to_fleet.rs b/tests/discovery_to_fleet.rs new file mode 100644 index 0000000..0ca21ad --- /dev/null +++ b/tests/discovery_to_fleet.rs @@ -0,0 +1,136 @@ +//! Proves the blocks `discovery` prints are blocks `fleet` can load. +//! +//! The two are coupled by nothing but a string - `Vendor::toml_value` has to +//! match what `fleet::build_printer` matches on, and the fields rendered have +//! to be the fields it requires. Unit tests on either side cannot catch a +//! drift between them, and the symptom of a drift is the worst kind: an +//! operator pastes in exactly what they were told to and the gateway refuses +//! to start. +//! +//! So this renders a candidate for every vendor, fills in the credential the +//! way a human would, and loads the result for real. + +use std::io::Write; + +use continuum_proxy::discovery::{Candidate, Source, Vendor}; +use continuum_proxy::fleet; +use continuum_proxy::printer::{GenericPrinter, PrinterHandle}; + +/// Fills in the credential a human is told to supply, so the entry loads. +fn with_credentials(entry: &str) -> String { + entry + .replace("access_code = \"\"", "access_code = \"12345678\"") + .replace("api_key = \"\"", "api_key = \"an-api-key\"") +} + +fn load(toml: &str) -> anyhow::Result> { + let mut file = tempfile(); + file.write_all(toml.as_bytes()).expect("write the fleet file"); + fleet::load(&file.path) +} + +/// A self-deleting temporary file, rather than a dependency for four lines. +struct TempToml { + path: std::path::PathBuf, + handle: Option, +} + +impl TempToml { + fn write_all(&mut self, bytes: &[u8]) -> std::io::Result<()> { + self.handle.as_mut().expect("still open").write_all(bytes)?; + self.handle.as_mut().expect("still open").flush() + } +} + +impl Drop for TempToml { + fn drop(&mut self) { + self.handle.take(); + let _ = std::fs::remove_file(&self.path); + } +} + +fn tempfile() -> TempToml { + // The pid and a counter, so two tests in one binary cannot collide. + use std::sync::atomic::{AtomicUsize, Ordering}; + static NEXT: AtomicUsize = AtomicUsize::new(0); + let path = + std::env::temp_dir().join(format!("continuum-fleet-{}-{}.toml", std::process::id(), NEXT.fetch_add(1, Ordering::Relaxed))); + let handle = std::fs::File::create(&path).expect("create a temp fleet file"); + TempToml { path, handle: Some(handle) } +} + +fn candidate(vendor: Vendor, host: &str) -> Candidate { + let mut candidate = Candidate::new(host, vendor, Source::HttpApi); + // Bambu needs one, and for every other vendor it is ignored. + candidate.serial = Some("01S09B441900143".to_string()); + candidate +} + +#[test] +fn every_rendered_entry_loads_as_the_printer_it_claims_to_be() { + let cases = [ + (Vendor::Bambu, "192.168.0.20"), + (Vendor::Klipper, "192.168.0.50"), + (Vendor::PrusaLink, "192.168.0.51"), + (Vendor::Octoprint, "192.168.0.60"), + (Vendor::PrusaSerial, "/dev/ttyACM0"), + ]; + for (vendor, host) in cases { + let entry = candidate(vendor, host).to_toml_entry("found"); + let fleet = load(&with_credentials(&entry)) + .unwrap_or_else(|err| panic!("{vendor:?} rendered an entry fleet::load rejects: {err}\n{entry}")); + assert_eq!(fleet.len(), 1, "{vendor:?}"); + assert_eq!(fleet[0].base().id, "found", "{vendor:?}"); + + // The vendor string has to resolve to the matching handle, not just + // to *a* handle - `octoprint` and `prusa_link` share a client, so a + // mapping that collapsed them would still load and still be wrong. + let matched = matches!( + (vendor, &fleet[0]), + (Vendor::Bambu, PrinterHandle::Bambu(_)) + | (Vendor::Klipper, PrinterHandle::Klipper(_)) + | (Vendor::PrusaLink | Vendor::Octoprint, PrinterHandle::OctoPrint(_)) + | (Vendor::PrusaSerial, PrinterHandle::PrusaSerial(_)) + ); + assert!(matched, "{vendor:?} loaded as the wrong handle"); + } +} + +/// The flavour is the operator's answer to a question no probe can settle, so +/// it has to survive the trip from the file into the printer. +#[test] +fn the_octoprint_flavour_an_operator_chose_reaches_the_client() { + use continuum_proxy::printer::OctoPrintFlavour; + + for (vendor, expected) in + [(Vendor::Octoprint, OctoPrintFlavour::Generic), (Vendor::PrusaLink, OctoPrintFlavour::PrusaLink)] + { + let entry = with_credentials(&candidate(vendor, "192.168.0.51").to_toml_entry("found")); + let fleet = load(&entry).expect("loads"); + let PrinterHandle::OctoPrint(printer) = &fleet[0] else { + panic!("{vendor:?} should load as an OctoPrint-family client"); + }; + assert_eq!(printer.flavour, expected, "{vendor:?}"); + } +} + +/// An entry straight out of discovery, credential still blank, must be +/// refused - and refused in a way that names the printer and the field. +#[test] +fn an_entry_with_the_credential_left_blank_is_refused_by_name() { + for vendor in [Vendor::Bambu, Vendor::PrusaLink, Vendor::Octoprint] { + let entry = candidate(vendor, "192.168.0.20").to_toml_entry("unfinished"); + let Err(err) = load(&entry) else { + panic!("{vendor:?}: a blank credential must not load:\n{entry}"); + }; + let message = err.to_string(); + // This is the whole reason discovery prints instead of writing the + // file: the failure is loud, and it says which printer and which + // field. + assert!(message.contains("unfinished"), "{vendor:?}: {message}"); + assert!( + message.contains("access_code") || message.contains("api_key"), + "{vendor:?} must name the missing field: {message}" + ); + } +}