mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-22 03:49:18 +02:00
fix(vp): gate the slicer's AMS pick behind the toggle and scope its badges (#2700)
Round-3 review of the "Save AMS mapping" PR. The queue item's ams_mapping was set unconditionally, on the reasoning that honouring the slicer's own pick is a correctness fix rather than a feature. It is both. Storing a resolved mapping makes _ensure_ams_mapping return early, so _compute_ams_mapping_for_printer never runs — and that function is where prefer_lowest_filament lives, along with the AMS-filament-backup gate that qualifies it (#1766), the inventory-remain overrides, and the per-slot force-colour overrides. Every existing queue-mode VP pointed at a printer would have quietly lost all of it on upgrade, without a setting to turn it back on. So save_ams_mapping now gates the queue item too, not just the archive persistence. Off is exactly the old behaviour. The correctness case the PR was written for — two spools of the same red PLA, and the slot the user picked in the slicer thrown away — is still fixed, for anyone who asks for it. Force color match wins over it when both are on. Its only effect on a fixed-printer item is the filament_overrides written onto the queue item, and those are read inside the function a stored mapping skips, so the two toggles sitting next to each other on the same card silently cancelled. The dispatch now matches strictly, as asked, while the slicer's pick is still saved onto the archive — that is what the toggle's name promises, and a later reprint is a separate decision from this print. The queue-add fallback applies the same rule to a request that carries force-colour overrides. A mapping shorter than a plate's highest slot id cannot address that plate's own slots, and _ensure_ams_mapping would have kept it anyway, since it only rejects an all-unresolved one. Each plate now checks the length it needs and falls back to a computed mapping if the array does not reach. Bambu Studio sends a file-global array, so this normally never fires; it also means a multi-plate Send All degrades safely if that ever stops being true. The badges claimed more than they delivered. Both rendered whenever a saved mapping existed, ignoring which printer it belonged to, while the tooltips promised the reprint would reuse those exact spools — true only on the printer the trays were resolved against. The queue row's flag is now computed against that row's own printer, which is precisely when dispatch reuses the mapping, and the archive card names the printer instead of implying any of them will do. It hides itself when that printer no longer exists. Retranslated in all 13 locales. Frontend tests, which the PR had none of. The printer-scoping rule is now a pure function rather than an inline expression, covered for the mismatched printer, the no-printer-selected case that would otherwise compare undefined against undefined, and malformed extra_data. The toggle's undo bookkeeping is covered for unresolved slots, short mappings, and hand-made picks — preserved when the toggle never wrote that slot, replaced when it did, which is behaviour worth pinning either way. Also reverts all three queue-mode switches when a save fails, not just the new one; without it the card shows a setting the server rejected.
This commit is contained in:
@@ -376,6 +376,93 @@ class TestPrintQueueAPI:
|
||||
result = response.json()
|
||||
assert result["ams_mapping"] is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_add_to_queue_force_color_match_overrides_beat_the_archive_fallback(
|
||||
self, async_client: AsyncClient, printer_factory, archive_factory, db_session
|
||||
):
|
||||
"""Force-color-match overrides are the caller asking the scheduler to
|
||||
match strictly against the printer's live trays, and they are only ever
|
||||
applied inside `_compute_ams_mapping_for_printer` — the function a
|
||||
stored mapping makes the scheduler skip. Inheriting the saved mapping
|
||||
here would silently retire the strictness that was just requested
|
||||
(#2700 review).
|
||||
"""
|
||||
printer = await printer_factory()
|
||||
archive = await archive_factory(
|
||||
extra_data={"slicer_ams_mapping": {"mapping": [5, -1, 2, -1], "printer_id": printer.id}}
|
||||
)
|
||||
|
||||
data = {
|
||||
"printer_id": printer.id,
|
||||
"archive_id": archive.id,
|
||||
"filament_overrides": [
|
||||
{"slot_id": 1, "type": "PLA", "color": "#FF0000", "force_color_match": True},
|
||||
],
|
||||
}
|
||||
response = await async_client.post("/api/v1/queue/", json=data)
|
||||
assert response.status_code == 200
|
||||
result = response.json()
|
||||
assert result["ams_mapping"] is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_add_to_queue_plain_overrides_still_allow_the_archive_fallback(
|
||||
self, async_client: AsyncClient, printer_factory, archive_factory, db_session
|
||||
):
|
||||
"""Only force_color_match stands the fallback down. A plain preference
|
||||
override is a filament swap, not a request for live colour matching, so
|
||||
the saved mapping is still the best starting point.
|
||||
"""
|
||||
printer = await printer_factory()
|
||||
archive = await archive_factory(
|
||||
extra_data={"slicer_ams_mapping": {"mapping": [5, -1, 2, -1], "printer_id": printer.id}}
|
||||
)
|
||||
|
||||
data = {
|
||||
"printer_id": printer.id,
|
||||
"archive_id": archive.id,
|
||||
"filament_overrides": [{"slot_id": 1, "type": "PLA", "color": "#FF0000"}],
|
||||
}
|
||||
response = await async_client.post("/api/v1/queue/", json=data)
|
||||
assert response.status_code == 200
|
||||
result = response.json()
|
||||
assert result["ams_mapping"] == [5, -1, 2, -1]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_queue_response_flags_saved_mapping_only_for_its_own_printer(
|
||||
self, async_client: AsyncClient, printer_factory, archive_factory, db_session
|
||||
):
|
||||
"""`archive_has_slicer_ams_mapping` drives a badge that claims the
|
||||
print reuses the slicer's exact trays. Global tray IDs mean nothing on
|
||||
another printer, so the flag must be false for a row targeting one —
|
||||
otherwise the badge is there while nothing is reused (#2700 review).
|
||||
"""
|
||||
origin_printer = await printer_factory()
|
||||
other_printer = await printer_factory()
|
||||
archive = await archive_factory(
|
||||
extra_data={"slicer_ams_mapping": {"mapping": [5, -1, 2, -1], "printer_id": origin_printer.id}}
|
||||
)
|
||||
|
||||
own = await async_client.post(
|
||||
"/api/v1/queue/", json={"printer_id": origin_printer.id, "archive_id": archive.id}
|
||||
)
|
||||
assert own.status_code == 200
|
||||
assert own.json()["archive_has_slicer_ams_mapping"] is True
|
||||
|
||||
foreign = await async_client.post(
|
||||
"/api/v1/queue/", json={"printer_id": other_printer.id, "archive_id": archive.id}
|
||||
)
|
||||
assert foreign.status_code == 200
|
||||
assert foreign.json()["archive_has_slicer_ams_mapping"] is False
|
||||
|
||||
# Model-based: the scheduler hasn't picked a printer yet, so the
|
||||
# mapping is not reused there either.
|
||||
model_based = await async_client.post("/api/v1/queue/", json={"target_model": "X1C", "archive_id": archive.id})
|
||||
assert model_based.status_code == 200
|
||||
assert model_based.json()["archive_has_slicer_ams_mapping"] is False
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_add_to_queue_with_plate_id(
|
||||
|
||||
Reference in New Issue
Block a user