From b0ecb8fd888a8e07f3af8d08d0784f41be4086b0 Mon Sep 17 00:00:00 2001 From: maziggy Date: Sat, 29 Aug 2026 09:23:48 +0200 Subject: [PATCH] Repair the bed temperature on archives written before the fix (issue #2989) The forward fix reads the array the fitted plate points at, but only for archives made after it. Everything already in the library stays blank, and preheat keeps falling back to the keep-warm bed temperature whenever one of those jobs is reprinted from the queue - 0 of 455 real 3MFs had resolved. A one-shot pass re-reads the 3MF already on disk, gated by a settings flag the way #2614's repair is: the rows it cannot fill are exactly the ones it would reopen every boot. It fills NULLs only. Nothing recorded is overwritten, an archive whose file is gone stays NULL, and a corrupted 3MF is skipped rather than failing startup. The plate mapping moves to threemf_tools.bed_temperature_from_config so the ingest path and the repair cannot read a 3MF differently - the same drift move. _extract_settings_from_content is deleted. Nothing called it anywhere in the repo, and it carried the old bed_temperature mapping this issue fixed. --- CHANGELOG.md | 2 +- backend/app/core/database.py | 93 +++++ backend/app/services/archive.py | 85 +--- backend/app/utils/threemf_tools.py | 90 +++++ .../test_bed_temperature_backfill_2989.py | 376 ++++++++++++++++++ 5 files changed, 564 insertions(+), 82 deletions(-) create mode 100644 backend/tests/unit/test_bed_temperature_backfill_2989.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a6ecb1ec..1953953c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,7 @@ All notable changes to Bambuddy will be documented in this file. - **A camera snapshot occasionally logged an asyncio ERROR with a traceback into the camera code (#2968, reported by @ceasley)** — `Task was destroyed but it is pending!`, naming `create_tls_proxy.._handle()`, once every few hundred snapshots. Nothing was actually broken by it — the snapshots on either side of each one succeeded — but it is the loudest thing in an otherwise clean log and it reads like a camera fault. `asyncio.start_server` wraps the connection callback in a task and keeps only a weak reference to it, so the RTSPS proxy's handler could be collected while still waiting on its two forwarders; that message is the garbage collector noticing. Teardown had the matching gap: `server.close()` stops the listener but leaves established connections running, so the close waited on a handler that only finishes when the *peer* drops the socket — and by then ffmpeg has already been reaped, so the connection is dead weight nobody is going to close. The proxy now holds its handlers for as long as they run and cancels them at shutdown, which is what `Server.close_clients()` would do natively if it had not landed in Python 3.13 (Bambuddy supports 3.10). Both the snapshot path and the streaming endpoint go through the same shutdown. 7 regression tests, against a real TLS listener rather than a mock, since the bug is in socket teardown. - **Deleting a print with no 3MF left its timelapse and its uploaded source on disk (#2968, reported by @ceasley)** — and logged the ordinary case as a security incident. An archive created without a 3MF carries an empty `file_path`, and both delete paths derived the directory to remove from that path: finding nothing, they removed nothing and wrote `SECURITY: Refusing to delete files for archive 7 - file_path is empty or invalid: ''` at ERROR. That was true when such an archive really was an empty row, and stopped being true once one could own files — `//` holds its timelapse and finish photos, and `archive/no_source//` holds a source 3MF uploaded onto it afterwards. Neither was ever removed, so the row went and the video stayed. That is not a corner case on an H2-series or P2S printer, where a print sent from Bambu Studio *always* archives without a 3MF — it is most of the library, and the space it used never came back. Both are now cleaned up, but by different means, because `/` **shares a namespace with the per-printer folders**: a normal archive lives at `//_/`, so `archive/1` is printer 1's folder *and* the directory the shared path helper hands archive id 1 — and archive ids and printer ids are small integers from unrelated sequences, so on every install the first few archives collide with the printers. A recursive delete there would take every print that printer ever made. So `archive/no_source//`, which is nested a level deeper under a name no printer id can take, is removed whole, while the id-named directory gives up only the files the row itself names — its `photos` subdirectory and its recorded video — and is then removed only if that left it empty, which a printer folder holding prints never is. Anything unrecognised keeps it alive and is left behind rather than guessed at. **The same collision is now refused for corrupted rows**: nothing one level under the archive directory is deleted at all, and since an archive directory has been two levels deep since the first commit, a `file_path` that has lost a path component can no longer point the delete at a printer folder. The shared `/photos` directory that every no-3MF archive once wrote into is never a candidate either, because removing it on one delete would take the others' photos. **Hard delete had its own copy of these rules** — the helper it was extracted from says in its docstring that it exists so the two cannot drift apart, and they had — so it now shares them, which also means it stops skipping the print-log thumbnail cleanup when a guard trips. The `SECURITY:` wording is kept for the two conditions that really are one, and both still remove the row so an archive cannot get stuck in the UI. 20 regression tests. - **Every ffmpeg failure logged its build banner instead of the error (#2968, reported by @ceasley)** — ffmpeg opens each run with about twenty lines of version, build and library banner and prints its diagnosis *last*. Seven of the eleven places Bambuddy logs ffmpeg or ffprobe output kept the first 200 to 500 characters of that, so the banner survived and the error did not. The reporter's log carried twelve camera-capture errors and every one of them read `ffmpeg frame bytes capture failed (code 183): ffmpeg version 7.1.4-0+deb13u1 … built with gcc 14 … configuration: --prefix=/usr --extra-version=` — identical on every install, and silent about why the capture failed. The exit code was the only usable information in the line. The banner-stripping summariser written for the camera streaming endpoint (#925) now lives in a shared module and every ffmpeg and ffprobe stderr goes through it: snapshot capture, last-frame extraction, the layer-timelapse stitch, the archive's MP4 conversion, external USB and RTSP capture and streaming, and timelapse post-processing. It drops the banner, keeps the last ten meaningful lines — the error plus the input analysis that explains it — and leaves indented `Duration:` and `Stream #0:0` lines alone, since those are diagnosis rather than boilerplate. **Two things the scattered copies also got wrong.** ffmpeg echoes its input URL back in its error output, and seven of those call sites logged it without masking, publishing a printer access code or an external camera's password into a log file people routinely attach to public issues; masking is now part of the summary, applied to the whole string before anything is dropped, so a credential straddling the cut cannot leave its tail behind. And ten sites called a bare `.decode()` on bytes ffmpeg had copied stream fragments into, which could raise `UnicodeDecodeError` while reporting an unrelated failure — the summariser takes bytes and decodes them safely. A failure whose output was banner only now says `no diagnostic output` rather than trailing off after the colon, and the whole line is capped, since ffmpeg quotes back what the peer sent it. 20 regression tests, including one that fails if a twelfth call site starts truncating or decoding by hand. -- **Every print archived from a Bambu slice had no bed temperature, so preheat guessed one (#2989, reported by @senguendk)** — `PrintArchive.bed_temperature` was parsed by looking for a `bed_temperature` key in the project settings. BambuStudio does not write one: it stores a separate per-filament array for each plate type — `cool_plate_temp`, `eng_plate_temp`, `hot_plate_temp`, `textured_plate_temp`, `supertack_plate_temp` — and names the plate the project is sliced for in `curr_bed_type`. `bed_temperature` is the Orca/PrusaSlicer spelling, so the lookup matched nothing at all: **0 of 455 real 3MFs** on a live install produced a bed temperature, and every archive stored NULL. That is why preheat logged "archive has no bed_temperature metadata" on jobs sliced perfectly normally, and then heated the bed to the configured keep-warm temperature — 90°C by default — for prints that asked for 55°C. The extractor now reads the array the fitted plate points at, taking the first-layer value in preference since that is what the printer heats to before the print starts; the same 455 files now all resolve, across the four plate types they were sliced for. The plate names and the plate-to-key mapping are BambuStudio's own, so a plate a future release adds reads as unknown rather than silently taking another plate's temperature — as does `Default Plate`, which BambuStudio maps to no key. The per-filament array resolves to its highest entry rather than its first: the bed has one temperature and a 0 in that array means "this filament cannot print on this plate", so the first entry would record a cold bed for any project whose first filament is not one the fitted plate is heated for. An array that is all zeros is left unrecorded rather than stored as 0. Orca-exported 3MFs keep parsing through the generic spelling, which is now the fallback rather than the only thing tried. 21 regression tests. +- **Every print archived from a Bambu slice had no bed temperature, so preheat guessed one (#2989, reported by @senguendk)** — `PrintArchive.bed_temperature` was parsed by looking for a `bed_temperature` key in the project settings. BambuStudio does not write one: it stores a separate per-filament array for each plate type — `cool_plate_temp`, `eng_plate_temp`, `hot_plate_temp`, `textured_plate_temp`, `supertack_plate_temp` — and names the plate the project is sliced for in `curr_bed_type`. `bed_temperature` is the Orca/PrusaSlicer spelling, so the lookup matched nothing at all: **0 of 455 real 3MFs** on a live install produced a bed temperature, and every archive stored NULL. That is why preheat logged "archive has no bed_temperature metadata" on jobs sliced perfectly normally, and then heated the bed to the configured keep-warm temperature — 90°C by default — for prints that asked for 55°C. The extractor now reads the array the fitted plate points at, taking the first-layer value in preference since that is what the printer heats to before the print starts; the same 455 files now all resolve, across the four plate types they were sliced for. The plate names and the plate-to-key mapping are BambuStudio's own, so a plate a future release adds reads as unknown rather than silently taking another plate's temperature — as does `Default Plate`, which BambuStudio maps to no key. The per-filament array resolves to its highest entry rather than its first: the bed has one temperature and a 0 in that array means "this filament cannot print on this plate", so the first entry would record a cold bed for any project whose first filament is not one the fitted plate is heated for. An array that is all zeros is left unrecorded rather than stored as 0. Orca-exported 3MFs keep parsing through the generic spelling, which is now the fallback rather than the only thing tried. **Archives you already have are repaired too**: a one-shot pass at startup re-reads the 3MF still on disk for every archive whose bed temperature is blank, so preheat gets the right figure when you reprint an old job from the queue rather than only on new ones. It fills blanks and nothing else — a temperature already recorded is never overwritten, an archive whose file is gone stays blank, and a corrupted 3MF is skipped rather than failing the boot. The plate mapping is now shared between the ingest path and the repair so the two cannot read a 3MF differently. A dead copy of the old, wrong lookup that nothing called was removed at the same time, so it cannot be wired back up by accident. 36 regression tests. - **A manual K-profile calibration left a print in your archive** — Bambuddy already recognises the printer's automatic pressure-advance run, `auto_pa_line_calib_mode`, and skips archiving and notifying for it. Started by hand instead of automatically before a print, the same calibration reports under its own name with no `auto_` prefix — and manual flow dynamics has *two* shapes, a line and a pattern, so neither `pa_line_calib_mode` nor `pa_pattern_calib_mode` matched anything. Both arrive exactly the way the automatic one does — a bare subtask name with no `/usr/` path — which meant the archive path swept FTP for a 3MF that cannot exist and then wrote a no-3MF archive named after the calibration, on a printer that was in the middle of calibrating. Both are now on the same list, which is the one place both the print-start and print-complete callbacks consult. Matching stays exact after normalising path, suffix and case, so a file you deliberately named `pa_pattern_calib_mode_v2.3mf` — or your own `pa_bracket.3mf` — is still archived as the print it is. Wiki updated. - **A spool you assigned to an AMS slot unassigned itself seconds later (#2987, reported by @frethop)** — and the slot's colour changed at the same time. It looked like Bambu Studio and Bambuddy fighting over the slot; the reporter's log shows Bambuddy losing to itself. P1S firmware 01.10.00.00 reads every **lowercase** hex letter in an AMS `tray_color` as a zero, and hides it completely: the command response echoes back the value you sent and reports `result: "success"`, so only the next AMS push reveals what was really stored. The spool-assign path sent `spool.rgba` verbatim, and that column stores lowercase — so `09ff00ff` became `09000000` on the printer and `ff5100ff` became `00510000`, while the one uppercase write in the same window round-tripped intact. That is the visible colour change. It is also what deleted the assignment: the auto-unlink sweep asks whether the slot still matches the spool it is assigned to, the mangled colour no longer did, and the assignment Bambuddy had created four seconds earlier was removed. Re-assigning could not help, because the **Configure Slot** dialog seeds its colour from whatever the printer currently reports — so it wrote the mangled colour straight back and cemented it, which is the loop in the report's steps 4 and 5. Colours are now uppercased at the single point the MQTT command is assembled rather than in each of the four routes that configure a slot, because a caller that forgets is exactly how this arrived. Nothing else about the command changes: no padding, no invented alpha, and `tray_type` / `tray_sub_brands` keep their case, where it carries meaning. **Two more things found in the same log.** A spool with a brand but no subtype was configured with the literal string `None` in its name — `"Sunlu PLA Matte None"` went on the wire, because the branded branch interpolated the subtype without checking it while the unbranded branch guarded it. And the FTP log is now readable: a `426` whose bytes Bambuddy has verified against the printer is how Bambu's FTPS normally ends a transfer, not a fault, so it is logged at INFO instead of WARNING. It fired 54 times in this one bundle, every single one followed by a completed upload, and it was burying the 26 TLS handshake failures in the same log that actually cost the reporter two prints. A `426` whose bytes do **not** verify is still an error and still fails the upload. 24 regression tests. - **One ASA spool parked in the AMS added 20 minutes to every PLA print (#2886, reported by @FirstRulez)** — Preheat's chamber target was the maximum across *every loaded AMS tray*, with no reference to the job. The reporter's P2S holds PETG Pro, PLA, ASA and PETG; the ASA row of the filament map says 45°C, so a PLA-only plate was dispatched with `chamber_target=45°C`, the bed driven to 90°C to reach it, and the full 900s max-wait plus 300s soak burned before the upload even started — every time, because a P2S has no chamber heater and the chamber tops out around 33°C, so the wait can only ever end on the timeout. Their log carries fifteen of these. The intent was never in doubt: the resolution order documented one screen above reads "PLA-only print derives 0 → chamber phase auto-skips", but it was implemented as PLA-only **AMS** rather than PLA-only **print**, and only misfires on a mixed-material load. The derivation now reads the trays the item's `ams_mapping` actually names — the same array the print command puts on the wire, `[-1, -1, -1, 1]` in their case, addressing exactly the PLA slot — so the ASA two slots over contributes nothing and the stage skips outright. Multi-material prints are unaffected: the maximum is still taken, just across the trays the plate loads, so an ASA the print really does use is still the binding constraint. An item whose mapping is missing or still unresolved keeps the whole-unit scan, since that is the only signal left and narrowing to nothing would disable preheat for prints that need it. The bed hold between jobs (`queue_keep_bed_warm`) is gated on the same derivation and was holding beds at 90°C for the same wrong reason; it now reads the next item's mapping too. **The external spool is no longer invisible to this**: the scan only ever looked at `raw_data['ams']`, so an ASA print fed from the external feed derived 0 and got no preheat at all — a mapping that names 254/255 is now honoured, while an item with no mapping still derives from the AMS alone so nothing starts preheating that did not before. That covers the mappings Bambuddy builds itself, from the print dialog or the dispatcher's own matcher. It does not cover a mapping captured from a slicer through a Virtual Printer: BambuStudio writes the external spool as `-1` there, which is the same value it writes for a slot the plate does not use, so the two cannot be told apart. A wholly external one carries no usable mapping and falls back to the whole-unit scan as before; a mixed one derives from its AMS trays and the external half stays unread — which is exactly what it did before this change, since nothing ever read `vt_tray`. 29 regression tests, built from the trays and mapping in the reporter's own support bundle. diff --git a/backend/app/core/database.py b/backend/app/core/database.py index a430c916a..22fdd3428 100644 --- a/backend/app/core/database.py +++ b/backend/app/core/database.py @@ -887,6 +887,94 @@ async def _migrate_scope_run_filament_to_plate(conn) -> None: ) +async def _backfill_archive_bed_temperature(conn) -> None: + """Fill in ``print_archives.bed_temperature`` for archives written before #2989. + + Bed temperature was read by looking for a ``bed_temperature`` key, which + BambuStudio does not write -- it stores a per-filament array per plate type + and names the fitted plate in ``curr_bed_type``. Every archive from a Bambu + slice therefore stored NULL: 0 of 455 real 3MFs resolved on the install this + was measured on. The forward fix reads the right array; without this, every + archive made before it stays blank, and preheat keeps falling back to the + keep-warm bed temperature when those jobs are reprinted from the queue. + + Only rows that are still NULL are touched, and only from the 3MF already on + disk -- nothing is invented and nothing already recorded is overwritten. An + archive whose file is gone (a no-3MF fallback, or one whose 3MF has been + cleaned up) is skipped and stays NULL, which is the honest answer. + + Gated to run exactly once via a settings flag, like #2614's repair. The + work itself is repeatable -- it only fills NULLs -- but the rows it cannot + fill are exactly the ones it would re-open on every boot, and that set grows + with print history. + """ + from pathlib import Path + + from sqlalchemy import text + + from backend.app.utils.threemf_tools import extract_bed_temperature_from_3mf + + flag = "_backfill_2989_bed_temperature_done" + + async with conn.begin_nested(): + already = ( + await conn.execute(text('SELECT value FROM settings WHERE "key" = :k'), {"k": flag}) + ).scalar_one_or_none() + if already is not None: + # Presence, not truthiness. A flag row that somehow holds an empty + # string would otherwise re-run and then fail the unique key on the + # INSERT below -- which, at startup, is a boot loop. + return + + rows = ( + await conn.execute( + text( + "SELECT id, file_path FROM print_archives " + "WHERE bed_temperature IS NULL " + "AND file_path IS NOT NULL AND file_path != ''" + ) + ) + ).fetchall() + + filled = 0 + for row in rows: + # Per row, and broad, for the reason in the extractor's docstring: + # nothing above this has a handler, so one unreadable archive must + # not cost the user their boot. #2614's repair guards its rows the + # same way. + try: + path = Path(row.file_path) + if not path.is_absolute(): + path = settings.base_dir / row.file_path + if not path.exists(): + continue + temperature = extract_bed_temperature_from_3mf(path) + except Exception as exc: + logger.warning("[#2989] could not read %s for archive %s: %s", row.file_path, row.id, exc) + continue + if not temperature: + continue + await conn.execute( + text("UPDATE print_archives SET bed_temperature = :t WHERE id = :id"), + {"t": temperature, "id": row.id}, + ) + filled += 1 + + if filled: + logger.info( + "[#2989] Read the bed temperature from the 3MF for %d archive(s) that had none", + filled, + ) + + # Marked done even when nothing matched, so the rows it could not fill -- + # which are exactly the ones it would re-open every boot -- are not + # rescanned forever. Same shape as #2614's one-shot. + await conn.execute( + text('INSERT INTO settings ("key", value) VALUES (:k, :v)'), + {"k": flag, "v": "true"}, + ) + + async def _migrate_drop_library_print_name(conn) -> None: """Strip the embedded 3MF Title (``print_name``) from library file metadata (#1489). @@ -4636,6 +4724,11 @@ async def run_migrations(conn): # #2603 archive plate_id backfill above so print_archives.plate_id is populated. await _migrate_scope_run_filament_to_plate(conn) + # Backfill: archives written before #2989 have no bed temperature, because + # the extractor looked for a key BambuStudio never writes. Re-reads the 3MF + # already on disk. One-shot; see the function for why it is gated. + await _backfill_archive_bed_temperature(conn) + # Migration: Add controls_printer_power to smart_plugs (#2629). Marks # whether a plug actually feeds the printer's own power — only then may an # auto-off mark the printer offline. Defaults to true so existing plugs diff --git a/backend/app/services/archive.py b/backend/app/services/archive.py index 48c49d540..77e3ca183 100644 --- a/backend/app/services/archive.py +++ b/backend/app/services/archive.py @@ -22,56 +22,11 @@ from backend.app.utils.archive_paths import archive_dir as resolve_archive_dir from backend.app.utils.ffmpeg_output import NO_FFMPEG_OUTPUT, summarize_ffmpeg_stderr from backend.app.utils.filename import clean_display_name from backend.app.utils.safe_path import PathTraversalError, assert_under, safe_join_under +from backend.app.utils.threemf_tools import bed_temperature_from_config logger = logging.getLogger(__name__) -# Bed temperature is not one key in a BambuStudio project. Every plate type has -# its own per-filament array, and the plate actually fitted is named separately -# in ``curr_bed_type`` -- so reading a bed temperature means picking the array -# the plate points at. Keys and mapping are BambuStudio's own -# ``get_bed_temp_1st_layer_key`` / ``get_bed_temp_key`` (PrintConfig.hpp), and -# the plate names are the ``curr_bed_type`` enum values (PrintConfig.cpp). -# First-layer temperature first: that is what the printer heats to before the -# print starts, which is what preheat is trying to reach. -# -# ``Default Plate`` is deliberately absent -- BambuStudio maps it to no key at -# all, so there is nothing to read and guessing a plate would invent a bed -# temperature the slice never specified. -_BED_TEMP_KEYS: dict[str, tuple[str, str]] = { - "Cool Plate": ("cool_plate_temp_initial_layer", "cool_plate_temp"), - "Engineering Plate": ("eng_plate_temp_initial_layer", "eng_plate_temp"), - "High Temp Plate": ("hot_plate_temp_initial_layer", "hot_plate_temp"), - "Textured PEI Plate": ("textured_plate_temp_initial_layer", "textured_plate_temp"), - "Supertack Plate": ("supertack_plate_temp_initial_layer", "supertack_plate_temp"), -} - -# Fallback for a config that names no plate: the Orca/PrusaSlicer spelling, -# which is a single value rather than a per-plate array. -_GENERIC_BED_TEMP_KEYS = ("bed_temperature_initial_layer", "bed_temperature") - - -def _plate_temperature(val) -> int | None: - """Bed temperature from one plate-temperature entry, or None. - - The plate arrays carry one entry per filament in the project, and a 0 means - that filament cannot print on this plate. The bed only has one temperature, - so the print runs at the highest its filaments ask for -- taking entry 0 the - way the neighbouring scalar settings do would store a 0 for any project - whose first filament is not one this plate is heated for. - """ - values = val if isinstance(val, list) else [val] - temps = [] - for entry in values: - if isinstance(entry, bool) or not isinstance(entry, (int, float, str)): - continue - try: - temps.append(int(float(entry))) - except (TypeError, ValueError): - continue - return max(temps) if temps else None - - def _copy_and_fsync(src: Path, dst: Path, chunk_size: int = 1024 * 1024) -> None: """Copy src to dst with an explicit chunked read/write and fsync the dst. @@ -550,17 +505,9 @@ class ThreeMFParser: # not write -- so every archive from a Bambu slice stored NULL, and # preheat fell back to a configured bed temperature on every job # (#2989). Orca-exported 3MFs keep working through the generic keys. - bed_type = str(data.get("curr_bed_type") or "").strip() - for key in (*_BED_TEMP_KEYS.get(bed_type, ()), *_GENERIC_BED_TEMP_KEYS): - if key not in data: - continue - temperature = _plate_temperature(data[key]) - # A plate array of all zeros means no filament in the project - # prints on this plate, which is not a bed temperature -- keep - # looking rather than recording a 0 that reads as "cold bed". - if temperature: - self.metadata["bed_temperature"] = temperature - break + bed_temperature = bed_temperature_from_config(data) + if bed_temperature is not None: + self.metadata["bed_temperature"] = bed_temperature # Nozzle temperature for key in ["nozzle_temperature_initial_layer", "nozzle_temperature"]: @@ -587,30 +534,6 @@ class ThreeMFParser: except Exception: pass # Print settings are optional; missing values are left unset - def _extract_settings_from_content(self, content: str): - """Extract print settings from config content.""" - settings_map = { - "layer_height": ("layer_height", float), - "nozzle_diameter": ("nozzle_diameter", float), - "bed_temperature": ("bed_temperature", int), - "nozzle_temperature": ("nozzle_temperature", int), - } - - for key, (search_key, converter) in settings_map.items(): - if key not in self.metadata: - try: - # Try JSON format - if f'"{search_key}"' in content: - start = content.find(f'"{search_key}"') - value_start = content.find(":", start) + 1 - value_end = content.find(",", value_start) - if value_end == -1: - value_end = content.find("}", value_start) - value = content[value_start:value_end].strip().strip('"') - self.metadata[key] = converter(value) - except (ValueError, TypeError): - pass # Skip settings with unconvertible values - def _parse_3dmodel(self, zf: zipfile.ZipFile): """Parse 3D/3dmodel.model for MakerWorld metadata.""" try: diff --git a/backend/app/utils/threemf_tools.py b/backend/app/utils/threemf_tools.py index ca016f05a..f7742cf88 100644 --- a/backend/app/utils/threemf_tools.py +++ b/backend/app/utils/threemf_tools.py @@ -1025,6 +1025,96 @@ def extract_bed_type_from_3mf(file_path: Path, plate_id: int | None = None) -> s return extract_plate_metadata_from_3mf(file_path, plate_id).bed_type +# Bed temperature is not one key in a BambuStudio project. Every plate type has +# its own per-filament array, and the plate actually fitted is named separately +# in ``curr_bed_type`` -- so reading a bed temperature means picking the array +# the plate points at. Keys and mapping are BambuStudio's own +# ``get_bed_temp_1st_layer_key`` / ``get_bed_temp_key`` (PrintConfig.hpp), and +# the plate names are the ``curr_bed_type`` enum values (PrintConfig.cpp). +# First-layer temperature first: that is what the printer heats to before the +# print starts, which is what preheat is trying to reach. +# +# ``Default Plate`` is deliberately absent -- BambuStudio maps it to no key at +# all, so there is nothing to read and guessing a plate would invent a bed +# temperature the slice never specified. +_BED_TEMP_KEYS: dict[str, tuple[str, str]] = { + "Cool Plate": ("cool_plate_temp_initial_layer", "cool_plate_temp"), + "Engineering Plate": ("eng_plate_temp_initial_layer", "eng_plate_temp"), + "High Temp Plate": ("hot_plate_temp_initial_layer", "hot_plate_temp"), + "Textured PEI Plate": ("textured_plate_temp_initial_layer", "textured_plate_temp"), + "Supertack Plate": ("supertack_plate_temp_initial_layer", "supertack_plate_temp"), +} + +# Fallback for a config that names no plate: the Orca/PrusaSlicer spelling, +# which is a single value rather than a per-plate array. +_GENERIC_BED_TEMP_KEYS = ("bed_temperature_initial_layer", "bed_temperature") + + +def _plate_temperature(val) -> int | None: + """Bed temperature from one plate-temperature entry, or None. + + The plate arrays carry one entry per filament in the project, and a 0 means + that filament cannot print on this plate. The bed only has one temperature, + so the print runs at the highest its filaments ask for -- taking entry 0 the + way the neighbouring scalar settings do would store a 0 for any project + whose first filament is not one this plate is heated for. + """ + values = val if isinstance(val, list) else [val] + temps = [] + for entry in values: + if isinstance(entry, bool) or not isinstance(entry, (int, float, str)): + continue + try: + temps.append(int(float(entry))) + except (TypeError, ValueError): + continue + return max(temps) if temps else None + + +def bed_temperature_from_config(data: dict) -> int | None: + """Bed temperature for the plate *data* is sliced for, or None (#2989). + + *data* is a parsed ``Metadata/project_settings.config``. Lives here rather + than beside the archive parser so the ingest path and the one-shot backfill + that repairs archives written before the fix read it exactly the same way. + """ + bed_type = str(data.get("curr_bed_type") or "").strip() + for key in (*_BED_TEMP_KEYS.get(bed_type, ()), *_GENERIC_BED_TEMP_KEYS): + if key not in data: + continue + temperature = _plate_temperature(data[key]) + # A plate array of all zeros means no filament in the project prints on + # this plate, which is not a bed temperature -- keep looking rather than + # recording a 0 that reads as "cold bed". + if temperature: + return temperature + return None + + +def extract_bed_temperature_from_3mf(file_path: Path) -> int | None: + """Read a 3MF's bed temperature straight off disk, or None. + + For the backfill, which has a ``file_path`` and nothing else. Opens only + ``Metadata/project_settings.config`` -- the archive parser reads thumbnails, + the model and the slice info as well, and none of that is wanted here. + + Every failure is None. Deliberately broader than the handful of exceptions a + malformed zip is expected to raise: the caller runs inside the startup + migration, which has no handler above it, so anything unlisted escaping here + does not skip one archive -- it stops Bambuddy from booting, and keeps + stopping it, because the one-shot flag is written in the same transaction + that just rolled back. A bed temperature is not worth that. + """ + try: + with zipfile.ZipFile(file_path, "r") as zf: + if "Metadata/project_settings.config" not in zf.namelist(): + return None + data = json.loads(zf.read("Metadata/project_settings.config").decode()) + return bed_temperature_from_config(data) if isinstance(data, dict) else None + except Exception: + return None + + # Header values exposed as `{placeholder}` substitutions inside snippets. # Aliases let users write Prusa-style names (`{max_layer_z}`) that map onto # Bambu/Orca header keys (`max_z_height`). diff --git a/backend/tests/unit/test_bed_temperature_backfill_2989.py b/backend/tests/unit/test_bed_temperature_backfill_2989.py new file mode 100644 index 000000000..2b00e3c76 --- /dev/null +++ b/backend/tests/unit/test_bed_temperature_backfill_2989.py @@ -0,0 +1,376 @@ +"""Archives written before #2989 get their bed temperature read back off disk. + +The extractor looked for a ``bed_temperature`` key BambuStudio does not write, +so every archive from a Bambu slice stored NULL -- 0 of 455 real 3MFs resolved +on the install this was measured on. The forward fix reads the array the fitted +plate points at, but only for archives made after it; everything already in the +library stays blank, and preheat keeps falling back to the keep-warm bed +temperature whenever one of those jobs is reprinted from the queue. + +This one-shot re-reads the 3MF that is already on disk. It fills NULLs and +nothing else: no value is invented, none is overwritten, and an archive whose +file is gone stays NULL rather than being guessed at. + +The 3MFs here are real zips rather than a patched extractor, because +``extract_bed_temperature_from_3mf`` is itself new code and stubbing it would +leave the only thing this migration depends on untested. +""" + +import json +import logging +import zipfile + +import pytest +from sqlalchemy import text +from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine + +import backend.app.models # noqa: F401 - populate Base.metadata +from backend.app.core import database as database_module +from backend.app.core.database import Base, _backfill_archive_bed_temperature +from backend.app.models.archive import PrintArchive + +# A Textured PEI slice of a two-filament project, in the shape BambuStudio +# writes: an array per plate type, and the fitted plate named separately. +_PEI_55 = { + "curr_bed_type": "Textured PEI Plate", + "cool_plate_temp": ["0", "0"], + "eng_plate_temp": ["0", "0"], + "hot_plate_temp": ["0", "0"], + "textured_plate_temp_initial_layer": ["55", "55"], + "textured_plate_temp": ["55", "55"], + "supertack_plate_temp": ["0", "0"], +} + + +@pytest.fixture +async def engine(tmp_path): + eng = create_async_engine(f"sqlite+aiosqlite:///{tmp_path}/t.db") + async with eng.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + try: + yield eng + finally: + await eng.dispose() + + +@pytest.fixture +def data_dir(tmp_path, monkeypatch): + monkeypatch.setattr(database_module.settings, "base_dir", tmp_path) + return tmp_path + + +def _write_3mf(data_dir, relative: str, config: dict | None) -> str: + """A 3MF on disk. ``config=None`` writes a file that is not a zip at all.""" + path = data_dir / relative + path.parent.mkdir(parents=True, exist_ok=True) + if config is None: + path.write_bytes(b"not a zip") + return relative + with zipfile.ZipFile(path, "w") as zf: + zf.writestr("Metadata/project_settings.config", json.dumps(config)) + return relative + + +async def _archive(db, file_path: str, *, bed_temperature=None) -> PrintArchive: + archive = PrintArchive( + filename="Benchy.gcode.3mf", + file_path=file_path, + file_size=1, + status="completed", + bed_temperature=bed_temperature, + ) + db.add(archive) + await db.flush() + return archive + + +async def _bed_temperature(engine, archive_id: int): + async with engine.begin() as conn: + return ( + await conn.execute(text("SELECT bed_temperature FROM print_archives WHERE id = :id"), {"id": archive_id}) + ).scalar_one() + + +class TestItFillsWhatItCan: + @pytest.mark.asyncio + async def test_a_null_is_read_from_the_plate_the_project_is_sliced_for(self, engine, data_dir): + sm = async_sessionmaker(engine, expire_on_commit=False) + async with sm() as db: + relative = _write_3mf(data_dir, "archive/1/20260828_Benchy/Benchy.gcode.3mf", _PEI_55) + archive = await _archive(db, relative) + await db.commit() + + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + assert await _bed_temperature(engine, archive.id) == 55 + + @pytest.mark.asyncio + async def test_an_orca_export_still_resolves(self, engine, data_dir): + """The generic spelling is the fallback, not a second-class citizen.""" + sm = async_sessionmaker(engine, expire_on_commit=False) + async with sm() as db: + relative = _write_3mf(data_dir, "archive/1/a/Benchy.gcode.3mf", {"bed_temperature": 60}) + archive = await _archive(db, relative) + await db.commit() + + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + assert await _bed_temperature(engine, archive.id) == 60 + + @pytest.mark.asyncio + async def test_several_archives_in_one_pass(self, engine, data_dir): + sm = async_sessionmaker(engine, expire_on_commit=False) + async with sm() as db: + first = await _archive(db, _write_3mf(data_dir, "archive/1/a/x.3mf", _PEI_55)) + second = await _archive( + db, + _write_3mf( + data_dir, "archive/1/b/y.3mf", {"curr_bed_type": "High Temp Plate", "hot_plate_temp": ["100"]} + ), + ) + await db.commit() + + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + assert await _bed_temperature(engine, first.id) == 55 + assert await _bed_temperature(engine, second.id) == 100 + + +class TestWhatItRefusesToTouch: + @pytest.mark.asyncio + async def test_a_value_already_recorded_is_left_alone(self, engine, data_dir): + """Only NULLs. A temperature somebody set, or one a later archive read + correctly, must not be rewritten from the file.""" + sm = async_sessionmaker(engine, expire_on_commit=False) + async with sm() as db: + relative = _write_3mf(data_dir, "archive/1/a/Benchy.gcode.3mf", _PEI_55) + archive = await _archive(db, relative, bed_temperature=90) + await db.commit() + + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + assert await _bed_temperature(engine, archive.id) == 90 + + @pytest.mark.asyncio + async def test_a_no_3mf_archive_stays_null(self, engine, data_dir): + """``file_path == ""`` is the ordinary shape of a Studio-sent H2 print. + There is no file to read, and inventing one is the whole bug.""" + sm = async_sessionmaker(engine, expire_on_commit=False) + async with sm() as db: + archive = await _archive(db, "") + await db.commit() + + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + assert await _bed_temperature(engine, archive.id) is None + + @pytest.mark.asyncio + async def test_a_file_that_is_gone_stays_null(self, engine, data_dir): + sm = async_sessionmaker(engine, expire_on_commit=False) + async with sm() as db: + archive = await _archive(db, "archive/1/a/deleted.gcode.3mf") + await db.commit() + + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + assert await _bed_temperature(engine, archive.id) is None + + @pytest.mark.asyncio + async def test_a_file_that_is_not_a_zip_stays_null(self, engine, data_dir): + """A truncated or corrupted 3MF must not take the whole boot down.""" + sm = async_sessionmaker(engine, expire_on_commit=False) + async with sm() as db: + archive = await _archive(db, _write_3mf(data_dir, "archive/1/a/broken.3mf", None)) + await db.commit() + + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + assert await _bed_temperature(engine, archive.id) is None + + @pytest.mark.asyncio + async def test_an_all_zero_plate_array_stays_null(self, engine, data_dir): + """0 means no filament in the project prints on this plate. Recording + it would read as a cold bed, which is worse than nothing.""" + sm = async_sessionmaker(engine, expire_on_commit=False) + async with sm() as db: + relative = _write_3mf( + data_dir, + "archive/1/a/zero.3mf", + {"curr_bed_type": "Cool Plate", "cool_plate_temp": ["0", "0"]}, + ) + archive = await _archive(db, relative) + await db.commit() + + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + assert await _bed_temperature(engine, archive.id) is None + + +class TestItCannotStopBambuddyBooting: + """The migration sequence has no handler above it. + + ``run_migrations`` is awaited straight from ``init_db`` with no try/except, + so anything escaping this function stops startup -- and keeps stopping it, + because the one-shot flag is written inside the transaction that just rolled + back. Measured: two consecutive boots, same failure, flag never written. So + the guards here are load-bearing rather than tidy. + """ + + @pytest.mark.asyncio + async def test_an_unexpected_exception_costs_one_archive_not_the_boot(self, engine, data_dir, monkeypatch, caplog): + """Not a listed zip error -- the point is that the guard does not depend + on having predicted which exception a bad file raises.""" + import backend.app.utils.threemf_tools as tools + + sm = async_sessionmaker(engine, expire_on_commit=False) + async with sm() as db: + bad = await _archive(db, _write_3mf(data_dir, "archive/1/a/bad.3mf", _PEI_55)) + await db.commit() + + def _explode(_path): + raise RecursionError("boom") + + monkeypatch.setattr(tools, "extract_bed_temperature_from_3mf", _explode) + + with caplog.at_level(logging.WARNING): + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + assert await _bed_temperature(engine, bad.id) is None + assert any("could not read" in r.getMessage() for r in caplog.records) + + @pytest.mark.asyncio + async def test_one_bad_archive_does_not_stop_the_others(self, engine, data_dir, monkeypatch): + """The guard is per row, so the rest of the library is still repaired.""" + import backend.app.utils.threemf_tools as tools + + sm = async_sessionmaker(engine, expire_on_commit=False) + async with sm() as db: + bad = await _archive(db, _write_3mf(data_dir, "archive/1/a/bad.3mf", _PEI_55)) + good = await _archive(db, _write_3mf(data_dir, "archive/1/b/good.3mf", _PEI_55)) + await db.commit() + + real = tools.extract_bed_temperature_from_3mf + + def _explode_on_bad(path): + if path.name == "bad.3mf": + raise RecursionError("boom") + return real(path) + + monkeypatch.setattr(tools, "extract_bed_temperature_from_3mf", _explode_on_bad) + + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + assert await _bed_temperature(engine, bad.id) is None + assert await _bed_temperature(engine, good.id) == 55 + + @pytest.mark.asyncio + async def test_the_extractor_swallows_anything_a_file_can_throw(self, tmp_path): + """Its callers are inside startup, so None is the only outcome.""" + from backend.app.utils.threemf_tools import extract_bed_temperature_from_3mf + + missing = tmp_path / "nope.3mf" + directory = tmp_path / "adir.3mf" + directory.mkdir() + truncated = tmp_path / "cut.3mf" + truncated.write_bytes(b"PK\x03\x04 and then nothing") + empty = tmp_path / "empty.3mf" + empty.write_bytes(b"") + + for candidate in (missing, directory, truncated, empty): + assert extract_bed_temperature_from_3mf(candidate) is None + + @pytest.mark.asyncio + async def test_the_extractor_swallows_what_no_one_predicted(self, tmp_path, monkeypatch): + """The four cases above all raise OSError or BadZipFile, so on their own + they would still pass with the guard narrowed back to that pair. This + one forces something outside it, which is the whole reason the catch is + broad -- the failure being guarded is an exception nobody listed.""" + import backend.app.utils.threemf_tools as tools + + good = tmp_path / "ok.3mf" + with zipfile.ZipFile(good, "w") as zf: + zf.writestr("Metadata/project_settings.config", json.dumps(_PEI_55)) + assert tools.extract_bed_temperature_from_3mf(good) == 55 + + class _Exploding: + def __init__(self, *a, **k): + raise RecursionError("boom") + + monkeypatch.setattr(tools.zipfile, "ZipFile", _Exploding) + + assert tools.extract_bed_temperature_from_3mf(good) is None + + @pytest.mark.asyncio + async def test_a_flag_row_with_an_empty_value_does_not_re_run(self, engine, data_dir): + """``if already:`` would treat "" as not-done, re-run, and then fail the + unique key on the INSERT -- a boot loop from a single odd row.""" + async with engine.begin() as conn: + await conn.execute( + text('INSERT INTO settings ("key", value) VALUES (:k, :v)'), + {"k": "_backfill_2989_bed_temperature_done", "v": ""}, + ) + + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + +class TestItRunsExactlyOnce: + @pytest.mark.asyncio + async def test_the_flag_is_written_even_when_nothing_matched(self, engine, data_dir): + """The rows it cannot fill are the ones it would reopen every boot.""" + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + async with engine.begin() as conn: + flag = ( + await conn.execute( + text('SELECT value FROM settings WHERE "key" = :k'), + {"k": "_backfill_2989_bed_temperature_done"}, + ) + ).scalar_one_or_none() + + assert flag == "true" + + @pytest.mark.asyncio + async def test_a_second_boot_does_not_rescan(self, engine, data_dir): + """An archive added after the one-shot has run is left to the forward + fix, which is what writes bed_temperature for anything new.""" + sm = async_sessionmaker(engine, expire_on_commit=False) + async with sm() as db: + first = await _archive(db, _write_3mf(data_dir, "archive/1/a/x.3mf", _PEI_55)) + await db.commit() + + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + assert await _bed_temperature(engine, first.id) == 55 + + async with sm() as db: + later = await _archive(db, _write_3mf(data_dir, "archive/1/b/y.3mf", _PEI_55)) + await db.commit() + + async with engine.begin() as conn: + await _backfill_archive_bed_temperature(conn) + + assert await _bed_temperature(engine, later.id) is None + # And the flag was not written twice, which the settings table's unique + # key would refuse anyway -- the guard is the SELECT, not the database. + async with engine.begin() as conn: + count = ( + await conn.execute( + text('SELECT COUNT(*) FROM settings WHERE "key" = :k'), + {"k": "_backfill_2989_bed_temperature_done"}, + ) + ).scalar_one() + assert count == 1