Compare commits

..
Author SHA1 Message Date
ChuckBuildsandClaude Opus 5 b7a7e26bfe fix(backup): stop a restore repointing the device at another panel
restore_backup copied the backup's config.json over the local one wholesale,
display.hardware included. That block is not configuration in the portable
sense -- it describes the panel physically wired to this machine: cols, rows,
chain_length, hardware_mapping, panel_type, multiplexing, and the refresh-rate
cap.

So restoring a backup taken on a 512x64 rig onto a 128x32 one repointed the
smaller panel at the larger one's geometry. Nothing on screen explains that;
the display simply stops being right, and the setting that broke it is one the
user never touched.

display.hardware is now held back by default and the rest of config.json is
restored as before. RestoreOptions.restore_hardware opts into the old
behaviour for the case it actually suits -- restoring onto identical hardware,
or onto a replacement for the machine the backup came from. When the two
differ, the kept geometry and the discarded one are both logged, so the choice
is visible afterwards.

A device with no local display.hardware takes the backup's, since there is
nothing to preserve. An unparseable file on either side falls back to the
plain copy: a restore must not fail because of this merge.

Reverting the guard fails the test that the local panel survives. 40 backup
and restore tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
2026-08-20 22:08:26 -04:00
6 changed files with 146 additions and 43 deletions
Binary file not shown.

Before

Width:  |  Height:  |  Size: 76 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 128 KiB

