mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-18 03:54:17 +02:00
fix(ams): don't wipe an AMS-HT spool on a partial state=9 update (#2594)
The AMS merge clears a tray on a partial {id, state} update when state != 11
(the 4-slot AMS "emptied slot" signal, #784). An AMS-HT (single-tray high-temp
dry box, id >= 128) reports its loaded tray as state=9, so the partial the
printer sends on power-on was misread as "emptied" and wiped the HT-A spool's
tray_type/RFID/assignment seconds after power-on.
Skip the state-heuristic for HT units (id >= 128). Genuine HT removal still
clears via the explicit tray_type="" update and tray_exist_bits cleanup;
regular AMS (id < 128) is unchanged.
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
|
||||
- **The HT-A (AMS-HT) spool vanished a few seconds after every power-on (#2594, reporter @GuillaumeHouba)** — On an H2C, the spool in the HT-A high-temp AMS on the left nozzle showed correctly with its RFID assignment on power-on, then disappeared seconds later; the regular AMS spools stayed. **Root cause.** The AMS merge in `_handle_ams_data` clears a tray when it receives a partial `{id, state}` update whose `state != 11` — the rule that lets 4-slot AMS units (e.g. H2D) report an emptied slot with just `{id, state}` and no `tray_type` (#784), where `11` = loaded. But an **AMS-HT** (single-tray high-temp dry box, unit id ≥ 128) reports its *loaded* tray as `state=9`, not 11 — it doesn't feed filament into a shared buffer the way a 4-slot AMS does. So the partial `{id:0, state:9}` the printer sends for the HT tray on power-on was misread as "slot emptied," and Bambuddy wiped the tray's `tray_type` / RFID / Spoolman assignment. The support log showed it plainly: every "state=9 (not loaded) — clearing stale tray data" was on AMS 128, never on the regular AMS unit 0 (which correctly reports 11). **Fix.** The `state != 11 → empty` heuristic is now skipped for AMS-HT units (id ≥ 128); their differing single-tray state semantics mean a partial state update must not clear a present spool. A genuine HT spool removal still clears through the explicit `tray_type == ""` update and the `tray_exist_bits` cleanup, both unchanged, and regular AMS behavior (id < 128) is untouched. Covered by tests for the HT tray surviving a `state=9` partial, the HT still clearing on an explicit empty, and the existing regular-AMS `state=9`/`10`/`11` cases.
|
||||
- **A start-print dispatched to an already-busy printer could cancel the running job (#2598, reporter @khaosdoctor)** — On an A1 mini across a night of prints, jobs were cancelled with no apparent cause; debug logs showed Bambuddy sending `project_file` twice ~3 minutes apart with no completion between, and the printer answering `0500_4004` ("Device is busy and cannot start a new task") — which on that model cancels the RUNNING print. **Root cause.** `start_print()` in the MQTT client guarded only on connection state (`self._client and self.state.connected`) — it published `project_file` with no check on the printer's `gcode_state`. The scheduler *does* gate dispatch on an idle check, but that check treats `FINISH` as idle, and a printer can keep reporting `FINISH` for tens of seconds *after* it accepted a `project_file`; combined with a dispatch watchdog that reverts a queue item and releases its dispatch hold when it doesn't observe the active-state transition in time (#2555), a re-selected item could reach the FTP upload while the printer had actually started, so the start command landed on a live print. **Fix (defense-in-depth).** (a) `start_print()` now refuses to publish `project_file` when the printer is in an active state (`PREPARE` / `SLICING` / `RUNNING` / `PAUSE`) and returns without sending — a single guard at the one publish choke point that every dispatch path (queue, manual, webhook, Virtual-Printer forward) funnels through. `IDLE` / `FINISH` / `FAILED` remain valid start targets. (b) The scheduler re-checks the live printer state right before the FTP upload and *defers* a busy printer (leaves the item pending for a later tick) instead of uploading and dispatching — no wasted transfer, no collision. (c) If the printer goes busy during the upload window and the start command is refused, the scheduler reverts the item to pending (a deferral) rather than marking it failed — a busy printer is not a failure. Covered by tests for the client-level guard (refused while busy, published while idle, guard precedes the connection check) and the scheduler deferring both before the upload and after a busy-refused start. Note: a transport-level MQTT QoS-1 replay on reconnect would bypass the client guard, but the dispatch/watchdog reconnect path already hard-resets the client with a fresh session so it has no inflight `project_file` to replay.
|
||||
- **Three more idle-in-transaction / thundering-herd paths surfaced by continued farm testing (#2572, reporter @Jostxxl)** — On `origin/dev` with 93 printers and multiple concurrent UI clients the reporter timestamp-correlated the surviving pool pressure to three remaining paths, none of them auth-related. **(a) The scheduler held its per-item session across preheat and the FTP upload.** `_dispatch_selected` opens one `async_session` per queue item and hands it to `_start_print`, which reads the printer/archive rows up front and then runs the preheat/heat-soak wait and the FTP delete+upload — all on the transaction opened by those first `SELECT`s. One correlated session's last statement was a `settings` `SELECT` at the exact moment the log showed "Starting queue item" → preheat → "FTP upload started" for a 96 MB 3MF; the transaction stayed open for the whole transfer. (This refines the earlier note that the scheduler paths were "already bounded" — the per-item session itself was the hold.) `_start_print` now commits right before the FTP delete/upload, and `_preheat_and_soak` commits after its read phase and before the up-to-15-minute soak wait (both loops touch only `printer_manager` state and `asyncio.sleep`, no DB). `expire_on_commit=False` keeps the loaded rows readable; the status writes afterward (upload-failure path and the pending→printing CAS) transparently open a fresh transaction. **(b) `/cloud/filament-info` held its request session across sequential Bambu Cloud round-trips and single-flighted nothing.** The route took its session via `Depends(get_db)` (held for the whole request), read the stored token, then looped over the uncached ids issuing one external `get_setting_detail` HTTP call each — so the session sat idle-in-transaction across N cloud calls, and because the printer overview mounts one filament-info request per printer card, several browsers hit the same uncached preset at once and each issued its own cloud call. The route now releases the transaction (`rollback`) right after the token read and before the cloud loop (Phase 3's local-preset read reopens a fresh one), and concurrent misses for the same id single-flight through one shared cloud call. **(c) `/printers/{id}/cover` had no in-flight coalescing.** The connection was already released before the download, but simultaneous clients could all miss the cache and each run the full multi-path FTP lookup + 3MF extraction (one observed transfer pulled an 81 MB 3MF while real print uploads were in flight). Identical concurrent cover requests now coalesce: the first becomes the leader and the rest await it, then serve from the positive/negative cache it filled. Also adds `pool_use_lifo` (PostgreSQL default on, `DB_POOL_USE_LIFO` override, shown in `/system/db-pool`) so a bursty farm keeps a small hot connection set busy and lets excess overflow connections age out via `pool_recycle` instead of churning the whole pool. Covered by tests: the scheduler releasing its connection before both the FTP upload and the soak wait, the filament-info single-flight (concurrent misses share one cloud call, cache-hit skips cloud, a failed fetch leaves no stuck in-flight entry), and concurrent cover requests downloading once.
|
||||
- **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.
|
||||
|
||||
@@ -2078,10 +2078,26 @@ class BambuMQTTClient:
|
||||
# 10=spool present but filament not in feeder) indicate
|
||||
# the slot should be cleared. Without this, old
|
||||
# tray_type/tray_color persist indefinitely (#784).
|
||||
#
|
||||
# BUT this is regular-AMS semantics. An AMS-HT (single-
|
||||
# tray high-temp dry box, id >= 128) reports its loaded
|
||||
# tray as state=9, not 11 — it doesn't feed filament into
|
||||
# a shared buffer the way a 4-slot AMS does. Applying the
|
||||
# `state != 11 → empty` rule to an HT unit wiped a present
|
||||
# spool on every power-on, when the printer sends a partial
|
||||
# {id, state=9} for the HT tray (#2594). Skip the state
|
||||
# heuristic for HT units — a genuine HT spool removal still
|
||||
# clears via the explicit tray_type=="" case above and the
|
||||
# tray_exist_bits cleanup below.
|
||||
try:
|
||||
_is_ht_unit = int(ams_id) >= 128
|
||||
except (TypeError, ValueError):
|
||||
_is_ht_unit = False
|
||||
tray_state = new_tray.get("state")
|
||||
if (
|
||||
tray_state is not None
|
||||
and tray_state != 11
|
||||
and not _is_ht_unit
|
||||
and "tray_type" not in new_tray
|
||||
and merged_tray.get("tray_type")
|
||||
):
|
||||
|
||||
@@ -1196,6 +1196,63 @@ class TestAMSTrayStateClearning:
|
||||
assert tray0["tray_color"] == "00FF00FF"
|
||||
assert tray0["remain"] == 75
|
||||
|
||||
def _seed_loaded_ht_tray(self, mqtt_client):
|
||||
"""Seed an AMS-HT unit (id 128, single tray) whose loaded tray reports
|
||||
state=9 — the real HT resting state, unlike a 4-slot AMS's state=11."""
|
||||
initial = {
|
||||
"ams": [
|
||||
{
|
||||
"id": 128,
|
||||
"tray": [
|
||||
{
|
||||
"id": 0,
|
||||
"tray_type": "PA",
|
||||
"tray_color": "161616FF",
|
||||
"tray_info_idx": "GFG99",
|
||||
"tag_uid": "AABBCCDD11223344",
|
||||
"tray_uuid": "AABBCCDD11223344AABBCCDD11223344",
|
||||
"remain": 60,
|
||||
"state": 9,
|
||||
}
|
||||
],
|
||||
}
|
||||
],
|
||||
"power_on_flag": True,
|
||||
}
|
||||
mqtt_client._handle_ams_data(initial)
|
||||
|
||||
def test_ht_unit_state_9_preserves_tray_data(self, mqtt_client):
|
||||
"""#2594: an AMS-HT reports its loaded tray as state=9. A partial
|
||||
{id, state=9} for the HT unit must NOT be read as 'empty' and wipe the
|
||||
present spool — that made the HT-A spool vanish on every power-on."""
|
||||
self._seed_loaded_ht_tray(mqtt_client)
|
||||
|
||||
# Printer sends a partial {id, state} for the HT tray on power-on.
|
||||
update = {
|
||||
"ams": [{"id": 128, "tray": [{"id": 0, "state": 9}]}],
|
||||
"power_on_flag": True,
|
||||
}
|
||||
mqtt_client._handle_ams_data(update)
|
||||
|
||||
tray0 = mqtt_client.state.raw_data["ams"][0]["tray"][0]
|
||||
assert tray0["tray_type"] == "PA", "HT state=9 must NOT clear a present spool (#2594)"
|
||||
assert tray0["tray_uuid"] == "AABBCCDD11223344AABBCCDD11223344", "HT RFID must survive"
|
||||
assert tray0["remain"] == 60
|
||||
|
||||
def test_ht_unit_explicit_empty_still_clears(self, mqtt_client):
|
||||
"""A genuine HT spool removal (explicit tray_type='') must still clear —
|
||||
the #2594 fix only skips the state-heuristic, not the explicit path."""
|
||||
self._seed_loaded_ht_tray(mqtt_client)
|
||||
|
||||
update = {
|
||||
"ams": [{"id": 128, "tray": [{"id": 0, "tray_type": ""}]}],
|
||||
"power_on_flag": False,
|
||||
}
|
||||
mqtt_client._handle_ams_data(update)
|
||||
|
||||
tray0 = mqtt_client.state.raw_data["ams"][0]["tray"][0]
|
||||
assert tray0["tray_type"] == "", "explicit tray_type='' must still clear an HT tray"
|
||||
|
||||
|
||||
class TestApplyTrayExistBitsHelper:
|
||||
"""Direct contract pinning for the shared ``apply_tray_exist_bits`` helper.
|
||||
|
||||
Reference in New Issue
Block a user