Sfoglia il codice sorgente

fix(spoolbuddy): apply screen-blank timeout changes live without kiosk restart

  Picking a new "Screen Blank Timeout" in SpoolBuddy Settings → Display
  didn't change actual blanking behaviour: whatever value was active when
  the kiosk last booted continued to fire. A user who started with the
  10m preset and switched to 1m or "Off" still saw the screen blank at 10m.

  Cause: blanking is driven by swayidle, started once by spoolbuddy-idle.sh
  at labwc autostart with the timeout passed as a command-line argument.
  The script fetched blank_timeout from the backend exactly once at startup
  and swayidle has no runtime control surface for changing its timeout.
  The daemon's display.set_blank_timeout() updated an in-memory variable
  that was never reached by swayidle, so UI changes were silently discarded
  until the next kiosk restart.

  Fix: extend the wake FIFO protocol with a "reload-timeout N" message.
  The daemon writes it whenever set_blank_timeout() sees a value that
  differs from the current one (the very first call is suppressed because
  the watchdog already fetched the same value at its own startup —
  signalling there would just thrash swayidle on every cold boot). The
  script's FIFO loop is restructured around start_swayidle / stop_swayidle
  helpers and a case statement: wake → wlopm --on + arm a re-blank at the
  current timeout; reload-timeout N → kill running swayidle, set TIMEOUT=N,
  restart swayidle, wlopm --on so the user sees the change took effect
  even if the screen was already blanked. The script de-dupes too — a
  reload-timeout whose N matches the current value is a no-op. Going from
  any positive timeout to 0 ("Off") stops swayidle and doesn't restart
  it; going from 0 to a positive value starts a fresh swayidle. Both work
  without a kiosk restart.

  The script's main loop opens the FIFO read+write (exec 3<>"$WAKE_FIFO")
  so bash read never sees EOF when the daemon momentarily disconnects
  between writes. A cleanup trap on TERM/INT/HUP stops swayidle, removes
  the FIFO, and exits cleanly.
maziggy 3 settimane fa
parent
commit
18238cfeb2

File diff suppressed because it is too large
+ 0 - 0
CHANGELOG.md


+ 40 - 8
spoolbuddy/daemon/display_control.py

@@ -6,7 +6,11 @@ Blanking:   swayidle is the sole authority on screen blanking (idle timeout →
             wlopm --off, touch → wlopm --on).  The daemon wakes the display by
             writing to a FIFO that the idle watchdog monitors — the watchdog
             runs inside the Wayland session and calls wlopm --on on behalf of
-            the daemon.
+            the daemon.  The same FIFO carries "reload-timeout N" lines: when
+            the user changes the blank-timeout setting in the UI, the daemon
+            picks up the new value over the heartbeat and signals the watchdog
+            to kill+restart swayidle with the new timeout, so changes take
+            effect live without a kiosk restart.
 """
 
 import logging
@@ -26,6 +30,7 @@ class DisplayControl:
         self._backlight_path = self._find_backlight()
         self._max_brightness = self._read_max_brightness()
         self._blank_timeout = 0  # seconds, 0 = disabled
+        self._timeout_initialized = False
         self._last_activity = time.monotonic()
         self._blanked = False
 
@@ -73,8 +78,24 @@ class DisplayControl:
             logger.warning("Failed to set brightness: %s", e)
 
     def set_blank_timeout(self, seconds: int):
-        """Set screen blank timeout in seconds. 0 = disabled."""
-        self._blank_timeout = max(0, seconds)
+        """Set screen blank timeout in seconds. 0 = disabled.
+
+        On every change after the first call, signals the idle watchdog
+        (spoolbuddy-idle.sh) to restart swayidle with the new value.
+        Without this, swayidle keeps running with whatever timeout it
+        was started with at autostart and UI changes only take effect
+        after a kiosk restart.
+
+        The first call (during daemon startup) is suppressed because the
+        watchdog already fetched the same value from the backend at its
+        own startup; signalling here would just thrash swayidle.
+        """
+        new_timeout = max(0, seconds)
+        changed = new_timeout != self._blank_timeout
+        self._blank_timeout = new_timeout
+        if changed and self._timeout_initialized:
+            self._signal_reload_timeout(new_timeout)
+        self._timeout_initialized = True
 
     def wake(self):
         """Wake screen on activity (NFC tag, scale weight change).
