소스 검색

fix(spoolbuddy): unregistered devices register again, refused requests stop blocking the buffer

maziggy 6 일 전
부모
커밋
e70fbfb961

+ 3 - 0
CHANGELOG.md

@@ -164,6 +164,9 @@ 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.
 - **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
 ### Fixed
+- **A spool Spoolman creates from the AMS is the size of the roll** — a 250 g refill matched to an existing 1000 g filament was created as a 1000 g spool with 250 g left, showing as 75% used.
+- **An unregistered SpoolBuddy that is still running registers again on its next heartbeat**, as the documentation says. It stayed missing until the daemon or the Pi restarted, and the refused heartbeats filled its send buffer, which then held back everything queued behind them.
+- **SpoolBuddy installer, full mode, installed under /home** — the Bambuddy service it writes now starts. The service hid /home from itself and failed with 203/EXEC, as the main installer's did before #1685.
 - **Spoolman costs** —
 - **Spoolman costs** —
   - A failed or stopped print in Spoolman mode is logged in Print History with what its Spoolman spools cost for the filament it used, as with the built-in inventory. It was logged with no cost.
   - A failed or stopped print in Spoolman mode is logged in Print History with what its Spoolman spools cost for the filament it used, as with the built-in inventory. It was logged with no cost.
   - The estimated cost of a queued library file is priced from the Spoolman spools in the trays it maps to. Every slot was priced at the default filament cost.
   - The estimated cost of a queued library file is priced from the Spoolman spools in the trays it maps to. Every slot was priced at the default filament cost.

+ 4 - 0
backend/app/services/spoolman.py

@@ -1513,6 +1513,10 @@ class SpoolmanClient:
                 remaining_weight=remaining,
                 remaining_weight=remaining,
                 comment="Created by Bambuddy",
                 comment="Created by Bambuddy",
                 extra=extra,
                 extra=extra,
+                # The roll's own net weight: the filament may be another size's
+                # (a 250 g refill of a 1000 g line), and Spoolman copies its
+                # weight when the spool gives none (#3194).
+                initial_weight=tray.tray_weight if tray.tray_weight > 0 else None,
             )
             )
             await self.add_native_tags(created, [tray.tray_uuid, chip_uid], tag_format)
             await self.add_native_tags(created, [tray.tray_uuid, chip_uid], tag_format)
             return created
             return created

+ 18 - 0
backend/tests/unit/services/test_spoolman_extra_field_registration_2903.py

@@ -351,3 +351,21 @@ class TestWhenRegistrationItselfFails:
         result = await client.create_spool(filament_id=7, extra={"tag": json.dumps("B")})
         result = await client.create_spool(filament_id=7, extra={"tag": json.dumps("B")})
 
 
         assert result["extra"]["tag"] == json.dumps("B")
         assert result["extra"]["tag"] == json.dumps("B")
+
+
+class TestAutoCreatedSpoolSize:
+    """A spool auto-created from the AMS is the size the tray reports. Left
+    off, Spoolman copies the filament's weight: a 250 g refill matched to a
+    1000 g filament was created as a 1000 g spool with 250 g left (#3194)."""
+
+    @pytest.mark.asyncio
+    async def test_the_spool_takes_the_trays_own_weight(self):
+        fake = FakeSpoolman()
+        client = _client(fake)
+        tray = _tray("D144798DEF394926ACAE9D69ABA910CC")
+        tray.tray_weight = 250
+
+        result = await client.sync_ams_tray(tray, "OJIMPO-X2D-01", _NoCatalog())
+
+        assert result["initial_weight"] == 250
+        assert result["remaining_weight"] == 250

+ 14 - 0
backend/tests/unit/test_systemd_backup_paths.py

@@ -69,3 +69,17 @@ class TestInstallersPreserveCustomPaths:
             f"{installer} no longer carries the previous unit's ReadWritePaths forward"
             f"{installer} no longer carries the previous unit's ReadWritePaths forward"
         )
         )
         assert ".bak-" in script, f"{installer} overwrites the unit without backing it up first"
         assert ".bak-" in script, f"{installer} overwrites the unit without backing it up first"
+
+
+class TestInstallersKeepHomeInstallsRunnable:
+    """ProtectHome=true hides /home from the service, so an install there fails
+    with 203/EXEC before Bambuddy starts (#1685). Both installers loosen it to
+    read-only for a /home install; the SpoolBuddy one kept a literal true."""
+
+    @pytest.mark.parametrize("installer", INSTALLERS)
+    def test_protect_home_follows_the_install_path(self, installer):
+        script = _read(installer)
+        line = next(line for line in script.splitlines() if line.startswith("ProtectHome="))
+        assert line == "ProtectHome=$protect_home", f"{installer} hard-codes {line}"
+        assert 'if [[ "$INSTALL_PATH" == /home/* ]]; then' in script
+        assert 'protect_home="read-only"' in script

