mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-21 18:47:13 +02:00
fix(hms): wrong-plate Ignore actually ignores + buttons read as buttons + ack-detection survives transient re-pause (#1869)
The HMS error modal had three compounding bugs that surfaced when a user forced a wrong-plate HMS (0500_8051) and tried to dispatch the per-fault actions. (1) IGNORE_RESUME did not ignore. Bambuddy redirected the action on state=PAUSE to a plain `resume` command, citing a #1830 verdict that BambuStudio's "err-bearing shape" was firmware-silently-rejected. BambuStudio source disagrees: DeviceErrorDialog.cpp:600 dispatches IGNORE_RESUME via command_hms_ignore, whose wire shape is {command:"ignore", err:"<decimal>", param:"reserve", job_id:...}. That's a distinct command from `resume` — the firmware suppresses the next re-check AND auto-resumes in one operation. Plain resume means "re-check normally", which is exactly why the wrong-plate detection re-fired 1-2 s after the user clicked Ignore. The #1830 "err-bearing shape rejected" test almost certainly sent the err as a hex shortcode; BambuStudio passes std::to_string(int m_error_code) i.e. the DECIMAL form, which is what the firmware matches against. (2) Action buttons read as inert badges. The button className used `hover:${buttonHoverColor}` — a template-literal interpolation Tailwind's JIT scanner can't see as a literal string, so the per-severity hover utility never reached the compiled CSS. Same bg/text color as the severity badge above and no border made it read as another label. No disabled state and no spinner during the 2.5 s ack wait left clicks sitting silently inert. (3) Ack-detection 502'd on legitimate ack. The route compared (gcode_state, hms_errors-len) before vs after publish; wrong-plate re-pause round-tripped both fields to their pre-publish values inside the 2.5 s window → false 502 even though the firmware fully ack'd. PROBLEM_SOLVED_RESUME working but IGNORE_RESUME 502'ing on the same fault was the same race resolving differently. Fixes: bambu_mqtt.py — new hms_ignore_command() publishes the BambuStudio shape; existing hms_ignore(persistent) renamed to hms_idle_ignore (unchanged shape, used by NO_REMINDER_NEXT_TIME per DeviceErrorDialog.cpp:588). Dispatch routes IGNORE_RESUME, IGNORE_NO_REMINDER_NEXT_TIME, and DONT_REMIND_NEXT_TIME to hms_ignore_command (BambuStudio routes all three to the same command_hms_ignore — the "don't remind" half is the firmware's job). NO_REMINDER_NEXT_TIME stays on hms_idle_ignore type=0. Hex → decimal err conversion at the helper layer with a defensive fallback. job_id=None → empty string (matches BambuStudio's std::string default). HMSErrorModal.tsx — getSeverityInfo loses the dead buttonHoverColor field. Action button uses static `bg-white/10 hover:bg-white/20 active:bg-white/30 text-white border border-white/20`, wires `disabled={!hasPermission||mutation.isPending}`, and renders `<Loader2/>` only on the button whose (action,print_error) matches mutation.variables. printers.py — ack-detection probes `client._last_message_time` (bumped on every MQTT push regardless of payload) rather than diffing state fields. The pushall that follows every command guarantees a fresh push lands inside the 2.5 s window on any healthy printer; only firmware-silent-drop leaves the timestamp untouched, which is the 502 path #1830 wanted.
This commit is contained in:
@@ -1770,24 +1770,26 @@ class TestExecuteHMSActionAPI:
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_execute_hms_action_success(self, async_client: AsyncClient, printer_factory):
|
||||
"""200 happy path — dispatcher returns True AND printer state moves
|
||||
within the ack-wait window. The state delta is the firmware's only
|
||||
proof that the command landed (publish success is necessary but not
|
||||
sufficient; see #1830 §(3))."""
|
||||
"""200 happy path — dispatcher returns True AND the printer pushes at
|
||||
least one MQTT message into the ack-wait window. A fresh inbound
|
||||
message is the firmware's proof that the command landed (publish
|
||||
success is necessary but not sufficient; see #1830 §(3))."""
|
||||
printer = await printer_factory(name="Test Printer")
|
||||
|
||||
mock_client = MagicMock()
|
||||
# Pre-action state — paused with a fault.
|
||||
# Pre-action state — paused with a fault, last message arrived at t=0.
|
||||
mock_client.state.state = "PAUSE"
|
||||
mock_client.state.print_error = 0x05008051
|
||||
mock_client.state.hms_errors = [object()]
|
||||
mock_client._last_message_time = 100.0
|
||||
|
||||
def _act(*_a, **_kw):
|
||||
# Simulate the printer accepting the command and clearing the fault
|
||||
# by the time the ack-wait expires.
|
||||
mock_client.state.state = "FAILED"
|
||||
mock_client.state.print_error = 0
|
||||
mock_client.state.hms_errors = []
|
||||
# Simulate the printer pushing a status update within the ack-wait
|
||||
# window. The pushall that follows every command is what produces
|
||||
# this — the actual state fields don't have to move (#1869: a
|
||||
# wrong-plate IGNORE_RESUME re-pauses with the same fault but the
|
||||
# printer DID push back).
|
||||
mock_client._last_message_time = 100.5
|
||||
return True
|
||||
|
||||
mock_client.execute_hms_action.side_effect = _act
|
||||
@@ -1830,8 +1832,8 @@ class TestExecuteHMSActionAPI:
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_execute_hms_action_no_printer_ack_returns_502(self, async_client: AsyncClient, printer_factory):
|
||||
"""502 when publish succeeded but printer state didn't move within the
|
||||
ack-wait window. This is the silent-rejection failure mode #1830
|
||||
"""502 when publish succeeded but no MQTT message arrives back within
|
||||
the ack-wait window. This is the silent-rejection failure mode #1830
|
||||
identifies: the broker ACKs the publish at QoS 1 but the firmware
|
||||
drops the command (err mismatch, wrong shape, state mismatch).
|
||||
Surfacing this as 502 instead of 200 stops the UI from claiming
|
||||
@@ -1842,8 +1844,9 @@ class TestExecuteHMSActionAPI:
|
||||
mock_client.state.state = "PAUSE"
|
||||
mock_client.state.print_error = 0x05008051
|
||||
mock_client.state.hms_errors = [object()]
|
||||
mock_client._last_message_time = 100.0
|
||||
mock_client.execute_hms_action.return_value = True # publish "succeeded"
|
||||
# Crucially: state does NOT change → ack-wait detects no movement.
|
||||
# Crucially: _last_message_time does NOT advance → no inbound push.
|
||||
|
||||
with (
|
||||
patch("backend.app.api.routes.printers.printer_manager") as mock_pm,
|
||||
@@ -1858,6 +1861,49 @@ class TestExecuteHMSActionAPI:
|
||||
assert response.status_code == 502
|
||||
assert "acknowledge" in response.json()["detail"].lower()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_execute_hms_action_ignore_resume_repauses_within_window_still_acks(
|
||||
self, async_client: AsyncClient, printer_factory
|
||||
):
|
||||
"""200 when the printer ack'd the command but immediately re-paused
|
||||
with the same fault — e.g. wrong-plate IGNORE_RESUME (#1869). The
|
||||
previous (gcode_state, hms_errors-len) diff produced a false 502
|
||||
because both fields round-tripped to their pre-publish values inside
|
||||
the ack window. Probing `_last_message_time` survives the round-trip
|
||||
because the printer's status push lands regardless of the eventual
|
||||
state."""
|
||||
printer = await printer_factory(name="Test Printer")
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.state.state = "PAUSE"
|
||||
mock_client.state.print_error = 0x05008051
|
||||
mock_client.state.hms_errors = [object()]
|
||||
mock_client._last_message_time = 100.0
|
||||
|
||||
def _act(*_a, **_kw):
|
||||
# Printer ack'd, briefly resumed, re-detected the wrong plate, and
|
||||
# re-paused with the same fault. Net diff on state fields is zero,
|
||||
# but a fresh status push DID arrive.
|
||||
mock_client._last_message_time = 100.4
|
||||
mock_client.state.state = "PAUSE" # round-tripped
|
||||
mock_client.state.hms_errors = [object()] # same length
|
||||
return True
|
||||
|
||||
mock_client.execute_hms_action.side_effect = _act
|
||||
|
||||
with (
|
||||
patch("backend.app.api.routes.printers.printer_manager") as mock_pm,
|
||||
patch("backend.app.api.routes.printers.HMS_ACTION_ACK_WAIT_SECONDS", 0.01),
|
||||
):
|
||||
mock_pm.get_client.return_value = mock_client
|
||||
|
||||
body = {"print_error": "05008051", "action": "IGNORE_RESUME", "job_id": None}
|
||||
response = await async_client.post(f"/api/v1/printers/{printer.id}/hms/execute-action", json=body)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["success"] is True
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_execute_hms_action_accepts_16_char_full_code(self, async_client: AsyncClient, printer_factory):
|
||||
@@ -1870,9 +1916,10 @@ class TestExecuteHMSActionAPI:
|
||||
mock_client.state.state = "RUNNING"
|
||||
mock_client.state.print_error = 0
|
||||
mock_client.state.hms_errors = [object()]
|
||||
mock_client._last_message_time = 100.0
|
||||
|
||||
def _act(*_a, **_kw):
|
||||
mock_client.state.hms_errors = []
|
||||
mock_client._last_message_time = 100.4
|
||||
return True
|
||||
|
||||
mock_client.execute_hms_action.side_effect = _act
|
||||
|
||||
Reference in New Issue
Block a user