Ver Fonte

HMS: no repeat notifications after a restart, right titles (#3486)

- The faults already notified were kept only in memory, so every
  restart sent each fault the printer still lists (power-loss notice,
  toolhead malfunction) again; the set is now stored per printer in
  hms_notified_faults and loaded before printers connect
- Writes happen only when the set changes; a failed write or load is
  logged and never holds back a notification
- Printer delete removes the rows on SQLite too (sqlite_fk APPLIED),
  so a reused id does not inherit them
- Module titles follow Bambu's own texts: 0x05 System, 0x0C Camera,
  0x12 AMS Lite (was Nozzle/Extruder, Motion Controller, Chamber);
  toolhead, AMS-HT, tool changer, air filter and AMS 3H added; unknown
  modules read "Printer Error"
maziggy há 2 dias atrás
pai
commit
7dbf02632e

+ 3 - 0
backend/app/core/sqlite_fk.py

@@ -93,6 +93,9 @@ APPLIED: frozenset[tuple[str, str]] = frozenset(
         ("slicer_pipelines", "target_printer_id"),
         # A deleted printer's spools would stay "assigned" to it (SA-1).
         ("spool_assignment", "printer_id"),
+        # A printer given a deleted one's id would never be told about the
+        # faults that printer was notified about (#3486).
+        ("hms_notified_faults", "printer_id"),
         # A profile default's K profile would select a calibration slot on
         # the next printer given the id (#1326).
         ("filament_profile_default_k_profile", "printer_id"),

+ 71 - 14
backend/app/main.py

@@ -2037,6 +2037,30 @@ def _hms_errors_to_notify(errors: list, new_error_codes: set[str]) -> list:
     return [e for e in errors if _hms_notify_key(e) in new_error_codes and _hms_fault_counts(e)]
 
 
+# What each HMS module byte is about, read off Bambu's own texts for its codes
+# (hms_catalog.json). The table used to call 0x05 "Nozzle/Extruder", 0x0C
+# "Motion Controller" and 0x12 "Chamber", which titled USB and MQTT errors as
+# nozzle faults and an A1 mini's AMS Lite runouts as chamber errors (#3486).
+_HMS_MODULE_NAMES = {
+    0x03: "Print/Task",  # bed, nozzle temperatures, motors, the print itself
+    0x05: "System",  # USB/SD, network, firmware, files, AMS communication
+    0x07: "AMS/Filament",
+    0x08: "Toolhead",
+    0x0C: "Camera",  # cameras, lidar, first-layer and AI inspection
+    0x12: "AMS Lite",
+    0x18: "AMS-HT",
+    0x1A: "Tool Changer",
+    0x29: "Air Filter",
+    0x2A: "AMS 3H",
+}
+
+
+def _hms_error_title(module: int) -> str:
+    """The notification title for a fault of *module*."""
+    name = _HMS_MODULE_NAMES.get(module)
+    return f"{name} Error" if name else "Printer Error"
+
+
 def _take_new_hms_faults(printer_id: int, errors: list, print_state: str | None = None) -> list:
     """The faults on this printer not notified yet, and record them as notified.
 
@@ -2080,6 +2104,47 @@ def _take_new_hms_faults(printer_id: int, errors: list, print_state: str | None
     return _hms_errors_to_notify(errors, new)
 
 
+async def _take_new_hms_faults_and_save(printer_id: int, errors: list, print_state: str | None = None) -> list:
+    """``_take_new_hms_faults``, and store the notified set when it changed (#3486).
+
+    The set lives in memory, so without this every restart sent each fault the
+    printer still lists once more. A failed write only costs that protection
+    for the next restart, so it is logged and the notification goes ahead.
+    """
+    before = set(_notified_hms_errors.get(printer_id, ()))
+    new = _take_new_hms_faults(printer_id, errors, print_state)
+    if set(_notified_hms_errors.get(printer_id, ())) != before:
+        from backend.app.services.hms_notified import save_notified_hms_faults
+
+        try:
+            await save_notified_hms_faults(printer_id, lambda: _notified_hms_errors.get(printer_id, {}).keys())
+        except Exception as e:
+            logging.getLogger(__name__).warning(
+                "Could not store the notified HMS faults of printer %s: %s", printer_id, e
+            )
+    return new
+
+
+async def _load_notified_hms_faults() -> None:
+    """Restore the faults notified before the restart (#3486).
+
+    Each counts as seen now: one still listed is not sent again, and one that
+    is gone is forgotten after the usual grace period.
+    """
+    from backend.app.services.hms_notified import load_notified_hms_faults
+
+    try:
+        loaded = await load_notified_hms_faults()
+    except Exception as e:
+        logging.getLogger(__name__).warning("Could not load the notified HMS faults: %s", e)
+        return
+    now = time.time()
+    for printer_id, keys in loaded.items():
+        seen = _notified_hms_errors.setdefault(printer_id, {})
+        for key in keys:
+            seen.setdefault(key, now)
+
+
 # `stg_cur` values that mean the printer is not in a preparation stage: 0 is
 # "Printing", -1 and 255 are "no stage" (#3211).
 _NO_PREPARATION_STAGES = frozenset({0, -1, 255})
@@ -2540,7 +2605,7 @@ async def on_printer_status_change(printer_id: int, state: PrinterState):
     # Check for new HMS errors and send notifications
     current_hms_errors = getattr(state, "hms_errors", []) or []
     if current_hms_errors:
-        new_errors = _take_new_hms_faults(printer_id, current_hms_errors, state.state)
+        new_errors = await _take_new_hms_faults_and_save(printer_id, current_hms_errors, state.state)
 
         if new_errors:
             try:
@@ -2554,16 +2619,6 @@ async def on_printer_status_change(printer_id: int, state: PrinterState):
 
                 printer_name = printer.name if printer else f"Printer {printer_id}"
 
-                # Format error details for notification
-                # Module 0x07 = AMS/Filament, 0x05 = Nozzle, 0x0C = Motion Controller, etc.
-                module_names = {
-                    0x03: "Print/Task",
-                    0x05: "Nozzle/Extruder",
-                    0x07: "AMS/Filament",
-                    0x0C: "Motion Controller",
-                    0x12: "Chamber",
-                }
-
                 # Capture camera snapshot once for all error notifications (no DB held).
                 error_image_data = await _capture_snapshot_for_notification(
                     printer_id, printer, logging.getLogger(__name__)
@@ -2573,7 +2628,6 @@ async def on_printer_status_change(printer_id: int, state: PrinterState):
                 async with async_session() as db:
                     sent_count = 0
                     for error in new_errors:
-                        module_name = module_names.get(error.module, f"Module 0x{error.module:02X}")
                         # Build short code like "0700_8010"
                         # Mask to 16 bits to handle printers that send larger values
                         error_code_int = int(error.code.replace("0x", ""), 16) if error.code else 0
@@ -2588,7 +2642,7 @@ async def on_printer_status_change(printer_id: int, state: PrinterState):
                         if not description or short_code in _HMS_NOTIFICATION_SUPPRESS:
                             continue
 
-                        error_type = f"{module_name} Error"
+                        error_type = _hms_error_title(error.module)
                         error_detail = description
 
                         await notification_service.on_printer_error(
@@ -2625,7 +2679,7 @@ async def on_printer_status_change(printer_id: int, state: PrinterState):
         # No HMS errors: nothing to send, but faults gone long enough (or gone
         # across a print state change) are forgotten. Some codes, e.g. chamber
         # temperature regulation during PETG prints, toggle every few seconds.
-        _take_new_hms_faults(printer_id, [], state.state)
+        await _take_new_hms_faults_and_save(printer_id, [], state.state)
 
     await ws_manager.send_printer_status(
         printer_id,
@@ -12083,6 +12137,9 @@ async def lifespan(app: FastAPI):
 
     # Rehydrate persisted awaiting-plate-clear gate (#961) so prompts survive restarts
     await printer_manager.load_awaiting_plate_clear_from_db()
+    # Before any printer connects, or its first report re-sends every fault
+    # still listed (#3486).
+    await _load_notified_hms_faults()
 
     # WLED is entirely optional. Seed its in-memory per-printer configuration
     # before MQTT connections begin emitting status callbacks.

+ 29 - 0
backend/app/models/hms_notified_fault.py

@@ -0,0 +1,29 @@
+"""HMS faults a notification has already gone out for, per printer (#3486).
+
+The printer keeps faults in its ``hms[]`` list long after they happened, and
+the de-duplication that stops them being sent again lived only in memory: every
+Bambuddy restart sent each still-listed fault once more. Kept here so a restart
+picks up where it left off. Only the set of fault keys is stored; when each was
+last seen stays in memory.
+"""
+
+from __future__ import annotations
+
+from datetime import datetime
+
+from sqlalchemy import DateTime, ForeignKey, Integer, String, UniqueConstraint, func
+from sqlalchemy.orm import Mapped, mapped_column
+
+from backend.app.core.database import Base
+
+
+class HmsNotifiedFault(Base):
+    __tablename__ = "hms_notified_faults"
+    __table_args__ = (UniqueConstraint("printer_id", "fault_key", name="uq_hms_notified_faults_printer_fault"),)
+
+    id: Mapped[int] = mapped_column(primary_key=True)
+    printer_id: Mapped[int] = mapped_column(
+        Integer, ForeignKey("printers.id", ondelete="CASCADE"), nullable=False, index=True
+    )
+    fault_key: Mapped[str] = mapped_column(String(64), nullable=False)
+    created_at: Mapped[datetime] = mapped_column(DateTime, server_default=func.now())

+ 58 - 0
backend/app/services/hms_notified.py

@@ -0,0 +1,58 @@
+"""Persistence for the HMS faults already notified (#3486).
+
+The notification de-duplication itself stays in ``main._take_new_hms_faults``,
+in memory. This only stores which fault keys it holds, so a restart does not
+send every fault a printer still lists again.
+"""
+
+import asyncio
+import logging
+
+from sqlalchemy import delete, select
+
+from backend.app.core.database import async_session
+from backend.app.models.hms_notified_fault import HmsNotifiedFault
+
+logger = logging.getLogger(__name__)
+
+# One writer at a time per printer, so the last write is the latest set.
+_locks: dict[int, asyncio.Lock] = {}
+
+
+async def load_notified_hms_faults() -> dict[int, set[str]]:
+    """Every printer's notified fault keys, as stored."""
+    async with async_session() as db:
+        rows = (await db.execute(select(HmsNotifiedFault.printer_id, HmsNotifiedFault.fault_key))).all()
+    loaded: dict[int, set[str]] = {}
+    for printer_id, fault_key in rows:
+        loaded.setdefault(printer_id, set()).add(fault_key)
+    return loaded
+
+
+async def save_notified_hms_faults(printer_id: int, current_keys) -> None:
+    """Store *current_keys()* as the printer's notified faults.
+
+    *current_keys* is called once the lock is held, so a write that waited
+    stores the set as it is then, not as it was when the write was asked for.
+    """
+    lock = _locks.setdefault(printer_id, asyncio.Lock())
+    async with lock:
+        keys = set(current_keys())
+        async with async_session() as db:
+            stored = set(
+                (
+                    await db.execute(
+                        select(HmsNotifiedFault.fault_key).where(HmsNotifiedFault.printer_id == printer_id)
+                    )
+                ).scalars()
+            )
+            gone = stored - keys
+            if gone:
+                await db.execute(
+                    delete(HmsNotifiedFault).where(
+                        HmsNotifiedFault.printer_id == printer_id, HmsNotifiedFault.fault_key.in_(gone)
+                    )
+                )
+            for key in keys - stored:
+                db.add(HmsNotifiedFault(printer_id=printer_id, fault_key=key))
+            await db.commit()

+ 1 - 0
backend/tests/unit/test_forget_printer.py

@@ -29,6 +29,7 @@ NOT_FORGOTTEN = {
     "_ams_assignment_locks": "a lock carries nothing over; popping one a task holds would let a second be made",
     "_fallback_recovery_locks": "a lock carries nothing over (see its comment on popping)",
     "_HMS_FAILURE_REASONS": "constant",
+    "_HMS_MODULE_NAMES": "constant",
     "_ACTIVE_PRINT_STATES": "constant",
     "_SENSOR_TARGET_KEYS": "constant",
     "PUBLIC_API_ROUTES": "constant",

+ 156 - 0
backend/tests/unit/test_hms_notified_restart_3486.py

@@ -0,0 +1,156 @@
+"""HMS notifications survive a restart without being sent again (#3486).
+
+Printers keep faults in ``hms[]`` long after they happened. The set of faults
+already notified lived only in memory, so every Bambuddy restart sent each
+still-listed fault once more. And the title's category came from a module table
+that called system faults "Nozzle/Extruder" and AMS Lite faults "Chamber".
+"""
+
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, patch
+
+import pytest
+from sqlalchemy import select
+from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
+
+from backend.app import main as main_module
+from backend.app.main import (
+    _hms_error_title,
+    _load_notified_hms_faults,
+    _take_new_hms_faults_and_save,
+)
+from backend.app.models.hms_notified_fault import HmsNotifiedFault
+from backend.app.models.printer import Printer
+
+
+def _fault(full_code: str):
+    return SimpleNamespace(
+        full_code=full_code,
+        attr=int(full_code[:8], 16),
+        code=f"0x{full_code[8:]}",
+        severity=2,
+        description="Described.",
+        actions=[],
+    )
+
+
+TOOLHEAD = _fault("0300010000010008")
+POWER_LOSS = _fault("0500020000020001")
+
+
+@pytest.fixture
+async def printer_id(db_session):
+    printer = Printer(name="P", serial_number="00M09A000000001", ip_address="192.168.1.100", access_code="12345678")
+    db_session.add(printer)
+    await db_session.commit()
+    pid = printer.id
+    main_module._notified_hms_errors.pop(pid, None)
+    main_module._hms_print_state.pop(pid, None)
+    main_module._hms_forget_when_gone.pop(pid, None)
+    yield pid
+    main_module._notified_hms_errors.pop(pid, None)
+    main_module._hms_print_state.pop(pid, None)
+    main_module._hms_forget_when_gone.pop(pid, None)
+
+
+@pytest.fixture(autouse=True)
+def app_session(test_engine):
+    """The service opens its own sessions; point them at the test database."""
+    maker = async_sessionmaker(test_engine, class_=AsyncSession, expire_on_commit=False)
+    with patch("backend.app.services.hms_notified.async_session", maker):
+        yield
+
+
+def _restart(printer_id: int) -> None:
+    """What a restart does to the in-memory tracking."""
+    main_module._notified_hms_errors.pop(printer_id, None)
+    main_module._hms_print_state.pop(printer_id, None)
+    main_module._hms_forget_when_gone.pop(printer_id, None)
+
+
+async def _stored(db_session, printer_id: int) -> set[str]:
+    rows = await db_session.execute(select(HmsNotifiedFault.fault_key).where(HmsNotifiedFault.printer_id == printer_id))
+    return set(rows.scalars())
+
+
+@pytest.mark.asyncio
+class TestRestart:
+    async def test_a_fault_still_listed_is_not_sent_again(self, printer_id):
+        assert await _take_new_hms_faults_and_save(printer_id, [TOOLHEAD, POWER_LOSS], "RUNNING") == [
+            TOOLHEAD,
+            POWER_LOSS,
+        ]
+
+        _restart(printer_id)
+        await _load_notified_hms_faults()
+
+        assert await _take_new_hms_faults_and_save(printer_id, [TOOLHEAD, POWER_LOSS], "RUNNING") == []
+
+    async def test_a_new_fault_after_the_restart_is_still_sent(self, printer_id):
+        await _take_new_hms_faults_and_save(printer_id, [TOOLHEAD], "RUNNING")
+
+        _restart(printer_id)
+        await _load_notified_hms_faults()
+
+        assert await _take_new_hms_faults_and_save(printer_id, [TOOLHEAD, POWER_LOSS], "RUNNING") == [POWER_LOSS]
+
+    async def test_without_the_stored_set_a_restart_sends_again(self, printer_id):
+        # The failure itself: nothing loaded, everything listed is new.
+        await _take_new_hms_faults_and_save(printer_id, [TOOLHEAD], "RUNNING")
+
+        _restart(printer_id)
+
+        assert await _take_new_hms_faults_and_save(printer_id, [TOOLHEAD], "RUNNING") == [TOOLHEAD]
+
+    async def test_a_forgotten_fault_leaves_the_stored_set(self, printer_id, db_session):
+        await _take_new_hms_faults_and_save(printer_id, [TOOLHEAD, POWER_LOSS], "RUNNING")
+        assert await _stored(db_session, printer_id) == {"0300010000010008", "0500020000020001"}
+
+        # A print state change forgets a fault as soon as it is gone.
+        await _take_new_hms_faults_and_save(printer_id, [TOOLHEAD], "FINISH")
+
+        db_session.expire_all()
+        assert await _stored(db_session, printer_id) == {"0300010000010008"}
+
+        _restart(printer_id)
+        await _load_notified_hms_faults()
+        assert await _take_new_hms_faults_and_save(printer_id, [TOOLHEAD, POWER_LOSS], "FINISH") == [POWER_LOSS]
+
+    async def test_a_failed_write_does_not_hold_back_the_notification(self, printer_id):
+        with patch(
+            "backend.app.services.hms_notified.save_notified_hms_faults",
+            AsyncMock(side_effect=RuntimeError("database is locked")),
+        ):
+            assert await _take_new_hms_faults_and_save(printer_id, [TOOLHEAD], "RUNNING") == [TOOLHEAD]
+
+    async def test_a_failed_load_starts_empty(self, printer_id):
+        with patch(
+            "backend.app.services.hms_notified.load_notified_hms_faults",
+            AsyncMock(side_effect=RuntimeError("no such table")),
+        ):
+            await _load_notified_hms_faults()
+
+        assert printer_id not in main_module._notified_hms_errors
+
+
+class TestTitle:
+    @pytest.mark.parametrize(
+        ("module", "title"),
+        [
+            # USB, MQTT and file errors were titled "Nozzle/Extruder Error".
+            (0x05, "System Error"),
+            # Cameras and lidar, not motion.
+            (0x0C, "Camera Error"),
+            # An A1 mini's AMS Lite runouts were titled "Chamber Error".
+            (0x12, "AMS Lite Error"),
+            (0x03, "Print/Task Error"),
+            (0x07, "AMS/Filament Error"),
+            (0x18, "AMS-HT Error"),
+            (0x2A, "AMS 3H Error"),
+            (0x1A, "Tool Changer Error"),
+            # Unknown modules get a plain title rather than a hex number.
+            (0x7F, "Printer Error"),
+        ],
+    )
+    def test_the_title_names_the_right_subsystem(self, module, title):
+        assert _hms_error_title(module) == title