+ 31 - 0
spoolbuddy/daemon/api_client.py

@@ -11,6 +11,15 @@ logger = logging.getLogger(__name__)
 MAX_BUFFER_SIZE = 100
 MAX_BUFFER_SIZE = 100
 
 
 
 
+def _says_unregistered(response: httpx.Response) -> bool:
+    """The backend's own answer for a device it does not know -- not a bare
+    404 from a reverse proxy while Bambuddy is restarting."""
+    try:
+        return response.json().get("detail") == "Device not registered"
+    except Exception:
+        return False
+
+
 class APIClient:
 class APIClient:
     def __init__(self, backend_url: str, api_key: str):
     def __init__(self, backend_url: str, api_key: str):
         self._base = backend_url.rstrip("/") + "/api/v1/spoolbuddy"
         self._base = backend_url.rstrip("/") + "/api/v1/spoolbuddy"
@@ -20,6 +29,10 @@ class APIClient:
         self._max_backoff = 30.0
         self._max_backoff = 30.0
         self._buffer: deque[dict] = deque(maxlen=MAX_BUFFER_SIZE)
         self._buffer: deque[dict] = deque(maxlen=MAX_BUFFER_SIZE)
         self._connected = False
         self._connected = False
+        # Set when a heartbeat is refused because the backend no longer knows
+        # this device (an admin unregistered it); the heartbeat loop registers
+        # again and clears it.
+        self.unregistered = False
 
 
     async def close(self):
     async def close(self):
         await self._client.aclose()
         await self._client.aclose()
@@ -31,6 +44,19 @@ class APIClient:
             self._backoff = 1.0
             self._backoff = 1.0
             self._connected = True
             self._connected = True
             return resp.json()
             return resp.json()
+        except httpx.HTTPStatusError as e:
+            if e.response.status_code < 500:
+                # Refused, not lost: sending it again later changes nothing,
+                # and a buffered refusal would block every item behind it.
+                logger.warning("POST %s refused (%s)", path, e.response.status_code)
+                if e.response.status_code == 404 and path.endswith("/heartbeat") and _says_unregistered(e.response):
+                    self.unregistered = True
+                return None
+            if self._connected:
+                logger.warning("Backend connection lost: %s", e)
+                self._connected = False
+            self._buffer.append({"path": path, "data": data})
+            return None
         except Exception as e:
         except Exception as e:
             if self._connected:
             if self._connected:
                 logger.warning("Backend connection lost: %s", e)
                 logger.warning("Backend connection lost: %s", e)
@@ -54,6 +80,11 @@ class APIClient:
                 resp = await self._client.post(f"{self._base}{item['path']}", json=item["data"])
                 resp = await self._client.post(f"{self._base}{item['path']}", json=item["data"])
                 resp.raise_for_status()
                 resp.raise_for_status()
                 self._buffer.popleft()
                 self._buffer.popleft()
+            except httpx.HTTPStatusError as e:
+                if e.response.status_code >= 500:
+                    break
+                # Refused: dropped, so the items behind it still go out.
+                self._buffer.popleft()
             except Exception:
             except Exception:
                 break
                 break
 
 

+ 40 - 25
spoolbuddy/daemon/main.py

@@ -234,6 +234,37 @@ async def scale_poll_loop(config: Config, api: APIClient, shared: dict):
         scale.close()
         scale.close()
 
 
 
 
+async def _register(config: Config, api: APIClient, nfc, scale, display, ip: str) -> None:
+    """Register with the backend (retries until it answers) and take its
+    calibration. At startup, and again when the device was unregistered."""
+    reg = await api.register_device(
+        device_id=config.device_id,
+        hostname=config.hostname,
+        ip_address=ip,
+        firmware_version=__version__,
+        has_nfc=True,
+        has_scale=True,
+        tare_offset=config.tare_offset,
+        calibration_factor=config.calibration_factor,
+        nfc_reader_type=nfc.reader_type if nfc else None,
+        nfc_connection=nfc.connection if nfc else None,
+        backend_url=config.backend_url,
+        has_backlight=display.has_backlight,
+    )
+
+    # Use server-side calibration if available
+    if reg:
+        config.tare_offset = reg.get("tare_offset", config.tare_offset)
+        config.calibration_factor = reg.get("calibration_factor", config.calibration_factor)
+        if scale:
+            scale.update_calibration(config.tare_offset, config.calibration_factor)
+
+        # Auto-deploy Bambuddy's SSH public key for remote updates
+        ssh_key = reg.get("ssh_public_key")
+        if ssh_key:
+            _deploy_ssh_key(ssh_key)
+
+
 async def heartbeat_loop(config: Config, api: APIClient, start_time: float, shared: dict):
 async def heartbeat_loop(config: Config, api: APIClient, start_time: float, shared: dict):
     """Periodic heartbeat to keep device registered and pick up commands."""
     """Periodic heartbeat to keep device registered and pick up commands."""
     display: DisplayControl = shared["display"]
     display: DisplayControl = shared["display"]
