From 26d6943b2d0f3488d9ee47178056234b350ef269 Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Tue, 1 Sep 2026 00:37:05 +0200 Subject: [PATCH] fix: seed --settings into app's own file, not device settings.txt `--settings` was merging the seed file into the device-root settings.txt (width/height/wifi), but most apps keep their own settings in a separate, relative-path file in their own directory (e.g. departures' `settings.txt`, clock's `clocksettings.txt`), so the seed was silently never read. Copy it verbatim into the app's own staged directory under its original filename instead, and warn when nothing in the app's own source references that filename. Also make the web UI's HTTP port ephemeral, like the frame server's, so it doesn't collide with another running `matrixbox` process. --- matrixbox_simulator/device/run_screenshot.py | 72 +++++++++++++++++--- tests/test_screenshot.py | 35 +++++++--- 2 files changed, 90 insertions(+), 17 deletions(-) diff --git a/matrixbox_simulator/device/run_screenshot.py b/matrixbox_simulator/device/run_screenshot.py index 498f002..a5108b7 100644 --- a/matrixbox_simulator/device/run_screenshot.py +++ b/matrixbox_simulator/device/run_screenshot.py @@ -90,14 +90,47 @@ def build_parser( return parser +def _warn_if_settings_filename_looks_unused(app_dir: Path, settings_src: Path) -> None: + # --settings is staged under its own filename (see _stage_for_screenshot), + # so it only ever gets read if the app's own code happens to open that + # exact name — a real, common per-app naming convention (departures + # wants "settings.txt", clock wants "clocksettings.txt", ...) that this + # tool has no way to look up ahead of time. A quick grep across the + # app's own source is a cheap, if imperfect, way to catch the likely + # mistake — an app that never mentions the given filename anywhere is + # not going to read it, no matter what it contains. + name = settings_src.name + for py_file in app_dir.rglob("*.py"): + try: + if name in py_file.read_text(errors="ignore"): + return + except OSError: + continue + + print( + f"matrixbox-simulator: warning: {app_dir.name}'s own code doesn't " + f"appear to reference {name!r} anywhere — it likely reads its " + "settings from a differently-named file (e.g. settings.txt, " + "clocksettings.txt, settings.txt, ...); if so, this seed " + "file has no effect. Check the app's own source for the exact " + "filename it opens, and rename --settings to match.", + file=sys.stderr, + ) + + def _stage_for_screenshot( app_dir: Path, framework_root: Path, settings_src: Path | None, args: argparse.Namespace, ) -> tuple[str, Path]: - """Stages `app_dir` fresh (whichever kernel style it uses) and seeds its - settings.txt, either from `settings_src` or plain defaults. Returns the + """Stages `app_dir` fresh (whichever kernel style it uses). If given, + `settings_src` is copied verbatim into the app's own staged directory + under its original filename — apps keep their own settings file there + (e.g. departures' `settings.txt`, clock's `clocksettings.txt`), a + plain relative-path file read straight off the app's own cwd, distinct + from the device-root /settings.txt this also seeds with plain + width/height/tiles defaults (see run_app._seed_settings). Returns the exec-ready (source, path) for the app's own entry point, ready for `run_app._exec_as_main`. Mirrors run_app's own _run_main_kernel/_run_package_kernel split, minus everything that's @@ -123,9 +156,16 @@ def _stage_monolithic_app_for_screenshot( run_app._install_chdir_path_tracking() run_app._install_lenient_bytes_import_hook(staged_root) - settings_path = staged_root / "settings.txt" if settings_src is not None: - settings_path.write_text(settings_src.read_text()) + # Seeded into the app's own *unflattened* apps/ copy, not + # wherever it ends up at runtime: the kernel only flattens apps/ + # to a top-level sibling when it actually boots one (main.py's + # own initialize_app, not this staging step), copying that app's + # whole directory — extra files included, same as clock's own + # code.py reading a sibling clock.html — so seeding here rides + # along with that copy. + unflattened_app_dir = staged_root / "apps" / app_dir.name + (unflattened_app_dir / settings_src.name).write_text(settings_src.read_text()) run_app._seed_monolithic_settings( staged_root, @@ -159,16 +199,20 @@ def _stage_package_app_for_screenshot( staged_app_dir = run_app._stage_app(app_dir, reset=True) run_app._install_path_sandbox(run_app.SANDBOX_ROOT) - # SANDBOX_ROOT (not staged_app_dir) is where a package-kernel app's + if settings_src is not None: + # The app's own settings file, seeded straight into its staged + # directory under its original filename — a plain relative-path + # file the app reads off its own cwd, distinct from the + # device-root /settings.txt below (width/height/tiles only). + (staged_app_dir / settings_src.name).write_text(settings_src.read_text()) + + # SANDBOX_ROOT (not staged_app_dir) is where the device-root # settings.txt actually lives, matching real hardware's single # flash-root settings file — see run_app._seed_settings. reset=True on # _stage_app above only wipes this app's own staged code, so drop any # leftover settings.txt from an earlier, unrelated run by hand: # screenshot mode always starts from a clean, known state. - settings_path = run_app.SANDBOX_ROOT / "settings.txt" - settings_path.unlink(missing_ok=True) - if settings_src is not None: - settings_path.write_text(settings_src.read_text()) + (run_app.SANDBOX_ROOT / "settings.txt").unlink(missing_ok=True) run_app._seed_settings( args.width, @@ -225,6 +269,14 @@ def run(args: argparse.Namespace) -> None: os.environ["MATRIXBOX_SIMULATOR_REFRESH_FPS"] = str(args.refresh_fps) os.environ["MATRIXBOX_SIMULATOR_GAMMA"] = str(args.gamma) + # A monolithic-kernel app's main.py stands up its own web UI on this + # port (remapped from the device's real port 80 — see socketpool.py). + # Nothing external ever needs to reach it in headless screenshot mode, + # so let the OS pick a free one instead of the fixed 8080 default, + # which would otherwise collide with any other already-running + # `matrixbox app`/`screenshot` process on the same machine — same + # reasoning as the frame server's port 0 below. + os.environ["MATRIXBOX_SIMULATOR_HTTP_PORT"] = "0" app_dir = run_app._resolve_app_dir(args.app) framework_root = run_app._framework_root_for(app_dir) @@ -248,6 +300,8 @@ def run(args: argparse.Namespace) -> None: except (OSError, ValueError) as exc: raise SystemExit(f"invalid settings file {settings_src}: {exc}") from exc + _warn_if_settings_filename_looks_unused(app_dir, settings_src) + # Resolved against the real launch directory, before staging below # os.chdir()s into the sandbox — a relative --output would otherwise # land inside it instead of where the caller actually meant. diff --git a/tests/test_screenshot.py b/tests/test_screenshot.py index a59caf7..318cdff 100644 --- a/tests/test_screenshot.py +++ b/tests/test_screenshot.py @@ -20,15 +20,25 @@ import framebufferio import rgbmatrix +# /settings.txt (absolute, device-root) carries panel geometry; a plain +# relative open() is this app's own settings file, living in its own +# staged directory — the two are unrelated, same as departures' own +# settings.txt (relative) vs. its wifi lookup at /settings.txt (absolute). try: with open("/settings.txt") as f: - settings = json.loads(f.read()) + device_settings = json.loads(f.read()) except OSError: - settings = {} + device_settings = {} -width = settings.get("width", 64) -height = settings.get("height", 32) -color = 0x00FF00 if settings.get("theme") == "green" else 0xFF0000 +try: + with open("app-settings.json") as f: + app_settings = json.loads(f.read()) +except OSError: + app_settings = {} + +width = device_settings.get("width", 64) +height = device_settings.get("height", 32) +color = 0x00FF00 if app_settings.get("theme") == "green" else 0xFF0000 matrix = rgbmatrix.RGBMatrix(width=width, height=height) display = framebufferio.FramebufferDisplay(matrix) @@ -119,15 +129,24 @@ def test_captures_a_drawn_frame_with_default_settings(tmp_path: Path) -> None: assert image.getpixel((0, 0)) == (255, 0, 0) # no --settings: app's own default -def test_settings_file_is_resolved_inside_the_app_directory(tmp_path: Path) -> None: +def test_settings_file_is_seeded_into_the_apps_own_staged_directory( + tmp_path: Path, +) -> None: + # Named to match what the fixture app itself opens (a plain relative + # "app-settings.json") — --settings copies the given file verbatim + # into the app's own staged directory under its original name, it + # doesn't merge it into the device-root settings.txt. app_dir = _make_app(tmp_path, "solid", _SOLID_FRAME_APP) - (app_dir / "ci.json").write_text(json.dumps({"theme": "green"})) + (app_dir / "app-settings.json").write_text(json.dumps({"theme": "green"})) output = tmp_path / "out.png" - result = _run_screenshot(str(app_dir), "--settings", "ci.json", "-o", str(output)) + result = _run_screenshot( + str(app_dir), "--settings", "app-settings.json", "-o", str(output) + ) assert result.returncode == 0, result.stderr assert Image.open(output).getpixel((0, 0)) == (0, 255, 0) + assert "doesn't appear to reference" not in result.stderr def test_missing_settings_file_fails_fast(tmp_path: Path) -> None: