diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a8edf34d..edf06fcdb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to Bambuddy will be documented in this file. ## [1.2.6b1] - Unreleased ### Added +- **A fault's description is in the status response, so a client no longer needs its own copy of the table (#2926, proposed and analysed by @sadontsev)** — `HMS_ERROR_DESCRIPTIONS` has been in the backend all along and the status response never carried it, so every consumer that wanted to tell a user *why* a print halted resolved the same 853 codes from its own duplicate of the same sentences — this repo's Python table, the frontend modal's, and at least one third-party iOS client whose catalogue exists purely because the server would not say. Each aged separately, and a push relay watching a printer could only manage "your printer needs attention" while the server already knew it was "Filament ran out. Please load new filament." `hms_errors[]` entries now carry `description`, defaulting to null so a client that has never seen the field is unaffected. It is resolved once, where the fault is parsed, rather than at the boundary that happened to prompt the request: there are three separate serializers of a fault — the status response, the WebSocket broadcast, and the print-completion payload the queue's failure reason is built from — and adding it to only the first would have delivered half the feature to a relay watching the stream, which is the likelier consumer. The queue's failure reason now quotes the same sentence instead of resolving the code a fourth time, and the notification path reads it rather than re-deriving its own. Resolving in one place is also what makes the three unable to drift, which is pinned by a test that asserts they agree. Resolution is exactly what the codebase already did, verified rather than assumed: an 8-char `print_error` is the catalogue's `MMMM_EEEE` key split in half, and a 16-char `hms[]` identifier is tried whole and then collapsed to its first and last groups, which is how the notification path, the queue's failure-reason helper and the frontend modal have always resolved those. The collapse is lossy — #2728 counts 65 documented faults falling onto `0300_0001` alone — and it is kept rather than tightened here because refusing it would not read the same data more strictly, it would stop describing faults that are described today and leave this field null while the UI shows text for the same fault. Narrowing it belongs with #2728, where both key spaces can move together. A fault the catalogue does not cover reports null and is still reported in full; `full_code` identifies it either way. The equivalence is pinned by a test that checks every catalogue code in both fault shapes across all three alert levels, so a future change to the lookup cannot silently stop notifications from firing. The text is English only and unlocalized, which the schema says next to the field. The frontend keeps resolving its own text for now; switching it over would change what `filterKnownHMSErrors` counts across eight call sites, which is #1840 and #2728's argument rather than this one's. - **A virtual printer can be told which address to advertise, so uploads work on Docker bridge networking (#2930, reported and diagnosed by @sebimarkgraf)** — `VIRTUAL_PRINTER_ADVERTISE_ADDRESS` sets the address written into the MQTT status that BambuStudio and OrcaSlicer read their FTP upload destination from. It exists for deployments where that address is not one of the container's own interfaces: on bridge networking the virtual printer is reached on the host's LAN IP but binds a private one like `172.24.0.2`, and that private address is what the slicer was handed — so it opened an FTP connection to an address that does not exist on its network, which is the upload stalling around 10% with "Failed to send" that the troubleshooting page has been describing as a limitation with no fix. Set it to the address slicers use, alongside the `VIRTUAL_PRINTER_PASV_ADDRESS` that already existed for the passive-data channel. The log line that arms the rewrite names its source, so `(VIRTUAL_PRINTER_ADVERTISE_ADDRESS)` versus `(bind_address)` says whether the variable reached the container. A value that is not a dotted-quad IPv4 is refused with one warning naming it and the address that would have been used before is used instead — deliberately, because refusing to rewrite at all would put the *real printer's* IP back in front of the slicer, which is the leak this path exists to close and strictly worse than the wrong local address. `0.0.0.0` counts as unset, and surrounding whitespace is tolerated for the sake of values pasted into a compose file. This is an environment variable rather than a change to how the advertised address is resolved, and that was the decision worth making carefully: the virtual printer already has a "Network Interface Override" field, but it feeds SSDP and the certificate's SAN list only, and reading it here would have moved the upload destination on every install that has one set — the multi-NIC, VLAN and Tailscale setups, which are the ones most likely to have been arrived at by hand and the least likely to survive being second-guessed. Unset, nothing about the resolution changes, which is pinned by a test. Host and macvlan networking still need none of this and remain what Virtual Printer is developed against; the variable removes one blocker rather than making bridge mode equivalent, and the wiki now says so in the three places that previously stated the host address could not be discovered at all. - **Printer file downloads can be selected in ranges and print-history videos can be downloaded (#2850, requested and contributed by @logikal in #2853)** — The printer file browser's multi-select download now prepares large selections on the app data volume instead of buffering them in server and browser memory, uses per-file compression (videos stored, G-code/3MF compressed), reports partial results and preparation progress, supports cancellation, rejects over-large or under-space selections, and preserves the legacy API contract. Shift-click selects a contiguous visible range and hidden selections are discarded when navigating or filtering. Print History now offers attached timelapses, matching printer timelapses, and `/ipcam` chunks when available; offline or unreadable storage is distinguished from an empty directory. Download tokens are single-use and resource-bound, API-key printer allowlists are enforced, FTP short reads are rejected, and abandoned staging is pruned. Translated in all locales; wiki updated. Covered by backend and frontend regression tests. - **Bambuddy now asks a printer that refuses FTPS what it actually said (#2780, measured by @grolmus)** — When a printer's file service answers port 990 with something that is not TLS, Python reports `[SSL: WRONG_VERSION_NUMBER]` and the bytes that caused it are gone, consumed by the TLS layer before the error surfaces. That has left #2780 open on a theory rather than a finding. The client now opens one plain connection straight afterwards and reads what the printer says, so the log carries the printer's own words — an FTP refusal such as `421 Too many connections` would identify the fault outright — and the line is marked as the one to quote in a report. Reading nothing is informative too, and says so: a healthy implicit-FTPS service stays silent until it gets a handshake, so silence means the refusal had already passed. It asks once per cool-off window rather than once per attempt, which keeps it to one extra connection per printer per five minutes — the suspected fault is a printer running out of connections, so the diagnosis must not add to it. What made this worth doing is a measurement from a nine-printer farm, reproduced here: a cleartext banner on the TLS port produces exactly the error the field reports, a genuine TLS version mismatch produces a different one, and a client with no version cap reaches a TLS-1.2-only peer unaided. So this failure was never a TLS-version problem, and the per-model `cap_tls_v1_2` knob cannot affect it. Two of the three entries carrying that knob were added on the belief that it could; they are kept, since their reporters saw the symptom clear and nobody here has the hardware to re-test on, but they are now marked for re-test and the reasoning recorded next to them is what was measured rather than what was assumed. Both measurements are pinned by tests, so the explanation stays falsifiable. @@ -15,6 +16,7 @@ All notable changes to Bambuddy will be documented in this file. - **The Windows installer build is split in two so a signing request can wait for a human (SignPath Foundation)** — Release tags are Authenticode-signed through the SignPath Foundation OSS programme, and the production certificate does not sign on demand the way the self-signed test certificate does: every request has to be approved by hand in the SignPath UI, because the Foundation verifies what is being signed and which build it came from. The submitting action waits for that approval with a default timeout of 600 seconds, which is ample when the test policy approves automatically in seconds and far too short once the wait is a person noticing a tag went out. A tag pushed at night would have failed the run ten minutes later with the installer already compiled and thrown away. The compile now ends in its own job that uploads the unsigned artifact and stops; a second job downloads it, signs it, and does the release-facing work, with the wait raised to an hour. Because the artifact is uploaded before the wait begins and is addressed by id, a missed approval window is recovered by re-running the second job alone rather than rebuilding the installer — which is the reason to separate them rather than simply raise the timeout in place. The second job runs for unsigned builds too, so the daily prereleases that are deliberately left unsigned to preserve the signing quota keep going out through exactly one set of alias, artifact and release steps. The property that matters is unchanged and now recorded next to the steps that depend on it: none of the alias, upload or release-attach steps carry `always()`, so GitHub skips all three when signing fails or times out, and an unsigned `.exe` cannot reach a release. Nothing about the signed output changes, and the restructure behaves identically under the test policy — the request simply completes immediately instead of waiting — so it can be proven green before the production certificate arrives. ### Fixed +- **A print that failed on an `hms[]` fault recorded an unlookupable error code** — The queue's failure reason is built by formatting the fault's module and error into `MMMM_EEEE`, and that one derivation never masked the error to 16 bits. A fault arriving from the printer's `hms[]` array carries its alert level in the code's high half, so the label came out as e.g. `0500_3000A` — five digits in a group that has four. It is not a code the user can look up on Bambu's HMS index, and because it matches no catalogue key the sentence explaining the failure was dropped along with it, leaving the bracketed number alone. The nozzle-size mismatch behind #1111 is exactly such a fault: it reads as `[0500_4038] The nozzle diameter in sliced file is not consistent...` when the printer reports it one way and read as a bare `[0500_24038]` when it reported it the other. There was already a helper that gets this right and is used by the archive's own failure-reason lookup; the queue's now calls it instead of keeping a fourth copy of the derivation. - **AMS slots were offered as places to store a spool** — The Storage Location dropdown in the spool editor listed entries like "H2D-1 - AMS A1" alongside real locations, and they could not be removed. They are not locations at all: Bambuddy used to record which slot a spool was loaded into by writing that string into Spoolman's `location` field, and although that writer went away when Storage Location became something the user picks, the strings stayed on people's Spoolman spools — where the location sync, which imports every distinct one it finds, has been reading them back ever since. A printer slot is where a spool is loaded, not where it is put away, and Bambuddy already tracks the first through slot assignments. Deleting one by hand did not work either, which is what made this a dead end rather than an annoyance: the delete route refuses a location that has spools, and in Spoolman mode it counts them by matching that same string, so every marker still sitting on a loaded spool answered 409 — and the two that were empty came back on the next sync a minute later. The import now skips them, and the ones already in the catalogue are removed on upgrade. The filter is deliberately narrow, matching only the shape Bambuddy itself wrote — an optional printer-name prefix followed by `AMS A1`, `AMS-HT A1` or `External Spool` — so "AMS Drybox" and "Spare AMS trays" are left alone; anything it swallowed would be a place the user could no longer file a spool under. A row is only removed when no spool in Bambuddy's own database points at it, by id or by legacy free-text name, so an internal-mode user who has deliberately filed spools under such a name keeps it. Spools in Spoolman are not touched: their location strings are the user's data on the user's server, and one that still reads "H2D-1 - AMS A1" in the inventory list is telling the truth about what Spoolman holds — it simply stops being offered as a destination. - **A wood, silk or gradient roll the AMS added for you was drawn as a flat disc** — A spool's swatch is composed from `effect_type` and `extra_colors`, and the RFID auto-add set neither. It reads the colour catalogue to name the colour and took the name alone, even though the row it had in hand also carries those two columns — the spool form's own colour picker hands both to a spool a user adds by hand, so the same roll rendered one way when you typed it in and another when the printer identified it for you. Both columns now travel with the name. That alone would have changed nothing on a stock install, because the shipped catalogue carries an effect on none of its 600-odd rows, so the subtype is read where the catalogue has none: it is already derived from what the printer reports, and the two vocabularies line up — Wood, Silk, Sparkle, Marble, Glow, Galaxy, Metal, Rainbow, Translucent, Matte, and the Gradient, Dual Color and Tri Color that the M*/T* colour codes upgrade a subtype to. "Silk+" is read as Silk, since the plus is on the product name rather than the finish. A subtype that names no effect — Basic, Tough, CF — leaves the column empty rather than inventing an overlay, and a value already set is never overwritten, so the column stays what it is documented to be: a rendering hint the user can override without touching Bambu's categorical label. - **The spool tare an RFID roll was added with is corrected on upgrade (#2909, diagnosed by @ojimpo)** — The lookup that gave an auto-added spool its `core_weight` asked for the first catalogue row whose name starts "Bambu Lab" and took whatever came back. There are three, and which is first is the database's business: SQLite returns insertion order in practice, Postgres promises nothing once a table has seen an update, so the same roll was recorded with the 216 g High Temp tare on one install and correctly with the 250 g Low Temp one on another. The forward fix picks the row by name; this repairs the rows already written, which the forward fix cannot reach. The tare is not cosmetic: a spool weighed on SpoolBuddy has its remaining filament worked out as the scale reading minus the tare, so a 34 g low tare credits the roll with 34 g that is not there and writes a used weight 34 g short. That error is a constant — every later print adds to the used weight on top of it — so adding the difference back is exact however much has been printed since, and it is applied only to spools that have actually been on the scale; one that never was has a used weight derived from the AMS remaining percentage, which the tare never entered into. The rows to repair are identified by the signature of the broken lookup — added by RFID, carrying the weight of one of the *other* Bambu catalogue rows — with the weights read out of the catalogue rather than hardcoded, so an install whose rows have been re-measured is repaired to its own numbers. One case cannot be told apart and is stated rather than hidden: someone who moved an RFID roll onto a genuine High Temp spool and set 216 g by hand looks identical to a row the lookup got wrong and is normalised with them. Keying on whether a catalogue row had been recorded would not have rescued them either — the weight picker auto-selects the only row matching the weight and writes its id on the next save, so that column says only whether the form was ever opened. Runs exactly once, so a tare set afterwards is kept. diff --git a/backend/app/api/routes/printers.py b/backend/app/api/routes/printers.py index d04ea28ef..8cf26b6e9 100644 --- a/backend/app/api/routes/printers.py +++ b/backend/app/api/routes/printers.py @@ -512,6 +512,7 @@ async def get_printer_status( actions=e.actions, job_id=e.job_id, full_code=e.full_code, + description=e.description, ) for e in (state.hms_errors or []) ] diff --git a/backend/app/main.py b/backend/app/main.py index 022864137..81b9ff669 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -1267,11 +1267,13 @@ def _maybe_start_layer_timelapse(printer, printer_id: int, archive_id: int) -> b def _format_hms_error_summary(hms_errors: list[dict]) -> str | None: """Build a human-readable failure reason from MQTT hms_errors for PrintQueueItem.error_message. - Each entry has keys: code ('0x4038'), attr (32-bit int), module, severity. - The short code used for the hms_errors.py lookup table is 'MMMM_EEEE' — module - from attr bits 16-31, error from the numeric part of code. Falls back to the raw - short code when no description is on file. Returns None for an empty list so - callers can leave error_message unset. + Each entry has keys: code ('0x4038'), attr (32-bit int), module, severity, and + — since #2926 — the description the parser already resolved, which is preferred + when present so the queue's failure reason reads the same as the status + response. The short code still produces the bracketed label, and still + resolves the sentence for a caller whose entries predate the field. Falls back + to the bare short code when no description is on file. Returns None for an + empty list so callers can leave error_message unset. """ if not hms_errors: return None @@ -1280,13 +1282,15 @@ def _format_hms_error_summary(hms_errors: list[dict]) -> str | None: parts: list[str] = [] for err in hms_errors: try: - code_str = str(err.get("code", "")).replace("0x", "") - error_num = int(code_str, 16) if code_str else 0 - module_num = (int(err.get("attr", 0)) >> 16) & 0xFFFF - short_code = f"{module_num:04X}_{error_num:04X}" + # `_hms_short_code` rather than a local derivation: this one used to + # format the error without masking it to 16 bits, so an `hms[]` entry + # whose code carries an alert-level group produced a five-digit label + # like "0500_3000A" — not a code the user can look up, and never a + # catalogue key, so the sentence was lost with it. + short_code = _hms_short_code(err.get("attr", 0), err.get("code", 0)) except (TypeError, ValueError): continue - description = get_error_description(short_code) + description = err.get("description") or get_error_description(short_code) parts.append(f"[{short_code}] {description}" if description else f"[{short_code}]") return "; ".join(parts) if parts else None @@ -1729,8 +1733,6 @@ async def on_printer_status_change(printer_id: int, state: PrinterState): 0x12: "Chamber", } - from backend.app.services.hms_errors import get_error_description - # Capture camera snapshot once for all error notifications (no DB held). error_image_data = await _capture_snapshot_for_notification( printer_id, printer, logging.getLogger(__name__) @@ -1749,7 +1751,9 @@ async def on_printer_status_change(printer_id: int, state: PrinterState): # Only notify for errors with known descriptions — printers # send many undocumented/phantom codes that aren't real errors. - description = get_error_description(short_code) + # Resolved at parse time (#2926); short_code is still needed + # for the suppression set below. + description = error.description if not description or short_code in _HMS_NOTIFICATION_SUPPRESS: continue diff --git a/backend/app/schemas/printer.py b/backend/app/schemas/printer.py index a24265681..094be343b 100644 --- a/backend/app/schemas/printer.py +++ b/backend/app/schemas/printer.py @@ -170,6 +170,15 @@ class HMSErrorResponse(BaseModel): # truncated short_code that historically caused silent command rejection # (#1830, H2D wrong-plate verification). full_code: str = "" + # The bundled catalogue's sentence for this fault, so a client does not have + # to carry its own copy of the same table to tell a user why a print halted + # (#2926). English only and not localized — the catalogue ships one language. + # None when the catalogue does not cover the code, which is common for + # `hms[]`-array faults: those resolve through a lossy collapse of their + # 16-char identifier and many land on no key at all (#2728). A client should + # treat null as "no text available", never as "no fault" — `full_code` is + # what identifies the fault, and it is always present. + description: str | None = None class AMSTray(BaseModel): diff --git a/backend/app/services/bambu_mqtt.py b/backend/app/services/bambu_mqtt.py index 194327722..db8e0ca54 100644 --- a/backend/app/services/bambu_mqtt.py +++ b/backend/app/services/bambu_mqtt.py @@ -22,6 +22,7 @@ from datetime import datetime, timezone import paho.mqtt.client as mqtt from backend.app.services.hms_actions import HMSAction, get_actions_for_error_code +from backend.app.services.hms_errors import describe_fault from backend.app.utils.ams_drying import ACTIVE_DRY_STATUSES logger = logging.getLogger(__name__) @@ -625,7 +626,13 @@ class HMSError: attr: int # Attribute value for constructing wiki URL module: int severity: int # 1=fatal, 2=serious, 3=common, 4=info - message: str = "" + # The bundled catalogue's sentence for this fault, resolved once here so + # every surface that reports it — the status response, the WebSocket + # broadcast, the completion payload, notifications — says the same thing. + # None when the catalogue does not cover the code; `describe_fault` documents + # the lookup and why the lossy `hms[]` collapse is kept as it was. + # Replaces a `message` field that was never set or read anywhere. + description: str | None = None # User-facing remediation actions from the bundled HMS catalog (e.g. "RESUME_PRINTING", # "CHECK_ASSISTANT"). Defaults to an empty list rather than None so the field always # satisfies HMSErrorResponse.actions: list[str] — a future code path that builds an @@ -4576,6 +4583,7 @@ class BambuMQTTClient: actions=actions, job_id=self.state.subtask_id, full_code=full_code, + description=describe_fault(full_code), ) ) self._apply_mqtt_verify_state(verify_failed) @@ -4651,6 +4659,7 @@ class BambuMQTTClient: # print_error is already 32-bit — `f"{print_error:08X}"` # is the firmware's matching key with no truncation. full_code=f"{print_error:08X}", + description=describe_fault(f"{print_error:08X}"), ) ) @@ -5228,7 +5237,16 @@ class BambuMQTTClient: # Include HMS errors for failure reason detection hms_errors_data = ( [ - {"code": e.code, "attr": e.attr, "module": e.module, "severity": e.severity} + { + "code": e.code, + "attr": e.attr, + "module": e.module, + "severity": e.severity, + # Carried so the queue's failure reason quotes the same + # sentence the status response and the broadcast do, + # rather than resolving the code a fourth time (#2926). + "description": e.description, + } for e in self.state.hms_errors ] if self.state.hms_errors diff --git a/backend/app/services/hms_errors.py b/backend/app/services/hms_errors.py index a9905cc87..323a9ce0d 100644 --- a/backend/app/services/hms_errors.py +++ b/backend/app/services/hms_errors.py @@ -873,3 +873,48 @@ def get_error_description(error_code: str) -> str | None: Human-readable description or None if not found """ return HMS_ERROR_DESCRIPTIONS.get(error_code.upper()) + + +def describe_fault(full_code: str | None) -> str | None: + """Resolve a fault's description from the canonical `full_code`. + + `full_code` is the identifier the firmware itself matches on: 8 hex chars + for a 32-bit `print_error`, 16 for a 64-bit `hms[]` entry. This is the one + place that maps either shape onto this table, so every surface that reports + a fault says the same thing about it. + + An 8-char code is this table's `MMMM_EEEE` key with the separator removed -- + the parser derives `full_code` and that key from the same 32-bit value -- so + it resolves exactly. + + A 16-char code is tried whole first, then collapsed to `G1_G4` (the first + and last of its four hex groups). That collapse is lossy and not injective: + it discards the Part No. and Alert level groups, and #2728 measured 65 + documented faults falling onto `0300_0001` alone, so a hit can in principle + attribute a neighbouring fault's sentence to this one. It is kept because it + is what this codebase has always done -- the notification path, the queue's + failure-reason helper and the frontend modal all resolve `hms[]` faults this + way, and it does resolve real ones (a `0500_4038` nozzle mismatch arrives in + that shape). Refusing to collapse would not be a stricter reading of the + same data; it would silently stop describing faults that are described + today, and leave this field null while the UI shows text for the same fault. + Narrowing it is #2728's subject, and belongs there where the key spaces can + be changed together. + + Returns None for an empty, malformed, or unknown code. + """ + if not full_code: + return None + code = full_code.strip().upper() + if len(code) == 8: + return HMS_ERROR_DESCRIPTIONS.get(f"{code[:4]}_{code[4:]}") + if len(code) == 16: + # `is not None` rather than truthiness: an entry whose text is empty is + # still an entry, and falling through on it would resolve the fault to a + # neighbour's sentence. No blank values ship today; the frontend lookup + # draws the same distinction and a regenerated catalogue could. + exact = HMS_ERROR_DESCRIPTIONS.get(code) + if exact is not None: + return exact + return HMS_ERROR_DESCRIPTIONS.get(f"{code[:4]}_{code[12:]}") + return None diff --git a/backend/app/services/printer_manager.py b/backend/app/services/printer_manager.py index 7e1c11fee..79803a883 100644 --- a/backend/app/services/printer_manager.py +++ b/backend/app/services/printer_manager.py @@ -1539,6 +1539,10 @@ def printer_state_to_dict( "actions": e.actions, "job_id": e.job_id, "full_code": e.full_code, + # Same field as the status response carries (#2926) — a relay + # watching the stream should not have to poll REST to find out + # what a fault means. + "description": e.description, } for e in (state.hms_errors or []) ], diff --git a/backend/tests/unit/services/test_bambu_mqtt.py b/backend/tests/unit/services/test_bambu_mqtt.py index 1e9e1f600..5619ac3e8 100644 --- a/backend/tests/unit/services/test_bambu_mqtt.py +++ b/backend/tests/unit/services/test_bambu_mqtt.py @@ -5452,6 +5452,37 @@ class TestHMSFullCode: assert len(mqtt_client.state.hms_errors) == 1 assert mqtt_client.state.hms_errors[0].full_code == "05008051" + def test_print_error_path_carries_the_catalogue_description(self, mqtt_client): + """The sentence is resolved once, at parse time, so every surface that + reports the fault quotes the same text (#2926).""" + mqtt_client._update_state({"print_error": 0x03008004}) + assert len(mqtt_client.state.hms_errors) == 1 + assert mqtt_client.state.hms_errors[0].description == "Filament ran out. Please load new filament." + + def test_print_error_path_leaves_description_none_for_an_uncatalogued_code(self, mqtt_client): + """An undocumented code gets no invented text — the field is the + catalogue's answer, not a placeholder.""" + mqtt_client._update_state({"print_error": 0x03009999}) + assert len(mqtt_client.state.hms_errors) == 1 + assert mqtt_client.state.hms_errors[0].description is None + + def test_hms_array_path_resolves_via_the_short_key(self, mqtt_client): + """`hms[]` faults resolve through the G1_G4 collapse — the same lookup + the notification path and the frontend modal have always used. 0500_4038 + is the nozzle-size mismatch behind #1111 and it arrives in this shape.""" + mqtt_client._update_state({"hms": [{"attr": 0x05000000, "code": 0x00004038}]}) + assert len(mqtt_client.state.hms_errors) == 1 + assert "nozzle diameter" in (mqtt_client.state.hms_errors[0].description or "") + + def test_hms_array_leaves_description_none_when_uncatalogued(self, mqtt_client): + """A real P2S fault (#2728) whose collapse is "0500_000A" — not a + catalogue key, since none has an error group below 0x4000. The fault is + still reported; only the text is absent.""" + mqtt_client._update_state({"hms": [{"attr": 0x05000200, "code": 0x0003000A}]}) + assert len(mqtt_client.state.hms_errors) == 1 + assert mqtt_client.state.hms_errors[0].full_code == "050002000003000A" + assert mqtt_client.state.hms_errors[0].description is None + def test_hms_array_catalog_lookup_tries_16_char_first(self, mqtt_client, monkeypatch): """When the catalog has both an 8-char and a 16-char entry for the same fault family, the 16-char (specific variant) wins. The 8-char diff --git a/backend/tests/unit/services/test_hms_errors.py b/backend/tests/unit/services/test_hms_errors.py index 50f82789d..fbdc926ea 100644 --- a/backend/tests/unit/services/test_hms_errors.py +++ b/backend/tests/unit/services/test_hms_errors.py @@ -1,6 +1,6 @@ """Tests for HMS error code translations.""" -from backend.app.services.hms_errors import HMS_ERROR_DESCRIPTIONS, get_error_description +from backend.app.services.hms_errors import HMS_ERROR_DESCRIPTIONS, describe_fault, get_error_description class TestHMSErrorDescriptions: @@ -72,3 +72,94 @@ class TestGetErrorDescription: for code in common_codes: result = get_error_description(code) assert result is not None, f"Missing description for common code: {code}" + + +class TestDescribeFault: + """`describe_fault` maps a fault's canonical `full_code` onto the catalogue, + so every surface that reports a fault resolves it the same way (#2926).""" + + def test_resolves_an_eight_char_print_error_code(self): + """The parser derives full_code and the catalogue key from the same + 32-bit value, so the split is exact rather than a guess.""" + assert describe_fault("03008004") == "Filament ran out. Please load new filament." + + def test_resolves_regardless_of_case(self): + """Firmware-facing code is uppercase, but a client echoing a value back + from its own store may not be.""" + assert describe_fault("0300400c") == "The task was canceled." + + def test_tolerates_surrounding_whitespace(self): + assert describe_fault(" 03008004 ") == "Filament ran out. Please load new filament." + + def test_returns_none_for_an_hms_code_outside_the_catalogue(self): + """A real P2S fault from #2728. Neither the whole 16-char key nor its + G1_G4 collapse ("0500_000A") is in the catalogue — no catalogue key has + an error group below 0x4000, and this family's is 0x000A.""" + assert describe_fault("050002000003000A") is None + + def test_collapses_a_sixteen_char_code_to_its_g1_g4_short_key(self): + """Lossy, and kept deliberately: this is how the notification path, the + queue's failure-reason helper and the frontend modal have always + resolved `hms[]` faults, and it resolves real ones. Refusing would stop + describing faults that are described today (see the module docstring).""" + key = next(iter(HMS_ERROR_DESCRIPTIONS)) # e.g. "0300_4000" + module, error = key.split("_") + forced = f"{module}02000003{error}" # four 4-hex groups; G1 and G4 are the key + assert len(forced) == 16 + assert describe_fault(forced) == HMS_ERROR_DESCRIPTIONS[key] + + def test_prefers_the_whole_sixteen_char_key_over_the_collapse(self): + """The full identifier is lossless, so it wins when the catalogue has + both. No 16-char keys ship today; this pins the order for when they do.""" + key = next(iter(HMS_ERROR_DESCRIPTIONS)) + module, error = key.split("_") + forced = f"{module}02000003{error}" + HMS_ERROR_DESCRIPTIONS[forced] = "specific variant" + try: + assert describe_fault(forced) == "specific variant" + finally: + del HMS_ERROR_DESCRIPTIONS[forced] + + def test_matches_the_derivation_it_replaced( + self, + ): + """The regression guard for the consolidation: for every fault shape the + codebase can produce, `describe_fault` returns exactly what the + attr/code short-code lookup in the notification path returned before it. + Covers both families and all three alert levels a real `hms[]` code + carries — a divergence here means notifications silently stop firing for + faults that used to raise them.""" + for key, expected in HMS_ERROR_DESCRIPTIONS.items(): + module, error = int(key[:4], 16), int(key[5:], 16) + + # print_error: attr is the whole 32-bit value, code its low half. + print_error = (module << 16) | error + assert describe_fault(f"{print_error:08X}") == expected + + # hms[]: attr is groups 1-2, code is groups 3-4 (alert level + id). + for alert_level in (0x0000, 0x0002, 0x0003): + attr = (module << 16) | 0x0200 + code = (alert_level << 16) | error + legacy = get_error_description(f"{(attr >> 16) & 0xFFFF:04X}_{code & 0xFFFF:04X}") + assert describe_fault(f"{attr:08X}{code:08X}") == legacy == expected + + def test_returns_none_for_an_unknown_eight_char_code(self): + assert describe_fault("99999999") is None + + def test_returns_none_for_empty_or_missing(self): + """The dataclass default is "" and the field is optional on the wire.""" + assert describe_fault("") is None + assert describe_fault(None) is None + + def test_returns_none_for_a_malformed_length(self): + """Neither 8 nor 16 chars — no shape to interpret, so no guess.""" + assert describe_fault("0300") is None + assert describe_fault("030080040") is None + + def test_agrees_with_the_short_code_lookup_for_print_error_codes(self): + """Pins the equivalence the consolidation rests on: for every 8-char + code the catalogue covers, describe_fault returns exactly what the + pre-existing short-code lookup did.""" + for key, expected in HMS_ERROR_DESCRIPTIONS.items(): + assert describe_fault(key.replace("_", "")) == expected + assert get_error_description(key) == expected diff --git a/backend/tests/unit/test_hms_description_surfaces.py b/backend/tests/unit/test_hms_description_surfaces.py new file mode 100644 index 000000000..a6705e19b --- /dev/null +++ b/backend/tests/unit/test_hms_description_surfaces.py @@ -0,0 +1,133 @@ +"""One fault, one sentence, on every surface that reports it (#2926). + +The catalogue in ``services/hms_errors.py`` has always held the text, and the +status response never carried it, so each client resolved the same codes from +its own copy of the same table. The description is now resolved once, at parse +time, and passed through by all three serializers of an ``HMSError``: the +status response, the WebSocket broadcast, and the print-completion payload the +queue's failure reason is built from. These tests pin that they agree — the +point of resolving it in one place is that they cannot drift apart. +""" + +import pytest + +from backend.app.main import _format_hms_error_summary +from backend.app.schemas.printer import HMSErrorResponse +from backend.app.services.bambu_mqtt import HMSError, PrinterState +from backend.app.services.printer_manager import printer_state_to_dict + +RUNOUT_SENTENCE = "Filament ran out. Please load new filament." + + +def _runout() -> HMSError: + """A `print_error` fault the catalogue covers, as the parser builds it.""" + return HMSError( + code="0x8004", + attr=0x03008004, + module=3, + severity=3, + full_code="03008004", + description=RUNOUT_SENTENCE, + ) + + +def _uncatalogued() -> HMSError: + """An `hms[]` fault the catalogue cannot describe — a real P2S code (#2728). + Its G1_G4 collapse is "0500_000A", which is not a key either.""" + return HMSError( + code="0x3000a", + attr=0x05000200, + module=5, + severity=2, + full_code="050002000003000A", + description=None, + ) + + +class TestStatusResponse: + def test_carries_the_description(self): + """What the route's mapper produces — the field a third-party client + needs so it does not have to ship the catalogue itself.""" + e = _runout() + assert ( + HMSErrorResponse( + code=e.code, + attr=e.attr, + module=e.module, + severity=e.severity, + actions=e.actions, + job_id=e.job_id, + full_code=e.full_code, + description=e.description, + ).description + == RUNOUT_SENTENCE + ) + + def test_defaults_to_none_when_not_supplied(self): + """A producer that never sets it still validates, so the field cannot + break an existing construction path.""" + assert HMSErrorResponse(code="0x8004", attr=0, module=3, severity=3).description is None + + def test_serializes_as_null_rather_than_being_dropped(self): + """A client distinguishing "no text" from "field absent" needs the key + present. Pydantic includes None by default; pin it so a later + `exclude_none` does not silently change the contract.""" + payload = HMSErrorResponse(code="0x3000a", attr=0, module=5, severity=2).model_dump() + assert "description" in payload + assert payload["description"] is None + + +class TestWebSocketBroadcast: + def test_carries_the_description(self): + """The broadcast is a separate hand-rolled serializer; a relay watching + the stream should not have to poll REST to find out what a fault means.""" + state = PrinterState() + state.hms_errors = [_runout()] + assert printer_state_to_dict(state, printer_id=1)["hms_errors"][0]["description"] == RUNOUT_SENTENCE + + def test_passes_none_through_for_an_uncatalogued_fault(self): + """The fault is still broadcast — only the text is missing.""" + state = PrinterState() + state.hms_errors = [_uncatalogued()] + entry = printer_state_to_dict(state, printer_id=1)["hms_errors"][0] + assert entry["full_code"] == "050002000003000A" + assert entry["description"] is None + + +class TestQueueFailureReason: + def test_prefers_the_resolved_description(self): + """Deliberately a sentence the local fallback would NOT produce, so the + preference is observable rather than coincidentally identical.""" + supplied = "Filament ran out, as resolved at parse time." + assert _format_hms_error_summary([{"code": "0x8004", "attr": 0x03008004, "description": supplied}]) == ( + f"[0300_8004] {supplied}" + ) + + def test_falls_back_for_an_entry_without_the_field(self): + """Entries predating the field still resolve, so the helper's own + contract is unchanged for any other caller.""" + assert _format_hms_error_summary([{"code": "0x8004", "attr": 0x03008004}]) == (f"[0300_8004] {RUNOUT_SENTENCE}") + + def test_bare_short_code_when_nothing_describes_it(self): + assert _format_hms_error_summary([{"code": "0x9999", "attr": 0x99990000, "description": None}]) == "[9999_9999]" + + +class TestSurfacesAgree: + @pytest.mark.parametrize("fault,expected", [(_runout(), RUNOUT_SENTENCE), (_uncatalogued(), None)]) + def test_the_same_fault_reads_the_same_everywhere(self, fault, expected): + """The reason to resolve once rather than at each boundary: these three + cannot report different text for one fault.""" + state = PrinterState() + state.hms_errors = [fault] + broadcast = printer_state_to_dict(state, printer_id=1)["hms_errors"][0]["description"] + rest = HMSErrorResponse( + code=fault.code, + attr=fault.attr, + module=fault.module, + severity=fault.severity, + full_code=fault.full_code, + description=fault.description, + ).description + assert broadcast == expected + assert rest == expected + assert fault.description == expected diff --git a/backend/tests/unit/test_hms_error_summary.py b/backend/tests/unit/test_hms_error_summary.py index faf2be25f..99984b700 100644 --- a/backend/tests/unit/test_hms_error_summary.py +++ b/backend/tests/unit/test_hms_error_summary.py @@ -52,3 +52,27 @@ def test_tolerates_malformed_entry_and_skips_it(): def test_all_malformed_returns_none(): assert _format([{"code": "not-hex", "attr": "also-not-int"}]) is None + + +def test_masks_a_32_bit_code_into_a_four_digit_label(): + """An `hms[]` entry's code carries the alert level in its high 16 bits. The + label used to be formatted from the unmasked value, producing "0500_3000A" — + five digits in a group that has four, so it matched no catalogue key and was + not a code anyone could look up either.""" + summary = _format([{"code": "0x3000a", "attr": 0x05000200, "module": 5, "severity": 2}]) + assert summary == "[0500_000A]" + + +def test_masking_lets_a_32_bit_code_resolve_its_description(): + """0500_4038 is the nozzle-size mismatch. Arriving as an `hms[]` entry with + an alert-level group, it went undescribed purely because of the formatting + above; now it reads the same as when it arrives via print_error.""" + summary = _format([{"code": "0x00024038", "attr": 0x05000200, "module": 5, "severity": 2}]) + assert summary is not None + assert summary.startswith("[0500_4038] ") + assert "nozzle diameter" in summary.lower() + + +def test_accepts_an_integer_code(): + """`_hms_short_code` takes both shapes; the raw MQTT payload carries ints.""" + assert _format([{"code": 0x4038, "attr": 0x05000000, "module": 5, "severity": 1}]).startswith("[0500_4038] ") diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 4ae676701..02d4e35b1 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -390,6 +390,12 @@ export interface HMSError { // this back as HmsActionBody.print_error so we don't truncate the 64-bit // identifier into the silent-rejection short code (#1830). full_code?: string; + // The backend's resolved catalogue sentence for this fault (#2926). English + // only, and null when the catalogue does not cover the code. Resolved with the + // same lookup order this file's consumers use (full_code, then the G1_G4 + // collapse), so it agrees with what HMSErrorModal renders — the modal still + // resolves its own text, and this is here for parity with the API. + description?: string | null; } export interface HMSActionBody {