Merge pull request #248 from screentinker/fix/245-pi5-wayland-installer

Pi installer: ask the operator, not the pipe; and stop assuming X11

README.md

@@ -662,13 +662,56 @@ keytool -genkey -v -keystore android/release-key.jks -keyalg RSA -keysize 2048 -
 2. Go to **Displays** and click **Add Display**
 3. Install the ScreenTinker app on your device:
    - **Android TV / tablets**: Download the APK from your instance (`/download/apk`) or build it from source (see above)
-   - **Raspberry Pi**: `curl -sSL https://your-instance/scripts/raspberry-pi-setup.sh | bash`
+   - **Raspberry Pi**: `curl -sSL https://your-instance/scripts/raspberry-pi-setup.sh | sudo bash` (see [Raspberry Pi notes](#raspberry-pi-notes))
    - **Debian 13 (headless)**: `curl -sSL https://your-instance/scripts/debian-13-setup.sh | sudo bash`
    - **Windows**: Run the setup script from `scripts/windows-setup.bat`
    - **Samsung Tizen TV / signage**: point the TV's URL Launcher (or browser) at `https://your-instance/player` - no signing needed. For an installed native app, see [tizen/README.md](tizen/README.md)
    - **Any browser**: Open `https://your-instance/player` in kiosk/fullscreen mode
 4. Enter the pairing code shown on the device
 
+### Raspberry Pi notes
+
+**Run it with `sudo`.** The script installs packages and writes systemd units, so it refuses to
+run otherwise. Piping is fine — prompts are read from your terminal, not from the pipe:
+
+```bash
+curl -sSL https://your-instance/scripts/raspberry-pi-setup.sh | sudo bash
+```
+
+To pick Player-Only without being asked:
+
+```bash
+curl -sSL https://your-instance/scripts/raspberry-pi-setup.sh | sudo bash -s -- --player-only https://your-server
+```
+
+**Pi 5 / Bookworm runs Wayland by default.** The kiosk launcher detects the session and does the
+right thing on either: `xset`/`unclutter` are X11-only and are skipped on Wayland (where they are
+no-ops that log an error and silently do nothing), Chromium is given `--ozone-platform=wayland`,
+and `--password-store=basic` stops it asking for a keyring password no kiosk has anyone to answer.
+
+Blanking and cursor-hiding belong to the compositor on Wayland. The launcher calls `wlopm` when it
+is present; if your image does not ship it, set the equivalent in your compositor's config
+(`~/.config/wayfire.ini` `[idle]` for wayfire, or the labwc equivalent).
+
+**A white page on every boot but the first** was Chromium restoring a session it believed crashed —
+a kiosk is killed by shutdown and never exits cleanly, so it came back with a restore surface on
+top of the player. The launcher now clears the stored session as well as the clean-exit flag.
+
+#### Read-only root (Overlay FS) on a Pi that loses power
+
+Worth enabling for **Player-Only** installs, where the Pi holds no state you cannot recreate: the
+overlay absorbs writes into RAM, so a power cut cannot corrupt the card and the flash does not wear
+out. Re-run the setup script (or `raspi-config` → Performance → Overlay File System) *after* the
+install, and remember that pairing is stored on the device — re-pair once with the overlay
+disabled, then enable it, or the pairing is lost at every reboot.
+
+**Do not enable it on an All-in-One install without moving the data first.** That Pi *is* the
+server: the SQLite database, uploaded media and the JWT secret live under `/opt/screentinker`, and
+an overlay discards every write at reboot — so content you upload and displays you pair vanish on
+the next power cycle. If you want both, put `DATA_DIR` on a writable partition or an external
+drive that is excluded from the overlay, and confirm the database file is genuinely outside it
+before trusting the setup.
+
 On the Android player, the setup screen lists the permissions it wants and lets you revisit any of
 them later — each row stays visible once granted and turns into **Manage**, so you can check or
 revoke what you gave it rather than having the option disappear.

scripts/raspberry-pi-setup.sh

@@ -58,17 +58,51 @@ while [[ $# -gt 0 ]]; do
     esac
 done
 
+# -- Prompting when we are being piped --
+#
+# The documented install is `curl -sL … | sudo bash`, which makes stdin the SCRIPT, not the
+# operator. bash has already consumed it by the time any `read` runs, so every prompt got EOF
+# instantly: the mode menu "chose" All-in-One without the operator touching anything, and the
+# Player-Only branch could never be reached that way at all. It looked like the menu was being
+# skipped, because it was.
+#
+# So prompts read from the controlling terminal instead. When there genuinely is no terminal
+# (cloud-init, a provisioning pipeline), we say so and take the documented default rather than
+# pretending a choice was made — the operator can pass --player-only / --server-url to decide
+# without a prompt.
+if [ -r /dev/tty ] && [ -t 1 ]; then
+    exec 3</dev/tty
+    HAVE_TTY=true
+else
+    HAVE_TTY=false
+fi
+
+# ask <varname> <prompt> [read-args…]
+ask() {
+    local __var="$1"; shift
+    local __prompt="$1"; shift
+    if [ "$HAVE_TTY" = true ]; then
+        read "$@" -u 3 -r -p "$__prompt" "$__var"
+    else
+        eval "$__var=''"
+    fi
+}
+
 # -- Root check --
 if [ "$(id -u)" -ne 0 ]; then
-    err "This script must be run as root. Try: sudo bash raspberry-pi-setup.sh"
+    err "This script must be run as root. Try:  curl -sL https://screentinker.com/scripts/raspberry-pi-setup.sh | sudo bash"
 fi
 
 # -- Architecture check --
 ARCH=$(uname -m)
 if [[ "$ARCH" != "aarch64" && "$ARCH" != "armv7l" ]]; then
     warn "Detected architecture: $ARCH (expected aarch64 or armv7l for Raspberry Pi)"
-    read -p "Continue anyway? (y/N) " -n 1 -r; echo
-    [[ ! $REPLY =~ ^[Yy]$ ]] && exit 1
+    if [ "$HAVE_TTY" = true ]; then
+        ask REPLY "Continue anyway? (y/N) " -n 1; echo
+        [[ ! $REPLY =~ ^[Yy]$ ]] && exit 1
+    else
+        err "Refusing to continue on $ARCH without a terminal to confirm at. Re-run interactively, or on the intended hardware."
+    fi
 fi
 
 # -- Interactive mode selection (if no flags passed) --
@@ -86,14 +120,24 @@ if [ "$PLAYER_ONLY" = false ] && [ -z "$SERVER_URL" ]; then
     echo "     Connects to an existing ScreenTinker server."
     echo "     This Pi just displays content."
     echo ""
-    read -p "Choose [1/2]: " MODE_CHOICE
-    case "$MODE_CHOICE" in
-        2)
-            PLAYER_ONLY=true
-            read -p "Server URL (e.g., https://screentinker.com): " SERVER_URL
-            ;;
-        *) ;;
-    esac
+    if [ "$HAVE_TTY" = false ]; then
+        # No terminal to ask at. Say which way we went, rather than letting an empty answer
+        # look like a decision — this is the exact confusion the piped-stdin bug produced.
+        warn "No terminal available for the menu — defaulting to All-in-One."
+        warn "To choose Player-Only non-interactively:  ... | sudo bash -s -- --player-only https://your-server"
+    else
+        ask MODE_CHOICE "Choose [1/2]: "
+        case "$MODE_CHOICE" in
+            2)
+                PLAYER_ONLY=true
+                while [ -z "$SERVER_URL" ]; do
+                    ask SERVER_URL "Server URL (e.g., https://screentinker.com): "
+                    [ -z "$SERVER_URL" ] && warn "Player-Only needs a server URL."
+                done
+                ;;
+            *) ;;
+        esac
+    fi
 fi
 
 # Strip trailing slash from server URL
