mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-20 09:03:43 +02:00
Explain a print that never reached the printer's card, instead of sweeping for it (#2780)
Bambuddy reads a print's 3MF, cover and timelapse over FTPS on port 990, which on every Bambu model serves external storage only. Under some configurations H2-series and P2S firmware keeps the sliced file on internal storage, where Bambu Studio put it over the port-6000 service, and then no path on 990 can find it. The print command has always said which of the two it used -- `url` reads ftp://<name> or brtc://emmc/<name>. We discarded it and swept anyway: ~110 connections per print, all certain to fail, ending in an archive card with nothing on it and no stated reason. In the reporter's bundle all 35 dispatches to their H2C and P2S said internal storage, all 25 to their X1C said external, and all 44 empty cards belonged to the first two. Read the field, skip the sweep when it cannot succeed, and record which reason applied. A printer that uses the card is unaffected, and so is one we have no answer for -- silence is not evidence, and reading it as bad news would break archives that work today. The answer is held per print and dropped when that print ends, rather than kept as a standing fact about the printer. Plenty of prints never announce themselves: 14 of the 79 print starts in that bundle arrived with nothing on the request topic, started from the printer's own screen or picked up after a restart. Left standing, one slicer print to internal storage would suppress the lookup for every screen-started print after it, on a printer whose files really are on the card. The sticky reading is kept for the connection diagnostic alone, which is run after the print that prompted it and would otherwise have nothing to report. Two things that pointed the wrong way go with it. The archives banner told everyone to enable "Store sent files on external storage"; the reporter had it on for the whole three weeks and it would not have helped. The diagnostic passed a printer whose slot was empty, because it read only the toggle -- an empty slot is now a failure naming the slot, and a printer that has storage and still used its own is a warning. On P1-series that empty-slot failure yields to the existing unsupported-model skip: the toggle cannot be switched on there at all, so telling the operator to insert a card would promise a fix inserting a card does not deliver (#2524). Also close FTP sockets on the failure paths, which dropped them for the garbage collector -- 1813 in a day in that bundle -- and drop the advice to restart the printer, which the reporter tried twice while a single manual connection to the same printer handshook cleanly. This does not make the affected prints archive in full; that needs the port-6000 protocol tracked in #2762.
This commit is contained in:
@@ -31,6 +31,7 @@ from backend.app.schemas.slicer import SliceRequest
|
||||
from backend.app.services.archive import ArchiveService
|
||||
from backend.app.services.design_settings import overrides_from_config
|
||||
from backend.app.services.filament_requirements import annotate_rack_groups
|
||||
from backend.app.services.print_storage import REASON_INTERNAL_STORAGE, REASON_NO_EXTERNAL_STORAGE
|
||||
from backend.app.utils.http import build_content_disposition
|
||||
from backend.app.utils.safe_path import safe_join_under
|
||||
from backend.app.utils.threemf_tools import (
|
||||
@@ -512,10 +513,23 @@ async def no_3mf_warning(
|
||||
)
|
||||
),
|
||||
):
|
||||
"""Whether to nudge the user about install step 4 ("Store sent files on
|
||||
external storage"). True iff any archive in the last 30 days was created
|
||||
via the no-3MF fallback path — that's the deterministic symptom of the
|
||||
slicer-side variant of the setting being off.
|
||||
"""Whether to nudge the user about a print that archived without its 3MF,
|
||||
and why. True iff any archive in the last 30 days was created via the
|
||||
no-3MF fallback path.
|
||||
|
||||
Also returns ``reason``, because the advice differs and the original
|
||||
single-cause wording sent people the wrong way. Historically the only
|
||||
known cause was install step 4 ("Store sent files on external storage")
|
||||
being off in the slicer, so the banner said so unconditionally. On
|
||||
H2-series and P2S that advice is actively wrong: the setting is already on
|
||||
and turning it on again changes nothing, because the printer keeps the
|
||||
sliced file on internal storage that FTPS does not serve at all (#2780).
|
||||
|
||||
``reason`` is the slug from :mod:`print_storage` when we recorded one,
|
||||
else None for the original slicer-setting case. When archives disagree the
|
||||
most specific known reason wins — one printer storing internally is a real
|
||||
finding worth explaining, and it should not be masked by another printer's
|
||||
plain missing-file fallback.
|
||||
|
||||
Complements the connection-diagnostic ``external_storage`` check, which
|
||||
only catches the printer-side variant of the setting. On older slicers
|
||||
@@ -536,10 +550,24 @@ async def no_3mf_warning(
|
||||
if user is not None and not can_read_all:
|
||||
conditions.append(PrintArchive.created_by_id == user.id)
|
||||
result = await db.execute(select(PrintArchive.extra_data).where(*conditions))
|
||||
reasons: set[str] = set()
|
||||
has_fallback = False
|
||||
for (extra_data,) in result.all():
|
||||
if extra_data and extra_data.get("no_3mf_available"):
|
||||
return {"has_fallback": True}
|
||||
return {"has_fallback": False}
|
||||
if not extra_data or not extra_data.get("no_3mf_available"):
|
||||
continue
|
||||
has_fallback = True
|
||||
reason = extra_data.get("no_3mf_reason")
|
||||
if reason:
|
||||
reasons.add(reason)
|
||||
if not has_fallback:
|
||||
return {"has_fallback": False, "reason": None}
|
||||
# Most specific first. Archives predating this field carry no reason at
|
||||
# all, so an install with one H2C and three older printers still gets the
|
||||
# H2C explanation rather than the generic one.
|
||||
for candidate in (REASON_INTERNAL_STORAGE, REASON_NO_EXTERNAL_STORAGE):
|
||||
if candidate in reasons:
|
||||
return {"has_fallback": True, "reason": candidate}
|
||||
return {"has_fallback": True, "reason": None}
|
||||
|
||||
|
||||
@router.get("/slim", response_model=list[ArchiveSlim])
|
||||
@@ -2347,14 +2375,14 @@ async def scan_timelapse(
|
||||
if not files:
|
||||
# "Couldn't reach the printer" and "the printer has no timelapse
|
||||
# directory" are different problems with different fixes, and both used
|
||||
# to come back as one 500 (#2780). A printer whose file service stopped
|
||||
# answering over TLS needs a restart, and nothing here will work until
|
||||
# it gets one.
|
||||
# to come back as one 500 (#2780). Nothing here will work while the
|
||||
# printer's file service is not answering over TLS, so say that rather
|
||||
# than reporting an empty directory.
|
||||
if ftps_handshake_blocked(printer.ip_address):
|
||||
raise HTTPException(
|
||||
503,
|
||||
f"Printer {printer.ip_address} is not answering its file service over TLS. "
|
||||
"Restart the printer and try again.",
|
||||
"Bambuddy will try again shortly.",
|
||||
)
|
||||
raise HTTPException(404, "No timelapse directory found on the printer")
|
||||
|
||||
|
||||
@@ -51,6 +51,7 @@ from backend.app.services.bambu_ftp import (
|
||||
get_storage_info_async,
|
||||
list_files_async,
|
||||
)
|
||||
from backend.app.services.print_storage import print_file_reachable_over_ftp
|
||||
from backend.app.services.printer_diagnostic import run_connection_diagnostic
|
||||
from backend.app.services.printer_manager import (
|
||||
display_temperatures,
|
||||
@@ -1217,6 +1218,19 @@ async def _produce_cover_image(
|
||||
break
|
||||
|
||||
if not downloaded:
|
||||
# The cover lives inside the 3MF, so it is only reachable if the 3MF is.
|
||||
# When the printer kept the print on internal storage there is nothing
|
||||
# at any of these paths, and walking all sixteen of them just to end on
|
||||
# a 404 that reads as "this print has no cover" helps nobody (#2780).
|
||||
storage = print_file_reachable_over_ftp(printer_manager.get_status(printer_id))
|
||||
if not storage.reachable:
|
||||
_cover_404_cache.setdefault(printer_id, set()).add(cache_key)
|
||||
raise HTTPException(
|
||||
404,
|
||||
f"The print file for '{subtask_name}' is not on storage Bambuddy can read over FTPS "
|
||||
f"({storage.reason}), so it has no cover to extract.",
|
||||
)
|
||||
|
||||
logger.info(
|
||||
f"Trying to download cover for '{subtask_name}' from {printer.ip_address} (trying {len(remote_paths)} paths)"
|
||||
)
|
||||
@@ -1234,7 +1248,7 @@ async def _produce_cover_image(
|
||||
raise HTTPException(
|
||||
503,
|
||||
f"Printer {printer.ip_address} is not answering its file service over TLS. "
|
||||
"Restart the printer and try again.",
|
||||
"Bambuddy will try again shortly.",
|
||||
)
|
||||
try:
|
||||
downloaded = await download_file_try_paths_async(
|
||||
|
||||
+45
-3
@@ -108,6 +108,7 @@ from backend.app.services.notification_service import notification_service
|
||||
from backend.app.services.obico_detection import obico_detection_service
|
||||
from backend.app.services.print_cost_estimate import plate_scoped_run_estimate as _plate_scoped_run_estimate
|
||||
from backend.app.services.print_scheduler import scheduler as print_scheduler
|
||||
from backend.app.services.print_storage import external_storage_present, print_file_reachable_over_ftp
|
||||
from backend.app.services.printer_manager import (
|
||||
init_printer_connections,
|
||||
parse_plate_id,
|
||||
@@ -3446,10 +3447,25 @@ async def on_print_start(printer_id: int, data: dict):
|
||||
downloaded_filename = try_filename
|
||||
break
|
||||
|
||||
# Does this printer keep the sliced file somewhere FTPS can reach? On
|
||||
# H2-series and P2S the answer is routinely no — the file stays on
|
||||
# internal eMMC and port 990 only ever serves external storage — and
|
||||
# then the whole sweep below (six filenames x five directories x four
|
||||
# retries, then the directory walk) is ~110 connections that cannot
|
||||
# succeed. Skip it and say why (#2780).
|
||||
storage = print_file_reachable_over_ftp(printer_manager.get_status(printer_id))
|
||||
if not storage.reachable and not downloaded_filename:
|
||||
logger.info(
|
||||
"Skipping the 3MF lookup for printer %s: %s — the print file is not on storage "
|
||||
"Bambuddy can read over FTPS, so no path would find it",
|
||||
printer_id,
|
||||
storage.reason,
|
||||
)
|
||||
|
||||
# Get FTP retry settings
|
||||
ftp_retry_enabled, ftp_retry_count, ftp_retry_delay, ftp_timeout = await get_ftp_retry_settings()
|
||||
|
||||
for try_filename in possible_names if not downloaded_filename else []:
|
||||
for try_filename in possible_names if not downloaded_filename and storage.reachable else []:
|
||||
if not try_filename.endswith(".3mf"):
|
||||
continue
|
||||
|
||||
@@ -3527,7 +3543,12 @@ async def on_print_start(printer_id: int, data: dict):
|
||||
# Different printer models use different directory structures. Skipped
|
||||
# when the printer's FTPS handshake is failing — the directory walk is
|
||||
# five more connections that cannot get further than the download did.
|
||||
if not downloaded_filename and (filename or subtask_name) and not ftps_handshake_blocked(printer.ip_address):
|
||||
if (
|
||||
not downloaded_filename
|
||||
and storage.reachable
|
||||
and (filename or subtask_name)
|
||||
and not ftps_handshake_blocked(printer.ip_address)
|
||||
):
|
||||
search_term = (subtask_name or filename).lower().replace(".gcode", "").replace(".3mf", "")
|
||||
logger.info("Direct FTP download failed, searching directories for '%s'", search_term)
|
||||
search_dirs = ["/cache", "/model", "/data", "/data/Metadata", "/"]
|
||||
@@ -3742,7 +3763,15 @@ async def on_print_start(printer_id: int, data: dict):
|
||||
subtask_id=subtask_id,
|
||||
filament_type=mqtt_filament_meta.get("filament_type"),
|
||||
filament_color=mqtt_filament_meta.get("filament_color"),
|
||||
extra_data={"no_3mf_available": True, "original_subtask": subtask_name, "_print_data": data},
|
||||
extra_data={
|
||||
"no_3mf_available": True,
|
||||
# Why the card is empty, when we know. The banner reads
|
||||
# this to stop telling H2/P2 owners to switch on a
|
||||
# setting that is already on and would not help (#2780).
|
||||
"no_3mf_reason": storage.reason,
|
||||
"original_subtask": subtask_name,
|
||||
"_print_data": data,
|
||||
},
|
||||
)
|
||||
|
||||
db.add(fallback_archive)
|
||||
@@ -3988,6 +4017,19 @@ async def _list_timelapse_videos(printer) -> tuple[list[dict], str | None]:
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# No card in the slot means no /timelapse to walk — four connections that
|
||||
# can only fail, on a path whose failures are swallowed and so would go on
|
||||
# costing time silently forever (#2780).
|
||||
#
|
||||
# ``getattr`` rather than ``printer.id``: every dereference below happens
|
||||
# inside the loop's own try/except, so a caller that passed something
|
||||
# unexpected used to get an empty listing rather than an exception. Keep
|
||||
# that, instead of making this gate the first thing that can raise here.
|
||||
printer_id = getattr(printer, "id", None)
|
||||
if printer_id is not None and not external_storage_present(printer_manager.get_status(printer_id)):
|
||||
logger.debug("[TIMELAPSE] Skipping the scan for printer %s: it reports no external storage", printer_id)
|
||||
return [], None
|
||||
|
||||
for timelapse_path in ["/timelapse", "/timelapse/video", "/record", "/recording"]:
|
||||
try:
|
||||
found_files = await list_files_async(
|
||||
|
||||
@@ -92,11 +92,20 @@ class DeleteResult(Enum):
|
||||
# handshake failed (#2780).
|
||||
#
|
||||
# ``WRONG_VERSION_NUMBER`` on port 990 means the printer answered with
|
||||
# something that is not a TLS record at all — its file service is wedged, and
|
||||
# no path, retry or SSL option can talk to it until the printer is restarted.
|
||||
# Two support bundles show that state lasting for days: one X2D served clean
|
||||
# FTPS for five days, flipped on 2026-07-19, and then failed every single
|
||||
# handshake for the next eight (zero successes, 3511 failures).
|
||||
# something that is not a TLS record at all, so no path, retry or SSL option
|
||||
# gets further. Two support bundles show that state lasting for days: one X2D
|
||||
# served clean FTPS for five days, flipped on 2026-07-19, and then failed every
|
||||
# single handshake for the next eight (zero successes, 3511 failures).
|
||||
#
|
||||
# What it is NOT is a wedged file service, which is what this comment used to
|
||||
# claim. #2780's reporter power-cycled both affected printers and the state
|
||||
# survived it, and ``openssl s_client`` against the same port completes a clean
|
||||
# handshake and returns a valid certificate while Bambuddy is failing. The
|
||||
# leading theory is now a connection-count refusal — vsFTPd answers one in
|
||||
# cleartext, which is exactly this error to an implicit-TLS client, and answers
|
||||
# the global limit by accepting and never speaking, which is the handshake
|
||||
# timeout we also see. Unproven: confirming it needs a capture taken while a
|
||||
# printer is in the failing state.
|
||||
#
|
||||
# Without a gate every candidate path re-runs the same doomed handshake: the
|
||||
# 3MF lookup alone walks 6 filename variants x 5 directories x 4 retries, and
|
||||
@@ -324,41 +333,77 @@ class BambuFTPClient:
|
||||
return True
|
||||
except ftplib.error_perm as e:
|
||||
logger.warning("FTP connection permission error to %s: %s", self.ip_address, e)
|
||||
self._ftp = None
|
||||
self._abandon_connection()
|
||||
return False
|
||||
except TimeoutError as e:
|
||||
logger.warning("FTP connection timed out to %s: %s", self.ip_address, e)
|
||||
self._ftp = None
|
||||
self._abandon_connection()
|
||||
return False
|
||||
except ssl.SSLError as e:
|
||||
# Not a transient failure and not something another path or another
|
||||
# retry can route around: the printer's file service answered port
|
||||
# 990 with something that isn't TLS. Say so once, in words the
|
||||
# reporter can act on, and stop knocking for a while (#2780).
|
||||
# 990 with something that isn't TLS. Say so once and stop knocking
|
||||
# for a while (#2780).
|
||||
#
|
||||
# Deliberately no advice about what to do. This message used to
|
||||
# tell the operator to restart the printer; #2780's reporter did
|
||||
# that twice, to no effect, and a single manual connect to the
|
||||
# same printer completes a clean handshake. We do not yet know the
|
||||
# trigger, so stating the observation and stopping there beats
|
||||
# sending people to do the one thing already known not to work.
|
||||
logger.warning(
|
||||
"FTP SSL error connecting to %s: %s — the printer's file service is not answering "
|
||||
"with TLS on port %s. Print files, covers and timelapses cannot be fetched from it "
|
||||
"until the printer is restarted. Pausing FTP to this printer for %.0fs.",
|
||||
"FTP SSL error connecting to %s: %s — the printer answered port %s with something "
|
||||
"that is not TLS, so print files, covers and timelapses cannot be fetched from it. "
|
||||
"Pausing FTP to this printer for %.0fs.",
|
||||
self.ip_address,
|
||||
e,
|
||||
self.FTP_PORT,
|
||||
_HANDSHAKE_COOLOFF_SECONDS,
|
||||
)
|
||||
self._handshake_blocked_until[self.ip_address] = time.monotonic() + _HANDSHAKE_COOLOFF_SECONDS
|
||||
self._ftp = None
|
||||
self._abandon_connection()
|
||||
return False
|
||||
except (OSError, ftplib.Error) as e:
|
||||
logger.warning("FTP connection failed to %s: %s (type: %s)", self.ip_address, e, type(e).__name__)
|
||||
self._ftp = None
|
||||
self._abandon_connection()
|
||||
return False
|
||||
|
||||
def _abandon_connection(self) -> None:
|
||||
"""Drop a connection that never became usable, closing its socket.
|
||||
|
||||
Every failure path in :meth:`connect` used to clear ``self._ftp`` and
|
||||
nothing else, leaving a connected socket for the garbage collector.
|
||||
That is survivable once; it is not survivable at this volume. A single
|
||||
print used to walk ~110 candidate paths, so a printer refusing FTPS
|
||||
got ~110 sockets opened and abandoned in a couple of minutes, and one
|
||||
support bundle recorded 1813 of them in a day (#2780). If the refusal
|
||||
is the printer running out of connection slots -- which fits the
|
||||
evidence better than a wedged service, since a single manual connect
|
||||
to the same printer succeeds -- then abandoning sockets is not just
|
||||
untidy, it is what keeps the printer refusing.
|
||||
|
||||
Uses ``close()`` rather than ``quit()``: QUIT is a command, and there
|
||||
is no working control channel to send it on.
|
||||
"""
|
||||
ftp = self._ftp
|
||||
self._ftp = None
|
||||
if ftp is None:
|
||||
return
|
||||
try:
|
||||
ftp.close()
|
||||
except (OSError, ftplib.Error, EOFError):
|
||||
pass # Best-effort; the socket may already be gone
|
||||
|
||||
def disconnect(self):
|
||||
"""Disconnect from the FTP server."""
|
||||
if self._ftp:
|
||||
try:
|
||||
self._ftp.quit()
|
||||
except (OSError, ftplib.Error, EOFError):
|
||||
pass # Best-effort FTP cleanup; connection may already be closed
|
||||
# ``quit()`` sends QUIT and only then closes; when the send
|
||||
# raises, ftplib never reaches its own close and the socket
|
||||
# stays open. Close it here rather than leaving it to the GC.
|
||||
self._abandon_connection()
|
||||
self._ftp = None
|
||||
|
||||
def list_files(self, path: str = "/") -> list[dict]:
|
||||
|
||||
@@ -760,7 +760,34 @@ class PrinterState:
|
||||
hms_errors: list = field(default_factory=list) # List of HMSError
|
||||
kprofiles: list = field(default_factory=list) # List of KProfile
|
||||
sdcard: bool = False # SD card inserted
|
||||
# Whether the printer has ever actually told us about `sdcard`. Without this
|
||||
# the default False is indistinguishable from a real "no card", and any
|
||||
# consumer that treats False as evidence would act on silence — which is how
|
||||
# a storage gate turns into a regression for every printer whose firmware
|
||||
# simply doesn't publish the field (#2780).
|
||||
sdcard_reported: bool = False
|
||||
store_to_sdcard: bool = False # Store sent files on SD card (home_flag bit 11)
|
||||
# Scheme+path of a `project_file` dispatch seen on the request topic, from
|
||||
# whoever sent it (the slicer or us). Bambu states where the sliced file
|
||||
# went: `ftp://<name>` is external storage, which FTPS serves, while
|
||||
# `brtc://emmc/<name>` is the printer's internal storage, which it does not.
|
||||
#
|
||||
# Two fields, because the two readers need different guarantees.
|
||||
# ``current_project_url`` belongs to the print now running and is cleared
|
||||
# when that print ends, so a print Bambuddy saw no dispatch for reads as
|
||||
# "unknown" rather than inheriting the previous job's answer. That matters:
|
||||
# 18% of the print starts in #2780's bundle had no dispatch on the request
|
||||
# topic at all (touchscreen reprints, restart recovery), and a stale
|
||||
# internal-storage URL would make those skip an FTPS sweep that could have
|
||||
# found the file — losing an archive that works today.
|
||||
#
|
||||
# ``last_project_url`` is sticky and exists for reporting only: the
|
||||
# connection diagnostic is usually run *after* the print that prompted it,
|
||||
# by which point the per-print value is rightly gone.
|
||||
#
|
||||
# None means we never saw a dispatch — say nothing, don't guess.
|
||||
current_project_url: str | None = None
|
||||
last_project_url: str | None = None
|
||||
timelapse: bool = False # Timelapse recording active
|
||||
ipcam: bool = False # Live view / camera streaming enabled
|
||||
wifi_signal: int | None = None # WiFi signal strength in dBm
|
||||
@@ -1781,6 +1808,14 @@ class BambuMQTTClient:
|
||||
return
|
||||
command = print_data.get("command", "")
|
||||
if command == "project_file":
|
||||
# Where the dispatcher put the sliced file. Captured for every
|
||||
# project_file, ours included: we publish to this same topic and
|
||||
# subscribe to it, so whoever dispatched last wins, which is exactly
|
||||
# the print the archive lookup is about to go looking for (#2780).
|
||||
url = print_data.get("url")
|
||||
if isinstance(url, str) and url:
|
||||
self.state.current_project_url = url
|
||||
self.state.last_project_url = url
|
||||
if "ams_mapping" in print_data:
|
||||
self._captured_ams_mapping = print_data["ams_mapping"]
|
||||
logger.info(
|
||||
@@ -4526,6 +4561,7 @@ class BambuMQTTClient:
|
||||
self.state.sdcard = "HAS_SDCARD" in raw_sdcard.upper() or raw_sdcard.lower() in ("true", "normal", "1")
|
||||
else:
|
||||
self.state.sdcard = bool(raw_sdcard)
|
||||
self.state.sdcard_reported = True
|
||||
|
||||
if home_flag is not None:
|
||||
store_to_sdcard = bool((home_flag >> 11) & 1)
|
||||
@@ -5098,6 +5134,11 @@ class BambuMQTTClient:
|
||||
}
|
||||
)
|
||||
self._captured_ams_mapping = None
|
||||
# Same lifecycle as the mapping above: it described *this* print.
|
||||
# Leaving it set would hand the next print an answer about where a
|
||||
# different file went, and a stale "internal storage" reading costs
|
||||
# an archive that the FTPS sweep would have found (#2780).
|
||||
self.state.current_project_url = None
|
||||
|
||||
self._previous_gcode_state = self.state.state
|
||||
if current_file:
|
||||
|
||||
@@ -0,0 +1,147 @@
|
||||
"""Can FTPS see the file this print is running from? (#2780)
|
||||
|
||||
Bambuddy reads a print's 3MF, cover and timelapse off the printer over implicit
|
||||
FTPS on port 990. On every Bambu model that port serves **external storage only**
|
||||
-- the SD card or USB stick. It is not a view of the printer's filesystem.
|
||||
|
||||
H2-series and P2S firmware default to keeping the sliced file on internal eMMC
|
||||
instead, and BambuStudio uploads there over a separate service on port 6000
|
||||
(the "BambuTunnelLocal" protocol -- see #2762, which tracks implementing it).
|
||||
When that happens there is no file on FTPS to find, at any path, and no TLS
|
||||
option, retry or directory guess changes that. The dispatch says so plainly:
|
||||
the ``project_file`` command carries ``url``, which is ``ftp://<name>`` for
|
||||
external storage and ``brtc://emmc/<name>`` for internal.
|
||||
|
||||
Before this module we ignored ``url`` and swept anyway: six filename variants
|
||||
across five directories with up to four retries for the 3MF, then sixteen more
|
||||
paths for the cover, then the timelapse scan -- roughly 110 FTPS connections per
|
||||
print, every one of them certain to 550. The user-visible result was an archive
|
||||
card with nothing on it and no stated reason, which read as a Bambuddy bug and
|
||||
was reported as one four times (#1170, #2524, #2762, #2780).
|
||||
|
||||
The rule here is deliberately one-sided: **skip only on positive evidence**.
|
||||
Silence is not evidence -- a printer that never publishes ``sdcard`` and never
|
||||
had a ``project_file`` pass through the request topic (some brokers refuse the
|
||||
subscription) must keep the old behaviour exactly, or this becomes a regression
|
||||
for installs whose archives work fine today.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass
|
||||
|
||||
# The one URL scheme that means "on external storage, reachable over FTPS".
|
||||
# Anything else -- brtc://emmc today, whatever Bambu ships next -- is somewhere
|
||||
# port 990 does not serve. Matching the reachable value rather than the
|
||||
# unreachable one is what keeps a new scheme from silently reading as fine.
|
||||
_EXTERNAL_STORAGE_SCHEME = "ftp"
|
||||
|
||||
# Reason slugs. These cross the API into the UI and into the connection
|
||||
# diagnostic, so they are part of the contract: the frontend maps each to its
|
||||
# own explanation and its own advice. Keep them stable.
|
||||
REASON_INTERNAL_STORAGE = "internal_storage"
|
||||
REASON_NO_EXTERNAL_STORAGE = "no_external_storage"
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class StorageVerdict:
|
||||
"""Whether an FTPS sweep for this print's file is worth running.
|
||||
|
||||
``reachable`` False always carries a ``reason``; True never does.
|
||||
"""
|
||||
|
||||
reachable: bool
|
||||
reason: str | None = None
|
||||
|
||||
|
||||
_REACHABLE = StorageVerdict(reachable=True)
|
||||
|
||||
|
||||
def url_is_external_storage(project_url: str | None) -> bool | None:
|
||||
"""Does *project_url* name a file on external storage?
|
||||
|
||||
``None`` when there is no URL to read, which is not the same answer as
|
||||
False and must not be collapsed into one by callers.
|
||||
"""
|
||||
# Type-checked, not just truth-checked: this value arrives straight off the
|
||||
# wire, so it is whatever the sender put there. Anything that is not a
|
||||
# string is not an answer.
|
||||
if not isinstance(project_url, str) or not project_url:
|
||||
return None
|
||||
scheme, separator, _ = project_url.partition("://")
|
||||
if not separator:
|
||||
# No scheme at all. Real dispatches always carry one, so rather than
|
||||
# guess at a bare path, decline to answer and let the caller fall
|
||||
# through to its existing behaviour.
|
||||
return None
|
||||
return scheme.lower() == _EXTERNAL_STORAGE_SCHEME
|
||||
|
||||
|
||||
def external_storage_present(state: object | None) -> bool:
|
||||
"""Does the printer have external storage for FTPS to serve at all?
|
||||
|
||||
Narrower than :func:`print_file_reachable_over_ftp` and deliberately so.
|
||||
The printer records its timelapse to the card itself, so *where the sliced
|
||||
file went* says nothing about whether a video exists -- an H2C that kept
|
||||
the 3MF on eMMC still writes ``/timelapse`` to an inserted card. Only the
|
||||
empty-slot case rules a scan out, and only when the printer said the slot
|
||||
is empty rather than never mentioning it.
|
||||
"""
|
||||
if state is None:
|
||||
return True
|
||||
return not (getattr(state, "sdcard_reported", False) and not getattr(state, "sdcard", False))
|
||||
|
||||
|
||||
def print_file_reachable_over_ftp(state: object | None) -> StorageVerdict:
|
||||
"""Decide whether to run an FTPS sweep for the print *state* is running.
|
||||
|
||||
*state* is a ``PrinterState`` (duck-typed so tests and callers can pass a
|
||||
stand-in). Reads ``current_project_url``, ``sdcard`` and ``sdcard_reported``.
|
||||
|
||||
Deliberately the *per-print* URL, not the sticky one: a print Bambuddy saw
|
||||
no dispatch for must read as unknown and sweep, rather than inherit the
|
||||
previous job's destination. Roughly a fifth of the print starts in #2780's
|
||||
bundle had no dispatch on the request topic -- touchscreen reprints and
|
||||
restart recovery -- and inheriting a stale internal-storage answer there
|
||||
would skip a sweep that could have found the file.
|
||||
|
||||
Returns :data:`_REACHABLE` unless something positively says otherwise.
|
||||
"""
|
||||
return _verdict(getattr(state, "current_project_url", None), state)
|
||||
|
||||
|
||||
def last_print_storage_verdict(state: object | None) -> StorageVerdict:
|
||||
"""Same question, asked of the last dispatch seen whenever that was.
|
||||
|
||||
For reporting only -- the connection diagnostic is normally run after the
|
||||
print that prompted it, by which point the per-print URL has been cleared.
|
||||
Never gate an FTPS sweep on this: it may describe a different print.
|
||||
"""
|
||||
return _verdict(getattr(state, "last_project_url", None), state)
|
||||
|
||||
|
||||
def _verdict(project_url: str | None, state: object | None) -> StorageVerdict:
|
||||
if state is None:
|
||||
return _REACHABLE
|
||||
|
||||
# Strongest signal, and specific to the print in question: the dispatcher
|
||||
# named the destination.
|
||||
external = url_is_external_storage(project_url)
|
||||
if external is False:
|
||||
return StorageVerdict(reachable=False, reason=REASON_INTERNAL_STORAGE)
|
||||
if external is True:
|
||||
# It said external storage, so sweep even if the card flags disagree.
|
||||
# Trusting the specific claim over the general one is what keeps a
|
||||
# printer that misreports `sdcard` from losing archives that work
|
||||
# today -- a false skip is a regression, a needless sweep is only slow.
|
||||
return _REACHABLE
|
||||
|
||||
# Model-independent fallback for printers whose broker refuses the request
|
||||
# topic, so we never see a `project_file` at all. An empty slot means FTPS
|
||||
# has nothing to serve from any path -- but only when the printer actually
|
||||
# said so. `sdcard` defaults to False, and acting on that default would
|
||||
# skip the sweep for every printer that simply doesn't publish the field.
|
||||
if getattr(state, "sdcard_reported", False) and not getattr(state, "sdcard", False):
|
||||
return StorageVerdict(reachable=False, reason=REASON_NO_EXTERNAL_STORAGE)
|
||||
|
||||
return _REACHABLE
|
||||
@@ -21,6 +21,7 @@ from backend.app.services.bambu_mqtt import CONNECT_ERROR_AUTH_REJECTED
|
||||
from backend.app.services.camera import get_camera_port
|
||||
from backend.app.services.discovery import is_running_in_docker
|
||||
from backend.app.services.ftp_profiles import get_ftp_profile
|
||||
from backend.app.services.print_storage import REASON_INTERNAL_STORAGE, last_print_storage_verdict
|
||||
from backend.app.services.printer_manager import printer_manager
|
||||
from backend.app.utils.printer_models import has_external_storage, has_remote_storage_toggle
|
||||
|
||||
@@ -300,11 +301,14 @@ async def run_connection_diagnostic(
|
||||
store_to_sdcard = getattr(state, "store_to_sdcard", None) if state else None
|
||||
if not model_has_slot or state is None or not state.connected:
|
||||
checks.append(DiagnosticCheck(id="external_storage", status="skip"))
|
||||
elif store_to_sdcard is True:
|
||||
checks.append(DiagnosticCheck(id="external_storage", status="pass"))
|
||||
elif store_to_sdcard is False and not has_remote_storage_toggle(model):
|
||||
# Slot present but no way to enable it on this firmware — don't nag
|
||||
# with an unresolvable fail; explain why via the reason param.
|
||||
#
|
||||
# Ahead of the empty-slot check below on purpose (#2524 over #2780):
|
||||
# on a P1-series the toggle cannot be switched on at all, so telling
|
||||
# the operator to insert a card would promise a fix that inserting a
|
||||
# card does not deliver.
|
||||
checks.append(
|
||||
DiagnosticCheck(
|
||||
id="external_storage",
|
||||
@@ -312,6 +316,33 @@ async def run_connection_diagnostic(
|
||||
params={"reason": "unsupported_model"},
|
||||
)
|
||||
)
|
||||
elif getattr(state, "sdcard_reported", False) and not getattr(state, "sdcard", False):
|
||||
# The toggle can be on and still achieve nothing with an empty slot,
|
||||
# and that combination used to report a clean pass — #2780's H2C had
|
||||
# `store_to_sdcard` set and `sdcard` False for three solid weeks while
|
||||
# every one of its archives came out blank. Report the empty slot,
|
||||
# which is the part the operator can actually act on.
|
||||
checks.append(
|
||||
DiagnosticCheck(
|
||||
id="external_storage",
|
||||
status="fail",
|
||||
params={"reason": "no_media"},
|
||||
)
|
||||
)
|
||||
elif not last_print_storage_verdict(state).reachable:
|
||||
# The toggle is on, a card is in, and the printer still put the last
|
||||
# print on internal storage — which is what H2-series and P2S firmware
|
||||
# does, and no setting here changes it (#2762 tracks reading that
|
||||
# storage). A pass here would be a lie; a fail would be unresolvable.
|
||||
checks.append(
|
||||
DiagnosticCheck(
|
||||
id="external_storage",
|
||||
status="warn",
|
||||
params={"reason": REASON_INTERNAL_STORAGE},
|
||||
)
|
||||
)
|
||||
elif store_to_sdcard is True:
|
||||
checks.append(DiagnosticCheck(id="external_storage", status="pass"))
|
||||
elif store_to_sdcard is False:
|
||||
checks.append(DiagnosticCheck(id="external_storage", status="fail"))
|
||||
else:
|
||||
|
||||
@@ -731,7 +731,7 @@ class TestNo3MFWarning:
|
||||
response = await async_client.get("/api/v1/archives/no-3mf-warning")
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"has_fallback": True}
|
||||
assert response.json() == {"has_fallback": True, "reason": None}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
@@ -739,7 +739,7 @@ class TestNo3MFWarning:
|
||||
response = await async_client.get("/api/v1/archives/no-3mf-warning")
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"has_fallback": False}
|
||||
assert response.json() == {"has_fallback": False, "reason": None}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
@@ -755,7 +755,7 @@ class TestNo3MFWarning:
|
||||
response = await async_client.get("/api/v1/archives/no-3mf-warning")
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"has_fallback": False}
|
||||
assert response.json() == {"has_fallback": False, "reason": None}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
@@ -775,7 +775,7 @@ class TestNo3MFWarning:
|
||||
response = await async_client.get("/api/v1/archives/no-3mf-warning")
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"has_fallback": False}
|
||||
assert response.json() == {"has_fallback": False, "reason": None}
|
||||
# Sanity: row really is in the DB, we just don't surface it.
|
||||
assert (await db_session.get(PrintArchive, archive.id)) is not None
|
||||
|
||||
@@ -796,7 +796,99 @@ class TestNo3MFWarning:
|
||||
assert response.status_code == 200
|
||||
# Soft-deleted fallbacks have been actioned (user clearing the
|
||||
# evidence). Stop nudging.
|
||||
assert response.json() == {"has_fallback": False}
|
||||
assert response.json() == {"has_fallback": False, "reason": None}
|
||||
|
||||
|
||||
class TestNo3MFWarningReason:
|
||||
"""Which explanation the banner shows (#2780).
|
||||
|
||||
The endpoint used to return a bare boolean and the banner had one wording:
|
||||
"Store sent files on external storage" is off in the slicer, go turn it on.
|
||||
For H2-series and P2S that is wrong twice over -- the setting is already on
|
||||
and turning it on changes nothing, because the printer keeps the file on
|
||||
internal storage FTPS does not serve. Sending people to re-do a step that
|
||||
cannot help is worse than saying nothing, so the reason travels with the
|
||||
flag.
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_internal_storage_is_reported(
|
||||
self, async_client: AsyncClient, archive_factory, printer_factory, db_session
|
||||
):
|
||||
printer = await printer_factory()
|
||||
await archive_factory(
|
||||
printer.id,
|
||||
extra_data={"no_3mf_available": True, "no_3mf_reason": "internal_storage"},
|
||||
)
|
||||
|
||||
response = await async_client.get("/api/v1/archives/no-3mf-warning")
|
||||
|
||||
assert response.json() == {"has_fallback": True, "reason": "internal_storage"}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_an_empty_slot_is_reported(
|
||||
self, async_client: AsyncClient, archive_factory, printer_factory, db_session
|
||||
):
|
||||
printer = await printer_factory()
|
||||
await archive_factory(
|
||||
printer.id,
|
||||
extra_data={"no_3mf_available": True, "no_3mf_reason": "no_external_storage"},
|
||||
)
|
||||
|
||||
response = await async_client.get("/api/v1/archives/no-3mf-warning")
|
||||
|
||||
assert response.json() == {"has_fallback": True, "reason": "no_external_storage"}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_a_known_reason_outranks_archives_that_carry_none(
|
||||
self, async_client: AsyncClient, archive_factory, printer_factory, db_session
|
||||
):
|
||||
"""Every archive written before this field exists carries no reason,
|
||||
and a farm usually has more than one printer. One H2C among four
|
||||
printers should still get the H2C explanation rather than have it
|
||||
drowned out by older rows.
|
||||
"""
|
||||
printer = await printer_factory()
|
||||
await archive_factory(printer.id, extra_data={"no_3mf_available": True})
|
||||
await archive_factory(printer.id, extra_data={"no_3mf_available": True, "no_3mf_reason": "internal_storage"})
|
||||
await archive_factory(printer.id, extra_data={"no_3mf_available": True})
|
||||
|
||||
response = await async_client.get("/api/v1/archives/no-3mf-warning")
|
||||
|
||||
assert response.json() == {"has_fallback": True, "reason": "internal_storage"}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_internal_storage_outranks_an_empty_slot(
|
||||
self, async_client: AsyncClient, archive_factory, printer_factory, db_session
|
||||
):
|
||||
"""Both are real, and only one of them is surprising. "Put a card in"
|
||||
is advice the user can act on unprompted; "this model keeps prints
|
||||
somewhere Bambuddy cannot read" is the one they need told.
|
||||
"""
|
||||
printer = await printer_factory()
|
||||
await archive_factory(printer.id, extra_data={"no_3mf_available": True, "no_3mf_reason": "no_external_storage"})
|
||||
await archive_factory(printer.id, extra_data={"no_3mf_available": True, "no_3mf_reason": "internal_storage"})
|
||||
|
||||
response = await async_client.get("/api/v1/archives/no-3mf-warning")
|
||||
|
||||
assert response.json() == {"has_fallback": True, "reason": "internal_storage"}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_a_reason_on_a_non_fallback_archive_is_not_reported(
|
||||
self, async_client: AsyncClient, archive_factory, printer_factory, db_session
|
||||
):
|
||||
"""The reason only means anything alongside the flag it explains."""
|
||||
printer = await printer_factory()
|
||||
await archive_factory(printer.id, extra_data={"no_3mf_reason": "internal_storage"})
|
||||
|
||||
response = await async_client.get("/api/v1/archives/no-3mf-warning")
|
||||
|
||||
assert response.json() == {"has_fallback": False, "reason": None}
|
||||
|
||||
|
||||
class TestPrintLogEntryDelete:
|
||||
|
||||
@@ -4181,7 +4181,20 @@ class TestCoverWhenFileServiceIsWedged:
|
||||
@pytest.mark.integration
|
||||
async def test_returns_503_naming_the_file_service(self, async_client: AsyncClient, printer_factory, db_session):
|
||||
printer = await printer_factory(name="Wedged P2S")
|
||||
state = MagicMock(subtask_name="job", state="RUNNING", gcode_file=None)
|
||||
# Spell out the storage fields rather than leaving them to MagicMock:
|
||||
# every attribute of a bare mock is truthy, so the storage gate added
|
||||
# in #2780 would read this stub as "the print is on internal storage"
|
||||
# and answer 404 before the handshake check ever ran. This test is
|
||||
# about a printer whose file service stopped answering, so say that
|
||||
# its last print went to the card.
|
||||
state = MagicMock(
|
||||
subtask_name="job",
|
||||
state="RUNNING",
|
||||
gcode_file=None,
|
||||
current_project_url="ftp://job.gcode.3mf",
|
||||
sdcard=True,
|
||||
sdcard_reported=True,
|
||||
)
|
||||
|
||||
with (
|
||||
patch("backend.app.api.routes.printers.printer_manager.get_status", return_value=state),
|
||||
@@ -4199,3 +4212,87 @@ class TestCoverWhenFileServiceIsWedged:
|
||||
assert "TLS" in response.json()["detail"]
|
||||
# The whole point: no FTP fan-out against a printer that cannot answer.
|
||||
mock_download.assert_not_called()
|
||||
|
||||
|
||||
class TestCoverWhenThePrintIsOnInternalStorage:
|
||||
"""The cover lives inside the 3MF, so it is only reachable if the 3MF is.
|
||||
|
||||
#2780: on an H2C or P2S the sliced file routinely stays on internal
|
||||
storage, which FTPS does not serve. Walking all sixteen candidate paths to
|
||||
arrive at a bare 404 spends connections the printer is already short of
|
||||
and tells the user "this print has no cover", which is not what happened.
|
||||
"""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clear_404_cache(self):
|
||||
"""The route remembers 404s per printer+print so repeat requests skip
|
||||
the fan-out. It is module-level, printer ids restart at 1 for each
|
||||
test, and both tests here use the same subtask name -- so without this
|
||||
the second test reads the first one's cached miss and never reaches
|
||||
the code it is checking.
|
||||
"""
|
||||
from backend.app.api.routes.printers import _cover_404_cache
|
||||
|
||||
_cover_404_cache.clear()
|
||||
yield
|
||||
_cover_404_cache.clear()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_skips_the_fan_out_and_says_why(self, async_client: AsyncClient, printer_factory, db_session):
|
||||
printer = await printer_factory(name="H2C")
|
||||
state = MagicMock(
|
||||
subtask_name="job",
|
||||
state="RUNNING",
|
||||
gcode_file=None,
|
||||
current_project_url="brtc://emmc/job.gcode.3mf",
|
||||
sdcard=True,
|
||||
sdcard_reported=True,
|
||||
)
|
||||
|
||||
with (
|
||||
patch("backend.app.api.routes.printers.printer_manager.get_status", return_value=state),
|
||||
patch("backend.app.api.routes.printers.resolve_plate_id", return_value=1),
|
||||
patch("backend.app.api.routes.printers.get_cached_3mf", return_value=None),
|
||||
patch("backend.app.api.routes.printers.ftps_handshake_blocked", return_value=False),
|
||||
patch(
|
||||
"backend.app.api.routes.printers.download_file_try_paths_async",
|
||||
new=AsyncMock(return_value=False),
|
||||
) as mock_download,
|
||||
):
|
||||
response = await async_client.get(f"/api/v1/printers/{printer.id}/cover")
|
||||
|
||||
assert response.status_code == 404, response.text
|
||||
assert "internal_storage" in response.json()["detail"]
|
||||
mock_download.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_a_print_on_external_storage_still_fans_out(
|
||||
self, async_client: AsyncClient, printer_factory, db_session
|
||||
):
|
||||
"""The regression guard. Every install whose covers work today reaches
|
||||
the download exactly as before."""
|
||||
printer = await printer_factory(name="X1C")
|
||||
state = MagicMock(
|
||||
subtask_name="job",
|
||||
state="RUNNING",
|
||||
gcode_file=None,
|
||||
current_project_url="ftp://job.gcode.3mf",
|
||||
sdcard=True,
|
||||
sdcard_reported=True,
|
||||
)
|
||||
|
||||
with (
|
||||
patch("backend.app.api.routes.printers.printer_manager.get_status", return_value=state),
|
||||
patch("backend.app.api.routes.printers.resolve_plate_id", return_value=1),
|
||||
patch("backend.app.api.routes.printers.get_cached_3mf", return_value=None),
|
||||
patch("backend.app.api.routes.printers.ftps_handshake_blocked", return_value=False),
|
||||
patch(
|
||||
"backend.app.api.routes.printers.download_file_try_paths_async",
|
||||
new=AsyncMock(return_value=False),
|
||||
) as mock_download,
|
||||
):
|
||||
await async_client.get(f"/api/v1/printers/{printer.id}/cover")
|
||||
|
||||
mock_download.assert_called()
|
||||
|
||||
@@ -1689,9 +1689,15 @@ class TestHandshakeCoolOff:
|
||||
assert bambu_ftp.ftps_handshake_blocked("127.0.0.1") is True
|
||||
messages = [r.getMessage() for r in caplog.records]
|
||||
assert any("WRONG_VERSION_NUMBER" in m for m in messages)
|
||||
# The warning has to name the remedy, not just the error: the port is
|
||||
# open, so "unblock port 990" is the wrong advice.
|
||||
assert any("restarted" in m for m in messages)
|
||||
# The warning has to say more than the error: the port is open, so
|
||||
# "unblock port 990" is the wrong reading and the operator needs to
|
||||
# know what it actually costs them.
|
||||
assert any("covers and timelapses cannot be fetched" in m for m in messages)
|
||||
# It must NOT prescribe a restart. It used to, and #2780's reporter
|
||||
# power-cycled both affected printers with no effect while a single
|
||||
# manual connection to the same printers completed a clean handshake.
|
||||
# Naming a remedy that is known not to work is worse than naming none.
|
||||
assert not any("restart" in m.lower() for m in messages)
|
||||
|
||||
def test_blocked_printer_is_not_contacted_again(self, plaintext_server):
|
||||
self._client(plaintext_server).connect()
|
||||
|
||||
@@ -0,0 +1,133 @@
|
||||
"""A connection that never came up still has to be closed (#2780).
|
||||
|
||||
Every failure path in ``BambuFTPClient.connect`` used to clear ``self._ftp``
|
||||
and stop there, leaving a connected socket for the garbage collector to
|
||||
notice. Once, that is untidy. At the volume this code runs at it is not: a
|
||||
single print used to walk ~110 candidate paths, so a printer refusing FTPS got
|
||||
~110 sockets opened and dropped in a couple of minutes, and one support bundle
|
||||
recorded 1813 in a day.
|
||||
|
||||
That matters beyond tidiness, because the leading explanation for the refusal
|
||||
is the printer running out of connection slots -- a single manual connect to
|
||||
the same printer completes a clean handshake while Bambuddy is failing, and
|
||||
vsFTPd answers a per-source limit in cleartext, which is exactly the
|
||||
``WRONG_VERSION_NUMBER`` we see. If that is right, abandoning sockets is not a
|
||||
side effect of the problem, it is part of what sustains it.
|
||||
|
||||
These tests assert the socket is closed, not merely dereferenced, because
|
||||
dereferencing is what the old code did and it looked identical from outside.
|
||||
"""
|
||||
|
||||
import ftplib
|
||||
import ssl
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from backend.app.services.bambu_ftp import BambuFTPClient
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _no_cooloff():
|
||||
"""The SSL path opens a per-IP cool-off that outlives the test."""
|
||||
BambuFTPClient._handshake_blocked_until.clear()
|
||||
yield
|
||||
BambuFTPClient._handshake_blocked_until.clear()
|
||||
|
||||
|
||||
def _client(ip="192.168.1.210"):
|
||||
return BambuFTPClient(ip, "12345678", printer_model="P2S")
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"error",
|
||||
[
|
||||
# The two the #2780 bundle actually recorded, 1813 and 49 times.
|
||||
ssl.SSLError("[SSL: WRONG_VERSION_NUMBER] wrong version number"),
|
||||
TimeoutError("_ssl.c:1015: The handshake operation timed out"),
|
||||
ftplib.error_perm("530 Login incorrect."),
|
||||
ftplib.error_temp("421 Service not available."),
|
||||
OSError("Connection reset by peer"),
|
||||
],
|
||||
ids=["ssl", "timeout", "perm", "temp", "oserror"],
|
||||
)
|
||||
def test_a_failed_connect_closes_its_socket(error):
|
||||
fake_ftp = MagicMock()
|
||||
fake_ftp.connect.side_effect = error
|
||||
|
||||
with patch("backend.app.services.bambu_ftp.ImplicitFTP_TLS", return_value=fake_ftp):
|
||||
client = _client()
|
||||
assert client.connect() is False
|
||||
|
||||
fake_ftp.close.assert_called_once()
|
||||
# Never QUIT: that is a command, and there is no working control channel
|
||||
# to send it on.
|
||||
fake_ftp.quit.assert_not_called()
|
||||
assert client._ftp is None
|
||||
|
||||
|
||||
def test_a_failure_after_connect_still_closes():
|
||||
"""Login and prot_p run on a live socket, so a failure there leaks a
|
||||
genuinely established connection -- the worst case for a session limit."""
|
||||
fake_ftp = MagicMock()
|
||||
fake_ftp.login.side_effect = ftplib.error_perm("530 Login incorrect.")
|
||||
|
||||
with patch("backend.app.services.bambu_ftp.ImplicitFTP_TLS", return_value=fake_ftp):
|
||||
client = _client()
|
||||
assert client.connect() is False
|
||||
|
||||
fake_ftp.close.assert_called_once()
|
||||
|
||||
|
||||
def test_close_raising_does_not_propagate():
|
||||
"""Cleanup is best-effort; the socket may already be gone. A raise here
|
||||
would turn a handled connect failure into an unhandled one."""
|
||||
fake_ftp = MagicMock()
|
||||
fake_ftp.connect.side_effect = OSError("boom")
|
||||
fake_ftp.close.side_effect = OSError("already closed")
|
||||
|
||||
with patch("backend.app.services.bambu_ftp.ImplicitFTP_TLS", return_value=fake_ftp):
|
||||
assert _client().connect() is False
|
||||
|
||||
|
||||
class TestDisconnect:
|
||||
def test_a_healthy_disconnect_quits(self):
|
||||
fake_ftp = MagicMock()
|
||||
|
||||
with patch("backend.app.services.bambu_ftp.ImplicitFTP_TLS", return_value=fake_ftp):
|
||||
client = _client()
|
||||
assert client.connect() is True
|
||||
client.disconnect()
|
||||
|
||||
fake_ftp.quit.assert_called_once()
|
||||
assert client._ftp is None
|
||||
|
||||
def test_a_failing_quit_falls_back_to_closing(self):
|
||||
"""``ftplib.FTP.quit`` sends QUIT and only then closes, so when the
|
||||
send raises it never reaches its own close and the socket stays open.
|
||||
The old handler swallowed that exception and left it there.
|
||||
"""
|
||||
fake_ftp = MagicMock()
|
||||
fake_ftp.quit.side_effect = OSError("Broken pipe")
|
||||
|
||||
with patch("backend.app.services.bambu_ftp.ImplicitFTP_TLS", return_value=fake_ftp):
|
||||
client = _client()
|
||||
assert client.connect() is True
|
||||
client.disconnect()
|
||||
|
||||
fake_ftp.close.assert_called_once()
|
||||
assert client._ftp is None
|
||||
|
||||
|
||||
def test_the_ssl_path_still_opens_the_cooloff():
|
||||
"""The cleanup change must not disturb the gate that stops the storm."""
|
||||
fake_ftp = MagicMock()
|
||||
fake_ftp.connect.side_effect = ssl.SSLError("[SSL: WRONG_VERSION_NUMBER] wrong version number")
|
||||
|
||||
with patch("backend.app.services.bambu_ftp.ImplicitFTP_TLS", return_value=fake_ftp):
|
||||
client = _client("192.168.1.211")
|
||||
assert client.connect() is False
|
||||
|
||||
assert BambuFTPClient.handshake_blocked("192.168.1.211") is True
|
||||
@@ -40,11 +40,25 @@ def _port_probe(overrides=None):
|
||||
return _probe
|
||||
|
||||
|
||||
def _state(*, connected=True, developer_mode=True, store_to_sdcard=True):
|
||||
def _state(
|
||||
*,
|
||||
connected=True,
|
||||
developer_mode=True,
|
||||
store_to_sdcard=True,
|
||||
sdcard=True,
|
||||
sdcard_reported=False,
|
||||
last_project_url=None,
|
||||
):
|
||||
# sdcard_reported defaults False — "the printer never mentioned its card",
|
||||
# which is what every pre-#2780 test in this file assumes and what keeps
|
||||
# the storage branches out of their way.
|
||||
return types.SimpleNamespace(
|
||||
connected=connected,
|
||||
developer_mode=developer_mode,
|
||||
store_to_sdcard=store_to_sdcard,
|
||||
sdcard=sdcard,
|
||||
sdcard_reported=sdcard_reported,
|
||||
last_project_url=last_project_url,
|
||||
)
|
||||
|
||||
|
||||
@@ -459,6 +473,88 @@ class TestExternalStorageCheck:
|
||||
result = await run_connection_diagnostic("192.168.1.50", printer=_printer(model="P1S"))
|
||||
assert _statuses(result)["external_storage"] == "pass"
|
||||
|
||||
# --- #2780: the toggle being on is not the same as it working ---------
|
||||
|
||||
async def test_fails_when_the_toggle_is_on_but_the_slot_is_empty(self):
|
||||
"""#2780's H2C reported `store_to_sdcard` set and `sdcard` False for
|
||||
three solid weeks. The check called that a clean pass while every one
|
||||
of its 23 archives came out blank -- the toggle can be on and still
|
||||
achieve nothing with nothing in the slot.
|
||||
"""
|
||||
with _Env(state=_state(store_to_sdcard=True, sdcard=False, sdcard_reported=True)):
|
||||
result = await run_connection_diagnostic("192.168.1.50", printer=_printer(model="H2C"))
|
||||
check = next(c for c in result.checks if c.id == "external_storage")
|
||||
assert check.status == "fail"
|
||||
assert check.params == {"reason": "no_media"}
|
||||
assert result.overall == "problems"
|
||||
|
||||
async def test_warns_when_the_printer_kept_the_print_internally(self):
|
||||
"""Toggle on, card in, and the printer still used internal storage --
|
||||
which is what H2-series and P2S firmware does. A pass would be a lie
|
||||
and a fail would be unresolvable, so it warns."""
|
||||
state = _state(
|
||||
store_to_sdcard=True,
|
||||
sdcard=True,
|
||||
sdcard_reported=True,
|
||||
last_project_url="brtc://emmc/Benchy.gcode.3mf",
|
||||
)
|
||||
with _Env(state=state):
|
||||
result = await run_connection_diagnostic("192.168.1.50", printer=_printer(model="H2C"))
|
||||
check = next(c for c in result.checks if c.id == "external_storage")
|
||||
assert check.status == "warn"
|
||||
assert check.params == {"reason": "internal_storage"}
|
||||
assert result.overall == "warnings"
|
||||
|
||||
async def test_an_external_storage_dispatch_still_passes(self):
|
||||
"""The regression guard: an X1C dispatch says `ftp://`, and that must
|
||||
read exactly as it did before any of this existed."""
|
||||
state = _state(
|
||||
store_to_sdcard=True,
|
||||
sdcard=True,
|
||||
sdcard_reported=True,
|
||||
last_project_url="ftp://Benchy.gcode.3mf",
|
||||
)
|
||||
with _Env(state=state):
|
||||
result = await run_connection_diagnostic("192.168.1.50", printer=_printer(model="X1C"))
|
||||
assert _statuses(result)["external_storage"] == "pass"
|
||||
|
||||
async def test_a_p1_series_keeps_its_non_nagging_skip_with_no_card(self):
|
||||
"""#2524 outranks #2780 here. Current P1 firmware exposes no way to
|
||||
turn the option on, so "insert a card" would promise a fix that
|
||||
inserting a card does not deliver -- the unresolvable-fail nagging
|
||||
that #2524 removed, back under a different label.
|
||||
"""
|
||||
with _Env(state=_state(store_to_sdcard=False, sdcard=False, sdcard_reported=True)):
|
||||
result = await run_connection_diagnostic("192.168.1.50", printer=_printer(model="P1S"))
|
||||
check = next(c for c in result.checks if c.id == "external_storage")
|
||||
assert check.status == "skip"
|
||||
assert check.params == {"reason": "unsupported_model"}
|
||||
assert result.overall == "ok"
|
||||
|
||||
async def test_a_model_with_a_reachable_toggle_does_report_the_empty_slot(self):
|
||||
"""The counterpart: on an X1C the advice is actionable, so give it."""
|
||||
with _Env(state=_state(store_to_sdcard=False, sdcard=False, sdcard_reported=True)):
|
||||
result = await run_connection_diagnostic("192.168.1.50", printer=_printer(model="X1C"))
|
||||
check = next(c for c in result.checks if c.id == "external_storage")
|
||||
assert check.status == "fail"
|
||||
assert check.params == {"reason": "no_media"}
|
||||
|
||||
async def test_an_empty_slot_outranks_the_internal_storage_warning(self):
|
||||
"""Both apply on a stickless H2C. "Put a card in" is the one the
|
||||
operator can act on, so it must not be masked by the softer warning.
|
||||
"""
|
||||
state = _state(
|
||||
store_to_sdcard=True,
|
||||
sdcard=False,
|
||||
sdcard_reported=True,
|
||||
last_project_url="brtc://emmc/Benchy.gcode.3mf",
|
||||
)
|
||||
with _Env(state=state):
|
||||
result = await run_connection_diagnostic("192.168.1.50", printer=_printer(model="H2C"))
|
||||
check = next(c for c in result.checks if c.id == "external_storage")
|
||||
assert check.status == "fail"
|
||||
assert check.params == {"reason": "no_media"}
|
||||
|
||||
|
||||
class TestFtpsTlsProbe:
|
||||
"""The FTPS probe must reach the handshake, not stop at the TCP accept.
|
||||
|
||||
@@ -0,0 +1,203 @@
|
||||
"""Capturing where the printer put the sliced file (#2780).
|
||||
|
||||
The ``project_file`` command has always carried a ``url`` saying which storage
|
||||
the file landed on -- ``ftp://<name>`` for the card, ``brtc://emmc/<name>`` for
|
||||
the printer's internal storage. Bambuddy discarded it and swept FTPS regardless,
|
||||
which on an H2C or P2S is ~110 connections that cannot succeed followed by a
|
||||
blank archive card with no stated reason.
|
||||
|
||||
These tests pin the capture itself. The decision made from it lives in
|
||||
:mod:`backend.app.services.print_storage` and is tested separately.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mqtt_client():
|
||||
from backend.app.services.bambu_mqtt import BambuMQTTClient
|
||||
|
||||
return BambuMQTTClient(ip_address="192.168.1.210", serial_number="TEST2780", access_code="12345678")
|
||||
|
||||
|
||||
class TestProjectUrlCapture:
|
||||
def test_nothing_is_claimed_before_a_dispatch_is_seen(self, mqtt_client):
|
||||
"""None is the "we don't know" answer, and consumers depend on it
|
||||
being distinguishable from a known-bad value."""
|
||||
assert mqtt_client.state.current_project_url is None
|
||||
|
||||
def test_an_internal_storage_dispatch_is_recorded(self, mqtt_client):
|
||||
"""The shape every H2C and P2S dispatch had in #2780's bundle."""
|
||||
mqtt_client._handle_request_message(
|
||||
{
|
||||
"print": {
|
||||
"command": "project_file",
|
||||
"url": "brtc://emmc/169356_204314.STEP.gcode.3mf",
|
||||
"sequence_id": "20002",
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
assert mqtt_client.state.current_project_url == "brtc://emmc/169356_204314.STEP.gcode.3mf"
|
||||
|
||||
def test_our_own_dispatch_is_recorded_too(self, mqtt_client):
|
||||
"""We publish to the request topic and subscribe to it, so our own
|
||||
commands come back. Capturing them is wanted, not incidental: after a
|
||||
Bambuddy-launched print the file really is on external storage, and a
|
||||
stale internal-storage URL from the slicer's last job would otherwise
|
||||
suppress a sweep that would have worked.
|
||||
|
||||
``sequence_id`` 20000 is ours; the external-payload log line keys off
|
||||
that, and this capture must not.
|
||||
"""
|
||||
mqtt_client._handle_request_message(
|
||||
{"print": {"command": "project_file", "url": "ftp://Benchy.gcode.3mf", "sequence_id": "20000"}}
|
||||
)
|
||||
|
||||
assert mqtt_client.state.current_project_url == "ftp://Benchy.gcode.3mf"
|
||||
|
||||
def test_the_latest_dispatch_wins(self, mqtt_client):
|
||||
for url in ("brtc://emmc/first.3mf", "ftp://second.3mf"):
|
||||
mqtt_client._handle_request_message({"print": {"command": "project_file", "url": url}})
|
||||
|
||||
assert mqtt_client.state.current_project_url == "ftp://second.3mf"
|
||||
|
||||
def test_a_dispatch_without_a_url_leaves_the_last_one_alone(self, mqtt_client):
|
||||
"""Better a slightly stale answer than a wrongly-cleared one: clearing
|
||||
would drop us to "unknown" and re-run the sweep we just learned was
|
||||
pointless."""
|
||||
mqtt_client._handle_request_message({"print": {"command": "project_file", "url": "brtc://emmc/x.3mf"}})
|
||||
mqtt_client._handle_request_message({"print": {"command": "project_file", "ams_mapping": [0]}})
|
||||
|
||||
assert mqtt_client.state.current_project_url == "brtc://emmc/x.3mf"
|
||||
|
||||
@pytest.mark.parametrize("url", [None, "", 12345, [], {}])
|
||||
def test_a_non_string_url_is_ignored(self, mqtt_client, url):
|
||||
"""Straight off the wire, so it is whatever the sender sent."""
|
||||
mqtt_client._handle_request_message({"print": {"command": "project_file", "url": url}})
|
||||
|
||||
assert mqtt_client.state.current_project_url is None
|
||||
|
||||
def test_other_commands_do_not_touch_it(self, mqtt_client):
|
||||
mqtt_client._handle_request_message({"print": {"command": "project_file", "url": "ftp://kept.3mf"}})
|
||||
for command in ("pause", "resume", "stop", "gcode_line"):
|
||||
mqtt_client._handle_request_message({"print": {"command": command, "url": "ftp://ignored.3mf"}})
|
||||
|
||||
assert mqtt_client.state.current_project_url == "ftp://kept.3mf"
|
||||
|
||||
|
||||
class TestTheTwoFieldsHaveDifferentLifetimes:
|
||||
"""``current_project_url`` describes the print now running and is cleared
|
||||
when it ends; ``last_project_url`` is sticky, for reporting after the fact.
|
||||
|
||||
The clearing is the load-bearing half. In #2780's support bundle 14 of 79
|
||||
print starts arrived with no ``project_file`` on the request topic at all
|
||||
-- touchscreen reprints and restart recovery. If those inherited the
|
||||
previous job's destination, one Studio print to internal storage would
|
||||
suppress the FTPS sweep for every screen-started print after it.
|
||||
"""
|
||||
|
||||
def test_a_dispatch_sets_both(self, mqtt_client):
|
||||
mqtt_client._handle_request_message({"print": {"command": "project_file", "url": "brtc://emmc/x.3mf"}})
|
||||
|
||||
assert mqtt_client.state.current_project_url == "brtc://emmc/x.3mf"
|
||||
assert mqtt_client.state.last_project_url == "brtc://emmc/x.3mf"
|
||||
|
||||
def test_finishing_a_print_clears_only_the_per_print_one(self, mqtt_client):
|
||||
mqtt_client._handle_request_message({"print": {"command": "project_file", "url": "brtc://emmc/x.3mf"}})
|
||||
# Drive a full RUNNING -> FINISH transition so the real completion
|
||||
# path runs, rather than reaching in and clearing the field by hand.
|
||||
mqtt_client.on_print_complete = lambda data: None
|
||||
mqtt_client._update_state({"gcode_state": "RUNNING"})
|
||||
mqtt_client._update_state({"gcode_state": "FINISH"})
|
||||
|
||||
assert mqtt_client.state.current_project_url is None, (
|
||||
"a print Bambuddy saw no dispatch for must read as unknown, not inherit this one"
|
||||
)
|
||||
assert mqtt_client.state.last_project_url == "brtc://emmc/x.3mf", (
|
||||
"the diagnostic runs after the print that prompted it and still needs the answer"
|
||||
)
|
||||
|
||||
def test_an_aborted_print_clears_it_too(self, mqtt_client):
|
||||
mqtt_client._handle_request_message({"print": {"command": "project_file", "url": "brtc://emmc/x.3mf"}})
|
||||
mqtt_client.on_print_complete = lambda data: None
|
||||
mqtt_client._update_state({"gcode_state": "RUNNING"})
|
||||
mqtt_client._update_state({"gcode_state": "FAILED"})
|
||||
|
||||
assert mqtt_client.state.current_project_url is None
|
||||
|
||||
def test_the_regression_sequence_end_to_end(self, mqtt_client):
|
||||
"""Studio print to internal storage, then a print started from the
|
||||
printer's own screen.
|
||||
|
||||
The capture and the gate are pinned separately; this is the sequence
|
||||
that made the split necessary, walked through both at once. The second
|
||||
print's file may well be on the card -- it has to be swept for, not
|
||||
written off on the strength of the first print's destination.
|
||||
"""
|
||||
from backend.app.services.print_storage import print_file_reachable_over_ftp
|
||||
|
||||
mqtt_client._update_state({"sdcard": True})
|
||||
mqtt_client.on_print_complete = lambda data: None
|
||||
|
||||
# 1. Studio dispatches to internal storage and the print runs.
|
||||
mqtt_client._handle_request_message({"print": {"command": "project_file", "url": "brtc://emmc/studio.3mf"}})
|
||||
mqtt_client._update_state({"gcode_state": "RUNNING"})
|
||||
assert print_file_reachable_over_ftp(mqtt_client.state).reachable is False
|
||||
|
||||
# 2. It finishes.
|
||||
mqtt_client._update_state({"gcode_state": "FINISH"})
|
||||
|
||||
# 3. The operator reprints from the touchscreen. No project_file
|
||||
# reaches the request topic at all -- 14 of 79 print starts in the
|
||||
# reporter's bundle looked exactly like this.
|
||||
mqtt_client._update_state({"gcode_state": "RUNNING"})
|
||||
|
||||
assert print_file_reachable_over_ftp(mqtt_client.state).reachable, (
|
||||
"the screen-started print inherited the Studio print's destination and lost its archive"
|
||||
)
|
||||
|
||||
|
||||
class TestSdcardReported:
|
||||
"""`sdcard` defaults to False, so "no card" and "never said" look alike.
|
||||
|
||||
Every consumer that treats False as evidence needs the difference, and
|
||||
getting it wrong skips FTP sweeps for printers whose archives work fine.
|
||||
"""
|
||||
|
||||
def test_nothing_is_claimed_before_a_status_frame(self, mqtt_client):
|
||||
assert mqtt_client.state.sdcard_reported is False
|
||||
assert mqtt_client.state.sdcard is False
|
||||
|
||||
def test_a_frame_carrying_the_field_marks_it_reported(self, mqtt_client):
|
||||
mqtt_client._update_state({"sdcard": False})
|
||||
|
||||
assert mqtt_client.state.sdcard_reported is True
|
||||
assert mqtt_client.state.sdcard is False
|
||||
|
||||
def test_a_frame_without_the_field_does_not(self, mqtt_client):
|
||||
mqtt_client._update_state({"gcode_state": "RUNNING"})
|
||||
|
||||
assert mqtt_client.state.sdcard_reported is False
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"raw,expected",
|
||||
[
|
||||
(True, True),
|
||||
(1, True),
|
||||
("HAS_SDCARD_NORMAL", True),
|
||||
("normal", True),
|
||||
("HAS_SDCARD_ABNORMAL", True),
|
||||
(False, False),
|
||||
(0, False),
|
||||
("", False),
|
||||
],
|
||||
)
|
||||
def test_the_reported_flag_is_set_for_every_accepted_shape(self, mqtt_client, raw, expected):
|
||||
"""Whatever the field means, having seen it is what is recorded."""
|
||||
mqtt_client._update_state({"sdcard": raw})
|
||||
|
||||
assert mqtt_client.state.sdcard_reported is True
|
||||
assert mqtt_client.state.sdcard is expected
|
||||
@@ -0,0 +1,219 @@
|
||||
"""The 3MF sweep is skipped when it cannot succeed, and the card says why (#2780).
|
||||
|
||||
The unit tests around ``print_storage`` pin the decision; this pins that
|
||||
``on_print_start`` actually acts on it. Two things have to hold and neither is
|
||||
visible from the helper alone:
|
||||
|
||||
* No FTP is attempted. The sweep is six filename variants across five
|
||||
directories with up to four retries, and on the reporter's P2S every one of
|
||||
those failed -- 1813 failures in a day against a printer that, on the leading
|
||||
theory, was refusing precisely because of the connection volume.
|
||||
* The fallback archive records *which* reason applied. Without it the archives
|
||||
banner falls back to its original wording, which tells the user to switch on
|
||||
a setting that in this case is already on and would not have helped.
|
||||
|
||||
The regression case matters as much: a printer whose file service is fine must
|
||||
still sweep exactly as before.
|
||||
"""
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from backend.app.main import (
|
||||
_active_prints,
|
||||
_expected_print_creators,
|
||||
_expected_print_registered_at,
|
||||
_expected_prints,
|
||||
_print_ams_mappings,
|
||||
_timelapse_baselines,
|
||||
)
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clear_dicts():
|
||||
for d in (
|
||||
_expected_prints,
|
||||
_expected_print_registered_at,
|
||||
_expected_print_creators,
|
||||
_print_ams_mappings,
|
||||
_active_prints,
|
||||
_timelapse_baselines,
|
||||
):
|
||||
d.clear()
|
||||
yield
|
||||
for d in (
|
||||
_expected_prints,
|
||||
_expected_print_registered_at,
|
||||
_expected_print_creators,
|
||||
_print_ams_mappings,
|
||||
_active_prints,
|
||||
_timelapse_baselines,
|
||||
):
|
||||
d.clear()
|
||||
|
||||
|
||||
def _printer():
|
||||
printer = MagicMock()
|
||||
printer.id = 1
|
||||
printer.auto_archive = True
|
||||
printer.external_camera_enabled = False
|
||||
printer.external_camera_url = None
|
||||
# Spell this one out: every unset MagicMock attribute is truthy, so leaving
|
||||
# it implicit runs the plate-detection camera grab and each test waits out
|
||||
# its 10s timeout against a printer that does not exist.
|
||||
printer.plate_detection_enabled = False
|
||||
printer.name = "H2C"
|
||||
printer.model = "H2C"
|
||||
printer.ip_address = "192.168.1.210"
|
||||
printer.access_code = "12345678"
|
||||
return printer
|
||||
|
||||
|
||||
def _state(current_project_url, sdcard=True, sdcard_reported=True):
|
||||
return MagicMock(
|
||||
current_project_url=current_project_url,
|
||||
sdcard=sdcard,
|
||||
sdcard_reported=sdcard_reported,
|
||||
)
|
||||
|
||||
|
||||
async def _run_print_start(state, added):
|
||||
"""Drive on_print_start for a print with no matching archive, capturing
|
||||
whatever rows it adds and whether it reached the FTP layer.
|
||||
"""
|
||||
printer = _printer()
|
||||
|
||||
def execute_router(stmt, *args, **kwargs):
|
||||
sql = str(stmt).lower()
|
||||
if "from printers" in sql or "from printer " in sql:
|
||||
return MagicMock(
|
||||
scalar_one_or_none=MagicMock(return_value=printer),
|
||||
scalars=MagicMock(return_value=MagicMock(all=MagicMock(return_value=[printer]))),
|
||||
)
|
||||
# No existing / expected archive anywhere: the fallback path is the
|
||||
# one under test.
|
||||
return MagicMock(
|
||||
scalar_one_or_none=MagicMock(return_value=None),
|
||||
scalars=MagicMock(return_value=MagicMock(all=MagicMock(return_value=[]))),
|
||||
)
|
||||
|
||||
session = AsyncMock()
|
||||
session.__aenter__ = AsyncMock(return_value=session)
|
||||
session.__aexit__ = AsyncMock()
|
||||
session.execute = AsyncMock(side_effect=execute_router)
|
||||
session.commit = AsyncMock()
|
||||
session.refresh = AsyncMock()
|
||||
session.add = MagicMock(side_effect=added.append)
|
||||
|
||||
download = AsyncMock(return_value=False)
|
||||
|
||||
with (
|
||||
patch("backend.app.main.async_session") as session_maker,
|
||||
patch("backend.app.main.notification_service") as notif,
|
||||
patch("backend.app.main.smart_plug_manager") as plug,
|
||||
patch("backend.app.main.ws_manager") as ws,
|
||||
patch("backend.app.main.mqtt_relay") as relay,
|
||||
patch("backend.app.main.printer_manager") as pm,
|
||||
patch("backend.app.main.download_file_async", new=download),
|
||||
patch("backend.app.main.with_ftp_retry", new=AsyncMock(return_value=False)) as retry,
|
||||
patch("backend.app.main.get_cached_3mf", return_value=None),
|
||||
# Imported inside the function, so it has to be patched at its source
|
||||
# or the directory-search fallback opens real sockets to a printer
|
||||
# that is not there and the test hangs on connect timeouts.
|
||||
patch("backend.app.services.bambu_ftp.list_files_async", new=AsyncMock(return_value=[])),
|
||||
patch("backend.app.main.ftps_handshake_blocked", return_value=False),
|
||||
patch("backend.app.main.get_ftp_retry_settings", new=AsyncMock(return_value=(False, 3, 2.0, 30))),
|
||||
patch("backend.app.main._record_energy_start", new_callable=AsyncMock),
|
||||
patch("backend.app.main._send_print_start_notification", new_callable=AsyncMock),
|
||||
patch("backend.app.main._maybe_start_layer_timelapse"),
|
||||
patch("backend.app.main._capture_timelapse_baseline_at_start", new_callable=AsyncMock),
|
||||
):
|
||||
session_maker.return_value = session
|
||||
notif.on_print_start = AsyncMock()
|
||||
plug.on_print_start = AsyncMock()
|
||||
ws.send_print_start = AsyncMock()
|
||||
ws.send_archive_updated = AsyncMock()
|
||||
relay.on_print_start = AsyncMock()
|
||||
pm.get_status = MagicMock(return_value=state)
|
||||
pm.get_printer = MagicMock(return_value=MagicMock(serial_number="TEST2780"))
|
||||
|
||||
from backend.app.main import on_print_start
|
||||
|
||||
await on_print_start(
|
||||
1,
|
||||
{"filename": "/data/Metadata/plate_1.gcode", "subtask_name": "Halterung"},
|
||||
)
|
||||
|
||||
return download, retry
|
||||
|
||||
|
||||
def _fallback(added):
|
||||
"""The archive row the fallback path created, if any."""
|
||||
for row in added:
|
||||
extra = getattr(row, "extra_data", None)
|
||||
if isinstance(extra, dict) and extra.get("no_3mf_available"):
|
||||
return row
|
||||
return None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_print_on_internal_storage_never_touches_ftp():
|
||||
added = []
|
||||
|
||||
download, retry = await _run_print_start(_state("brtc://emmc/Halterung.gcode.3mf"), added)
|
||||
|
||||
download.assert_not_called()
|
||||
retry.assert_not_called()
|
||||
|
||||
archive = _fallback(added)
|
||||
assert archive is not None, "the print still has to be archived, just without slice data"
|
||||
assert archive.extra_data["no_3mf_reason"] == "internal_storage"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_an_empty_slot_never_touches_ftp():
|
||||
added = []
|
||||
|
||||
download, retry = await _run_print_start(_state(None, sdcard=False, sdcard_reported=True), added)
|
||||
|
||||
download.assert_not_called()
|
||||
retry.assert_not_called()
|
||||
assert _fallback(added).extra_data["no_3mf_reason"] == "no_external_storage"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_print_on_external_storage_still_sweeps():
|
||||
"""The regression guard: every install whose archives work today reaches
|
||||
the download exactly as it did before the gate existed."""
|
||||
added = []
|
||||
|
||||
download, _ = await _run_print_start(_state("ftp://Halterung.gcode.3mf"), added)
|
||||
|
||||
download.assert_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_printer_that_said_nothing_still_sweeps():
|
||||
"""Silence is not evidence. A printer whose broker refuses the request
|
||||
topic never gives us a URL, and its firmware may never publish `sdcard`
|
||||
either -- that install must behave exactly as before."""
|
||||
added = []
|
||||
|
||||
download, _ = await _run_print_start(_state(None, sdcard=False, sdcard_reported=False), added)
|
||||
|
||||
download.assert_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_sweep_that_simply_found_nothing_records_no_reason():
|
||||
"""The original cause -- the slicer left no file on a card that is present
|
||||
and working -- is still reported with the original wording, because that
|
||||
advice is right for it."""
|
||||
added = []
|
||||
|
||||
await _run_print_start(_state("ftp://Halterung.gcode.3mf"), added)
|
||||
|
||||
assert _fallback(added).extra_data["no_3mf_reason"] is None
|
||||
@@ -0,0 +1,257 @@
|
||||
"""Which prints are worth an FTPS sweep, and which are not (#2780).
|
||||
|
||||
The gate this module guards is one-sided on purpose, and both sides matter:
|
||||
|
||||
* Missing it costs ~110 doomed FTP connections per print and an archive card
|
||||
that is blank with no stated reason -- the reported bug.
|
||||
* Over-applying it costs archives that work today. A printer that never
|
||||
publishes ``sdcard`` and never had a ``project_file`` reach us must sweep
|
||||
exactly as before, or the fix is a regression for everyone else.
|
||||
|
||||
So the tests below spend most of their weight on the second failure mode.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from backend.app.services.print_storage import (
|
||||
REASON_INTERNAL_STORAGE,
|
||||
REASON_NO_EXTERNAL_STORAGE,
|
||||
external_storage_present,
|
||||
last_print_storage_verdict,
|
||||
print_file_reachable_over_ftp,
|
||||
url_is_external_storage,
|
||||
)
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
|
||||
class FakeState:
|
||||
"""Stand-in for PrinterState with only the fields the helper reads."""
|
||||
|
||||
def __init__(self, current_project_url=None, sdcard=False, sdcard_reported=False, last_project_url=None):
|
||||
self.current_project_url = current_project_url
|
||||
# Defaults to the per-print value: for every test that does not care
|
||||
# about the distinction, the two readings agree.
|
||||
self.last_project_url = current_project_url if last_project_url is None else last_project_url
|
||||
self.sdcard = sdcard
|
||||
self.sdcard_reported = sdcard_reported
|
||||
|
||||
|
||||
class TestUrlScheme:
|
||||
@pytest.mark.parametrize(
|
||||
"url",
|
||||
[
|
||||
"ftp://Benchy.gcode.3mf",
|
||||
# Real dispatches carry names with spaces and non-ASCII; the scheme
|
||||
# is all that is being read and none of that should disturb it.
|
||||
"ftp://Halterung Kühlschrank V2.gcode.3mf",
|
||||
"FTP://Benchy.gcode.3mf",
|
||||
],
|
||||
)
|
||||
def test_ftp_means_external_storage(self, url):
|
||||
assert url_is_external_storage(url) is True
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"url",
|
||||
[
|
||||
# The scheme every H2C and P2S dispatch in #2780's bundle carried,
|
||||
# 35 out of 35.
|
||||
"brtc://emmc/169356_204314.STEP.gcode.3mf",
|
||||
"brtc://emmc/Benchy.gcode.3mf",
|
||||
],
|
||||
)
|
||||
def test_brtc_means_internal_storage(self, url):
|
||||
assert url_is_external_storage(url) is False
|
||||
|
||||
def test_an_unknown_scheme_is_not_assumed_reachable(self):
|
||||
"""Matching the reachable value, not the unreachable one.
|
||||
|
||||
If Bambu ships a third scheme, the safe reading is "somewhere we can't
|
||||
see", not "fine" -- an unrecognised scheme that read as reachable would
|
||||
put the storm straight back.
|
||||
"""
|
||||
assert url_is_external_storage("sftp://Benchy.3mf") is False
|
||||
|
||||
@pytest.mark.parametrize("url", [None, "", "Benchy.gcode.3mf"])
|
||||
def test_no_usable_url_declines_to_answer(self, url):
|
||||
"""None is a third answer and must not collapse into False."""
|
||||
assert url_is_external_storage(url) is None
|
||||
|
||||
@pytest.mark.parametrize("url", [12345, [], {}, object()])
|
||||
def test_a_non_string_url_declines_too(self, url):
|
||||
"""The value arrives straight off the wire, so it is whatever the
|
||||
sender put there. Truth-testing alone would let a non-string fall
|
||||
through to the scheme comparison and read as internal storage --
|
||||
which is a silent skip of a sweep that should have run.
|
||||
"""
|
||||
assert url_is_external_storage(url) is None
|
||||
|
||||
|
||||
class TestSweepIsSkipped:
|
||||
def test_a_print_kept_on_internal_storage(self):
|
||||
verdict = print_file_reachable_over_ftp(FakeState(current_project_url="brtc://emmc/Benchy.gcode.3mf"))
|
||||
|
||||
assert verdict.reachable is False
|
||||
assert verdict.reason == REASON_INTERNAL_STORAGE
|
||||
|
||||
def test_a_printer_that_says_its_slot_is_empty(self):
|
||||
"""#2780's H2C: `sdcard` False for three weeks, 800 clean FTPS
|
||||
connections, and a 550 on every single path it asked for."""
|
||||
verdict = print_file_reachable_over_ftp(FakeState(sdcard=False, sdcard_reported=True))
|
||||
|
||||
assert verdict.reachable is False
|
||||
assert verdict.reason == REASON_NO_EXTERNAL_STORAGE
|
||||
|
||||
|
||||
class TestSweepStillRuns:
|
||||
"""The regression guard. Every case here worked before the gate existed."""
|
||||
|
||||
def test_a_print_on_external_storage(self):
|
||||
assert print_file_reachable_over_ftp(FakeState(current_project_url="ftp://Benchy.gcode.3mf")).reachable
|
||||
|
||||
def test_a_printer_that_never_mentioned_its_card(self):
|
||||
"""Silence is not evidence.
|
||||
|
||||
`sdcard` defaults to False, so a printer whose firmware simply never
|
||||
publishes the field looks identical to an empty slot unless the
|
||||
"did it ever say so" flag is honoured. Reading the default as an
|
||||
answer would skip the sweep for every one of them.
|
||||
"""
|
||||
assert print_file_reachable_over_ftp(FakeState(sdcard=False, sdcard_reported=False)).reachable
|
||||
|
||||
def test_a_printer_with_a_card_and_no_dispatch_seen(self):
|
||||
"""Some brokers refuse the request-topic subscription, so no URL ever
|
||||
arrives. That install must behave exactly as it did before."""
|
||||
assert print_file_reachable_over_ftp(FakeState(sdcard=True, sdcard_reported=True)).reachable
|
||||
|
||||
def test_an_explicit_ftp_url_outranks_a_disagreeing_card_flag(self):
|
||||
"""A false skip is a regression; a needless sweep is only slow.
|
||||
|
||||
When the dispatcher says the file went to external storage, believe
|
||||
the specific claim over the general one.
|
||||
"""
|
||||
state = FakeState(current_project_url="ftp://Benchy.gcode.3mf", sdcard=False, sdcard_reported=True)
|
||||
|
||||
assert print_file_reachable_over_ftp(state).reachable
|
||||
|
||||
def test_no_state_at_all(self):
|
||||
"""Printer not connected, or status not yet populated."""
|
||||
assert print_file_reachable_over_ftp(None).reachable
|
||||
|
||||
def test_a_state_missing_the_fields_entirely(self):
|
||||
"""The helper is duck-typed, and a PrinterState from a pickled or
|
||||
partially-constructed source may predate these fields."""
|
||||
|
||||
class Bare:
|
||||
pass
|
||||
|
||||
assert print_file_reachable_over_ftp(Bare()).reachable
|
||||
|
||||
|
||||
class TestReasonIsAlwaysPresentWhenUnreachable:
|
||||
@pytest.mark.parametrize(
|
||||
"state",
|
||||
[
|
||||
FakeState(current_project_url="brtc://emmc/x.3mf"),
|
||||
FakeState(sdcard=False, sdcard_reported=True),
|
||||
],
|
||||
)
|
||||
def test_unreachable_carries_a_reason(self, state):
|
||||
"""The reason crosses into the API and picks the banner text. An
|
||||
unreachable verdict without one would render the generic advice --
|
||||
which is the wrong advice, and the whole point of the change."""
|
||||
verdict = print_file_reachable_over_ftp(state)
|
||||
|
||||
assert verdict.reachable is False
|
||||
assert verdict.reason
|
||||
|
||||
def test_reachable_carries_no_reason(self):
|
||||
assert print_file_reachable_over_ftp(FakeState(sdcard=True, sdcard_reported=True)).reason is None
|
||||
|
||||
|
||||
class TestTheGateUsesThePerPrintUrlOnly:
|
||||
"""A stale URL must never gate a sweep.
|
||||
|
||||
``current_project_url`` is cleared when a print ends; ``last_project_url``
|
||||
is sticky for reporting. The gate reads only the first, and that is
|
||||
load-bearing rather than tidiness: 18% of the print starts in #2780's
|
||||
support bundle (14 of 79) had no ``project_file`` on the request topic at
|
||||
all -- touchscreen reprints, restart recovery, anything Bambuddy did not
|
||||
see dispatched. If those inherited the previous job's destination, a
|
||||
printer that ran one Studio print to internal storage would skip the FTPS
|
||||
sweep for every subsequent screen-started print, losing archives that work
|
||||
today.
|
||||
|
||||
The asymmetry is what makes it worth pinning: a stale ``ftp://`` costs
|
||||
only a pointless sweep, while a stale ``brtc://`` costs an archive.
|
||||
"""
|
||||
|
||||
def test_a_print_with_no_dispatch_of_its_own_still_sweeps(self):
|
||||
"""The previous print went to internal storage; this one Bambuddy
|
||||
never saw dispatched. Unknown, so sweep."""
|
||||
state = FakeState(
|
||||
current_project_url=None,
|
||||
last_project_url="brtc://emmc/previous.gcode.3mf",
|
||||
sdcard=True,
|
||||
sdcard_reported=True,
|
||||
)
|
||||
|
||||
assert print_file_reachable_over_ftp(state).reachable
|
||||
|
||||
def test_the_sticky_reading_still_reports_it(self):
|
||||
"""The diagnostic is normally run after the print that prompted it, so
|
||||
it needs the answer the gate has rightly forgotten."""
|
||||
state = FakeState(
|
||||
current_project_url=None,
|
||||
last_project_url="brtc://emmc/previous.gcode.3mf",
|
||||
sdcard=True,
|
||||
sdcard_reported=True,
|
||||
)
|
||||
|
||||
verdict = last_print_storage_verdict(state)
|
||||
|
||||
assert verdict.reachable is False
|
||||
assert verdict.reason == REASON_INTERNAL_STORAGE
|
||||
|
||||
def test_an_empty_slot_is_reported_by_both(self):
|
||||
"""Not URL-derived, so clearing the per-print value changes nothing."""
|
||||
state = FakeState(sdcard=False, sdcard_reported=True)
|
||||
|
||||
assert print_file_reachable_over_ftp(state).reason == REASON_NO_EXTERNAL_STORAGE
|
||||
assert last_print_storage_verdict(state).reason == REASON_NO_EXTERNAL_STORAGE
|
||||
|
||||
def test_the_sticky_reading_never_gates_a_sweep(self):
|
||||
"""Guard against someone swapping the two back: if the gate ever reads
|
||||
the sticky field, the case above starts failing -- and so does this."""
|
||||
import inspect
|
||||
|
||||
from backend.app.services import print_storage
|
||||
|
||||
source = inspect.getsource(print_storage.print_file_reachable_over_ftp)
|
||||
assert "current_project_url" in source
|
||||
assert "last_project_url" not in source.split('"""')[-1]
|
||||
|
||||
|
||||
class TestTimelapseUsesTheNarrowerRule:
|
||||
"""The printer writes its timelapse to the card itself.
|
||||
|
||||
Where the *sliced file* went says nothing about whether a video exists, so
|
||||
gating the timelapse scan on the URL would silently stop finding videos on
|
||||
every H2C and P2S that has a card in -- a new bug, introduced by the fix
|
||||
for this one.
|
||||
"""
|
||||
|
||||
def test_internal_storage_does_not_suppress_the_timelapse_scan(self):
|
||||
state = FakeState(current_project_url="brtc://emmc/x.3mf", sdcard=True, sdcard_reported=True)
|
||||
|
||||
assert print_file_reachable_over_ftp(state).reachable is False
|
||||
assert external_storage_present(state) is True
|
||||
|
||||
def test_an_empty_slot_does_suppress_it(self):
|
||||
assert external_storage_present(FakeState(sdcard=False, sdcard_reported=True)) is False
|
||||
|
||||
def test_silence_does_not(self):
|
||||
assert external_storage_present(FakeState(sdcard=False, sdcard_reported=False)) is True
|
||||
|
||||
def test_no_state_does_not(self):
|
||||
assert external_storage_present(None) is True
|
||||
Reference in New Issue
Block a user