From 0d21239e18cb3672c8b931a211608840543deea0 Mon Sep 17 00:00:00 2001 From: maziggy Date: Fri, 28 Aug 2026 12:18:27 +0200 Subject: [PATCH] Send AMS tray colours as uppercase hex (issue #2987) Assigning a spool to an AMS slot unassigned it again seconds later, and the slot's colour changed at the same time. It presented as Bambu Studio and Bambuddy fighting over the slot. The reporter's log shows Bambuddy losing to itself. P1S firmware 01.10.00.00 reads every lowercase hex letter in an AMS tray_color as a zero, and hides it completely: the command response echoes back the value that was sent and reports result "success", so only the next AMS push says what was really stored. The spool-assign path sent spool.rgba verbatim and that column stores lowercase. From the bundle: sent 09ff00ff -> AMS reports 09000000 sent ff5100ff -> AMS reports 00510000 sent 090000FF -> AMS reports 090000FF That is the visible colour change, and it is also what deleted the assignment. The auto-unlink sweep asks whether the slot still matches the spool assigned to it; the mangled colour no longer did, so the assignment Bambuddy had made four seconds earlier was removed. colors_similar('09000000', '09FF00FF') is False, which is the whole of it. Re-assigning could not recover, because the Configure Slot dialog seeds its colour from whatever the printer currently reports. It wrote the mangled colour back and cemented it, which is the loop the report describes in its steps 4 and 5. Colours are now uppercased where the command is assembled rather than in each of the four routes that configure a slot. A caller that forgets is exactly how this arrived. Nothing else changes: no padding, no invented alpha, no six-to-eight widening, and tray_type and tray_sub_brands keep their case, where it carries meaning -- "PLA Matte" is a product line, "PLA MATTE" is not. Two paths deliberately left alone. The developer-mode probe re-sends the colour the printer itself just reported so that the probe is inert; uppercasing there would turn it into a write. And the Virtual Printer forwards the slicer's own command verbatim -- Studio could in principle hit the same firmware bug, but nothing here evidences that it sends lowercase, and rewriting a slicer payload inside a transparent proxy is not a change to make on a hunch. Two more defects from the same log. A spool with a brand and no subtype was configured with the string "None" in its name: the branded branch interpolated spool.subtype without checking it while the unbranded branch guarded it, so "Sunlu PLA Matte None" went on the wire and into Studio's display. And the FTP log is readable again. A 426 whose bytes Bambuddy has already verified against the printer is how Bambu FTPS normally ends a transfer, not a fault, so it drops from WARNING to INFO. It fired 54 times in this one bundle, every one followed by a completed upload, and it was burying the 26 TLS handshake failures in the same log that actually cost the reporter two prints. A 426 whose bytes do not verify is still an error and still fails the upload. The handshake failures themselves are printer-side FTPS cool-off under load and are not touched here. --- CHANGELOG.md | 1 + backend/app/api/routes/inventory.py | 13 +- backend/app/services/bambu_ftp.py | 11 +- backend/app/services/bambu_mqtt.py | 32 ++++- .../test_ams_slot_sub_brand_none_2987.py | 105 ++++++++++++++++ .../services/test_ams_tray_color_case_2987.py | 113 ++++++++++++++++++ backend/tests/unit/services/test_bambu_ftp.py | 56 +++++++++ 7 files changed, 321 insertions(+), 10 deletions(-) create mode 100644 backend/tests/integration/test_ams_slot_sub_brand_none_2987.py create mode 100644 backend/tests/unit/services/test_ams_tray_color_case_2987.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 769d0eb32..154c20458 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,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 spool you assigned to an AMS slot unassigned itself seconds later (#2987, reported by @frethop)** — and the slot's colour changed at the same time. It looked like Bambu Studio and Bambuddy fighting over the slot; the reporter's log shows Bambuddy losing to itself. P1S firmware 01.10.00.00 reads every **lowercase** hex letter in an AMS `tray_color` as a zero, and hides it completely: the command response echoes back the value you sent and reports `result: "success"`, so only the next AMS push reveals what was really stored. The spool-assign path sent `spool.rgba` verbatim, and that column stores lowercase — so `09ff00ff` became `09000000` on the printer and `ff5100ff` became `00510000`, while the one uppercase write in the same window round-tripped intact. That is the visible colour change. It is also what deleted the assignment: the auto-unlink sweep asks whether the slot still matches the spool it is assigned to, the mangled colour no longer did, and the assignment Bambuddy had created four seconds earlier was removed. Re-assigning could not help, because the **Configure Slot** dialog seeds its colour from whatever the printer currently reports — so it wrote the mangled colour straight back and cemented it, which is the loop in the report's steps 4 and 5. Colours are now uppercased at the single point the MQTT command is assembled rather than in each of the four routes that configure a slot, because a caller that forgets is exactly how this arrived. Nothing else about the command changes: no padding, no invented alpha, and `tray_type` / `tray_sub_brands` keep their case, where it carries meaning. **Two more things found in the same log.** A spool with a brand but no subtype was configured with the literal string `None` in its name — `"Sunlu PLA Matte None"` went on the wire, because the branded branch interpolated the subtype without checking it while the unbranded branch guarded it. And the FTP log is now readable: a `426` whose bytes Bambuddy has verified against the printer is how Bambu's FTPS normally ends a transfer, not a fault, so it is logged at INFO instead of WARNING. It fired 54 times in this one bundle, every single one followed by a completed upload, and it was burying the 26 TLS handshake failures in the same log that actually cost the reporter two prints. A `426` whose bytes do **not** verify is still an error and still fails the upload. 24 regression tests. - **One ASA spool parked in the AMS added 20 minutes to every PLA print (#2886, reported by @FirstRulez)** — Preheat's chamber target was the maximum across *every loaded AMS tray*, with no reference to the job. The reporter's P2S holds PETG Pro, PLA, ASA and PETG; the ASA row of the filament map says 45°C, so a PLA-only plate was dispatched with `chamber_target=45°C`, the bed driven to 90°C to reach it, and the full 900s max-wait plus 300s soak burned before the upload even started — every time, because a P2S has no chamber heater and the chamber tops out around 33°C, so the wait can only ever end on the timeout. Their log carries fifteen of these. The intent was never in doubt: the resolution order documented one screen above reads "PLA-only print derives 0 → chamber phase auto-skips", but it was implemented as PLA-only **AMS** rather than PLA-only **print**, and only misfires on a mixed-material load. The derivation now reads the trays the item's `ams_mapping` actually names — the same array the print command puts on the wire, `[-1, -1, -1, 1]` in their case, addressing exactly the PLA slot — so the ASA two slots over contributes nothing and the stage skips outright. Multi-material prints are unaffected: the maximum is still taken, just across the trays the plate loads, so an ASA the print really does use is still the binding constraint. An item whose mapping is missing or still unresolved keeps the whole-unit scan, since that is the only signal left and narrowing to nothing would disable preheat for prints that need it. The bed hold between jobs (`queue_keep_bed_warm`) is gated on the same derivation and was holding beds at 90°C for the same wrong reason; it now reads the next item's mapping too. **The external spool is no longer invisible to this**: the scan only ever looked at `raw_data['ams']`, so an ASA print fed from the external feed derived 0 and got no preheat at all — a mapping that names 254/255 is now honoured, while an item with no mapping still derives from the AMS alone so nothing starts preheating that did not before. That covers the mappings Bambuddy builds itself, from the print dialog or the dispatcher's own matcher. It does not cover a mapping captured from a slicer through a Virtual Printer: BambuStudio writes the external spool as `-1` there, which is the same value it writes for a slot the plate does not use, so the two cannot be told apart. A wholly external one carries no usable mapping and falls back to the whole-unit scan as before; a mixed one derives from its AMS trays and the external half stays unread — which is exactly what it did before this change, since nothing ever read `vt_tray`. 29 regression tests, built from the trays and mapping in the reporter's own support bundle. - **Spoolman reset your renamed extra fields on every restart (#2983, reported by @ngreatorex)** — Bambuddy checked whether one of its four custom spool fields existed by calling `GET /field/spool/{name}`. Spoolman has never served that: its API declares only `POST` and `DELETE` at that path, so the check answered **405 Method Not Allowed** every single time and could never succeed. Each call then fell through to `POST /field/spool/{name}` — and that endpoint is an *upsert*, not a create. It answers 200 whether or not the field is already there, so a field you had renamed, retyped or given a default to in Spoolman's own UI was silently reset to Bambuddy's version of it, and an untrue `Created Spoolman extra field` was logged alongside. The reporter's log carried 60 of those lines in three days. Existence now comes from the documented `GET /field/spool` listing, matched on the field's `key` rather than its display `name` — a rename is the same field, and treating it as a missing one is what caused the overwrite. An existing field is left completely alone. **You can now rename these fields in Spoolman and the name will stick.** Registering all four also costs one request instead of four, and none at all on a client that has already looked once. If the listing can't be read at all, Bambuddy falls back to attempting the write exactly as before, so an unexpected Spoolman build is no worse off than today. - **The internal slicer picked PETG for a PLA plate, and an A1 process for a P1S (#2982, reported by @Igiegel)** — Both traced to one line of the slicer sidecar: its bundled-profile listing read `filament_type` off the leaf preset and never reported `compatible_printers` at all. Measured against the shipped trees, the leaf read finds nothing — zero of the 1156 filament presets in OrcaSlicer 2.4.2 carry a material, and zero of the 1792 in BambuStudio 02.08.02.61 — because it lives one to four hops up the `inherits` chain (`Bambu ABS @BBL A1` → `Bambu ABS @base` → `fdm_filament_abs`). With no material on any of them the pre-pick had only colour and source to go on, so a white PLA plate drew `Bambu PETG Basic` on an A1 mini and `Bambu PC` on a P1S. The sidecar now walks the chain, which recovers the material for 1124 of the 1156 (the other 32 name a parent Bambu's own bundle does not contain, and stay listed with no material rather than being dropped). **`compatible_printers` is the second half**, and it is the only truthful account of which printer a preset belongs to: the bundle ships no process preset named after a P1S, an X1, an X1E or an H2D Pro — all ten of the P1S's are named `@BBL X1C` and name the P1S only in that list. Reading the printer out of the preset *name* therefore made a P1S look like it had no compatible process at all: all 198 were hidden behind “Show all” and the auto-pick fell through to an alphabetically-first `0.06mm Fine @BBL A1 0.2 nozzle` the CLI then refused with “the selected printer is not compatible with the process preset”. A P1S now gets the `0.20mm Standard @BBL X1C` it should always have had, and 73 filaments instead of 4. **This half needs the updated sidecar image** — an older one simply reports nothing and Bambuddy stays on the name matcher, degraded exactly as before rather than broken. Three hardenings ride along so a stale sidecar fails more gracefully: a filament preset that states a *different* material than the plate asks for is now skipped outright rather than merely scored down (a preset stating no material stays eligible — unknown is not wrong), a slot is no longer held on printer-compatibility alone once its material turns out to disagree (a preset you chose yourself is exempt — printing PETG on a plate labelled PLA is a legitimate thing to do), and a dropdown the printer filter would empty now shows everything instead, since a visible preset for the wrong printer can be changed and an empty list cannot. `filament_colour` stays null throughout, which is correct: no BBL profile carries a colour at any depth, because colour is a spool attribute rather than a profile one. diff --git a/backend/app/api/routes/inventory.py b/backend/app/api/routes/inventory.py index c18192f69..18eadec7a 100644 --- a/backend/app/api/routes/inventory.py +++ b/backend/app/api/routes/inventory.py @@ -124,13 +124,12 @@ async def apply_spool_to_slot_via_mqtt( # may actually hold ("PLA+", "HTPLA") stays in tray_sub_brands below, which # is where Bambu puts it too (issue #2902). tray_type = printer_filament_type(spool.material) - tray_sub_brands = ( - f"{spool.brand} {spool.material} {spool.subtype}".strip() - if spool.brand - else f"{spool.material} {spool.subtype}" - if spool.subtype - else spool.material - ) + # Join only the parts that exist. The previous shape interpolated + # `spool.subtype` into the branded string without checking it, so a spool + # with a brand and no subtype went to the printer as + # "Sunlu PLA Matte None" -- the string "None", on the wire (#2987). The + # unbranded branch guarded subtype; the branded one did not. + tray_sub_brands = " ".join(p for p in (spool.brand, spool.material, spool.subtype) if p) or spool.material tray_color = spool.rgba or "FFFFFFFF" _generic_id_values = _GENERIC_ID_VALUES diff --git a/backend/app/services/bambu_ftp.py b/backend/app/services/bambu_ftp.py index 2c1c7517e..3650a59d4 100644 --- a/backend/app/services/bambu_ftp.py +++ b/backend/app/services/bambu_ftp.py @@ -976,7 +976,13 @@ class BambuFTPClient: logger.debug("Post-error SIZE check failed: %s", size_err) server_size = None if server_size is not None and server_size == file_size: - logger.warning( + # INFO, not WARNING: a 426 whose bytes verify is the normal + # way Bambu FTPS ends a transfer, not a fault. It fired 54 + # times in one support bundle and every one was followed by + # a completed upload, which buried the 26 handshake failures + # in the same log that actually cost the user two prints + # (#2987). The unverified branch below is still an error. + logger.info( "FTP STOR returned %s for %s but file is intact on the " "printer (%s bytes match) — proceeding: %s", type(e).__name__, @@ -1110,7 +1116,8 @@ class BambuFTPClient: logger.debug("Post-error SIZE check failed: %s", size_err) server_size = None if server_size is not None and server_size == len(data): - logger.warning( + # INFO for the same reason as upload_file above (#2987). + logger.info( "FTP STOR returned %s for %s but file is intact on the " "printer (%s bytes match) — proceeding: %s", type(e).__name__, diff --git a/backend/app/services/bambu_mqtt.py b/backend/app/services/bambu_mqtt.py index d1806afe4..53cadeb5a 100644 --- a/backend/app/services/bambu_mqtt.py +++ b/backend/app/services/bambu_mqtt.py @@ -111,6 +111,34 @@ def normalize_am_unit_id(ams_id: int) -> int: return A2L_LITE_NORMALIZED_AMS_ID if ams_id == A2L_LITE_PHYSICAL_AMS_ID else ams_id +def wire_tray_color(tray_color: str | None) -> str: + """Normalise a colour to the form AMS firmware actually parses: UPPERCASE hex. + + P1S firmware 01.10.00.00 parses every lowercase hex letter in ``tray_color`` + as a zero, and does it silently: the command response echoes the value you + sent and reports ``result: "success"``, so only the next AMS push shows what + was really stored. Measured on the reporter's machine (#2987), where the + spool's own ``rgba`` is stored lowercase and went out verbatim: + + sent 09ff00ff -> AMS reports 09000000 + sent ff5100ff -> AMS reports 00510000 + sent 090000FF -> AMS reports 090000FF + + A mangled colour is not merely cosmetic. The auto-unlink sweep compares the + tray against the spool it is assigned to, so the tray Bambuddy just wrote no + longer matches the spool that asked for it and the assignment is deleted + seconds after being made -- and re-assigning through the slot modal writes + the mangled colour back, because the modal seeds itself from the tray. + + Applied here, at the one place the command is built, rather than in each of + the four callers: a caller that forgets is exactly how this arrived. + + A leading ``#`` is stripped -- the wire format carries bare hex -- and a + blank stays blank, which is how a slot is cleared. + """ + return (tray_color or "").strip().lstrip("#").upper() + + def a2l_lite_wire_ids(ams_id: int, tray_id: int) -> tuple[int, int, int] | None: """Translate a normalised A2L slot back to the physical wire form. @@ -7680,7 +7708,9 @@ class BambuMQTTClient: "tray_info_idx": tray_info_idx, "tray_type": tray_type, "tray_sub_brands": tray_sub_brands, - "tray_color": tray_color, + # UPPERCASE, always: lowercase hex is silently read as zeros by + # P1S firmware and acknowledged as a success (#2987). + "tray_color": wire_tray_color(tray_color), "nozzle_temp_min": nozzle_temp_min, "nozzle_temp_max": nozzle_temp_max, "sequence_id": "0", diff --git a/backend/tests/integration/test_ams_slot_sub_brand_none_2987.py b/backend/tests/integration/test_ams_slot_sub_brand_none_2987.py new file mode 100644 index 000000000..25f6532f4 --- /dev/null +++ b/backend/tests/integration/test_ams_slot_sub_brand_none_2987.py @@ -0,0 +1,105 @@ +"""A spool with a brand and no subtype must not send the string "None" (#2987). + +``tray_sub_brands`` was built by interpolating brand, material and subtype into +one f-string whenever the spool had a brand -- without checking that the subtype +existed. The unbranded branch guarded it; the branded one did not. The +reporter's Sunlu roll has no subtype, so the slot was configured as + + "tray_sub_brands": "Sunlu PLA Matte None" + +which is what the printer stored and what Bambu Studio then displayed. +""" + +from unittest.mock import MagicMock, patch + +import pytest +from httpx import AsyncClient +from sqlalchemy.ext.asyncio import AsyncSession + +from backend.app.models.printer import Printer +from backend.app.models.spool import Spool + + +def _mqtt_mock(): + client = MagicMock() + client.ams_set_filament_setting.return_value = True + client.extrusion_cali_sel.return_value = True + return client + + +def _status(): + status = MagicMock() + status.raw_data = {"ams": {"ams": []}} + status.nozzles = [MagicMock(nozzle_diameter="0.4")] + status.ams_extruder_map = None + status.kprofiles = [] + return status + + +async def _assign(async_client, db_session, serial, **spool_kwargs): + printer = Printer(name="P1S", serial_number=serial, ip_address="192.168.1.78", access_code="12345678") + db_session.add(printer) + spool = Spool(rgba="09ff00ff", label_weight=1000, weight_used=0, **spool_kwargs) + db_session.add(spool) + await db_session.commit() + await db_session.refresh(printer) + await db_session.refresh(spool) + + client = _mqtt_mock() + with patch("backend.app.services.printer_manager.printer_manager") as pm: + pm.get_client.return_value = client + pm.get_status.return_value = _status() + response = await async_client.post( + "/api/v1/inventory/assignments", + json={"spool_id": spool.id, "printer_id": printer.id, "ams_id": 0, "tray_id": 2}, + ) + assert response.status_code == 200 + client.ams_set_filament_setting.assert_called_once() + return client.ams_set_filament_setting.call_args.kwargs + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_a_branded_spool_without_a_subtype_sends_no_none(async_client: AsyncClient, db_session: AsyncSession): + """The reported case.""" + sent = await _assign(async_client, db_session, "SB2987A", brand="Sunlu", material="PLA Matte", subtype=None) + + assert sent["tray_sub_brands"] == "Sunlu PLA Matte" + assert "None" not in sent["tray_sub_brands"] + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_a_branded_spool_with_a_subtype_still_carries_all_three( + async_client: AsyncClient, db_session: AsyncSession +): + """The case that already worked, pinned so the fix does not drop the subtype.""" + sent = await _assign(async_client, db_session, "SB2987B", brand="Sunlu", material="PLA", subtype="Silk") + + assert sent["tray_sub_brands"] == "Sunlu PLA Silk" + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_an_unbranded_spool_with_a_subtype_is_unchanged(async_client: AsyncClient, db_session: AsyncSession): + sent = await _assign(async_client, db_session, "SB2987C", brand=None, material="PLA", subtype="Silk") + + assert sent["tray_sub_brands"] == "PLA Silk" + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_a_spool_with_only_a_material_is_unchanged(async_client: AsyncClient, db_session: AsyncSession): + sent = await _assign(async_client, db_session, "SB2987D", brand=None, material="PLA", subtype=None) + + assert sent["tray_sub_brands"] == "PLA" + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_an_empty_string_subtype_is_treated_as_absent(async_client: AsyncClient, db_session: AsyncSession): + """The old branded branch stripped a trailing space away, so "" already + behaved; None was the only broken input and both must stay correct.""" + sent = await _assign(async_client, db_session, "SB2987E", brand="Sunlu", material="PLA Matte", subtype="") + + assert sent["tray_sub_brands"] == "Sunlu PLA Matte" diff --git a/backend/tests/unit/services/test_ams_tray_color_case_2987.py b/backend/tests/unit/services/test_ams_tray_color_case_2987.py new file mode 100644 index 000000000..0dba8f3ea --- /dev/null +++ b/backend/tests/unit/services/test_ams_tray_color_case_2987.py @@ -0,0 +1,113 @@ +"""AMS tray colours must go to the printer in UPPERCASE hex (#2987). + +P1S firmware 01.10.00.00 reads every lowercase hex letter in ``tray_color`` as a +zero, and says nothing about it: the command response echoes the value that was +sent and reports ``result: "success"``. Only the next AMS push shows what was +really stored. From the reporter's support bundle, where the spool's ``rgba`` +column holds lowercase and went out verbatim: + + sent 09ff00ff -> AMS reports 09000000 + sent ff5100ff -> AMS reports 00510000 + sent 090000FF -> AMS reports 090000FF + +The damage is not cosmetic. The auto-unlink sweep asks whether the tray still +matches the spool assigned to it, so the tray Bambuddy had just written stopped +matching the spool that asked for it and the assignment was deleted seconds +later -- which is the disappearing assignment the issue reports. +""" + +import json +from unittest.mock import MagicMock + +import pytest + +from backend.app.services.bambu_mqtt import BambuMQTTClient, wire_tray_color + + +def _client(model: str = "P1S") -> BambuMQTTClient: + client = BambuMQTTClient(ip_address="10.0.0.1", serial_number="P1S2987", access_code="c", model=model) + client._client = MagicMock() + client.state.connected = True + return client + + +def _sent(client: BambuMQTTClient) -> dict: + return json.loads(client._client.publish.call_args[0][1])["print"] + + +def _set(client: BambuMQTTClient, tray_color: str, **overrides): + kwargs = { + "ams_id": 0, + "tray_id": 2, + "tray_info_idx": "GFSNL03", + "tray_type": "PLA Matte", + "tray_sub_brands": "Sunlu PLA Matte", + "tray_color": tray_color, + "nozzle_temp_min": 200, + "nozzle_temp_max": 240, + } + kwargs.update(overrides) + assert client.ams_set_filament_setting(**kwargs) + return _sent(client) + + +class TestWireTrayColor: + @pytest.mark.parametrize( + ("stored", "expected"), + [ + pytest.param("09ff00ff", "09FF00FF", id="the-reported-spool"), + pytest.param("ff5100ff", "FF5100FF", id="the-second-spool-in-the-bundle"), + pytest.param("090000FF", "090000FF", id="already-uppercase-is-untouched"), + pytest.param("AbCdEf12", "ABCDEF12", id="mixed-case"), + pytest.param(" 09ff00ff ", "09FF00FF", id="surrounding-whitespace"), + pytest.param("#09ff00ff", "09FF00FF", id="css-style-hash-is-stripped"), + ], + ) + def test_the_wire_form_is_uppercase_bare_hex(self, stored: str, expected: str): + assert wire_tray_color(stored) == expected + + @pytest.mark.parametrize("blank", ["", " ", None]) + def test_a_blank_colour_stays_blank(self, blank): + """Clearing a slot sends an empty colour; it must not become "NONE".""" + assert wire_tray_color(blank) == "" + + def test_only_the_case_changes(self): + """No padding, no alpha invented, no six-to-eight widening -- the + firmware's own format is whatever the caller resolved, and guessing at + it here would be a second bug wearing the first one's clothes.""" + assert wire_tray_color("09ff00") == "09FF00" + assert wire_tray_color("09ff00ff00") == "09FF00FF00" + + +class TestWhatReachesThePrinter: + def test_a_lowercase_colour_is_uppercased_on_the_wire(self): + """The reported case, end to end through the publisher.""" + assert _set(_client(), "09ff00ff")["tray_color"] == "09FF00FF" + + def test_an_uppercase_colour_is_unchanged(self): + assert _set(_client(), "090000FF")["tray_color"] == "090000FF" + + def test_every_other_field_is_left_alone(self): + """Only the colour is normalised. tray_type and tray_sub_brands are + free text the printer stores verbatim, and case carries meaning there -- + "PLA Matte" is a product line, "PLA MATTE" is not.""" + sent = _set(_client(), "09ff00ff") + assert sent["tray_type"] == "PLA Matte" + assert sent["tray_sub_brands"] == "Sunlu PLA Matte" + assert sent["tray_info_idx"] == "GFSNL03" + assert (sent["nozzle_temp_min"], sent["nozzle_temp_max"]) == (200, 240) + + def test_the_external_spool_is_normalised_too(self): + """ams_id 255 takes a different branch to build its wire ids, so the + colour has to be normalised where the command is assembled rather than + inside any one of them.""" + assert _set(_client("X1C"), "09ff00ff", ams_id=255, tray_id=0)["tray_color"] == "09FF00FF" + + def test_an_ams_ht_is_normalised_too(self): + assert _set(_client(), "09ff00ff", ams_id=128, tray_id=0)["tray_color"] == "09FF00FF" + + def test_an_a2l_lite_slot_is_normalised_too(self): + assert _set(_client("A2L"), "09ff00ff", ams_id=6, tray_id=2)["tray_color"] == "09FF00FF" + + def test_a_blank_colour_reaches_the_printer_blank(self): + assert _set(_client(), "")["tray_color"] == "" diff --git a/backend/tests/unit/services/test_bambu_ftp.py b/backend/tests/unit/services/test_bambu_ftp.py index ad0cf6e57..5b736cd22 100644 --- a/backend/tests/unit/services/test_bambu_ftp.py +++ b/backend/tests/unit/services/test_bambu_ftp.py @@ -487,6 +487,62 @@ class TestUpload: assert result is True, "intact file (SIZE match) tolerates 426 noise" client.disconnect() + def test_upload_426_with_intact_file_logs_at_info_not_warning( + self, ftp_client_factory, ftp_server, tmp_path, caplog + ): + """A verified-intact 426 is how Bambu FTPS normally ends a transfer, + not a fault, so it must not be a WARNING (#2987). + + It fired 54 times in one support bundle, every one followed by a + completed upload, and buried the 26 TLS handshake failures in the same + log that actually cost the reporter two prints. The truncated case below + is still an error -- this only moves the one we have already verified. + """ + import ftplib # nosec B402 — tests need the real ftplib to construct mock 426 responses + import logging + + local = tmp_path / "test.bin" + local.write_bytes(b"data" * 256) # 1024 bytes + client = ftp_client_factory() + client.connect() + + def raise_426(): + raise ftplib.error_temp("426 Failure reading network stream.") + + client._ftp.voidresp = raise_426 + client._ftp.size = lambda _path: 1024 + + with caplog.at_level(logging.INFO, logger="backend.app.services.bambu_ftp"): + assert client.upload_file(local, "/cache/test.bin") is True + client.disconnect() + + intact = [r for r in caplog.records if "file is intact on the" in r.getMessage()] + assert intact, "the proceed path must still say why it proceeded" + assert [r.levelno for r in intact] == [logging.INFO] * len(intact) + + def test_upload_426_with_truncated_file_still_logs_an_error(self, ftp_client_factory, ftp_server, tmp_path, caplog): + """The half that must stay loud: bytes that did not verify.""" + import ftplib # nosec B402 — tests need the real ftplib to construct mock 426 responses + import logging + + local = tmp_path / "test.bin" + local.write_bytes(b"data" * 256) + client = ftp_client_factory() + client.connect() + + def raise_426(): + raise ftplib.error_temp("426 Failure reading network stream.") + + client._ftp.voidresp = raise_426 + client._ftp.size = lambda _path: 100 + + with caplog.at_level(logging.INFO, logger="backend.app.services.bambu_ftp"): + assert client.upload_file(local, "/cache/test.bin") is False + client.disconnect() + + rejected = [r for r in caplog.records if "rejected by printer" in r.getMessage()] + assert rejected and all(r.levelno == logging.ERROR for r in rejected) + def test_upload_426_with_truncated_file_returns_false(self, ftp_client_factory, ftp_server, tmp_path): """The original #1401 fix is preserved: when SIZE confirms the file isn't on the server at full size (or SIZE itself fails), the upload