@@ -100,20 +121,31 @@ class DisplayControl:
 
     def _signal_wake(self) -> None:
         """Write to the wake FIFO to request display power-on."""
+        if self._write_fifo(b"wake\n"):
+            logger.info("Wake signal sent via FIFO")
+
+    def _signal_reload_timeout(self, seconds: int) -> None:
+        """Tell the idle watchdog to apply a new timeout to swayidle."""
+        if self._write_fifo(f"reload-timeout {seconds}\n".encode()):
+            logger.info("Reload-timeout signal sent (timeout=%ds)", seconds)
+
+    def _write_fifo(self, payload: bytes) -> bool:
+        """Best-effort write to the wake FIFO. Returns True on success."""
         if not WAKE_FIFO.exists():
-            return
+            return False
         try:
             # Verify it's actually a FIFO, not a regular file
             if not stat.S_ISFIFO(WAKE_FIFO.stat().st_mode):
-                return
+                return False
             # Open non-blocking so we don't hang if no reader is attached
             fd = os.open(str(WAKE_FIFO), os.O_WRONLY | os.O_NONBLOCK)
             try:
-                os.write(fd, b"wake\n")
-                logger.info("Wake signal sent via FIFO")
+                os.write(fd, payload)
+                return True
             finally:
                 os.close(fd)
         except OSError as e:
             # ENXIO = no reader on the FIFO (idle script not running) — expected
             if e.errno != 6:  # ENXIO
-                logger.debug("Wake FIFO write failed: %s", e)
+                logger.debug("FIFO write failed: %s", e)
+            return False

+ 90 - 36
spoolbuddy/install/spoolbuddy-idle.sh

@@ -4,8 +4,13 @@
 # Powers the HDMI output off via wlopm after the configured inactivity
 # timeout, driven by swayidle inside the labwc Wayland session. The timeout
 # value is fetched once from the Bambuddy backend on startup so it matches
-# whatever the user picked in SpoolBuddy Settings → Display. Changes made
-# in the UI take effect on the next reboot / kiosk restart.
+# whatever the user picked in SpoolBuddy Settings → Display.
+#
+# Changes made in the UI are applied live: the daemon writes a
+# "reload-timeout N" line to /tmp/spoolbuddy-wake whenever it sees a new
+# value over the heartbeat, and the FIFO loop below kills the current
+# swayidle and starts a fresh one with the new timeout. No kiosk restart
+# is required.
 #
 # Runs in labwc's autostart file as the kiosk user — needs access to
 # WAYLAND_DISPLAY, which it inherits from the parent labwc process.
@@ -85,45 +90,94 @@ if [ -n "$BACKEND_URL" ] && [ -n "$API_KEY" ] && [ -n "$DEVICE_ID" ]; then
     fi
 fi
 
-# FIFO for the SpoolBuddy daemon to request display wake from outside the
-# Wayland session (NFC tag scan, scale weight change).  The daemon writes
-# "wake\n" to this pipe; the monitor loop below calls wlopm --on.
+# FIFO for the SpoolBuddy daemon to talk to this watchdog from outside the
+# Wayland session.  Two messages are understood:
+#   wake               — turn the display on (NFC tag scan, scale weight change)
+#   reload-timeout N   — kill swayidle and restart it with timeout=N
 WAKE_FIFO="/tmp/spoolbuddy-wake"
 rm -f "$WAKE_FIFO"
 mkfifo -m 622 "$WAKE_FIFO"
 echo "wake FIFO created at $WAKE_FIFO"
 
-if [ "$TIMEOUT" -le 0 ]; then
-    # Blanking explicitly disabled — just monitor the wake FIFO so NFC/scale
-    # wake still works even without swayidle.
-    echo "timeout<=0, monitoring wake FIFO only (no swayidle)"
-    while read -r _ < "$WAKE_FIFO"; do
-        wlopm --on "$OUTPUT" 2>/dev/null || true
-    done
+SWAYIDLE_PID=""
+REBLANK_PID=""
+
+start_swayidle() {
+    [ "$TIMEOUT" -gt 0 ] || return 0
+    swayidle -w \
+        timeout "$TIMEOUT" "wlopm --off $OUTPUT" \
+        resume "wlopm --on $OUTPUT" &
+    SWAYIDLE_PID=$!
+    echo "swayidle started (pid=$SWAYIDLE_PID, timeout=$TIMEOUT, output=$OUTPUT)"
+}
+
+stop_swayidle() {
+    if [ -n "$SWAYIDLE_PID" ]; then
+        kill "$SWAYIDLE_PID" 2>/dev/null || true
+        wait "$SWAYIDLE_PID" 2>/dev/null || true
+        SWAYIDLE_PID=""
+    fi
+    if [ -n "$REBLANK_PID" ]; then
+        kill "$REBLANK_PID" 2>/dev/null || true
+        REBLANK_PID=""
+    fi
+}
+
+cleanup() {
+    stop_swayidle
+    rm -f "$WAKE_FIFO"
     exit 0
-fi
+}
+trap cleanup TERM INT HUP
 