@@ -268,22 +312,49 @@ KIOSK_URL="${KIOSK_URL}"
 # Wait for display
 sleep 2
 
-# Disable screen blanking and power management
-xset s off
-xset s noblank
-xset -dpms
-xset s 0 0
+# Which display server are we actually on? Pi 5 on Bookworm defaults to WAYLAND, where every
+# X11 tool below is a no-op that prints an error into the journal and silently does nothing —
+# so a Wayland Pi got no blanking suppression and no cursor hiding while appearing configured.
+SESSION_TYPE="\${XDG_SESSION_TYPE:-}"
+if [ -z "\$SESSION_TYPE" ]; then
+    if [ -n "\${WAYLAND_DISPLAY:-}" ]; then SESSION_TYPE=wayland
+    elif [ -n "\${DISPLAY:-}" ]; then SESSION_TYPE=x11
+    fi
+fi
+echo "Display server: \${SESSION_TYPE:-unknown}"
 
-# Hide cursor after 3 seconds of inactivity
-unclutter -idle 3 -root &
+if [ "\$SESSION_TYPE" = "wayland" ]; then
+    # Blanking/DPMS belong to the compositor here, not to us. wlopm is present on Pi OS
+    # (wlroots-based wayfire/labwc); if it is not, the compositor's own idle config is the
+    # documented fallback and README says so.
+    command -v wlopm >/dev/null 2>&1 && wlopm --on '*' 2>/dev/null || true
+    # unclutter is X11-only. Under wayfire the cursor is hidden by the compositor
+    # (hide_cursor / idle plugin), which the installer writes below when wayfire.ini exists.
+else
+    # Disable screen blanking and power management
+    xset s off
+    xset s noblank
+    xset -dpms
+    xset s 0 0
 