@@ -259,6 +290,14 @@ async def heartbeat_loop(config: Config, api: APIClient, start_time: float, shar
             system_stats=stats,
             system_stats=stats,
         )
         )
 
 
+        if api.unregistered is True:
+            # An admin removed the device while it kept running: register again
+            # so it reappears (the route's documented behaviour).
+            logger.info("Backend no longer knows this device, registering again")
+            api.unregistered = False
+            await _register(config, api, nfc, scale, display, ip)
+            continue
+
         if result:
         if result:
             ssh_key = result.get("ssh_public_key")
             ssh_key = result.get("ssh_public_key")
             if ssh_key:
             if ssh_key:
@@ -441,31 +480,7 @@ async def main():
     display = DisplayControl()
     display = DisplayControl()
 
 
     # Register with backend (retries until success)
     # Register with backend (retries until success)
-    reg = await api.register_device(
-        device_id=config.device_id,
-        hostname=config.hostname,
-        ip_address=ip,
-        firmware_version=__version__,
-        has_nfc=True,
-        has_scale=True,
-        tare_offset=config.tare_offset,
-        calibration_factor=config.calibration_factor,
-        nfc_reader_type=nfc.reader_type,
-        nfc_connection=nfc.connection,
-        backend_url=config.backend_url,
-        has_backlight=display.has_backlight,
-    )
-
-    # Use server-side calibration if available
-    if reg:
-        config.tare_offset = reg.get("tare_offset", config.tare_offset)
-        config.calibration_factor = reg.get("calibration_factor", config.calibration_factor)
-        scale.update_calibration(config.tare_offset, config.calibration_factor)
-
-        # Auto-deploy Bambuddy's SSH public key for remote updates
-        ssh_key = reg.get("ssh_public_key")
-        if ssh_key:
-            _deploy_ssh_key(ssh_key)
+    await _register(config, api, nfc, scale, display, ip)
 
 
     logger.info("Device registered, starting poll loops")
     logger.info("Device registered, starting poll loops")
 
 

+ 10 - 1
spoolbuddy/install/install.sh

