mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 03:01:21 +02:00
Load and Unload in the AMS slot menu did nothing on an H2C with the switch fitted. The ams_change_filament command carries an optional extruder_id and Bambuddy never sent it. That is correct on every printer without the switch, and is what BambuStudio does there too -- each AMS is wired to one hotend, so the firmware works the target out for itself and an explicit value would only be a guess at something it already knows. Fit the switch and every AMS is bound to one of its two inlets instead, either hotend is reachable from any slot, and a command naming neither leaves the firmware nothing to act on. It was discarded in silence. Load now asks which hotend to feed, on the same terms as Bambu Studio: no preselection, so a stray Enter cannot feed the wrong one, and the hotend already fed from that very slot greyed out. Printers without a switch send a byte-identical command and still load in one click. A switch fitted but not yet set up -- any AMS still unassigned to an inlet -- refuses the load up front rather than publishing one the firmware will drop, mirroring DevFilaSwitch::IsReady, which likewise demands a switcher position on every AMS. Unload was addressed at the same time. It was aimed with tray_now, a single value for the whole printer, so on any dual-nozzle machine with both hotends loaded it unloaded whichever that field happened to name regardless of which slot's menu was used. It now names the slot and resolves the holding hotend from device.extruder.info, previously read for temperatures only. That resolution is gated on the printer having reported two extruders: single-nozzle machines do send the block, but nobody has read a single-nozzle snow value off the wire, and staking every X1C, P1S and A1 unload on an unverified encoding buys nothing where tray_now is already unambiguous. Both new state fields ride the WebSocket and are in the broadcast key, and both are computed in the REST status route as well -- that response is what the page has before any push arrives, and leaving them at their defaults would have told a correctly set-up machine that its switch was not set up. Verified on H2C-1, AMS-A slot 3: loaded and unloaded from each hotend in turn, all four correct. Covered by 18 MQTT unit tests, 4 status-dict tests, 7 integration tests and 6 component tests. Two known stragglers, both deliberately left alone. Load on an AMS-HT slot has never worked -- an HT unit is addressed by its unit id rather than ams*4+slot, which these endpoints do not accept -- so unload there keeps the printer-wide form it always used instead of gaining a slot it cannot name. And a slot's K-profile still follows the AMS's plumbing rather than the nozzle just loaded, so loading to the far hotend leaves the other one's calibration bound; that is the same per-nozzle problem the filament and K-profile redesign is scoped to fix.
309 lines
13 KiB
Python
309 lines
13 KiB
Python
"""Regression tests for ``PrinterManager._broadcast_status_change`` and
|
|
its wiring from ``set_awaiting_plate_clear`` (#1128).
|
|
|
|
The bug: ``awaiting_plate_clear`` is a Bambuddy-side flag, so toggling it
|
|
doesn't produce an MQTT push from the printer. Before the fix,
|
|
``set_awaiting_plate_clear()`` mutated state and persisted to DB but never
|
|
notified WebSocket subscribers. The plate-clear button on the printer card
|
|
disappeared "immediately" only because of an optimistic React Query cache
|
|
update on the click path; any other caller (admin script, second tab, an
|
|
automation that hits ``POST /printers/{id}/clear-plate``) silently left
|
|
the UI stale until the next coincidental status refresh.
|
|
|
|
These tests pin the contract: every flip of the flag must schedule a
|
|
``printer_status`` broadcast, and the broadcast must carry the new flag
|
|
value so subscribers see the right state without polling.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
from types import SimpleNamespace
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
from backend.app.services.printer_manager import PrinterManager
|
|
|
|
|
|
@pytest.fixture
|
|
def manager():
|
|
"""Fresh manager per test; the awaiting-plate-clear set is per-instance."""
|
|
return PrinterManager()
|
|
|
|
|
|
def _close_unawaited(coro):
|
|
"""Side effect for mocked ``_schedule_async``.
|
|
|
|
``set_awaiting_plate_clear`` evaluates the coroutine expressions
|
|
``self._persist_awaiting_plate_clear(...)`` and
|
|
``self._broadcast_status_change(...)`` before passing them to
|
|
``_schedule_async``. When that target is patched, the coroutine objects
|
|
leak — Python's ``__del__`` then emits ``coroutine was never awaited``
|
|
during GC, and when GC runs late enough that warning hits the interpreter
|
|
shutdown path with ``KeyError: '__import__'``. Closing the coroutine here
|
|
prevents both. Returns ``None`` so the mock's call signature is unchanged.
|
|
"""
|
|
if asyncio.iscoroutine(coro):
|
|
coro.close()
|
|
return None
|
|
|
|
|
|
def _fake_state(**overrides):
|
|
"""Stand-in for a ``PrinterState``.
|
|
|
|
The tests below patch ``printer_state_to_dict`` so the fake doesn't need
|
|
to satisfy every attribute access — but the patch was observed to race on
|
|
parallel CI runners (pytest-xdist), and when it didn't catch the call the
|
|
real ``printer_state_to_dict`` ran against this fake and ``AttributeError``'d
|
|
on ``.kprofiles``. The fake now carries every attribute the real function
|
|
reads, so it remains correct even if the patch is somehow bypassed — the
|
|
test no longer depends on a fragile monkeypatch landing in time.
|
|
|
|
Iterables (``kprofiles``, ``printable_objects``, ``hms_errors``,
|
|
``temperatures``, etc.) default to empty so the function's loops are
|
|
no-ops; scalars default to ``None`` so any "if state.x is None" guard
|
|
falls through cleanly.
|
|
"""
|
|
base = {
|
|
# State the existing test bodies explicitly set / read
|
|
"connected": True,
|
|
"state": "FINISH",
|
|
"raw_data": {},
|
|
"progress": 100.0,
|
|
# Iterables — must be iterable for the loops inside printer_state_to_dict
|
|
"kprofiles": [],
|
|
"printable_objects": [],
|
|
"hms_errors": [],
|
|
"temperatures": {},
|
|
"nozzle_rack": [],
|
|
# Nullable scalars — printer_state_to_dict tolerates None for these
|
|
"active_extruder": None,
|
|
"ams_status_main": None,
|
|
"ams_status_sub": None,
|
|
"big_fan1_speed": None,
|
|
"big_fan2_speed": None,
|
|
"chamber_light": None,
|
|
"cooling_fan_speed": None,
|
|
"current_print": None,
|
|
"door_open": None,
|
|
"firmware_version": None,
|
|
"gcode_file": None,
|
|
"heatbreak_fan_speed": None,
|
|
"left_aux_fan_speed": None,
|
|
"exhaust_fan_present": False,
|
|
"layer_num": None,
|
|
"remaining_time": None,
|
|
"speed_level": None,
|
|
"stg_cur": 0, # get_derived_status_name does ``0 <= state.stg_cur < 255``
|
|
"subtask_name": None,
|
|
"total_layers": None,
|
|
"tray_now": None,
|
|
"wifi_signal": None,
|
|
"wired_network": None,
|
|
"ams_filament_backup": None,
|
|
# Filament Track Switch. None means "no accessory", which is what
|
|
# printer_state_to_dict gates both of these on.
|
|
"fila_switch": None,
|
|
"ams_switch_inlet": {},
|
|
"extruder_slots": {},
|
|
}
|
|
base.update(overrides)
|
|
return SimpleNamespace(**base)
|
|
|
|
|
|
def _scheduled_names(mock) -> list[str]:
|
|
"""Coroutine names passed to the patched ``_schedule_async``.
|
|
|
|
Asserting on names rather than a bare call count keeps this file pinned to
|
|
#1128's contract (persist + broadcast on every flag mutation) without
|
|
breaking every time another emission is hung off the same setter — #2525
|
|
added an edge-triggered MQTT/notification relay, which is covered by its
|
|
own test module.
|
|
"""
|
|
return [call.args[0].__qualname__.rsplit(".", 1)[-1] for call in mock.call_args_list]
|
|
|
|
|
|
class TestSchedulingFromSetAwaitingPlateClear:
|
|
"""The hook from the public flag-mutation method into the broadcast."""
|
|
|
|
def test_schedules_broadcast_when_loop_running(self, manager):
|
|
"""When a real event loop is attached, every call to
|
|
``set_awaiting_plate_clear`` must enqueue both the persistence
|
|
coroutine and the broadcast coroutine. Both are needed: persist
|
|
survives restarts, broadcast notifies live subscribers."""
|
|
manager._loop = MagicMock()
|
|
manager._loop.is_running.return_value = True
|
|
|
|
with patch.object(manager, "_schedule_async", side_effect=_close_unawaited) as scheduled:
|
|
manager.set_awaiting_plate_clear(7, True)
|
|
|
|
# Persist + broadcast, in either order.
|
|
names = _scheduled_names(scheduled)
|
|
assert "_persist_awaiting_plate_clear" in names
|
|
assert "_broadcast_status_change" in names
|
|
|
|
def test_does_not_schedule_when_no_loop_attached(self, manager):
|
|
"""Sync unit-test path (no loop attached): nothing must be
|
|
scheduled, otherwise Python emits 'coroutine was never awaited'
|
|
runtime warnings and the test suite goes red on harmless flag
|
|
twiddling."""
|
|
manager._loop = None
|
|
|
|
with patch.object(manager, "_schedule_async") as scheduled:
|
|
manager.set_awaiting_plate_clear(7, True)
|
|
|
|
scheduled.assert_not_called()
|
|
|
|
def test_does_not_schedule_when_loop_not_running(self, manager):
|
|
"""A loop attached-but-stopped is the same situation as no loop —
|
|
scheduling onto a dead loop would never fire."""
|
|
manager._loop = MagicMock()
|
|
manager._loop.is_running.return_value = False
|
|
|
|
with patch.object(manager, "_schedule_async") as scheduled:
|
|
manager.set_awaiting_plate_clear(7, True)
|
|
|
|
scheduled.assert_not_called()
|
|
|
|
def test_both_true_and_false_flips_schedule_broadcast(self, manager):
|
|
"""The bug only became visible on ``False`` flips (clear), but a
|
|
regression that broadcasts only on ``True`` would re-introduce
|
|
the original symptom for any future flag mutation that goes
|
|
``False → True`` outside the printer-card optimistic-update
|
|
path. Make both directions a contract."""
|
|
manager._loop = MagicMock()
|
|
manager._loop.is_running.return_value = True
|
|
|
|
with patch.object(manager, "_schedule_async", side_effect=_close_unawaited) as scheduled:
|
|
manager.set_awaiting_plate_clear(7, True)
|
|
scheduled.reset_mock()
|
|
manager.set_awaiting_plate_clear(7, False)
|
|
|
|
# The False flip persists and broadcasts just like the True flip did.
|
|
names = _scheduled_names(scheduled)
|
|
assert "_persist_awaiting_plate_clear" in names
|
|
assert "_broadcast_status_change" in names
|
|
|
|
|
|
class TestBroadcastStatusChange:
|
|
"""The broadcast coroutine itself."""
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_emits_ws_update_when_state_present(self, manager):
|
|
"""Happy path: printer has a known status, broadcast goes out
|
|
with the dict produced by ``printer_state_to_dict``.
|
|
|
|
Note: we deliberately don't patch ``printer_state_to_dict`` here.
|
|
The patch was observed to race on parallel xdist runners — when it
|
|
didn't catch the call the real function ran, leaving the test
|
|
comparing the patched return value against the real dict shape.
|
|
Letting the real function run (against a complete ``_fake_state``)
|
|
makes the test deterministic; we assert structural shape, not the
|
|
exact ~36 keys, because pinning those couples the test to the
|
|
evolving ``printer_state_to_dict`` body and adds zero value over
|
|
what ``test_printer_manager.py`` already covers."""
|
|
state = _fake_state()
|
|
with (
|
|
patch.object(manager, "get_status", return_value=state),
|
|
patch.object(manager, "get_model", return_value="P1S"),
|
|
patch(
|
|
"backend.app.core.websocket.ws_manager.send_printer_status",
|
|
new_callable=AsyncMock,
|
|
) as send_status,
|
|
):
|
|
await manager._broadcast_status_change(7)
|
|
|
|
send_status.assert_awaited_once()
|
|
printer_id_arg, payload_arg = send_status.await_args.args
|
|
assert printer_id_arg == 7
|
|
assert isinstance(payload_arg, dict)
|
|
# The ``awaiting_plate_clear`` key is the whole point of this broadcast
|
|
# path (#1128). Any future restructuring that drops it from the dict
|
|
# would silently break the UI; pin its presence.
|
|
assert "awaiting_plate_clear" in payload_arg
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_skips_when_status_unknown(self, manager):
|
|
"""Printer not connected / unknown ID → no point broadcasting a
|
|
snapshot we don't have. A future reconnect will produce a fresh
|
|
status push anyway, so we'd only be forcing a stale or bogus
|
|
payload onto subscribers right now."""
|
|
with (
|
|
patch.object(manager, "get_status", return_value=None),
|
|
patch(
|
|
"backend.app.core.websocket.ws_manager.send_printer_status",
|
|
new_callable=AsyncMock,
|
|
) as send_status,
|
|
):
|
|
await manager._broadcast_status_change(999)
|
|
|
|
send_status.assert_not_awaited()
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_swallows_websocket_errors(self, manager):
|
|
"""The broadcast is a courtesy, not a correctness path — if the
|
|
WS layer is down, the flag is already mutated in-memory and
|
|
persisted. Letting an exception bubble out of
|
|
``_broadcast_status_change`` would surface as an
|
|
``Exception in scheduled callback`` traceback in the log AND
|
|
prevent the persistence coroutine from completing if both were
|
|
gathered together. Swallow + warn instead."""
|
|
with (
|
|
patch.object(manager, "get_status", return_value=_fake_state()),
|
|
patch.object(manager, "get_model", return_value="P1S"),
|
|
patch(
|
|
"backend.app.core.websocket.ws_manager.send_printer_status",
|
|
new_callable=AsyncMock,
|
|
side_effect=RuntimeError("websocket layer unavailable"),
|
|
),
|
|
):
|
|
# Must not raise.
|
|
await manager._broadcast_status_change(7)
|
|
|
|
|
|
class TestEndToEndUnderRunningLoop:
|
|
"""Verify the full flow under a real running event loop — schedule
|
|
→ broadcast → ws_manager.send_printer_status — without mocking
|
|
``_schedule_async``. Catches regressions where individual pieces
|
|
pass but the wiring breaks (e.g. ``_schedule_async`` swallowing the
|
|
broadcast coroutine)."""
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_set_false_eventually_emits_broadcast(self, manager):
|
|
"""Reproduces the #1128 fix path end-to-end: set the flag to
|
|
False under a live loop, give the scheduler a tick, the
|
|
ws broadcast must have fired with the new payload."""
|
|
loop = asyncio.get_running_loop()
|
|
manager._loop = loop
|
|
# Pretend the printer has been seen — without a state present
|
|
# the broadcast short-circuits before reaching ws_manager.
|
|
manager._awaiting_plate_clear.add(7)
|
|
|
|
# _fake_state defaults awaiting_plate_clear=False via printer_state_to_dict's
|
|
# is_awaiting_plate_clear(printer_id) lookup, which reads from
|
|
# manager._awaiting_plate_clear (the in-memory set). Since we just
|
|
# removed 7 from that set by calling set_awaiting_plate_clear(7, False),
|
|
# the broadcast payload's awaiting_plate_clear field will be False.
|
|
with (
|
|
patch.object(manager, "get_status", return_value=_fake_state()),
|
|
patch.object(manager, "get_model", return_value="P1S"),
|
|
patch(
|
|
"backend.app.core.websocket.ws_manager.send_printer_status",
|
|
new_callable=AsyncMock,
|
|
) as send_status,
|
|
# Persistence path opens a DB session; stub it out so this
|
|
# stays a pure unit test.
|
|
patch.object(manager, "_persist_awaiting_plate_clear", new_callable=AsyncMock),
|
|
):
|
|
manager.set_awaiting_plate_clear(7, False)
|
|
# Yield repeatedly so run_coroutine_threadsafe has a chance
|
|
# to land its scheduled coroutine on this loop.
|
|
for _ in range(10):
|
|
await asyncio.sleep(0)
|
|
|
|
send_status.assert_awaited()
|
|
printer_id_arg, payload_arg = send_status.await_args.args
|
|
assert printer_id_arg == 7
|
|
assert payload_arg["awaiting_plate_clear"] is False
|