-# Clean Chromium crash flags (prevents restore session dialogs)
+    # Hide cursor after 3 seconds of inactivity (X11 only — no Wayland equivalent)
+    unclutter -idle 3 -root &
+fi
+
+# Clean Chromium crash flags (prevents restore session dialogs).
+#
+# The white page on every boot after the first is Chromium restoring a session it thinks
+# crashed: a kiosk is killed by the shutdown, never exits cleanly, and comes back with a
+# restore surface on top of the player — which is why ALT+F4 "fixed" it (it closed the
+# surface, not the player). Rewriting the flags is not enough on its own because Chromium
+# also replays the previous window set from Sessions/, so those go too.
 CDIR="\$HOME/.config/chromium/Default"
 mkdir -p "\$CDIR"
 if [ -f "\$CDIR/Preferences" ]; then
     sed -i 's/"exited_cleanly":false/"exited_cleanly":true/' "\$CDIR/Preferences" 2>/dev/null || true
     sed -i 's/"exit_type":"Crashed"/"exit_type":"Normal"/' "\$CDIR/Preferences" 2>/dev/null || true
 fi
+rm -rf "\$CDIR/Sessions" "\$CDIR/Session Storage" 2>/dev/null || true
 
 # Wait for local server if running all-in-one
 if echo "\$KIOSK_URL" | grep -q "localhost"; then
@@ -306,8 +377,15 @@ if [ -z "\$SCREEN_W" ] || [ -z "\$SCREEN_H" ]; then
     SCREEN_H=1080
 fi
 
+# Wayland needs the ozone backend named explicitly on some Bookworm builds; on X11 the flag
+# is absent so nothing changes there.
+OZONE=""
+[ "\$SESSION_TYPE" = "wayland" ] && OZONE="--ozone-platform=wayland"
+
 exec ${CHROMIUM_BIN} \\
     --kiosk \\
+    \$OZONE \\
+    --password-store=basic \\
     --window-position=0,0 \\
     --window-size=\${SCREEN_W},\${SCREEN_H} \\
     --noerrdialogs \\