-echo "starting swayidle with timeout=$TIMEOUT output=$OUTPUT"
-swayidle -w \
-    timeout "$TIMEOUT" "wlopm --off $OUTPUT" \
-    resume "wlopm --on $OUTPUT" &
-SWAYIDLE_PID=$!
+start_swayidle
 
-# Monitor wake FIFO — when the daemon writes to it, turn the display on
-# and schedule a re-blank after TIMEOUT seconds (swayidle doesn't know about
-# FIFO wakes so it won't re-blank on its own).
-REBLANK_PID=""
-while read -r _ < "$WAKE_FIFO"; do
-    wlopm --on "$OUTPUT" 2>/dev/null || true
-    # Cancel any pending re-blank timer, then start a new one
-    [ -n "$REBLANK_PID" ] && kill "$REBLANK_PID" 2>/dev/null || true
-    (sleep "$TIMEOUT" && wlopm --off "$OUTPUT" 2>/dev/null) &
-    REBLANK_PID=$!
-done &
-FIFO_PID=$!
-
-# If either process exits, clean up and exit.
-wait -n "$SWAYIDLE_PID" "$FIFO_PID" 2>/dev/null
-echo "child exited, cleaning up"
-kill "$SWAYIDLE_PID" "$FIFO_PID" 2>/dev/null || true
-rm -f "$WAKE_FIFO"
+# Open the FIFO read+write so EOF never arrives even when the daemon
+# (the writer) momentarily disconnects between messages — without this,
+# `read` would return immediately the first time the daemon closes its
+# write end and the loop would spin.
+exec 3<>"$WAKE_FIFO"
+
+while IFS= read -r line <&3; do
+    case "$line" in
+        wake)
+            wlopm --on "$OUTPUT" 2>/dev/null || true
+            # Cancel any pending re-blank timer, then start a new one
+            # at the *current* timeout (swayidle doesn't know about
+            # FIFO wakes so it won't re-blank on its own).
+            [ -n "$REBLANK_PID" ] && kill "$REBLANK_PID" 2>/dev/null || true
+            REBLANK_PID=""
+            if [ "$TIMEOUT" -gt 0 ]; then
+                (sleep "$TIMEOUT" && wlopm --off "$OUTPUT" 2>/dev/null) &
+                REBLANK_PID=$!
+            fi
+            ;;
+        reload-timeout\ *)
+            new_timeout="${line#reload-timeout }"
+            # Validate: must be a non-negative integer.
+            if [ "$new_timeout" -eq "$new_timeout" ] 2>/dev/null && [ "$new_timeout" -ge 0 ]; then
+                if [ "$new_timeout" != "$TIMEOUT" ]; then
+                    echo "reload-timeout: $TIMEOUT -> $new_timeout"
+                    stop_swayidle
+                    TIMEOUT="$new_timeout"
+                    start_swayidle
+                    # Bring the display back on so the user sees the
+                    # change took effect (a setting saved while the
+                    # screen was already blanked would otherwise look
+                    # ignored until the next touch).
+                    wlopm --on "$OUTPUT" 2>/dev/null || true
+                fi
+            else
+                echo "ignoring invalid reload-timeout payload: $new_timeout"
+            fi
+            ;;
+        '')
+            : # ignore empty lines (e.g. opening the FIFO with no payload)
+            ;;
+        *)
+            echo "unknown FIFO message: $line"
+            ;;
+    esac
+done
+
+cleanup

+ 93 - 0
spoolbuddy/tests/test_display_control.py

@@ -1,5 +1,6 @@
 """Tests for daemon.display_control — DisplayControl brightness and blanking."""
 
+import os
 import time
 
 import pytest
