mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-21 02:19:07 +00:00
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
This commit is contained in:
co-authored by
Claude Opus 5
parent
cf0a551f7b
commit
b7a7e26bfe
+63
-1
@@ -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)
|
||||||
|
|||||||
@@ -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")
|
||||||
Reference in New Issue
Block a user