@@ -580,11 +658,11 @@ fi
 # ============================================================
 cat > /etc/motd << 'MOTDEOF'
 
-  ____                        _____          _
- / ___|  ___ _ __ ___  ___  |_   _|_ _ __ | | _____ _ __
- \___ \ / __| '__/ _ \/ _ \   | || | '_ \| |/ / _ \ '__|
-  ___) | (__| | |  __/  __/   | || | | | |   <  __/ |
- |____/ \___|_|  \___|\___|   |_||_|_| |_|_|\_\___|_|
+ ____                                  _____  _         _
+/ ___|   ___  _ __   ___   ___  _ __  |_   _|(_) _ __  | | __  ___  _ __
+\___ \  / __|| '__| / _ \ / _ \| '_ \   | |  | || '_ \ | |/ / / _ \| '__|
+ ___) || (__ | |   |  __/|  __/| | | |  | |  | || | | ||   < |  __/| |
+|____/  \___||_|    \___| \___||_| |_|  |_|  |_||_| |_||_|\_\ \___||_|
 
  Open-Source Digital Signage for Any Screen
 

server/test/raspberry-pi-setup.test.js

@@ -0,0 +1,112 @@
+'use strict';
+
+/*
+ * The Pi installer generates another script (the kiosk launcher) and writes it to disk. Nothing
+ * ever executed either one in CI, so every defect in them was found by a user on real hardware —
+ * which is how #245 arrived: a menu that ignored the operator, a keyring prompt, X11 tools
+ * no-oping on a Wayland Pi, and a banner spelling the product's own name wrong.
+ *
+ * These tests check the two things a repo can check without a Pi: that the generated script is
+ * syntactically valid bash, and that the flags/guards the bug reports turned on are actually
+ * present. `bash -n` on the OUTER script would not have caught any of it — the kiosk script lives
+ * inside a heredoc, where a syntax error is just text until it reaches a screen.
+ */
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('fs');
+const os = require('os');
+const path = require('path');
+const { execFileSync } = require('child_process');
+
+const ROOT = path.join(__dirname, '..', '..');
+const SCRIPT = path.join(ROOT, 'scripts', 'raspberry-pi-setup.sh');
+const SRC = fs.readFileSync(SCRIPT, 'utf8');
+
+// The kiosk launcher as the installer will write it, with the install-time expansions applied.
+function generatedKioskScript() {
+  const m = SRC.match(/cat > "\$PI_HOME\/screentinker-kiosk\.sh" << KIOSKEOF\n([\s\S]*?)\nKIOSKEOF/);
+  assert.ok(m, 'kiosk heredoc not found — did the installer restructure?');
+  return m[1]
+    .replace(/\$\{KIOSK_URL\}/g, 'http://localhost:3001/player')
+    .replace(/\$\{SCREENTINKER_PORT\}/g, '3001')
+    .replace(/\$\{CHROMIUM_BIN\}/g, '/usr/bin/chromium-browser')
+    .replace(/\\\$/g, '$')
+    .replace(/\\\\/g, '\\');
+}
+
+function bashSyntaxOk(text) {
+  const p = path.join(os.tmpdir(), `st-kiosk-${process.pid}-${Math.abs(text.length)}.sh`);
+  fs.writeFileSync(p, text);
+  try {
+    execFileSync('bash', ['-n', p], { stdio: 'pipe' });
+    return true;
+  } catch (e) {
+    throw new Error(`generated script is not valid bash:\n${e.stderr?.toString() || e.message}`);
+  } finally {
+    try { fs.unlinkSync(p); } catch { /* best-effort */ }
+  }
+}
+
+test('#245: the installer itself is valid bash', () => {
+  assert.ok(bashSyntaxOk(SRC));
+});
+
+test('#245: the kiosk launcher it generates is valid bash', () => {
+  assert.ok(bashSyntaxOk(generatedKioskScript()));
+});
+
+test('#245: prompts read the terminal, not the pipe', () => {
+  // `curl … | sudo bash` makes stdin the SCRIPT. bash has consumed it by the time any read
+  // runs, so a plain `read` gets EOF instantly and the menu "chooses" the default without the
+  // operator touching anything — reported as the menu being skipped, because it was.
+  assert.match(SRC, /exec 3<\/dev\/tty/, 'prompts must come from the controlling terminal');
+  // Any prompting `read` that is not the one inside ask() itself (which is the tty read).
+  const reads = SRC.match(/^\s*read (?!.*-u 3).*-p /gm) || [];
+  assert.equal(reads.length, 0, `every prompt must go through ask(); found a raw read: ${reads[0]}`);
+  assert.match(SRC, /read .*-u 3/, 'ask() must read from the tty fd');
+  // And when there is genuinely no terminal, it must SAY which way it went rather than let an
+  // empty answer look like a decision.
+  assert.match(SRC, /No terminal available for the menu/);
+});
+
+test('#245: Chromium is told not to ask for a keyring', () => {
+  // "Choose password for keyring" on every boot: Chromium reaching for gnome-keyring on a
+  // desktop session. A kiosk has nobody to answer it.
+  assert.match(generatedKioskScript(), /--password-store=basic/);
+});
+
+test('#245: X11-only tools are guarded by the session type', () => {
+  // Pi 5 on Bookworm defaults to Wayland, where xset/unclutter/xrandr are no-ops that log an
+  // error and silently do nothing — so the Pi got no blanking suppression and no cursor
+  // hiding while looking configured.
+  const kiosk = generatedKioskScript();
+  assert.match(kiosk, /SESSION_TYPE/, 'the launcher must detect the display server');
+  const x11Block = kiosk.slice(kiosk.indexOf('if [ "$SESSION_TYPE" = "wayland" ]'), kiosk.indexOf('# Clean Chromium crash flags'));
+  assert.ok(x11Block.length > 0, 'session-type branch not found');
+  for (const tool of ['xset', 'unclutter']) {
+    assert.ok(x11Block.includes(tool), `${tool} must live inside the session-type branch`);
+  }
+  assert.match(kiosk, /--ozone-platform=wayland/, 'Wayland needs the ozone backend named');
+});
+
+test('#245: the crash-restore surface is cleared, not just flagged', () => {
+  // The white page on every boot after the first: a kiosk is killed by shutdown, never exits
+  // cleanly, and Chromium returns with a restore surface over the player. ALT+F4 "fixed" it
+  // because it closed that surface, not the player. Rewriting the flags is not enough on its
+  // own — Chromium also replays the previous window set from Sessions/.
+  const kiosk = generatedKioskScript();
+  assert.match(kiosk, /exited_cleanly/, 'the clean-exit flag must be rewritten');
+  assert.match(kiosk, /rm -rf .*Sessions/, 'the stored session must be removed too');
+  assert.match(kiosk, /--disable-session-crashed-bubble/);
+});
+
+test('#245: the login banner spells the product name', () => {
+  // It read "Scree Tinker" — the n was missing from the ASCII art, and it is the first thing
+  // anyone sees over SSH.
+  const motd = SRC.slice(SRC.indexOf("cat > /etc/motd << 'MOTDEOF'"), SRC.indexOf('MOTDEOF\n', SRC.indexOf("cat > /etc/motd") + 30));
+  const lines = motd.split('\n').filter((l) => /[_\\\/|()]/.test(l) && l.trim().length > 20);
+  assert.ok(lines.length >= 5, 'expected the 5-row banner');
+  // Row 4 of figlet "standard" carries the distinguishing strokes: 'n' contributes "| | | |".
+  const banner = lines.join('\n');
+  assert.ok(banner.includes('| | | |'), 'the n glyph is missing from the banner — it reads "Scree Tinker"');
+});