diff --git a/CHANGELOG.md b/CHANGELOG.md index 60acc7149..ed5052d6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ All notable changes to Bambuddy will be documented in this file. - **Trivy DS-0026 (`Dockerfile.test` missing HEALTHCHECK): silenced via `HEALTHCHECK NONE`** — The test image runs `pytest` and exits; there is no long-running service to probe, so any HEALTHCHECK we added would be cargo-cult noise. `HEALTHCHECK NONE` is the documented Docker directive to explicitly opt out of any inherited healthcheck and is the way Trivy expects projects to signal "this image is not a service." Closes code-scanning alert #813. ### Fixed +- **Fallback-archive MQTT filament extraction now actually fires for real prints (#1533 follow-up, reported by @JmanB52D)** — Reporter updated to 0.2.5b1 expecting the #1533 fix to populate filament fields on his P2S virtual-printer prints when the .3mf is locked. His support bundle showed Bambuddy still creating fallback archives with NULL filament fields even though the print-start log line proved AMS-0-T0 had PETG loaded at the moment the helper should have read it (`AMS 0: T0(type=PETG, color=FFFFFFFF, …)`). Cause: the #1533 helper `_extract_filament_data_from_mqtt(data)` in `backend/app/main.py` only looked at `data["ams"]`, but the dict that `on_print_start` actually receives at runtime is the wrapper shape `{"filename", "subtask_name", "remaining_time", "raw_data": , "ams_mapping"}` that `backend/app/services/bambu_mqtt.py:2971-2980` constructs — so `data["ams"]` was undefined on every real call and the helper silently returned `{}`, leaving the fallback archive's `filament_type` / `filament_color` NULL. The 15 unit tests that shipped with #1533 all passed the bare inner shape directly and never exercised the callback wiring, so the regression slipped through the green build. **Fix**: the helper now resolves `data["raw_data"]["ams"]` first (the callback shape) and only falls back to `data["ams"]` when the wrapper isn't present (preserves the inner-shape callers from the existing tests). Defensive: a non-dict `raw_data` (e.g. partial MQTT decode failure) falls through to the inner lookup instead of crashing. **Tests**: 5 new in `TestOnPrintStartCallbackShape` (`backend/tests/unit/test_fallback_archive_mqtt_filament.py`) — wrapper payload with ams_mapping resolves to the inner data; wrapper with no ams_mapping lists all loaded slots; the existing inner-shape callers still work after the additive wrapper lookup; missing `raw_data` returns `{}` instead of raising; junk `raw_data` (string) doesn't shadow a present inner `ams`. Full 5378-test backend suite green. Backend ruff clean. **What this does NOT fix**: per-filament gram usage still needs the actual .3mf — the printer locks it during print (P-line firmware behaviour, not a Bambuddy bug), and the existing 19 FTP candidate paths + directory probes are expected to 550 in that window. Per-print filament type and colour are the data point that drives the AMS-expansion planning the reporter explicitly called out, so this is the fix that moves the needle for him. - **Assigning a spool no longer shows a profile-mismatch warning when only the slicer profile differs, and the warning now states the AMS slot will be reconfigured (#1552, reported by @anthonyma94)** — Reporter assigned a spool to a slot whose stored slicer profile (e.g. "Bambu PLA Matte") differed from the new spool's profile (e.g. "Bambu PLA Basic"), got a warning popup with only Cancel / Assign Anyway, and was under the impression that confirming the popup just linked the spool in Bambuddy's DB without touching the AMS — i.e. that he then had to manually open Configure AMS Slot to push the new profile to the printer. The auto-push has actually been in place since the assign route existed: `backend/app/api/routes/inventory.py::assign_spool` calls `apply_spool_to_slot_via_mqtt` after upserting the SpoolAssignment row, which publishes both `ams_filament_setting` (tray_info_idx, tray_sub_brands, color, temps) and `extrusion_cali_sel` (K profile) over MQTT, and `backend/app/api/routes/spoolman_inventory.py::assign_spoolman_slot` does the same on the Spoolman side. The only short-circuit is when the firmware explicitly reports the slot empty (`tray_state ∈ {9, 10}`), in which case `main.py::on_ams_change` deferred-replays the configure as soon as a spool appears. So the popup was creating friction without revealing what it actually did. **Two changes**: (1) `AssignSpoolModal.tsx` + `spoolbuddy/AssignToAmsModal.tsx` no longer fire the mismatch popup for *profile-only* mismatches — `if (materialMatchResult !== 'exact')` replaces the old `materialMatchResult !== 'exact' || !profileMatches`, and the `'profile'` member is dropped from the `mismatchType` union (the standalone profile branch in both popup render bodies is removed as dead code). Material mismatch — where Bambu firmware can refuse the print because the type is wrong — still warns. (2) Every firing warning (material, partial, material+profile, partial+profile) now appends a new line via the new `inventory.assignReconfigureNote` i18n key: "The AMS slot will be reconfigured to use the spool's profile." This makes the Assign Anyway button's effect explicit instead of leaving users to guess. **i18n**: real translations across all 9 locales per [[feedback_translate_dont_fallback]]; parity script clean at 4999 leaves per locale. **Tests**: existing 14 `AssignSpoolModal` + 7 `AssignToAmsModal` tests pass unchanged — no test asserted on the profile-only popup firing. Frontend build clean, full 2043-test suite green. **Open follow-up**: if anthonyma94 confirms after this change that his slot *still* shows the old profile after Assign Anyway, the real bug is in `apply_spool_to_slot_via_mqtt`'s tray_info_idx / setting_id resolution for his specific spool shape — would need his spool's `slicer_filament` value plus the live tray state to diagnose. - **Transparent / clear filament now selectable and rendered as transparent end-to-end in the built-in inventory (#1545, reported by @Synec5, confirmed by @CMW-ISS)** — Reporter wanted to select a transparent filament colour in the spool editor; CMW-ISS independently confirmed on v0.2.5b1 that AMS-detected transparent spools were silently labelled "Black" in the filament-mapping dropdown because the colour name resolver dropped the alpha byte and the underlying RGB `000000` HSL-bucketed to "Black". Spoolman already supported 8-digit `RRGGBBAA` hex; the built-in inventory didn't. Five distinct sites collapsed alpha → 6-char RGB and had to be fixed together: (a) `frontend/src/utils/colors.ts` — `hexToColorName`, `getColorName`, `resolveSpoolColorName`, and `isLightColor` now short-circuit to `"Clear"` when the input is 8 chars with alpha `00`, before either the catalog lookup or the HSL fallback can mislabel transparent as black; `isLightColor` returns `true` for clear so text contrast matches the light/mid-gray checkerboard underlay the swatch paints. (b) `frontend/src/utils/amsHelpers.ts::normalizeColor` no longer unconditionally strips the alpha byte — it preserves `#RRGGBBAA` when alpha < `FF` so the AMS-side colour reaches CSS `fill=` / `backgroundColor` as a translucent value instead of a solid one; opaque colours still emit `#RRGGBB` and `normalizeColorForCompare` (which DOES strip alpha) is unchanged so type/colour matching for auto-mapping is unaffected. (c) `backend/app/api/routes/printers.py::get_available_filaments` no longer truncates `tray_color` to 6 chars before emitting it on `/printers/available-filaments` — both the AMS and `vt_tray` branches now pass the full `#RRGGBBAA` through; the dedup key still uses the 6-char RGB so two slots that share an RGB but differ only in alpha still merge into one filament requirement. (d) `frontend/src/components/spool-form/constants.ts` gained a `{ name: 'Clear', hex: '00000000' }` entry to `QUICK_COLORS` — the only 8-char preset, because the native `` can't pick alpha and a dedicated swatch is the only UX that lets the user actually choose transparency. (e) `frontend/src/components/spool-form/ColorSection.tsx` reworked the hex draft contract: previously the hex input was hardcoded to 6 chars and every commit path unconditionally appended `'FF'`, so even pasting `00000000` got truncated to `000000FF` (solid black). Now: the draft accepts up to 8 hex chars; a 6-char commit appends `FF`, an 8-char commit passes through verbatim; on blur a 7-char draft (RGB + one alpha nibble) right-pads the nibble to `0` instead of jumping back to 6-char-pad-RGB; the `selectColor()` helper that the preset swatches call only appends `FF` when the preset is 6 chars, so the new `Clear` swatch lands as `00000000` in `formData.rgba` instead of `00000000FF`. `currentRgba` is canonicalised to 8 chars uppercase and `isSelected()` matches on the full rgba so `Clear` (`00000000`) doesn't collide with `Black` (`000000FF`) in the swatch highlight. (f) Two new shared helpers in `frontend/src/utils/colors.ts` — `getSwatchStyle(rgba)` returns a `{ backgroundColor }` for opaque colours and a `{ backgroundImage, backgroundSize }` 8px checkerboard for alpha=00 (use for div / button backgrounds); `spoolColorString(rgba)` returns a hex string that preserves the alpha byte when alpha < FF (use for SVG `fill=` props and other single-string colour contexts where the consumer can interpret 8-char hex natively). Applied to every simple-swatch site that previously did `style={{ backgroundColor: '#' + rgba.slice(0, 6) }}` or passed a 6-char fill to an SVG icon — those sites would have rendered Clear spools as solid black after the cream rewrite was removed: the three preset rows in `ColorSection.tsx` (recent / catalog / fallback), the spool checkbox swatch in `LabelTemplatePickerModal.tsx`, the per-card colour dot + the SVG `SpoolCircle` in `SpoolBuddyInventoryPage.tsx`, the assigned-spool indicators in `SpoolBuddyAmsPage.tsx` (both internal and Spoolman branches), the four selected-spool summary swatches + the simple-view's spool dot in `SpoolBuddyWriteTagPage.tsx`, the lead-spool indicator in `ForecastPanel.tsx`, the header swatch in `AssignToAmsModal.tsx`, the two spool-list dots in `AssignSpoolModal.tsx` (internal + Spoolman columns), and the `SpoolIcon` fed by `InventorySpoolInfoCard.tsx` / `TagDetectedModal.tsx` / `SpoolInfoCard.tsx` / `LinkSpoolModal.tsx` (which now pass the full 8-char rgba — SVG `fill=` interprets translucent values correctly). `FilamentSwatch.tsx`'s tooltip title fallback also widened so the on-hover hex code shows `#00000000` for a Clear spool instead of misreporting it as `#000000`. **What is intentionally NOT changed**: the native `` value in `SpoolBuddyWriteTagPage.tsx`'s simple-view picker keeps its 6-char hex — that input element doesn't support alpha, and its onChange handler still sets rgba back to opaque `FF` (which is correct behaviour: the user explicitly picked a colour via the picker, not transparency). The colour-sort comparator in `LabelTemplatePickerModal.tsx::colorSortKey` keeps its 6-char alpha-strip — transparent spools sort into the same bucket as black/neutrals which is the right behaviour for ordering. The label-renderer in `backend/app/services/label_renderer.py` keeps its 6-char alpha-strip in `_hex_code_label` because the printed text on a physical label can't show transparency — `_color_from_hex` does honour the alpha byte for the printed swatch fill (alpha=00 → invisible swatch on the label, which is the honest physical answer). The Spoolman auto-sync's `_find_or_create_filament` in `backend/app/services/spoolman.py` still strips alpha when looking up the Spoolman catalog because Spoolman's filament catalog schema only supports 6-char `color_hex` — a transparent AMS spool synced into Spoolman will now match against a `000000` (Black) Bambu Lab filament entry instead of the pre-fix synthetic "PLA Basic" cream entry (RGB `F5E6D3`); both are inaccurate, the post-fix behaviour is at least honest about which colour the catalog has chosen rather than silently inventing a cream spool — users on the Spoolman backend can manually correct the filament assignment if desired. (g) **Removed the cream rewrite** at `backend/app/services/spoolman.py::parse_ams_tray` that silently replaced AMS-reported `00000000` with `F5E6D3FF` ("Light cream/natural color"). That rewrite was a workaround from when the swatch renderer couldn't show alpha — `filamentSwatchHelpers.ts::buildFilamentBackground` already paints a checkerboard underlay for alpha < FF (added in #1154), so the rewrite has been hidden technical debt that made every AMS-detected transparent spool land in inventory as cream instead of clear, with no signal to the user that a colour was substituted. AMS-synced spools now keep their true `00000000` value; the swatch renders the checkerboard; `getColorName` resolves to "Clear". (h) `backend/app/services/spool_tag_matcher.py::create_spool_from_tray` short-circuits the colour-catalog lookup when `rgba` is alpha=00 and stores `color_name="Clear"` directly — without this, a Bambu-RFID transparent spool would resolve against the `#000000` row in the catalog (or `Black` via the HSL fallback) before the frontend's name resolver ever sees it, defeating the alpha-aware fix in `colors.ts`. **Tests** — `src/__tests__/utils/colors.test.ts`: 5 new assertions covering alpha=00 → "Clear" for `hexToColorName`, `getColorName` (including precedence over a catalog entry on the same RGB), and `resolveSpoolColorName`; one existing assertion changed from `12345600` (which now correctly resolves to "Clear") to `123456FF` to keep its intent of "unknown opaque colour returns null". `src/__tests__/components/spool-form/ColorSectionHexInput.test.tsx`: header docblock rewritten to reflect the new 0–8 char draft contract; the "truncates 7–8 char pastes to RGB" test replaced with two new tests — `'0011223344'` paste now truncates to the leading 8 chars (`00112233`) and commits verbatim with no `FF` append, and a 7-char draft on blur pads to 8 with a trailing `0` instead of jumping back to RGB. 17 colours tests, 9 hex-input tests, 53 useFilamentMapping tests, 14 FilamentOverride tests, 10 FilamentSlotCircle tests, 6 `/printers/available-filaments` integration tests, 50 Spoolman API integration tests all green. Backend ruff clean; frontend build clean; i18n parity clean at 4998 leaves per locale. **What this does NOT change**: Spoolman-mode parity is preserved — Spoolman's own picker already supported 8-digit hex and `inventory.py:119` / `spoolman.py:887-889` already passed `00000000` through verbatim (the 6→8 char `FF` pad only fires when `len == 6`), so no parallel mutation is needed on the Spoolman-mode write path. Existing inventory rows that were *already* rewritten to `F5E6D3FF` stay as cream until the next AMS sync overwrites them — a one-time edit is the only path to recover them, and dropping the rewrite means future AMS syncs land the true value. - **Virtual-printer MQTT no longer drops idle slicer connections at exactly 60 s (#1548, reported by @hollajandro)** — Reporter pointed OrcaSlicer at a Bambuddy virtual printer and got a clean MQTT/TLS connect, successful auth, and a normal pushall/get_version exchange — then the slicer dropped exactly ~60 s later, every time, even after a fresh trust of the VP CA, a logged-out Bambu account, and toggling VP mode. Trace from his support bundle: 5 consecutive connect→disconnect cycles all exactly 60 s apart, with no intervening client packets after the initial exchange. Root cause: `backend/app/services/virtual_printer/mqtt_server.py::_handle_client` used a **hardcoded `timeout=60` on every per-packet read**, and `_handle_connect` two functions below explicitly skipped the keepalive field from the CONNECT payload (`# Skip keepalive` / `idx += 2`). So no matter what the client negotiated, the VP server would close the socket after 60 s of silence — and OrcaSlicer's normal pattern after the initial exchange is to sit quietly waiting for the printer to push status updates, which a virtual printer with no real state changes doesn't do. The real Bambu firmware honours the client's keepalive (MQTT spec §3.1.2.10 / §4.4: server must allow 1.5× the negotiated value before disconnecting), which is why Orca works against a real P1S but failed at exactly 60 s against the VP. **Fix**: `_handle_connect` now parses the 2-byte big-endian keepalive value from the CONNECT payload and returns it alongside the auth bool (`tuple[bool, int]`). `_handle_client` uses that to set its per-packet read timeout to `1.5 × keep_alive` after a successful CONNECT, or `None` (no timeout) when the client opted out with `keep_alive == 0` per spec. The 60 s default is retained for the *initial* read before CONNECT arrives, so a TCP-connect-but-never-send still gets reaped. **Tests**: 7 in `test_vp_mqtt_server.py` — `TestHandleConnectKeepalive` (4: returns negotiated value on success, returns 0 for opt-out, returns `(False, 0)` on auth fail / parse error so the caller's tuple-unpack never crashes), `TestHandleClientHonoursKeepalive` (3: idle client with `keep_alive=180` is still alive past the old 60 s boundary; `keep_alive=2` closes idle in ~3 s; a PINGREQ inside the window resets the timeout and the connection exits via DISCONNECT instead of timeout). The integration-style tests feed a synthetic CONNECT into a real `asyncio.StreamReader` and drive the handler on an event loop, so the timeout math is exercised end-to-end, not just unit-mocked. Backend ruff clean. diff --git a/backend/app/main.py b/backend/app/main.py index f880c821a..d66ad648c 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -644,9 +644,24 @@ def _extract_filament_data_from_mqtt(data: dict, ams_mapping: list[int] | None = entries). When supplied, only the slots actually consumed by this print contribute. Without it the function falls back to every loaded AMS slot — less accurate but still useful. + + Accepts both the raw inner payload (``{"ams": {"ams": [...]}, ...}``) + that the unit tests pass directly, AND the on_print_start callback + shape (``{"raw_data": {"ams": {"ams": [...]}, ...}, ...}``) the + bambu_mqtt service hands to main.py at runtime. The original + ``_extract_filament_data_from_mqtt(data)`` shipped in #1533 only + handled the inner shape and silently returned ``{}`` for every real + print start, leaving fallback archives' filament fields NULL — the + exact regression the fix was meant to close. Reported with a log + proving the AMS state was right there at + ``data["raw_data"]["ams"]["ams"][0]["tray"][0]`` (#1533 follow-up). """ result: dict[str, str] = {} - ams_root = (data or {}).get("ams") or {} + # Look at the on_print_start wrapper first, then the inner shape. + raw_data = (data or {}).get("raw_data") + ams_root = (raw_data or {}).get("ams") if isinstance(raw_data, dict) else None + if not isinstance(ams_root, dict): + ams_root = (data or {}).get("ams") or {} ams_units = ams_root.get("ams") if isinstance(ams_root, dict) else None if not isinstance(ams_units, list) or not ams_units: return result diff --git a/backend/tests/unit/test_fallback_archive_mqtt_filament.py b/backend/tests/unit/test_fallback_archive_mqtt_filament.py index 7d3b6c521..ccbeb85a5 100644 --- a/backend/tests/unit/test_fallback_archive_mqtt_filament.py +++ b/backend/tests/unit/test_fallback_archive_mqtt_filament.py @@ -204,3 +204,90 @@ class TestExtractFilamentDataFromMqtt: @pytest.mark.parametrize("data", [None, {}, {"ams": "weird-string"}]) def test_garbage_top_level_is_empty(self, data): assert _extract_filament_data_from_mqtt(data or {}) == {} + + +class TestOnPrintStartCallbackShape: + """Regression: the callback wrapper shape the bambu_mqtt service + actually hands to ``on_print_start`` at runtime (#1533 follow-up). + + The original #1533 fix only handled the bare ``{"ams": {"ams": [...]}}`` + inner shape, but the call site at ``backend/app/main.py::on_print_start`` + receives the wrapper + ``{"filename", "subtask_name", "remaining_time", "raw_data": , + "ams_mapping"}`` from ``backend/app/services/bambu_mqtt.py:2971-2980``. + The lookup at ``data["ams"]`` therefore missed every real print and + fallback archives kept their filament fields NULL — the exact regression + the fix was supposed to close. Reproduced from JmanB52D's support + bundle whose print start log line showed + ``AMS 0: T0(type=PETG, color=FFFFFFFF, …)`` was sitting right there at + ``data["raw_data"]["ams"]["ams"][0]["tray"][0]``. + """ + + def test_callback_wrapper_payload_resolves_raw_data_path(self): + """The wrapper-shape payload must produce the same result the + inner-shape payload would.""" + inner = { + "ams": { + "ams": [ + _ams_unit(0, [_tray(0, "PETG", "FFFFFFFF")]), + ], + }, + } + wrapper = { + "filename": "/data/Metadata/plate_1.gcode", + "subtask_name": "xyz-10mm-calibration-cube", + "remaining_time": 1200, + "raw_data": inner, + "ams_mapping": [0], + } + result = _extract_filament_data_from_mqtt(wrapper, ams_mapping=[0]) + # The 6-char rgba `FFFFFF` is what the AMS reports (with `FF` alpha + # tail trimmed by the catalog) — the helper preserves whatever the + # firmware sends. + assert result == {"filament_type": "PETG", "filament_color": "FFFFFFFF"} + + def test_wrapper_with_no_ams_mapping_falls_back_to_all_loaded(self): + """Wrapper shape without an ams_mapping behaves the same as the + inner-shape no-mapping path: lists every loaded slot.""" + inner = { + "ams": { + "ams": [ + _ams_unit(0, [_tray(0, "PLA", "FF0000"), _tray(1, "PETG", "00FF00")]), + ], + }, + } + wrapper = {"raw_data": inner} + result = _extract_filament_data_from_mqtt(wrapper) + assert result == {"filament_type": "PLA,PETG", "filament_color": "FF0000,00FF00"} + + def test_inner_shape_still_supported_after_wrapper_lookup(self): + """Existing callers that pass the inner shape directly (e.g. the + unit tests above) must keep working — the new lookup is additive.""" + inner = { + "ams": { + "ams": [_ams_unit(0, [_tray(0, "ASA", "112233")])], + }, + } + assert _extract_filament_data_from_mqtt(inner) == { + "filament_type": "ASA", + "filament_color": "112233", + } + + def test_wrapper_with_missing_raw_data_returns_empty(self): + """No raw_data wrapper AND no top-level ams → empty, no raise.""" + wrapper = {"filename": "foo.gcode", "ams_mapping": [0]} + assert _extract_filament_data_from_mqtt(wrapper, ams_mapping=[0]) == {} + + def test_wrapper_with_non_dict_raw_data_falls_through_to_inner_lookup(self): + """Defensive: a junk raw_data value (string / None) shouldn't crash + and shouldn't shadow a present inner ``ams`` either. Lets us catch + the case where MQTT decoding partially fails but the rest of the + payload is fine.""" + wrapper = { + "raw_data": "garbage", + "ams": {"ams": [_ams_unit(0, [_tray(0, "PLA", "FF0000")])]}, + } + assert _extract_filament_data_from_mqtt(wrapper) == { + "filament_type": "PLA", + "filament_color": "FF0000", + }