@@ -174,3 +175,95 @@ class TestDisplayControlBlanking:
         time.sleep(0.01)
         display.wake()
         assert display._last_activity > old_time
+
+
+class TestDisplayControlFifoMessages:
+    """The wake FIFO carries two messages: `wake` and `reload-timeout N`.
+
+    These tests pin both — they're the only way the daemon can talk to
+    the idle watchdog (spoolbuddy-idle.sh) running in the Wayland session.
+    Regression target: a one-shot swayidle started with a stale timeout
+    value would never pick up UI changes without these signals.
+    """
+
+    @pytest.fixture
+    def display_with_fifo(self, monkeypatch, tmp_path):
+        import daemon.display_control as dc_mod
+
+        empty_dir = tmp_path / "backlight"
+        empty_dir.mkdir()
+        monkeypatch.setattr(dc_mod, "BACKLIGHT_BASE", empty_dir)
+
+        fifo_path = tmp_path / "spoolbuddy-wake"
+        os.mkfifo(str(fifo_path), 0o622)
+        monkeypatch.setattr(dc_mod, "WAKE_FIFO", fifo_path)
+
+        # Hold a non-blocking reader open so the daemon's writes don't hit ENXIO.
+        reader_fd = os.open(str(fifo_path), os.O_RDONLY | os.O_NONBLOCK)
+        try:
+            yield dc_mod.DisplayControl(), reader_fd
+        finally:
+            os.close(reader_fd)
+
+    @staticmethod
+    def _drain(fd: int) -> bytes:
+        """Read whatever is queued on the FIFO without blocking."""
+        try:
+            return os.read(fd, 4096)
+        except BlockingIOError:
+            return b""
+
+    def test_wake_writes_wake_line(self, display_with_fifo):
+        dc, reader_fd = display_with_fifo
+        dc.wake()
+        assert self._drain(reader_fd) == b"wake\n"
+
+    def test_first_set_blank_timeout_does_not_signal(self, display_with_fifo):
+        """The watchdog already fetched this value at its own startup —
+        signalling here would just thrash swayidle for nothing."""
+        dc, reader_fd = display_with_fifo
+        dc.set_blank_timeout(300)
+        assert self._drain(reader_fd) == b""
+        assert dc._blank_timeout == 300
+
+    def test_subsequent_change_signals_reload(self, display_with_fifo):
+        dc, reader_fd = display_with_fifo
+        dc.set_blank_timeout(300)  # init — no signal
+        dc.set_blank_timeout(60)
+        assert self._drain(reader_fd) == b"reload-timeout 60\n"
+
+    def test_same_value_does_not_signal(self, display_with_fifo):
+        dc, reader_fd = display_with_fifo
+        dc.set_blank_timeout(300)
+        dc.set_blank_timeout(300)
+        assert self._drain(reader_fd) == b""
+
+    def test_disable_after_enable_signals_zero(self, display_with_fifo):
+        """Going from "blanking on" to "blanking off" must reach the watchdog
+        so it can stop swayidle — otherwise the screen keeps blanking even
+        after the user picks 'Off'."""
+        dc, reader_fd = display_with_fifo
+        dc.set_blank_timeout(300)  # init
+        dc.set_blank_timeout(0)
+        assert self._drain(reader_fd) == b"reload-timeout 0\n"
+
+    def test_negative_clamped_to_zero_in_signal(self, display_with_fifo):
+        dc, reader_fd = display_with_fifo
+        dc.set_blank_timeout(300)  # init
+        dc.set_blank_timeout(-5)
+        assert self._drain(reader_fd) == b"reload-timeout 0\n"
+
+    def test_signal_no_op_when_fifo_missing(self, monkeypatch, tmp_path):
+        """No watchdog running = no FIFO. Writes must not raise."""
+        import daemon.display_control as dc_mod
+
+        empty_dir = tmp_path / "backlight"
+        empty_dir.mkdir()
+        monkeypatch.setattr(dc_mod, "BACKLIGHT_BASE", empty_dir)
+        monkeypatch.setattr(dc_mod, "WAKE_FIFO", tmp_path / "no-such-fifo")
+
+        dc = dc_mod.DisplayControl()
+        dc.set_blank_timeout(300)
+        dc.set_blank_timeout(60)  # would signal if FIFO existed
+        dc.wake()
+        # No assertion needed — surviving without raising is the contract.

Some files were not shown because too many files changed in this diff