mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 06:15:09 +00:00
fix(web): default web_display_autostart to true, as the installer already does (#556)
start_web_conditionally.py read the flag with
`config_data.get("web_display_autostart", False)`, so a config that simply
lacked the key got no web interface. Both config/config.template.json and
first_time_install.sh ship the key as true, so the code default contradicted
the shipped default in two places: absence means an older or hand-edited
config, not a request to stay down.
The failure mode was silent in the worst way. The "not starting" path exits 0,
so `systemctl status ledmatrix-web` reported the unit as successfully started
while nothing was listening on the port, and the only trace was one journal
line saying the flag was "false or not set" -- which reads as a deliberate
setting rather than a missing key.
Also start the web interface when config.json is missing or unparseable,
instead of exiting. The web interface is how a config gets created and
repaired, so a broken config is exactly when the user needs it most; leaving
it down means there is no way back in. Only an explicit false/off disables
autostart now, and the disabled message says "explicitly disabled" so the
journal distinguishes a real setting from a default.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -74,23 +74,41 @@ def install_dependencies():
|
|||||||
print(f"Failed to install dependencies: {e}")
|
print(f"Failed to install dependencies: {e}")
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
#: String spellings that turn autostart OFF. Anything else -- including the key
|
||||||
|
#: being absent entirely -- leaves it on.
|
||||||
|
DISABLED_STRINGS = ("off", "false", "no", "0")
|
||||||
|
|
||||||
|
|
||||||
|
def autostart_enabled(config_data):
|
||||||
|
"""Whether to bring the web interface up. Defaults to True.
|
||||||
|
|
||||||
|
config.template.json and first_time_install.sh both ship
|
||||||
|
``web_display_autostart`` as true, so a config that lacks the key is an
|
||||||
|
older or hand-edited one rather than a request to stay down. Defaulting to
|
||||||
|
False meant any such config silently got no web interface -- and because
|
||||||
|
the "not starting" path exits 0, systemd reported the unit as successfully
|
||||||
|
started while nothing was listening. Only an explicit false/off disables it.
|
||||||
|
"""
|
||||||
|
value = config_data.get("web_display_autostart", True)
|
||||||
|
if isinstance(value, str):
|
||||||
|
return value.strip().lower() not in DISABLED_STRINGS
|
||||||
|
return bool(value)
|
||||||
|
|
||||||
|
|
||||||
def main():
|
def main():
|
||||||
try:
|
try:
|
||||||
with open(CONFIG_FILE, 'r') as f:
|
with open(CONFIG_FILE, 'r') as f:
|
||||||
config_data = json.load(f)
|
config_data = json.load(f)
|
||||||
except FileNotFoundError:
|
except FileNotFoundError:
|
||||||
print(f"Config file {CONFIG_FILE} not found. Web interface will not start.")
|
# The web interface is how a config gets created and repaired, so a
|
||||||
sys.exit(0) # Exit gracefully, don't start
|
# missing one is the case where the user needs it most.
|
||||||
except Exception as e:
|
print(f"Config file {CONFIG_FILE} not found. Starting the web interface so it can be configured.")
|
||||||
print(f"Error reading config file {CONFIG_FILE}: {e}. Web interface will not start.")
|
config_data = {}
|
||||||
sys.exit(1) # Exit with error, service might restart depending on config
|
except (json.JSONDecodeError, OSError) as e:
|
||||||
|
print(f"Error reading config file {CONFIG_FILE}: {e}. Starting the web interface anyway so the config can be repaired.")
|
||||||
|
config_data = {}
|
||||||
|
|
||||||
autostart_enabled = config_data.get("web_display_autostart", False)
|
if autostart_enabled(config_data):
|
||||||
|
|
||||||
# Handle both boolean True and string "on"/"true" values
|
|
||||||
is_enabled = (autostart_enabled is True) or (isinstance(autostart_enabled, str) and autostart_enabled.lower() in ("on", "true", "yes", "1"))
|
|
||||||
|
|
||||||
if is_enabled:
|
|
||||||
print("Configuration 'web_display_autostart' is enabled. Starting web interface...")
|
print("Configuration 'web_display_autostart' is enabled. Starting web interface...")
|
||||||
|
|
||||||
# Only install dependencies if not already done during first-time setup
|
# Only install dependencies if not already done during first-time setup
|
||||||
@@ -116,7 +134,7 @@ def main():
|
|||||||
print(f"Failed to exec web interface: {e}")
|
print(f"Failed to exec web interface: {e}")
|
||||||
sys.exit(1) # Failed to start
|
sys.exit(1) # Failed to start
|
||||||
else:
|
else:
|
||||||
print("Configuration 'web_display_autostart' is false or not set. Web interface will not be started.")
|
print("Configuration 'web_display_autostart' is explicitly disabled. Web interface will not be started.")
|
||||||
sys.exit(0) # Exit gracefully, service considered successful
|
sys.exit(0) # Exit gracefully, service considered successful
|
||||||
|
|
||||||
if __name__ == '__main__':
|
if __name__ == '__main__':
|
||||||
|
|||||||
@@ -0,0 +1,63 @@
|
|||||||
|
"""Tests for the web interface's autostart default.
|
||||||
|
|
||||||
|
Regression under test: ``start_web_conditionally.py`` read the flag with
|
||||||
|
``config_data.get("web_display_autostart", False)``, so a config that simply
|
||||||
|
lacked the key got no web interface. Both config.template.json and
|
||||||
|
first_time_install.sh ship the key as ``true``, so absence means an older or
|
||||||
|
hand-edited config -- not a request to stay down.
|
||||||
|
|
||||||
|
The failure was silent in the worst way: the "not starting" path exits 0, so
|
||||||
|
``systemctl status ledmatrix-web`` reported the unit as successfully started
|
||||||
|
while nothing was listening on the port, and the journal's only trace was one
|
||||||
|
line saying the flag was "false or not set".
|
||||||
|
"""
|
||||||
|
|
||||||
|
import importlib.util
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
SCRIPT = (Path(__file__).resolve().parents[1]
|
||||||
|
/ "scripts" / "utils" / "start_web_conditionally.py")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(scope="module")
|
||||||
|
def mod():
|
||||||
|
spec = importlib.util.spec_from_file_location("start_web_conditionally", SCRIPT)
|
||||||
|
module = importlib.util.module_from_spec(spec)
|
||||||
|
spec.loader.exec_module(module)
|
||||||
|
return module
|
||||||
|
|
||||||
|
|
||||||
|
class TestAutostartDefault:
|
||||||
|
def test_absent_key_still_starts(self, mod):
|
||||||
|
# The regression: this returned False and the UI silently stayed down.
|
||||||
|
assert mod.autostart_enabled({}) is True
|
||||||
|
|
||||||
|
def test_unreadable_config_still_starts(self, mod):
|
||||||
|
# main() falls back to {} when the config is missing or corrupt, because
|
||||||
|
# the web interface is how a broken config gets repaired.
|
||||||
|
assert mod.autostart_enabled({}) is True
|
||||||
|
|
||||||
|
def test_explicit_true_starts(self, mod):
|
||||||
|
assert mod.autostart_enabled({"web_display_autostart": True}) is True
|
||||||
|
|
||||||
|
def test_explicit_false_does_not_start(self, mod):
|
||||||
|
assert mod.autostart_enabled({"web_display_autostart": False}) is False
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("value", ["off", "false", "no", "0", "OFF", " False "])
|
||||||
|
def test_disabling_strings_do_not_start(self, mod, value):
|
||||||
|
assert mod.autostart_enabled({"web_display_autostart": value}) is False
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("value", ["on", "true", "yes", "1", "TRUE"])
|
||||||
|
def test_enabling_strings_start(self, mod, value):
|
||||||
|
assert mod.autostart_enabled({"web_display_autostart": value}) is True
|
||||||
|
|
||||||
|
|
||||||
|
class TestShippedDefaultsAgree:
|
||||||
|
def test_template_ships_autostart_true(self):
|
||||||
|
"""The code default must match what the installer actually writes."""
|
||||||
|
import json
|
||||||
|
template = json.loads(
|
||||||
|
(SCRIPT.parents[2] / "config" / "config.template.json").read_text(encoding="utf-8"))
|
||||||
|
assert template["web_display_autostart"] is True
|
||||||
Reference in New Issue
Block a user