Przeglądaj źródła

fix(virtual-printer): proxy VPs stop restarting on every sync, renames and printer IP changes reach the slicer

- The proxy branch of sync_from_db built its instance without the row's
  target printer id and queue toggles, which the change check compares,
  so every VP create/update/delete restarted every running proxy VP and
  dropped the slicer's connection.
- A rename restarts the VP in every mode: the running SSDP and bind
  servers kept advertising the old name.
- Changing a printer's IP syncs the virtual printers, so proxy VPs aimed
  at it forward to the new address instead of the old one.
maziggy 6 dni temu
rodzic
commit
c23c073dd1

+ 1 - 0
CHANGELOG.md

@@ -164,6 +164,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **The frontend build no longer warns about `path` and `crypto` being externalized for the STEP previewer (#2976)** — `occt-import-js`, the Emscripten build behind STEP previews, requires both modules, but only inside its `ENVIRONMENT_IS_NODE` branches; in the browser it loads its `.wasm` from the URL the preview worker passes and draws randomness from `crypto.getRandomValues`. Vite still externalized both and printed two warnings on every build. `vite.config.ts` now drops exactly those two warnings for that one package through `build.rolldownOptions.onLog`, so an externalization anywhere else, or of any other module, still shows.
 
 ### Fixed
+- **Proxy virtual printers no longer restart when another virtual printer is saved** — Creating, editing or deleting any virtual printer restarted every running proxy-mode virtual printer, dropping the slicer's connection and any upload in progress. They now restart only when their own settings change. Changing a printer's IP now restarts the proxy virtual printers aimed at it on the new address; before, they kept forwarding to the old one until a virtual printer was saved. Renaming a virtual printer now restarts it, in every mode, so the slicer sees the new name; before, a running virtual printer kept advertising its old name until Bambuddy restarted.
 - **Prints queued through a virtual printer keep the slicer's nozzle choices** —
   - **Nozzle offset calibration** from the slicer, or the default from Settings → Workflow, is used. It always ran on "Auto".
   - **H2C rack nozzles** picked in the slicer are kept only by a virtual printer bound to one printer. A virtual printer for any H2C sent those positions to whichever H2C took the job, whatever that printer's rack held; the dispatch now picks them from that printer's rack.

+ 12 - 0
backend/app/api/routes/printers.py

@@ -434,6 +434,8 @@ async def update_printer(
             update_data["plate_detection_roi_w"] = None
             update_data["plate_detection_roi_h"] = None
 
+    ip_changed = "ip_address" in update_data and update_data["ip_address"] != printer.ip_address
+
     for field, value in update_data.items():
         setattr(printer, field, value)
 
@@ -443,6 +445,16 @@ async def update_printer(
     if access_moved:
         await ws_manager.refresh_printer_scopes()
 
+    # Proxy virtual printers forward to the IP they were started with; the
+    # sync restarts the ones aimed at this printer on the new one.
+    if ip_changed:
+        from backend.app.services.virtual_printer import virtual_printer_manager
+
+        try:
+            await virtual_printer_manager.sync_from_db()
+        except Exception as e:
+            logger.error("Failed to sync virtual printers after printer IP change: %s", e)
+
     # Reconnect if connection settings changed
     if any(k in update_data for k in ["ip_address", "access_code", "is_active"]):
         printer_manager.disconnect_printer(printer_id)

+ 10 - 0
backend/app/services/virtual_printer/manager.py

@@ -1843,6 +1843,9 @@ class VirtualPrinterManager:
             db_mode = normalize_vp_mode(vp.mode)
             changed = (
                 instance.mode != db_mode
+                # The name is advertised to the slicer (SSDP, bind) by the
+                # running services, so a rename only reaches it on restart.
+                or instance.name != vp.name
                 or instance.model != (vp.model or DEFAULT_VIRTUAL_PRINTER_MODEL)
                 or instance.access_code != (vp.access_code or "")
                 or instance.bind_ip != (vp.bind_ip or "")
@@ -1887,7 +1890,14 @@ class VirtualPrinterManager:
                     serial_suffix=vp.serial_suffix,
                     target_printer_ip=target_ip,
                     target_printer_serial=target_serial,
+                    # Carried although the proxy does not use them: the change
+                    # check above compares them with the row, and a missing
+                    # value restarted every proxy VP on every sync.
+                    target_printer_id=vp.target_printer_id,
                     auto_dispatch=vp.auto_dispatch,
+                    queue_force_color_match=vp.queue_force_color_match,
+                    save_ams_mapping=vp.save_ams_mapping,
+                    gcode_injection=vp.gcode_injection,
                     bind_ip=vp.bind_ip or "",
                     remote_interface_ip=vp.remote_interface_ip or "",
                     tailscale_disabled=vp.tailscale_disabled,

+ 60 - 0
backend/tests/integration/test_printer_ip_change_syncs_vps.py

@@ -0,0 +1,60 @@
+"""A printer's new IP reaches the proxy virtual printers aimed at it.
+
+Proxy VPs forward to the IP they were started with. The sync that restarts
+them on a changed target IP ran only when a virtual printer was saved, so
+editing the printer's IP left them forwarding to the old address until then.
+"""
+
+from unittest.mock import AsyncMock, patch
+
+import pytest
+from httpx import AsyncClient
+
+
+@pytest.fixture
+def vp_sync():
+    with (
+        patch("backend.app.api.routes.printers.printer_manager.disconnect_printer"),
+        patch("backend.app.api.routes.printers.printer_manager.connect_printer", new=AsyncMock()),
+        patch(
+            "backend.app.services.virtual_printer.virtual_printer_manager.sync_from_db",
+            new_callable=AsyncMock,
+        ) as sync,
+    ):
+        yield sync
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_ip_change_syncs_virtual_printers(async_client: AsyncClient, printer_factory, vp_sync):
+    printer = await printer_factory(ip_address="192.168.1.50")
+
+    response = await async_client.patch(f"/api/v1/printers/{printer.id}", json={"ip_address": "192.168.1.77"})
+
+    assert response.status_code == 200
+    vp_sync.assert_awaited_once()
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_unchanged_ip_does_not_sync(async_client: AsyncClient, printer_factory, vp_sync):
+    printer = await printer_factory(ip_address="192.168.1.50")
+
+    response = await async_client.patch(
+        f"/api/v1/printers/{printer.id}", json={"name": "Renamed", "ip_address": "192.168.1.50"}
+    )
+
+    assert response.status_code == 200
+    vp_sync.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_a_failed_sync_does_not_fail_the_update(async_client: AsyncClient, printer_factory, vp_sync):
+    printer = await printer_factory(ip_address="192.168.1.50")
+    vp_sync.side_effect = RuntimeError("sync broke")
+
+    response = await async_client.patch(f"/api/v1/printers/{printer.id}", json={"ip_address": "192.168.1.77"})
+
+    assert response.status_code == 200
+    assert response.json()["ip_address"] == "192.168.1.77"

+ 129 - 0
backend/tests/unit/services/test_virtual_printer.py

@@ -8,6 +8,7 @@ import json
 import shutil
 import zipfile
 from pathlib import Path
+from types import SimpleNamespace
 from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
@@ -3475,6 +3476,134 @@ class TestVirtualPrinterManager:
 
             mock_remove.assert_called_once_with(1)
 
+    def _setup_proxy_sync_mocks(
+        self, manager, enabled_vps, tmp_path, printer_ip="192.168.1.50", serial="0948AB012345678"
+    ):
+        """Like _setup_sync_mocks, but the target printer lookup returns a real IP and serial."""
+        vps_result = MagicMock()
+        vps_result.scalars.return_value.all.return_value = enabled_vps
+        printer_result = MagicMock()
+        printer_result.scalar_one_or_none.return_value = SimpleNamespace(ip_address=printer_ip, serial_number=serial)
+
+        async def execute(stmt):
+            entity = stmt.column_descriptions[0]["entity"]
+            return printer_result if entity.__name__ == "Printer" else vps_result
+
+        mock_db = AsyncMock()
+        mock_db.execute = AsyncMock(side_effect=execute)
+        mock_db.__aenter__ = AsyncMock(return_value=mock_db)
+        mock_db.__aexit__ = AsyncMock(return_value=False)
+
+        manager._session_factory = MagicMock(return_value=mock_db)
+        manager._base_dir = tmp_path
+
+    @pytest.mark.asyncio
+    async def test_sync_from_db_leaves_a_running_proxy_vp_alone(self, manager, tmp_path):
+        """A proxy VP that sync started must survive the next sync unchanged.
+
+        The proxy branch built its instance without the row's target printer
+        id, so every later sync saw a change and restarted every proxy VP,
+        dropping the slicer's connection, whenever any VP or printer was saved.
+        The queue-mode toggles are set away from their defaults, as on a VP
+        switched from queue mode to proxy, which keeps them in its row.
+        """
+        from backend.app.services.virtual_printer.manager import VirtualPrinterInstance
+
+        db_vp = self._make_db_vp(
+            mode="proxy",
+            model="O1D",
+            target_printer_id=7,
+            bind_ip="10.0.0.5",
+            auto_dispatch=False,
+            queue_force_color_match=True,
+            save_ams_mapping=True,
+            gcode_injection=True,
+        )
+        self._setup_proxy_sync_mocks(manager, [db_vp], tmp_path)
+
+        with (
+            patch.object(VirtualPrinterInstance, "start_proxy", new_callable=AsyncMock) as mock_start,
+            patch.object(VirtualPrinterInstance, "stop_proxy", new_callable=AsyncMock) as mock_stop,
+        ):
+            await manager.sync_from_db()
+            assert mock_start.await_count == 1
+            assert manager._instances[1].target_printer_id == 7
+
+            await manager.sync_from_db()
+
+        assert mock_start.await_count == 1
+        mock_stop.assert_not_awaited()
+
+    @pytest.mark.asyncio
+    async def test_sync_from_db_restarts_a_proxy_vp_on_target_ip_change(self, manager, tmp_path):
+        from backend.app.services.virtual_printer.manager import VirtualPrinterInstance
+
+        db_vp = self._make_db_vp(mode="proxy", model="O1D", target_printer_id=7, bind_ip="10.0.0.5")
+        self._setup_proxy_sync_mocks(manager, [db_vp], tmp_path)
+
+        with (
+            patch.object(VirtualPrinterInstance, "start_proxy", new_callable=AsyncMock) as mock_start,
+            patch.object(VirtualPrinterInstance, "stop_proxy", new_callable=AsyncMock) as mock_stop,
+        ):
+            await manager.sync_from_db()
+            self._setup_proxy_sync_mocks(manager, [db_vp], tmp_path, printer_ip="192.168.1.77")
+            await manager.sync_from_db()
+
+        mock_stop.assert_awaited_once()
+        assert mock_start.await_count == 2
+        assert manager._instances[1].target_printer_ip == "192.168.1.77"
+
+    @pytest.mark.asyncio
+    async def test_sync_from_db_restarts_a_proxy_vp_on_rename(self, manager, tmp_path):
+        """The name is advertised to the slicer (SSDP, bind), so a rename only
+        reaches the slicer through a restart."""
+        from backend.app.services.virtual_printer.manager import VirtualPrinterInstance
+
+        db_vp = self._make_db_vp(mode="proxy", model="O1D", target_printer_id=7, bind_ip="10.0.0.5")
+        self._setup_proxy_sync_mocks(manager, [db_vp], tmp_path)
+
+        with (
+            patch.object(VirtualPrinterInstance, "start_proxy", new_callable=AsyncMock) as mock_start,
+            patch.object(VirtualPrinterInstance, "stop_proxy", new_callable=AsyncMock) as mock_stop,
+        ):
+            await manager.sync_from_db()
+            db_vp.name = "Workshop H2D"
+            await manager.sync_from_db()
+
+        mock_stop.assert_awaited_once()
+        assert mock_start.await_count == 2
+        assert manager._instances[1].name == "Workshop H2D"
+
+    @pytest.mark.asyncio
+    async def test_sync_from_db_restarts_on_rename(self, manager, tmp_path):
+        """Same for a server-mode VP: the running SSDP and bind servers keep
+        advertising the old name until the instance restarts."""
+        from backend.app.services.virtual_printer.manager import VirtualPrinterInstance
+
+        inst = VirtualPrinterInstance(
+            vp_id=1,
+            name="TestVP",
+            mode="archive",
+            model="C11",
+            access_code="12345678",
+            serial_suffix="391800001",
+            base_dir=tmp_path,
+        )
+        manager._instances[1] = inst
+
+        db_vp = self._make_db_vp(name="Renamed VP")
+        self._setup_sync_mocks(manager, [db_vp], tmp_path)
+
+        with patch.object(manager, "remove_instance", new_callable=AsyncMock) as mock_remove:
+            with patch("backend.app.services.virtual_printer.manager.VirtualPrinterInstance") as MockInst:
+                mock_new = MagicMock()
+                mock_new.start_server = AsyncMock()
+                MockInst.return_value = mock_new
+
+                await manager.sync_from_db()
+
+            mock_remove.assert_called_once_with(1)
+
 
 class TestFTPSession:
     """Tests for FTP session handling."""