diff --git a/src/printer/bambu.rs b/src/printer/bambu.rs index 6748207..a9c4519 100644 --- a/src/printer/bambu.rs +++ b/src/printer/bambu.rs @@ -244,10 +244,16 @@ const SLOTS_PER_AMS: i32 = 4; /// External spools, which `ams_mapping` has to be padded for even though it /// cannot address them. /// -/// 1 is right for every printer this has been used with. A dual-nozzle H2C -/// has two, and this should come from the report rather than a constant once -/// `vt_tray` is modelled on `PrintReport` - the ams_mapping schema spells -/// out the arithmetic. +/// Still a constant now that `vt_tray` is modelled, because modelling it did +/// not make the count derivable. A v1 P1P reports one `vt_tray` object, which +/// confirms 1 for that printer - but a v2 H2C omits the key entirely (see +/// `PrintReport::vt_tray`), so on v2 an absent `vt_tray` means "not reported" +/// and not "no external spool". Counting what is reported would therefore +/// give 0 on exactly the printer the ams_mapping schema says has *two*. +/// +/// 1 is right for every printer this has run against. A dual-nozzle H2C has +/// two, and getting that right needs a v2 capture that actually shows how a +/// v2 printer describes its spools. const EXTERNAL_SPOOLS: usize = 1; /// Flattens the AMS report into trays with global slot numbers, plus the @@ -291,6 +297,47 @@ fn available_trays(ams: Option<&super::AmsState>) -> (Vec, (trays, total) } +/// Whether the printer's external spool holds something every one of this +/// project's filaments could be printed from. +/// +/// Reuses `map_to_trays` with the spool presented as the single candidate +/// tray, rather than reimplementing the match. With exactly one candidate, +/// an `Ok` means every filament matched that one spool - which is the +/// question being asked - and the slot numbers it returns are thrown away, +/// since the external spool is not addressable by slot. Hence the `0` +/// placeholder. +/// +/// The point of this is to turn "a project wants the external spool" from an +/// error into a correct answer. Before `vt_tray` was modelled there was no +/// way to see what the spool held, and claiming a match without looking is +/// how the `use_ams: false` bug happened in the first place. +fn external_spool_satisfies( + filaments: &[super::ProjectFilament], + spool: &super::VtTray, +) -> bool { + let (Some(tray_info_idx), Some(filament_type), Some(color)) = ( + spool.tray_info_idx.as_ref(), + spool.tray_type.as_ref(), + spool.tray_color.as_ref(), + ) else { + // An empty spool reports these as absent or blank. Nothing to match. + return false; + }; + + if tray_info_idx.is_empty() || filament_type.is_empty() { + return false; + } + + let candidate = [super::AvailableTray { + slot: 0, + tray_info_idx: tray_info_idx.clone(), + filament_type: filament_type.clone(), + color: super::normalise_color(color), + }]; + + super::map_to_trays(filaments, &candidate).is_ok() +} + /// What a printer's light should be doing. /// /// Only the two steady states. Bambu's `system.ledctrl` also has a @@ -770,14 +817,18 @@ impl BambuPrinter { /// project "work". An unmatched filament is an error, because every /// alternative is a guess about what someone meant to print. /// - /// The one case it does answer `ExternalSpool` for is a printer - /// reporting no AMS trays at all, where there is nothing else it could - /// mean. Note the asymmetry that leaves: a printer *with* an AMS whose - /// project wants the external spool is an error here, not an - /// `ExternalSpool`, because `vt_tray` is not modelled on `PrintReport` - - /// so there is no way to check that the spool holds the right material, - /// and claiming a match without checking is how this went wrong the - /// first time. Modelling `vt_tray` is what fixes that. + /// It answers `ExternalSpool` when the spool is what the project asks + /// for - checked against `vt_tray`, not assumed. A printer with an AMS + /// whose project wants the spool used to be an error here purely because + /// `vt_tray` was not modelled; now that it is, the question can be + /// answered instead of refused. + /// + /// The one place it still answers `ExternalSpool` without being able to + /// verify anything is a v2 printer that reports no AMS trays and no + /// `vt_tray` either - v2 omits the key entirely (see + /// `PrintReport::vt_tray`), so there is nothing to check against and + /// nothing else it could mean. That one is logged as unverified rather + /// than passed off as a match. pub async fn filament_source_for( &self, archive: &[u8], @@ -794,26 +845,68 @@ impl BambuPrinter { })?; let (trays, tray_count) = available_trays(report.ams.as_ref()); + let spool = report.vt_tray.as_ref(); - if trays.is_empty() { - tracing::info!( - printer = %self.base.name, - "no AMS trays reported; printing from the external spool" - ); - return Ok(FilamentSource::ExternalSpool); + // The AMS first, when there is one: a project sliced for the AMS + // naming a filament that also happens to be on the spool should + // still use the AMS, because that is what it was sliced for. + if !trays.is_empty() { + match super::map_to_trays(&filaments, &trays) { + Ok(mapping) => { + tracing::info!( + printer = %self.base.name, + ?mapping, + tray_count, + "mapped the project's filaments onto AMS slots" + ); + return FilamentSource::ams(mapping, tray_count); + } + Err(ams_err) => { + // Nothing in the AMS fits. The spool is the only other + // thing it could have meant, so check it rather than + // failing outright - but report the AMS error if the + // spool does not fit either, since that one names what + // was wanted and what was loaded. + if spool.is_some_and(|spool| external_spool_satisfies(&filaments, spool)) { + tracing::info!( + printer = %self.base.name, + "no AMS tray fits this project, but the external spool does" + ); + return Ok(FilamentSource::ExternalSpool); + } + return Err(PrinterError(format!("{}: {ams_err}", self.base.name))); + } + } } - let mapping = super::map_to_trays(&filaments, &trays) - .map_err(|err| PrinterError(format!("{}: {err}", self.base.name)))?; - - tracing::info!( - printer = %self.base.name, - ?mapping, - tray_count, - "mapped the project's filaments onto AMS slots" - ); - - FilamentSource::ams(mapping, tray_count) + match spool { + Some(spool) if external_spool_satisfies(&filaments, spool) => { + tracing::info!( + printer = %self.base.name, + "no AMS; the external spool matches this project" + ); + Ok(FilamentSource::ExternalSpool) + } + // A spool is reported and does not match. Now that `vt_tray` + // exists this is a real negative rather than an unknown, so it + // is worth refusing on. + Some(spool) => Err(PrinterError(format!( + "{}: no AMS trays, and the external spool holds {} {} ({}), \ + which does not match this project", + self.base.name, + spool.tray_type.clone().unwrap_or_else(|| "nothing".into()), + spool.tray_info_idx.clone().unwrap_or_default(), + spool.tray_color.clone().unwrap_or_default(), + ))), + // v2 omits `vt_tray`, so this is "not reported", not "empty". + None => { + tracing::warn!( + printer = %self.base.name, + "no AMS trays and no vt_tray reported; assuming the external spool, unverified" + ); + Ok(FilamentSource::ExternalSpool) + } + } } /// Reads one file back off the printer's SD card. @@ -1312,6 +1405,82 @@ mod tests { assert_eq!(body["ams_mapping"], serde_json::json!("")); } + fn real_report() -> super::super::PrintReport { + let raw = include_str!("../../tests/fixtures/push_status_v1_p1p_idle.json"); + let value: serde_json::Value = serde_json::from_str(raw).expect("fixture is json"); + let body = value.get("print").unwrap_or(&value); + serde_json::from_value(body.clone()).expect("fixture should parse") + } + + fn filament(tray_info_idx: &str, filament_type: &str, color: &str) -> super::super::ProjectFilament { + super::super::ProjectFilament { + index: 0, + tray_info_idx: tray_info_idx.into(), + filament_type: filament_type.into(), + color: color.into(), + } + } + + /// The real P1P's external spool: PETG `P9599a25` in `D3C5A3FF`. + #[test] + fn the_real_external_spool_satisfies_a_project_asking_for_it() { + let report = real_report(); + let spool = report.vt_tray.as_ref().expect("the v1 fixture has a vt_tray"); + + assert!(external_spool_satisfies( + &[filament("P9599a25", "PETG", "D3C5A3")], + spool + )); + } + + /// The case that used to be indistinguishable from the one above: the + /// project wants a filament the spool does not hold. Before `vt_tray` + /// was modelled there was no way to tell these apart, so the code + /// assumed a match - which is the whole `use_ams: false` bug. + #[test] + fn the_real_external_spool_does_not_satisfy_a_different_filament() { + let report = real_report(); + let spool = report.vt_tray.as_ref().expect("the v1 fixture has a vt_tray"); + + // Right material, wrong product and colour - the AMS's blue GFG99. + assert!(!external_spool_satisfies( + &[filament("GFG99", "PETG", "2850E0")], + spool + )); + } + + #[test] + fn an_empty_external_spool_satisfies_nothing() { + let empty = super::super::VtTray { + id: "254".into(), + tray_info_idx: Some(String::new()), + tray_type: Some(String::new()), + tray_color: Some("00000000".into()), + bed_temp: None, + bed_temp_type: None, + nozzle_temp_max: None, + nozzle_temp_min: None, + remain: None, + tag_uid: None, + tray_diameter: None, + tray_id_name: None, + tray_sub_brands: None, + tray_uuid: None, + tray_weight: None, + xcam_info: None, + tray_temp: None, + tray_time: None, + cali_idx: None, + k: None, + n: None, + }; + + assert!(!external_spool_satisfies( + &[filament("P9599a25", "PETG", "D3C5A3")], + &empty + )); + } + /// The AMS half of the mapping, against the real P1P report in /// `tests/fixtures/push_status_v1_p1p_idle.json`: one unit, slots 0 and /// 1 empty, slots 2 and 3 holding PETG in two different colours. diff --git a/src/printer/bambu_commands/mod.rs b/src/printer/bambu_commands/mod.rs index e97195f..7c74397 100644 --- a/src/printer/bambu_commands/mod.rs +++ b/src/printer/bambu_commands/mod.rs @@ -25,7 +25,7 @@ mod print; mod pushall; pub use get_version::{GetVersionRequest, GetVersionResponse, ModuleInfo}; -pub use print::{AmsState, AmsTray, AmsUnit, EmptyTray, LoadedTray, PrintReport, XcamSettings}; +pub use print::{AmsState, AmsTray, AmsUnit, EmptyTray, LoadedTray, PrintReport, VtTray, XcamSettings}; pub use pushall::PushallRequest; /// Fields every Bambu command/response shares. This is the "inheritance" diff --git a/src/printer/bambu_commands/print.rs b/src/printer/bambu_commands/print.rs index 865de0a..448a548 100644 --- a/src/printer/bambu_commands/print.rs +++ b/src/printer/bambu_commands/print.rs @@ -89,6 +89,30 @@ pub struct PrintReport { /// against Bambu's HMS tables before it's worth a typed shape. pub hms: Option>, + // --- Stage. Without these a print that is busy preparing is + // --- indistinguishable from one that has stalled. + /// The stage the printer is in right now; `0` is "none in particular" + /// (idle, or actively printing). Confirmed live on a P1P mid-print: it + /// walked `54 -> 14 -> 13 -> 0` while `layer_num` stayed `0` and + /// `mc_percent` climbed to 14%. That is the only signal separating "doing + /// bed levelling and the like before the first layer" from "stuck at + /// layer 0", so a stall detector built on `layer_num` alone would raise + /// a false alarm on every healthy print start. Deliberately a bare + /// number: the code-to-meaning table is Bambu's, and an unknown code + /// should still show up as itself. + pub stg_cur: Option, + /// The stages planned for this print, in order - `[54, 14, 13]` on the + /// live P1P above, `[]` on an idle one, twelve entries on the H2C + /// fixture. `stg_cur` walks through these. + pub stg: Option>, + /// The external spool ("virtual tray"). **Observed on v1 only:** the P1P + /// fixture carries it, the H2C one has no `vt_tray` key at all. That is a + /// genuine generation divergence, not a gap in the capture, so this must + /// stay `Option` - the `tray_temp`/`drying_temp` split on `LoadedTray` + /// is the precedent, and making that one required broke every report from + /// the other generation. + pub vt_tray: Option, + // There are still ~60 more top-level fields in a real v2 payload (see // `tests/fixtures/push_status_v2_h2c_printing.json` for the full set): // the `job`/`device`/`net`/`upload` sub-objects, queue state, the @@ -153,6 +177,10 @@ impl PrintReport { self.wifi_signal = newer.wifi_signal.or(self.wifi_signal.take()); self.hms = newer.hms.or(self.hms.take()); + + self.stg_cur = newer.stg_cur.or(self.stg_cur.take()); + self.stg = newer.stg.or(self.stg.take()); + self.vt_tray = newer.vt_tray.or(self.vt_tray.take()); } } @@ -259,6 +287,44 @@ pub struct LoadedTray { pub n: Option, } +/// The external spool. Kept separate from `LoadedTray` rather than shared: +/// the shapes differ (no `cols`/`state`, but `k`/`n`/`cali_idx` are always +/// there), and bending `LoadedTray` to fit would loosen its required fields +/// for AMS trays, which have been seen on both generations. Only `id` is +/// required; the rest come from a single P1P capture, and any of them being +/// required would fail the whole report the first time another model or +/// firmware omits one - the same silent failure `LoadedTray` already hit. +/// Not `deny_unknown_fields`: a new key here should be ignored, not take +/// every `push_status` down with it. +#[derive(serde::Deserialize, Debug, Clone)] +pub struct VtTray { + /// `"254"` on the P1P - Bambu's fixed id for the external spool. + pub id: String, + pub bed_temp: Option, + pub bed_temp_type: Option, + pub nozzle_temp_max: Option, + pub nozzle_temp_min: Option, + pub remain: Option, + pub tag_uid: Option, + pub tray_color: Option, + pub tray_diameter: Option, + pub tray_id_name: Option, + pub tray_info_idx: Option, + pub tray_sub_brands: Option, + pub tray_type: Option, + pub tray_uuid: Option, + pub tray_weight: Option, + pub xcam_info: Option, + /// v1 spelling; v2 AMS trays call these `drying_temp`/`drying_time`, + /// and it is unknown what a v2 external spool would send. + pub tray_temp: Option, + pub tray_time: Option, + /// Flow-calibration profile index (`-1` when none) and coefficients. + pub cali_idx: Option, + pub k: Option, + pub n: Option, +} + /// Camera-based AI print monitoring settings (spaghetti detection, /// first-layer inspection, etc.). #[derive(serde::Deserialize, Debug, Clone)] diff --git a/src/printer/mod.rs b/src/printer/mod.rs index eed40ee..fa9f22c 100644 --- a/src/printer/mod.rs +++ b/src/printer/mod.rs @@ -49,7 +49,7 @@ pub use bambu::{ }; pub use bambu_commands::{ AmsState, AmsTray, AmsUnit, BambuReport, BambuRequest, CommandEnvelope, EmptyTray, GetVersionRequest, - GetVersionResponse, LoadedTray, ModuleInfo, PrintReport, PushallRequest, XcamSettings, + GetVersionResponse, LoadedTray, ModuleInfo, PrintReport, PushallRequest, VtTray, XcamSettings, }; pub use klipper::KlipperPrinter; pub use prusa::{PrusaLinkPrinter, PrusaSerialPrinter}; diff --git a/src/uplink/mod.rs b/src/uplink/mod.rs index 73634c4..783b5af 100644 --- a/src/uplink/mod.rs +++ b/src/uplink/mod.rs @@ -186,6 +186,7 @@ async fn collect_telemetry(fleet: &SharedFleet) -> Vec { name: base.name.clone(), state: report.gcode_state.clone(), percent: report.mc_percent, + stage: report.stg_cur, layer: report.layer_num, total_layers: report.total_layer_num, remaining_minutes: report.mc_remaining_time, diff --git a/src/uplink/protocol.rs b/src/uplink/protocol.rs index 37e3fe2..38f1d76 100644 --- a/src/uplink/protocol.rs +++ b/src/uplink/protocol.rs @@ -59,6 +59,18 @@ pub struct PrinterTelemetry { pub state: Option, #[serde(skip_serializing_if = "Option::is_none")] pub percent: Option, + /// What the printer is doing when it is not laying down plastic - bed + /// levelling, nozzle cleaning, extrusion calibration. Bambu's `stg_cur`, + /// passed through as the printer's own number rather than decoded, + /// because the code-to-name table is not something we have. 0 means + /// printing. + /// + /// Here because progress without it is ambiguous in a way that reads as + /// a fault. Confirmed on a P1P (2026-10-11): `percent` reached 14% while + /// `layer` stayed 0 for minutes, the printer walking `stg_cur` 54 -> 14 + /// -> 13 -> 0 before starting layer 1. + #[serde(skip_serializing_if = "Option::is_none")] + pub stage: Option, #[serde(skip_serializing_if = "Option::is_none")] pub layer: Option, #[serde(skip_serializing_if = "Option::is_none")] diff --git a/tests/push_status_parsing.rs b/tests/push_status_parsing.rs index e49dca6..42d5f34 100644 --- a/tests/push_status_parsing.rs +++ b/tests/push_status_parsing.rs @@ -86,3 +86,61 @@ fn both_protocol_generations_parse_their_ams_trays() { assert!(!ams.ams.is_empty(), "{name}: expected at least one AMS unit"); } } + +fn parse(name: &str) -> PrintReport { + serde_json::from_value(load(name)).unwrap_or_else(|err| panic!("{name}: {err}")) +} + +/// The P1P sends an external spool; pin the fields a farm would show. +#[test] +fn v1_report_carries_the_external_spool() { + let report = parse("push_status_v1_p1p_idle.json"); + let vt = report.vt_tray.expect("v1 fixture carries vt_tray"); + + assert_eq!(vt.id, "254"); + assert_eq!(vt.tray_info_idx.as_deref(), Some("P9599a25")); + assert_eq!(vt.tray_type.as_deref(), Some("PETG")); + assert_eq!(vt.tray_color.as_deref(), Some("D3C5A3FF")); + assert_eq!(vt.cali_idx, Some(-1)); + assert_eq!(vt.n, Some(1)); +} + +/// The H2C has no `vt_tray` key at all. That is the observed v1/v2 +/// divergence, NOT a parse failure: if a change makes `vt_tray` required, +/// `parse` panics here instead of every v2 report silently vanishing in +/// `handle_incoming_report`. +#[test] +fn v2_report_has_no_external_spool_and_still_parses() { + let report = parse("push_status_v2_h2c_printing.json"); + assert!(report.vt_tray.is_none()); +} + +/// `stg_cur` is what tells "preparing" from "stalled" while `layer_num` is 0. +#[test] +fn stage_fields_parse_on_both_generations() { + let v1 = parse("push_status_v1_p1p_idle.json"); + assert_eq!(v1.stg_cur, Some(0)); + assert_eq!(v1.stg, Some(vec![])); + + let v2 = parse("push_status_v2_h2c_printing.json"); + assert_eq!(v2.stg_cur, Some(0)); + assert_eq!(v2.stg, Some(vec![64, 29, 13, 74, 72, 4, 54, 39, 8, 14, 1, 3])); +} + +/// A v1 partial push without these must not erase what a full one set. +#[test] +fn merge_keeps_stage_and_spool_when_a_partial_update_omits_them() { + let mut base = parse("push_status_v1_p1p_idle.json"); + let mut partial = parse("push_status_v1_p1p_idle.json"); + partial.stg_cur = None; + partial.stg = None; + partial.vt_tray = None; + base.stg_cur = Some(54); + base.stg = Some(vec![54, 14, 13]); + + base.merge_from(partial); + + assert_eq!(base.stg_cur, Some(54)); + assert_eq!(base.stg, Some(vec![54, 14, 13])); + assert!(base.vt_tray.is_some()); +} diff --git a/tests/uplink_protocol.rs b/tests/uplink_protocol.rs index 1e09dad..2d2ead3 100644 --- a/tests/uplink_protocol.rs +++ b/tests/uplink_protocol.rs @@ -131,6 +131,9 @@ fn empty_telemetry_fields_are_omitted_not_null() { name: "P1P".into(), state: Some("IDLE".into()), percent: Some(0), + // An idle printer is in no stage, so this is the second field + // after `job_name` that must be omitted rather than nulled. + stage: None, layer: Some(0), total_layers: Some(0), remaining_minutes: Some(0), @@ -151,6 +154,38 @@ fn empty_telemetry_fields_are_omitted_not_null() { // the absence of "null" alone would pass on an empty object. assert!(json.contains(r#""state":"IDLE""#)); assert!(json.contains(r#""bed_temp":22.40625"#)); + assert!(!json.contains("stage"), "an absent stage should not appear at all"); +} + +/// A stage of 0 means "printing" and must survive the wire, because +/// `skip_serializing_if = "Option::is_none"` on an `Option` is one +/// `unwrap_or_default` away from turning "printing" into "no stage". The +/// distinction matters: a printer at 14% and layer 0 is mid-bed-levelling if +/// the stage is 54 and genuinely stuck if it is 0. +#[test] +fn a_zero_stage_is_sent_rather_than_treated_as_absent() { + let msg = ClientMessage::Telemetry { + printers: vec![PrinterTelemetry { + printer_id: "p1p".into(), + name: "P1P".into(), + state: Some("RUNNING".into()), + percent: Some(14), + stage: Some(0), + layer: Some(1), + total_layers: Some(150), + remaining_minutes: Some(39), + job_name: Some("cube-test".into()), + nozzle_temp: Some(255.2), + nozzle_target: Some(255.0), + bed_temp: Some(79.3), + bed_target: Some(80.0), + print_error: Some(0), + active_alerts: Some(0), + }], + }; + + let json = serde_json::to_string(&msg).expect("should serialize"); + assert!(json.contains(r#""stage":0"#), "a zero stage must be on the wire, got:\n {json}"); } #[test]