+63 -1
View File
@@ -111,6 +111,12 @@ class RestoreOptions:
"""Which sections of a backup should be restored.""" """Which sections of a backup should be restored."""
restore_config: bool = True restore_config: bool = True
#: Whether to take the backup's display.hardware block as well.
#: Off by default: that block describes the panel physically wired to
#: *this* device -- its size, chain length, mapping, multiplexing and
#: refresh cap. A backup carries the panel of the machine it was taken
#: on, and restoring one onto a different rig drives the wrong geometry.
restore_hardware: bool = False
restore_secrets: bool = True restore_secrets: bool = True
restore_wifi: bool = True restore_wifi: bool = True
restore_fonts: bool = True restore_fonts: bool = True
@@ -549,6 +555,60 @@ def _copy_file(src: Path, dst: Path) -> None:
raise raise
_HARDWARE_PATH = ("display", "hardware")
def _restore_config_preserving_hardware(src: Path, dst: Path, keep_hardware: bool) -> None:
"""Copy a backed-up config.json, optionally keeping the local panel block.
display.hardware describes the panel physically attached to this device:
cols, rows, chain_length, hardware_mapping, panel_type, multiplexing and
the refresh-rate cap. None of that travels with a configuration -- it is a
property of the machine. Restoring a backup taken on a 512x64 rig onto a
128x32 one used to overwrite the smaller panel's geometry with the larger
one's, which is not a setting the user can see going wrong; the display
simply stops being right.
Falls back to a plain copy when either file cannot be parsed, so a restore
never fails because of this.
"""
if not keep_hardware:
_copy_file(src, dst)
return
try:
incoming = json.loads(src.read_text(encoding="utf-8"))
local = json.loads(dst.read_text(encoding="utf-8")) if dst.exists() else {}
except (OSError, ValueError) as exc:
logger.warning(
"[Backup] Could not merge local panel config (%s); restoring the "
"backup's config.json as-is", exc)
_copy_file(src, dst)
return
section, key = _HARDWARE_PATH
local_hw = (local.get(section) or {}).get(key)
if not isinstance(local_hw, dict) or not local_hw:
_copy_file(src, dst)
return
if not isinstance(incoming.get(section), dict):
incoming[section] = {}
incoming_hw = incoming[section].get(key)
incoming[section][key] = local_hw
if isinstance(incoming_hw, dict) and incoming_hw != local_hw:
logger.info(
"[Backup] Kept this device's display.hardware; the backup's panel "
"was %sx%s chain %s, this one is %sx%s chain %s",
incoming_hw.get("cols"), incoming_hw.get("rows"),
incoming_hw.get("chain_length"),
local_hw.get("cols"), local_hw.get("rows"),
local_hw.get("chain_length"))
tmp_path = dst.with_suffix(dst.suffix + ".restore-tmp")
tmp_path.write_text(json.dumps(incoming, indent=2) + "\n", encoding="utf-8")
os.replace(tmp_path, dst)
def restore_backup( def restore_backup(
zip_path: Path, zip_path: Path,
project_root: Path, project_root: Path,
@@ -584,7 +644,9 @@ def restore_backup(
# Main config. # Main config.
if options.restore_config and (tmp_dir / _CONFIG_REL).exists(): if options.restore_config and (tmp_dir / _CONFIG_REL).exists():
try: try:
_copy_file(tmp_dir / _CONFIG_REL, project_root / _CONFIG_REL) _restore_config_preserving_hardware(
tmp_dir / _CONFIG_REL, project_root / _CONFIG_REL,
keep_hardware=not options.restore_hardware)
result.restored.append("config") result.restored.append("config")
except OSError as e: except OSError as e:
logger.error("[Backup] Failed to restore config.json: %s", e, exc_info=True) logger.error("[Backup] Failed to restore config.json: %s", e, exc_info=True)
+1 -5
View File
@@ -328,11 +328,7 @@ class ScrollHelper:
elapsed_time = current_time - (self.scroll_start_time or current_time) elapsed_time = current_time - (self.scroll_start_time or current_time)
# The image already includes display_width padding, so we only need total_scroll_width # The image already includes display_width padding, so we only need total_scroll_width
required_total_distance = self.total_scroll_width required_total_distance = self.total_scroll_width
# Progress telemetry, emitted every few seconds for the whole of self.logger.info(
# every scroll. It says how far along a marquee is, which is what
# you turn debug on to watch and not something an operator needs
# in the journal on a device that scrolls all day.
self.logger.debug(
"Scroll progress: elapsed=%.2fs, target=%.2fs, total_scrolled=%.0f/%d px (%.1f%%)", "Scroll progress: elapsed=%.2fs, target=%.2fs, total_scrolled=%.0f/%d px (%.1f%%)",
elapsed_time, elapsed_time,
self.calculated_duration, self.calculated_duration,
+3 -33
View File
@@ -31,14 +31,6 @@ if TYPE_CHECKING:
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
#: A frame rate this close to target is not news; below it is.
_FPS_HEALTHY_FRACTION = 0.9
#: A healthy marquee still reports this often, so silence means stopped
#: rather than fine.
_FPS_HEARTBEAT_INTERVAL = 300.0
def _percentile(ordered: List[float], fraction: float) -> float: def _percentile(ordered: List[float], fraction: float) -> float:
"""Nearest-rank percentile of an already-sorted list. """Nearest-rank percentile of an already-sorted list.
@@ -403,9 +395,7 @@ class VegasModeCoordinator:
duration = self.render_pipeline.get_dynamic_duration() duration = self.render_pipeline.get_dynamic_duration()
start_time = time.time() start_time = time.time()
frame_count = 0 frame_count = 0
fps_log_interval = 5.0 # Sample FPS every 5 seconds fps_log_interval = 5.0 # Log FPS every 5 seconds
last_fps_health_log = 0.0 # last INFO-level report
was_degraded = False # so the recovery is reported too
last_fps_log_time = start_time last_fps_log_time = start_time
fps_frame_count = 0 fps_frame_count = 0
# A mean hides stutter completely. At 120fps a five-second window is # A mean hides stutter completely. At 120fps a five-second window is
@@ -458,36 +448,16 @@ class VegasModeCoordinator:
frame_count += 1 frame_count += 1
fps_frame_count += 1 fps_frame_count += 1
# Periodic FPS logging. Reported at INFO only when the frame rate # Periodic FPS logging
# is actually worth an operator's attention -- a shortfall against
# target, or the recovery from one -- with a slow heartbeat so a
# healthy marquee still shows a pulse.
#
# Measured over two hours on a running rig: 1410 samples, 98.5%
# of them within 10% of target. The 1.5% that were not included a
# reading of 8.6fps against a target of 60 -- a real stall, and
# completely invisible inside 1389 lines reading "59.6".
current_time = time.time() current_time = time.time()
if current_time - last_fps_log_time >= fps_log_interval: if current_time - last_fps_log_time >= fps_log_interval:
fps = fps_frame_count / (current_time - last_fps_log_time) fps = fps_frame_count / (current_time - last_fps_log_time)
p99 = _percentile(sorted(frame_times), 0.99) p99 = _percentile(sorted(frame_times), 0.99)
target = self.vegas_config.target_fps
degraded = target > 0 and fps < target * _FPS_HEALTHY_FRACTION
due = current_time - last_fps_health_log >= _FPS_HEARTBEAT_INTERVAL
if degraded or was_degraded or due:
logger.info( logger.info(
"Vegas FPS: %.1f (target: %d, frames: %d) p99 %.1fms worst %.1fms", "Vegas FPS: %.1f (target: %d, frames: %d) p99 %.1fms worst %.1fms",
fps, target, fps_frame_count, fps, self.vegas_config.target_fps, fps_frame_count,
p99 * 1000.0, frame_worst * 1000.0 p99 * 1000.0, frame_worst * 1000.0
) )
last_fps_health_log = current_time
else:
logger.debug(
"Vegas FPS: %.1f (target: %d, frames: %d) p99 %.1fms worst %.1fms",
fps, target, fps_frame_count,
p99 * 1000.0, frame_worst * 1000.0
)
was_degraded = degraded
last_fps_log_time = current_time last_fps_log_time = current_time
fps_frame_count = 0 fps_frame_count = 0
frame_worst = 0.0 frame_worst = 0.0
+75
View File
@@ -0,0 +1,75 @@
#!/usr/bin/env python3
"""A restore must not repoint this device at another machine's panel.
display.hardware describes the panel physically wired to this device -- cols,
rows, chain_length, hardware_mapping, panel_type, multiplexing, the refresh
cap. A backup carries the panel of the machine it was taken on. Restoring a
512x64 rig's backup onto a 128x32 one used to overwrite the smaller panel's
geometry with the larger one's, and nothing on screen explains why: the
display just stops being right.
That is not hypothetical. It happened, and the rig it happened to had to be
reflashed.
"""
import json
import sys
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
from src.backup_manager import _restore_config_preserving_hardware # noqa: E402
BIG = {"display": {"hardware": {"cols": 128, "rows": 64, "chain_length": 4,
"hardware_mapping": "adafruit-hat-pwm"},
"runtime": {"gpio_slowdown": 4}},
"timezone": "America/New_York", "some-plugin": {"enabled": True}}
SMALL = {"display": {"hardware": {"cols": 64, "rows": 32, "chain_length": 2,
"hardware_mapping": "regular"},
"runtime": {"gpio_slowdown": 2}},
"timezone": "UTC"}
def _run(tmp, keep):
src = tmp / "backup_config.json"; src.write_text(json.dumps(BIG))
dst = tmp / "config.json"; dst.write_text(json.dumps(SMALL))
_restore_config_preserving_hardware(src, dst, keep_hardware=keep)
return json.loads(dst.read_text())
def test_local_panel_survives(tmp_path):
out = _run(tmp_path, keep=True)
hw = out["display"]["hardware"]
assert (hw["cols"], hw["rows"], hw["chain_length"]) == (64, 32, 2), (
"the restore repointed this device at the backup's panel")
assert hw["hardware_mapping"] == "regular", "panel wiring came from the backup"
def test_everything_else_is_restored(tmp_path):
out = _run(tmp_path, keep=True)
assert out["timezone"] == "America/New_York", "config was not restored"
assert out["some-plugin"] == {"enabled": True}, "plugin config was not restored"
assert out["display"]["runtime"] == {"gpio_slowdown": 4}, (
"only display.hardware should be held back")
def test_opting_in_takes_the_backups_panel(tmp_path):
out = _run(tmp_path, keep=False)
hw = out["display"]["hardware"]
assert (hw["cols"], hw["rows"], hw["chain_length"]) == (128, 64, 4)
def test_a_device_with_no_local_hardware_takes_the_backups(tmp_path):
src = tmp_path / "b.json"; src.write_text(json.dumps(BIG))
dst = tmp_path / "c.json"; dst.write_text(json.dumps({"timezone": "UTC"}))
_restore_config_preserving_hardware(src, dst, keep_hardware=True)
out = json.loads(dst.read_text())
assert out["display"]["hardware"]["cols"] == 128, (
"nothing local to preserve, so the backup's panel should be used")
def test_unparseable_local_config_still_restores(tmp_path):
src = tmp_path / "b.json"; src.write_text(json.dumps(BIG))
dst = tmp_path / "c.json"; dst.write_text("{ not json")
_restore_config_preserving_hardware(src, dst, keep_hardware=True)
assert json.loads(dst.read_text())["timezone"] == "America/New_York", (
"a restore must never fail because of this merge")