mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 11:12:35 +02:00
fix(dispatch): honor a resolved AMS mapping over a stale use_ams=false (#2595)
A print sliced against a Virtual Printer carries use_ams=false — a VP advertises no AMS, so the slicer sends it and VP intake stamps it on the queue item. But an "Any [model]" item is colour-matched to a real printer at dispatch, resolving a real AMS slot in ams_mapping. The command builder only ever forced use_ams off (all-external) and never back on, so the stale false shipped with a real-tray mapping and the printer aborted at layer 0 on the empty external spool. For single-nozzle printers the mapping is now authoritative: a real tray (0-253) forces use_ams=true, explicit external (254/255) forces it false, and an unresolved -1 does neither (preserving the #2589 contract). Dual-nozzle is untouched — use_ams is nozzle routing there. The correction sits at the single command-builder choke point, covering the VP, queue, and manual paths.
This commit is contained in:
@@ -8,6 +8,7 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
- **Orca Cloud profile sync now connects by approving a code instead of the copy-paste sign-in** — Connecting Bambuddy to Orca Cloud used to mean opening an OAuth sign-in in a new tab, watching it redirect to a `localhost` URL that fails to load, then copying that dead URL out of the address bar and pasting it back into Bambuddy. That dance existed only because Orca's auth backend (Supabase) accepts no redirect target other than `localhost`, and the deliberately-broken redirect page confused nearly everyone who reached it. OrcaSlicer has since shipped a first-class external-app pairing API (the OAuth 2.0 Device Authorization Grant, RFC 8628), so the flow is now: click **Connect**, approve a short code on your Orca Cloud settings page, and Bambuddy pairs itself — no redirect, no paste, no client secret, and it behaves identically from a LAN IP, `localhost`, or behind a reverse proxy. Bambuddy requests **read-only** access (it only lists and views your Orca Cloud profiles), keeps the pairing alive with the API's rotating refresh tokens (validated end-to-end against Orca's staging and production servers), and stores nothing beyond the issued token pair. The profile list and detail views are unchanged, so nothing downstream of the connect step looks different. The old paste-based sign-in and the email/password fallback are removed. Points at production Orca Cloud by default; `ORCA_CLOUD_API_BASE` overrides the endpoint for testing.
|
||||
|
||||
### Fixed
|
||||
- **An "Any [model]" queue job dispatched from a Virtual Printer printed to the empty external spool and aborted at layer 0 (#2595, diagnosed by @Sawtaytoes, PR #2596)** — On a farm of identical X1Cs with different filaments loaded per AMS, the intended flow — VP in Queue mode, auto-dispatch, target **Any X1C**, force-colour-match picks the printer that has the right spool — sent the job to the correctly-matched printer and then failed: the printer ignored the AMS, pulled the empty external spool, and aborted with "not enough filament", even though the mapped slot was loaded (the same print via a specific printer, or straight from the slicer, worked). **Root cause.** A slicer talking to a Virtual Printer only ever sees the VP's external spool — a VP advertises no AMS — so the slicer sends `use_ams=false`, and VP intake stamps that onto the queue item. But an "Any [model]" item is colour-matched to a real printer *at dispatch*, resolving a real AMS slot in `ams_mapping`; the scheduler still forwarded the stale `use_ams=false`. The print-command builder only ever forced `use_ams` **off** (the all-external case) and never back **on**, so `use_ams=false` shipped alongside `ams_mapping=[<real tray>]` → external spool → abort. **Fix.** For single-nozzle printers the resolved mapping is now authoritative: a real AMS tray (0-253) forces `use_ams=true`; an explicit external selection (254/255) still forces it false; an unresolved `-1` mapping does neither (preserving the #2589 contract — it should have been recomputed upstream, and must not be silently promoted to AMS or downgraded to external). Dual-nozzle printers are untouched, where `use_ams` encodes nozzle routing rather than an on/off flag. Because the correction lives at the single command-builder choke point, it fixes the VP, queue, and manual paths alike. Covered by tests for the VP `false`+real-tray promotion, padded mappings, all-external staying off, unresolved `-1` staying put, the original all-external downgrade, and the dual-nozzle bypass.
|
||||
- **Reconnecting or restarting inflated Stats → Total Print Time by hundreds of hours on large farms (#2592, reporter @Jostxxl)** — On the reporter's farm a restart pushed Total Print Time from ~1,500h to 3,215h. When a printer reconnects, the connected edge runs `reconcile_stale_active_prints`, which closes out every archive still stuck in `status="printing"` (missed completions, disconnects, restarts) by synthesising an aborted `on_print_complete`. That wrote a `PrintLogEntry` whose duration was `completed_at - started_at` — but for a reconciled archive the real end time is unknown: the print stopped somewhere during the disconnect, and `completed_at` is only the reconnect moment. So each stale archive banked its entire multi-day gap as print time (one row was 51.9h), and a printer with several stale archives contributed hundreds of fabricated hours at once. Worse, the Stats total *recomputed* `completed_at - started_at` whenever the stored duration was falsy, so storing NULL wouldn't have helped. Reconciled completions now log an explicit `duration_seconds = 0` (honest "no measured runtime") and the two Stats time paths trust a stored 0 instead of recomputing from the stale timestamps — legacy rows that never recorded a duration still fall back as before. Reconciled aborts also get a truthful `failure_reason` ("Stale - reconciled after reconnect, end time unknown") instead of being mislabelled "User cancelled". Genuine long prints are untouched: nothing is capped, a still-running >24h print is never treated as stale, and a real >24h run keeps its full measured duration. Re-running reconciliation is already idempotent (the archive flips to `aborted`, so it isn't re-selected). Existing inflated rows from before this fix are not auto-corrected — they're indistinguishable from real cancellations in the database, and a blanket cap would clobber genuine long prints; the reporter repaired his own rows by hand. Covered by tests for the multi-day reconcile, multiple stale archives per printer, a retained >24h print, and the Stats total ignoring reconciled time while still counting real runtime.
|
||||
- **H2C prints intermittently recorded no filament and never deducted from inventory (#2582, reporter @gyrene2083)** — On an H2C (firmware `01.02.00.00`) filament usage sometimes wasn't deducted and the Print Log showed no filament for that print; the reporter confirmed the tell-tale detail — the failed print's archived `.3mf` didn't exist to download. Filament totals, the Print Log filament column, and the weight deduction all read the sliced 3MF's data, so when that file can't be pulled off the printer the print drops to the no-3MF fallback archive and every one of them comes up empty. The download itself was the failure: the H2C is the same H2 generation and the same firmware line as the P2S, whose FTPS data channel trips a vsFTPd + TLS 1.3 session-reuse bug on Python 3.13 (#1401) — and the X2D hit the sibling handshake variant (#1638). Both were fixed by capping that model's FTP control/data channel to TLS 1.2 via the per-model FTP profile registry, but the H2C had no entry and so ran on the Python-default TLS 1.3, leaving its 3MF downloads to fail the same way (intermittently, matching the "sometimes works, sometimes doesn't" report — the session-reuse race rather than a hard handshake failure). The H2C now gets the same `cap_tls_v1_2` profile as the P2S/X2D (with its `O1C`/`O1C2` SSDP codes mapped to it), so the sliced 3MF comes off the printer reliably and the slice data — filament total, Print Log filament, and the inventory deduction — is populated again. H2D is deliberately left on the default profile; it negotiates TLS 1.3 without this fault.
|
||||
- **An unresolved AMS mapping silently dispatched a P1S print to the empty external spool (#2589, reporter @Jostxxl)** — A queued P1S job with a regular AMS attached, two compatible PETG spools loaded, and nothing on the external spool holder started against the *external* feed and paused seconds later with a filament-runout HMS. The queue row was correct on its face — `use_ams=true` — but carried `ams_mapping=[-1]`, and Bambuddy turned that into a print with no AMS. Two faults combined. **A `-1` was read as "external spool."** The command builder's rule for "all slots are external, so drop `use_ams`" tested `t < 0 or t >= 254` — folding the *unresolved* sentinel (`-1`) in with a genuine external selection (`254`/`255`). An explicit external print serializes as `[254]`; an unresolved slot serializes as `[-1]`, and the two mean opposite things — one is "use the spool holder", the other is "we never worked out which tray." Only `>= 254` may now force `use_ams=False`; `-1` never does. **The unresolved mapping was trusted instead of recomputed.** The scheduler only computes a mapping when the row has *none*; a stored `[-1]` is non-empty, so it looked "already resolved" and was passed through verbatim — even though the backend had the live AMS trays and the plate's filament requirements right there and could have matched them. Dispatch now recomputes whenever the stored mapping is entirely unresolved, so a bogus `[-1]` self-heals against the trays actually loaded (and any pre-existing stuck row heals on the next scheduler pass); if nothing compatible is loaded it is cleared rather than sent, so the firmware reports a clear mapping error instead of quietly printing to an empty feed. **Where the `[-1]` came from.** The Print dialog builds the mapping from the selected printer's live status; if you submitted a single-printer job in the instant before that status query resolved, it matched against zero known trays and serialized every required slot as `-1`. The dialog now waits for the printer's AMS status before it will submit (showing a brief "Waiting for AMS status from …" notice), and the mapping hook returns *no* mapping rather than an all-`-1` one while the trays are unknown — so the scheduler resolves it at dispatch. A genuine no-match with trays present still serializes `-1` and surfaces the mismatch as before. **Tests.** Backend: the command builder keeps `use_ams=true` for `[-1]`/`[-1,-1]` and a padded `[-1,-1,5]`, still drops it for an explicit `[254]`; the scheduler recomputes a stored `[-1]`, leaves a resolved (or manually-overridden) mapping untouched, and clears an unresolvable one. An existing test that asserted the old `[-1] → use_ams=False` behaviour was corrected to the fixed contract. Frontend: the mapping hook returns `undefined` while status is loading, resolves to the AMS tray once it arrives (type-only match with strict colour off), and still emits `-1` for a real mismatch. Full backend suite and the PrintModal/mapping frontend suites green.
|
||||
|
||||
@@ -3916,23 +3916,46 @@ class BambuMQTTClient:
|
||||
flat_ams_mapping.append(tray_id)
|
||||
ams_mapping2.append({"ams_id": ams_id, "slot_id": slot_id})
|
||||
|
||||
# If all mapped slots are external spool (no real AMS trays), force use_ams=False.
|
||||
# P1S/P1P with no AMS rejects use_ams=True with "Failed to get AMS mapping table".
|
||||
# Skip for dual-nozzle printers — use_ams controls nozzle routing there.
|
||||
# H2S falls through this gate now (#1386): it is single-nozzle and was
|
||||
# Reconcile use_ams against the resolved ams_mapping for single-nozzle
|
||||
# printers — the mapping is authoritative about whether this print
|
||||
# actually feeds from the AMS. Skip for dual-nozzle printers, where
|
||||
# use_ams encodes nozzle routing rather than an AMS on/off flag.
|
||||
# H2S falls through here now (#1386): it is single-nozzle and was
|
||||
# hitting the dual-nozzle bypass, which caused 07FF_8012 when printing
|
||||
# without an AMS attached.
|
||||
#
|
||||
# Only an *explicit* external/virtual spool (254/255) may downgrade to
|
||||
# use_ams=False. An unresolved slot (-1) must NOT: it means the mapping
|
||||
# was never resolved — e.g. a frontend status-load race that persisted
|
||||
# [-1] (#2589) — and treating that as "external" silently started the
|
||||
# print against an empty external feed, pausing with a runout. A genuine
|
||||
# external selection is >=254; unresolved is -1. Keep them distinct so an
|
||||
# unresolved mapping fails loudly (or is recomputed upstream) instead of
|
||||
# silently going external.
|
||||
if ams_mapping and use_ams and not is_dual_nozzle:
|
||||
if all(t is None or int(t) >= 254 for t in ams_mapping):
|
||||
# Two symmetric corrections:
|
||||
#
|
||||
# (a) A mapping that resolves a *real* AMS tray (0-253) forces
|
||||
# use_ams=True even if it arrived False. A print sent to a Virtual
|
||||
# Printer is sliced against the VP, which advertises no AMS, so the
|
||||
# slicer sends use_ams=false and that gets stamped on the queue item
|
||||
# — but at dispatch the scheduler colour-matches a real printer and
|
||||
# resolves a real AMS slot. Without this, the stale False reaches the
|
||||
# printer, which ignores the mapped slot and aborts at layer 0 on the
|
||||
# empty external spool ("not enough filament"). Diagnosed by
|
||||
# @Sawtaytoes (#2595, PR #2596).
|
||||
#
|
||||
# (b) Only an *explicit* external/virtual spool (254/255) may downgrade
|
||||
# to use_ams=False. P1S/P1P with no AMS rejects use_ams=True with
|
||||
# "Failed to get AMS mapping table". An unresolved slot (-1) does
|
||||
# NEITHER: it means the mapping was never resolved — e.g. a frontend
|
||||
# status-load race that persisted [-1] (#2589) — and treating it as
|
||||
# external silently started the print against an empty feed. A genuine
|
||||
# external selection is >=254; unresolved is -1; a loaded tray is
|
||||
# 0-253. Keeping them distinct means an unresolved mapping fails loudly
|
||||
# (or is recomputed upstream) instead of silently going external, and
|
||||
# never gets force-enabled by (a) either.
|
||||
if ams_mapping and not is_dual_nozzle:
|
||||
has_real_tray = any(t is not None and 0 <= int(t) <= 253 for t in ams_mapping)
|
||||
all_external = all(t is None or int(t) >= 254 for t in ams_mapping)
|
||||
if has_real_tray and not use_ams:
|
||||
use_ams = True
|
||||
logger.info(
|
||||
"[%s] AMS mapping resolved a real slot — setting use_ams=True (#2595)",
|
||||
self.serial_number,
|
||||
)
|
||||
elif use_ams and all_external:
|
||||
use_ams = False
|
||||
logger.info(
|
||||
"[%s] All filament slots use external spool — setting use_ams=False",
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
"""use_ams must be reconciled against the resolved ams_mapping at dispatch (#2595).
|
||||
|
||||
A print sent to a Virtual Printer is sliced against the VP, which advertises no
|
||||
AMS, so the slicer sends ``use_ams=false`` and that flag is stamped onto the
|
||||
queue item. But an "Any [model]" queue item is colour-matched to a real printer
|
||||
at dispatch, resolving a *real* AMS slot in ``ams_mapping``. The stale
|
||||
``use_ams=False`` used to survive to the print command, so the printer ignored
|
||||
the mapped slot and aborted at layer 0 on the empty external spool ("not enough
|
||||
filament"). Diagnosed by @Sawtaytoes.
|
||||
|
||||
The command builder now treats the mapping as authoritative for single-nozzle
|
||||
printers: a real tray forces ``use_ams=True``; an explicit external selection
|
||||
forces it False; an unresolved ``-1`` mapping (#2589) does neither. Dual-nozzle
|
||||
is untouched (``use_ams`` is nozzle routing there).
|
||||
"""
|
||||
|
||||
import json
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from backend.app.services.bambu_mqtt import BambuMQTTClient
|
||||
|
||||
|
||||
class TestUseAmsReconcile:
|
||||
@pytest.fixture
|
||||
def mqtt_client(self):
|
||||
client = BambuMQTTClient(
|
||||
ip_address="192.168.1.100",
|
||||
serial_number="01P00A452600691",
|
||||
access_code="12345678",
|
||||
)
|
||||
# Single-nozzle X1C so the dual-nozzle bypass does not apply.
|
||||
client.model = "X1C"
|
||||
client._client = MagicMock()
|
||||
client.state.connected = True
|
||||
return client
|
||||
|
||||
def _sent_command(self, mqtt_client) -> dict:
|
||||
assert mqtt_client._client.publish.called, "start_print did not publish"
|
||||
payload = mqtt_client._client.publish.call_args.args[1]
|
||||
return json.loads(payload)["print"]
|
||||
|
||||
def test_vp_false_with_real_tray_forces_use_ams_true(self, mqtt_client):
|
||||
"""The reported bug: use_ams=False (VP-stamped) + a real AMS slot -> True."""
|
||||
assert mqtt_client.start_print("shell.3mf", ams_mapping=[4], use_ams=False) is True
|
||||
cmd = self._sent_command(mqtt_client)
|
||||
assert cmd["use_ams"] is True
|
||||
|
||||
def test_false_with_padded_real_tray_forces_true(self, mqtt_client):
|
||||
"""A padded mapping ([-1, -1, tray]) still has a real slot -> True."""
|
||||
assert mqtt_client.start_print("shell.3mf", ams_mapping=[-1, -1, 5], use_ams=False) is True
|
||||
cmd = self._sent_command(mqtt_client)
|
||||
assert cmd["use_ams"] is True
|
||||
|
||||
def test_false_all_external_stays_false(self, mqtt_client):
|
||||
"""An explicit external selection must NOT be force-enabled."""
|
||||
assert mqtt_client.start_print("shell.3mf", ams_mapping=[254], use_ams=False) is True
|
||||
cmd = self._sent_command(mqtt_client)
|
||||
assert cmd["use_ams"] is False
|
||||
|
||||
def test_false_mixed_external_stays_false(self, mqtt_client):
|
||||
assert mqtt_client.start_print("shell.3mf", ams_mapping=[255, 254], use_ams=False) is True
|
||||
cmd = self._sent_command(mqtt_client)
|
||||
assert cmd["use_ams"] is False
|
||||
|
||||
def test_false_unresolved_stays_false(self, mqtt_client):
|
||||
"""An unresolved [-1] is neither external nor a real tray — leave it alone
|
||||
(preserves the #2589 contract; it should have been recomputed upstream)."""
|
||||
assert mqtt_client.start_print("shell.3mf", ams_mapping=[-1], use_ams=False) is True
|
||||
cmd = self._sent_command(mqtt_client)
|
||||
assert cmd["use_ams"] is False
|
||||
|
||||
def test_true_all_external_still_downgrades(self, mqtt_client):
|
||||
"""The original #2589 all-external downgrade still fires."""
|
||||
assert mqtt_client.start_print("shell.3mf", ams_mapping=[254], use_ams=True) is True
|
||||
cmd = self._sent_command(mqtt_client)
|
||||
assert cmd["use_ams"] is False
|
||||
|
||||
def test_true_real_tray_stays_true(self, mqtt_client):
|
||||
assert mqtt_client.start_print("shell.3mf", ams_mapping=[5], use_ams=True) is True
|
||||
cmd = self._sent_command(mqtt_client)
|
||||
assert cmd["use_ams"] is True
|
||||
|
||||
def test_dual_nozzle_use_ams_untouched(self, mqtt_client):
|
||||
"""Dual-nozzle: use_ams is nozzle routing, not an AMS flag — never coerced."""
|
||||
mqtt_client._is_dual_nozzle = True
|
||||
assert mqtt_client.start_print("shell.3mf", ams_mapping=[4], use_ams=False) is True
|
||||
cmd = self._sent_command(mqtt_client)
|
||||
assert cmd["use_ams"] is False
|
||||
Reference in New Issue
Block a user