@@ -759,6 +759,15 @@ create_bambuddy_directories() {
 create_bambuddy_service() {
 create_bambuddy_service() {
     info "Creating Bambuddy systemd service..."
     info "Creating Bambuddy systemd service..."
 
 
+    # ProtectHome=true hides /home/* from the service, which breaks ExecStart
+    # when INSTALL_PATH lives under /home (issue #1685, as in install/install.sh).
+    # Read-only there keeps the venv binary resolvable; ReadWritePaths below
+    # re-grants writes for the install's data and log dirs.
+    local protect_home="true"
+    if [[ "$INSTALL_PATH" == /home/* ]]; then
+        protect_home="read-only"
+    fi
+
     # Overwriting the unit used to silently drop any ReadWritePaths the operator
     # Overwriting the unit used to silently drop any ReadWritePaths the operator
     # had added — a NAS share for Scheduled Backups, typically — after which the
     # had added — a NAS share for Scheduled Backups, typically — after which the
     # backups failed with EROFS, which looks like a permission problem and is not
     # backups failed with EROFS, which looks like a permission problem and is not
@@ -821,7 +830,7 @@ AmbientCapabilities=CAP_NET_BIND_SERVICE
 NoNewPrivileges=true
 NoNewPrivileges=true
 PrivateTmp=true
 PrivateTmp=true
 ProtectSystem=strict
 ProtectSystem=strict
-ProtectHome=true
+ProtectHome=$protect_home
 # ProtectSystem=strict makes every path outside the ones below read-only for this
 # ProtectSystem=strict makes every path outside the ones below read-only for this
 # service. To back up to a NAS share, add it here or in a drop-in that survives a
 # service. To back up to a NAS share, add it here or in a drop-in that survives a
 # reinstall: sudo systemctl edit bambuddy → [Service] → ReadWritePaths=/mnt/share
 # reinstall: sudo systemctl edit bambuddy → [Service] → ReadWritePaths=/mnt/share

+ 56 - 0
spoolbuddy/tests/test_api_client.py

@@ -221,3 +221,59 @@ class TestReportUpdateStatus:
         result = await api.report_update_status("dev-1", "error", "oops")
         result = await api.report_update_status("dev-1", "error", "oops")
 
 
         assert result is None
         assert result is None
+
+
+def _status_error(code: int, detail: str | None = None) -> httpx.HTTPStatusError:
+    request = httpx.Request("POST", "http://localhost:5000/x")
+    body = {"json": {"detail": detail}} if detail is not None else {}
+    return httpx.HTTPStatusError("rejected", request=request, response=httpx.Response(code, request=request, **body))
+
+
+class TestRejectedRequests:
+    """A request the backend refuses is not a connection problem: buffering it
+    replays it forever, and the flush stops at the first item that fails."""
+
+    @pytest.mark.asyncio
+    async def test_a_heartbeat_for_an_unregistered_device_is_not_buffered(self, api):
+        resp = MagicMock()
+        resp.raise_for_status = MagicMock(side_effect=_status_error(404, "Device not registered"))
+        api._client.post = AsyncMock(return_value=resp)
+
+        result = await api.heartbeat(device_id="dev-1", nfc_ok=True, scale_ok=True, uptime_s=1)
+
+        assert result is None
+        assert len(api._buffer) == 0
+        assert api.unregistered is True
+
+    @pytest.mark.asyncio
+    async def test_a_proxys_bare_404_is_not_taken_for_an_unregistered_device(self, api):
+        """A reverse proxy answers 404 while Bambuddy restarts."""
+        resp = MagicMock()
+        resp.raise_for_status = MagicMock(side_effect=_status_error(404))
+        api._client.post = AsyncMock(return_value=resp)
+
+        await api.heartbeat(device_id="dev-1", nfc_ok=True, scale_ok=True, uptime_s=1)
+
+        assert api.unregistered is False
+
+    @pytest.mark.asyncio
+    async def test_a_connection_error_is_still_buffered(self, api):
+        api._client.post = AsyncMock(side_effect=httpx.ConnectError("refused"))
+
+        await api.heartbeat(device_id="dev-1", nfc_ok=True, scale_ok=True, uptime_s=1)
+
+        assert len(api._buffer) == 1
+        assert api.unregistered is False
+
+    @pytest.mark.asyncio
+    async def test_a_rejected_buffered_item_does_not_block_the_rest(self, api):
+        api._buffer.append({"path": "/devices/dev-1/heartbeat", "data": {}})
+        api._buffer.append({"path": "/scale/reading", "data": {"w": 1}})
+        rejected = MagicMock(raise_for_status=MagicMock(side_effect=_status_error(404)))
+        accepted = MagicMock(raise_for_status=MagicMock())
+        api._client.post = AsyncMock(side_effect=[rejected, accepted])
+
+        await api._flush_buffer()
+
+        assert len(api._buffer) == 0
+        assert api._client.post.await_count == 2

+ 33 - 0
spoolbuddy/tests/test_main.py

@@ -320,3 +320,36 @@ class TestScalePollLoopWakeGating:
         # would still wake (delta 120 > 50), so this asserts both gating AND
         # would still wake (delta 120 > 50), so this asserts both gating AND
         # the absence of poisoning.
         # the absence of poisoning.
         assert display.wake.call_count == 2
         assert display.wake.call_count == 2
+
+
+class TestReRegistration:
+    """An admin removed the device while the daemon kept running: the next
+    heartbeat is refused, and the daemon registers again (#sb-1)."""
+
+    @pytest.mark.asyncio
+    async def test_a_refused_heartbeat_registers_the_device_again(self):
+        config = _make_config()
+        api = _make_api()
+        api.unregistered = False
+        api.register_device = AsyncMock(return_value={"tare_offset": 5, "calibration_factor": 2.0})
+
+        async def mock_heartbeat(*args, **kwargs):
+            api.unregistered = True
+            return None
+
+        api.heartbeat = mock_heartbeat
+        display = MagicMock()
+        display.has_backlight = False
+        shared = {"nfc": None, "scale": None, "display": display}
+
+        task = asyncio.create_task(heartbeat_loop(config, api, time.monotonic(), shared))
+        await asyncio.sleep(0.05)
+        task.cancel()
+        try:
+            await task
+        except asyncio.CancelledError:
+            pass
+
+        api.register_device.assert_awaited()
+        assert api.register_device.await_args.kwargs["device_id"] == "dev-1"
+        assert config.tare_offset == 5