ninharp/ninlight-w600
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(station): Panel liest den Modus aus einer dauerhaften Datei, nicht aus RUN_DIR

Browse Source 
Fixrunde 1, Befund 3: RUN_DIR/mode ueberlebt nur einen "systemctl restart"
(RuntimeDirectoryPreserve=restart) - "ninlight-station off" fuehrt aber einen
echten "systemctl stop" aus, der das RuntimeDirectory leert. build_state()
fiel danach unbemerkt auf "recovery" zurueck, auch wenn tatsaechlich eine
Migration anstand - die Fusszeile haette dann "in Ordnung" gezeigt, obwohl der
Migrationssatz leer ist. Ein Docstring behauptete ausdruecklich das Gegenteil.

_read_mode_file() liest jetzt eine neue, dauerhafte LAST_MODE_FILE (unter
/var/lib/ninlight-station, wie der Bildsatz-Umschalter selbst) statt
RUN_DIR/mode; write_mode() im CLI schreibt beide. Ausserdem: _read_imageset()
faengt jetzt auch UnicodeDecodeError ab (Fixrunde 1, Kleinigkeit).

Ein neuer Test bildet den echten Ablauf nach (Modus schreiben, RUN_DIR wie bei
einem echten Stop leeren, pruefen dass der Modus trotzdem stimmt) - der alte
Test fuer den Rettungs-Bildsatz liess die Modusdatei ganz weg und pruefte
damit nur den zufaellig gleichen Rueckfallwert, nicht die Wirklichkeit.
This commit is contained in:
Michael Sauer
2026-09-23 21:59:09 +02:00
parent e67fc53b20
commit 1562a7983a
2 changed files with 132 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tools/station/display/display.py
+53 -20
View File
@@ -67,6 +67,30 @@ UI_FILE = Path(os.environ.get("NINLIGHT_UI_FILE", "/run/ninlight-button/ui.json"
# schreibt server.py nichts mehr.
IMAGESET_FILE = Path(os.environ.get("NINLIGHT_IMAGESET_FILE",
"/var/lib/ninlight-station/imageset"))
# Der Modus, den "recovery on"/"migrate on" zuletzt angefordert haben - fuer die
# Fusszeile GENAUSO dauerhaft gebraucht wie der Bildsatz-Umschalter oben, aus demselben
# Grund und nach demselben Muster (eigene Datei unter /var/lib, NICHT unter RUN_DIR).
#
# Fixrunde 1, Befund 3: RUN_DIR/mode taugt dafuer NICHT. Die Unit setzt
# RuntimeDirectoryPreserve=restart - das erhaelt /run/ninlight-station nur ueber einen
# "systemctl restart" hinweg (das, was "recovery on"/"migrate on" fuer einen bereits
# laufenden Dienst ausloesen). "ninlight-station off" aber STOPPT den Dienst
# (systemctl stop), und GENAU DANN raeumt systemd das RuntimeDirectory leer, "restart"
# deckt einen echten Stop nicht ab. Ein frueherer Docstring hier behauptete das
# Gegenteil ("die Datei ueberlebt das Abschalten") - das war schlicht falsch, und der
# Fehler war unauffaellig, weil eine fehlende Datei auf "recovery" faellt: genau der
# Modus, den viele Tests ohnehin erwarteten, ohne dass die Faelligkeit der Datei
# je geprueft wurde. Die Folge am Geraet waere die falsche Beruhigung, gegen die die
# Fusszeile ueberhaupt gebaut ist - "in Ordnung" vor einer geplanten Migration, obwohl
# der Migrationssatz leer ist, weil das Panel mangels Datei den Rettungssatz geprueft
# haette.
#
# write_mode() im CLI schreibt deshalb NEBEN der ephemeren RUN_DIR/mode (die
# ausschliesslich server.py's naechsten Start betrifft) auch hierher - dieselbe
# Zeichenkette, zwei Ablagen fuer zwei verschiedene Leser mit verschiedenen
# Haltbarkeitsanforderungen.
LAST_MODE_FILE = Path(os.environ.get("NINLIGHT_LAST_MODE_FILE",
"/var/lib/ninlight-station/mode"))
# Dieselben Namen und dieselbe Vorgabe wie in server.py und im CLI - im Zweifel der
# Satz OHNE Zugangsdaten (dort steht die Begruendung).
IMAGESET_DIRS = {"ohne-wlan": "migrate-nocreds", "mit-wlan": "migrate-wlan"}
@@ -215,13 +239,19 @@ def _recent_request(status: dict, now: float | None = None) -> bool:
return 0 <= age_s <= ACTIVITY_WINDOW_S
def _read_mode_file(run_dir: Path) -> str:
"""Modus, in dem der Dienst laut RUN_DIR/mode tatsaechlich gestartet wurde. Fehlt die
Datei oder ist ihr Inhalt kein bekannter Modus, gilt "recovery" - derselbe Vorgabewert
wie in server.py:resolve_mode()."""
def _read_mode_file(path: Path) -> str:
"""Zuletzt angeforderter Modus laut `path`. Fehlt die Datei, ist sie unlesbar
(inklusive ungueltigem UTF-8 - eine halb geschriebene/beschaedigte Datei ist nicht
nur per fehlendem Inhalt kaputt), oder ist ihr Inhalt kein bekannter Modus, gilt
"recovery" - derselbe Vorgabewert wie in server.py:resolve_mode(). Wirft nie: eine
kaputte Datei darf weder das Panel noch "off" aufhalten.
`path` ist bewusst eine Datei, kein Verzeichnis (anders als frueher, wo hier
`run_dir / "mode"` gelesen wurde) - siehe LAST_MODE_FILE oben, warum die Fusszeile
NICHT mehr aus RUN_DIR liest."""
try:
value = (run_dir / "mode").read_text().strip()
except OSError:
value = path.read_text().strip()
except (OSError, UnicodeDecodeError):
return "recovery"
return value if value in ("recovery", "migration") else "recovery"
@@ -275,7 +305,7 @@ def _read_imageset(imageset_path: Path, conf: dict[str, str], mode: str) -> tupl
dann gilt "ok", denn eine unbelegte Warnung ist schlechter als keine."""
try:
value = imageset_path.read_text().strip()
except OSError:
except (OSError, UnicodeDecodeError):
value = ""
name = value if value in IMAGESET_DIRS else IMAGESET_DEFAULT
image_dir = conf.get("IMAGE_DIR")
@@ -320,16 +350,18 @@ def local_ip() -> str:
def build_state(run_dir: Path = RUN_DIR, conf_path: Path = CONF_PATH,
service_name: str = SERVICE_NAME,
imageset_path: Path = IMAGESET_FILE) -> dict:
imageset_path: Path = IMAGESET_FILE,
last_mode_path: Path = LAST_MODE_FILE) -> dict:
"""Baut den zusammengefuehrten Zustand, den render.lines()/compose() erwarten.
Modus kommt vorrangig aus RUN_DIR/mode (Vorgabe "recovery", wie
Modus kommt vorrangig aus `last_mode_path` (Vorgabe "recovery", wie
server.py:resolve_mode()) - das ist das, was "ninlight-station on"/"migrate on"
zuletzt geschrieben haben und unabhaengig davon verfuegbar, ob/wann der Serverprozess
seit dem letzten Start schon einmal status.json geschrieben hat. status.json selbst
liefert nur served/recent/last_seen; sein "mode"-Feld wird bewusst NICHT als
Modusquelle verwendet (Fixrunde 1: die urspruengliche Fassung hatte den Vorrang
umgekehrt).
zuletzt geschrieben haben, dauerhaft (siehe LAST_MODE_FILE oben - NICHT die ephemere
RUN_DIR/mode, die server.py fuer den naechsten Start liest) und unabhaengig davon
verfuegbar, ob/wann der Serverprozess seit dem letzten Start schon einmal
status.json geschrieben hat. status.json selbst liefert nur served/recent/
last_seen; sein "mode"-Feld wird bewusst NICHT als Modusquelle verwendet
(Fixrunde 1: die urspruengliche Fassung hatte den Vorrang umgekehrt).
Phase/armed_target kommen aus control.json, nur im Migrationsmodus massgeblich - der
Rettungsmodus ignoriert control.json ebenso wie server.py das tut. "off" ueberschreibt
@@ -342,17 +374,18 @@ def build_state(run_dir: Path = RUN_DIR, conf_path: Path = CONF_PATH,
Ob "imageset_ok" den Rettungs- oder den Migrations-Bildsatz prueft, richtet sich nach
dem MODUS, in dem die Station beim naechsten Einschalten anlaufen wuerde - also
RUN_DIR/mode, nicht danach, ob sie gerade laeuft. Deshalb wird der Modus HIER, vor der
"ist der Dienst aktiv"-Abfrage, gelesen: er ist auch im Zustand "off" verfuegbar (die
Datei ueberlebt das Abschalten) und genau dort soll die Vorschau ja gelten - "off"
zeigt ja=noch keinen Modus im engeren Sinn, aber der naechste Start faehrt in genau
diesen Modus, und die Fusszeile soll VORHER zeigen, ob dessen Bildsatz bereitliegt.
`last_mode_path`, nicht danach, ob sie gerade laeuft. Deshalb wird der Modus HIER,
vor der "ist der Dienst aktiv"-Abfrage, gelesen: er ist auch im Zustand "off"
verfuegbar (die Datei ist dauerhaft, siehe LAST_MODE_FILE) und genau dort soll die
Vorschau ja gelten - "off" zeigt ja noch keinen Modus im engeren Sinn, aber der
naechste Start faehrt in genau diesen Modus, und die Fusszeile soll VORHER zeigen,
ob dessen Bildsatz bereitliegt.
"activity" (nur in Recovery/Migration, siehe _recent_request()) sagt, ob die letzte
Anfrage jeder Art hoechstens ACTIVITY_WINDOW_S Sekunden her ist - das Lebenszeichen,
das render.blocks() an die Servierzeile anhaengt."""
conf = _read_conf(conf_path)
mode = _read_mode_file(run_dir)
mode = _read_mode_file(last_mode_path)
imageset, imageset_ok = _read_imageset(imageset_path, conf, mode)
if not _service_active(service_name):
tools/station/tests/test_render.py
+79 -32
View File
@@ -840,14 +840,15 @@ def test_autooff_line_says_so_when_the_deadline_has_arrived():
def test_build_state_reads_the_autooff_file(tmp_path, monkeypatch):
monkeypatch.setattr(display, "_service_active", lambda service_name: True)
(tmp_path / "mode").write_text("recovery")
(tmp_path / "last-mode").write_text("recovery")
conf_path = tmp_path / "station.conf"
conf_path.write_text("SSID='ninlight-recovery'\n")
# Noch 6 min 1 s - aufgerundet 7, wie es "ninlight-station status" auch rechnet.
(tmp_path / "autooff.json").write_text(json.dumps(
{"minutes": 30, "deadline": time.time() + 361, "deferrals": 0}))
state = display.build_state(run_dir=tmp_path, conf_path=conf_path, service_name="x")
state = display.build_state(run_dir=tmp_path, conf_path=conf_path, service_name="x",
last_mode_path=tmp_path / "last-mode")
assert state["autooff_minutes"] == 7
assert "Auto-off in 7 min" in render.lines(state)
@@ -855,11 +856,12 @@ def test_build_state_reads_the_autooff_file(tmp_path, monkeypatch):
def test_build_state_without_autooff_file_has_no_timer_field(tmp_path, monkeypatch):
monkeypatch.setattr(display, "_service_active", lambda service_name: True)
(tmp_path / "mode").write_text("recovery")
(tmp_path / "last-mode").write_text("recovery")
conf_path = tmp_path / "station.conf"
conf_path.write_text("SSID='ninlight-recovery'\n")
state = display.build_state(run_dir=tmp_path, conf_path=conf_path, service_name="x")
state = display.build_state(run_dir=tmp_path, conf_path=conf_path, service_name="x",
last_mode_path=tmp_path / "last-mode")
assert "autooff_minutes" not in state
assert not any("Auto-off" in line for line in render.lines(state))
@@ -913,12 +915,13 @@ def test_a_switched_off_station_shows_no_timer(tmp_path, monkeypatch):
def test_control_json_is_ignored_outside_migration_mode(tmp_path, monkeypatch):
monkeypatch.setattr(display, "_service_active", lambda service_name: True)
(tmp_path / "mode").write_text("recovery")
(tmp_path / "last-mode").write_text("recovery")
(tmp_path / "control.json").write_text(json.dumps({"phase": "armed", "armed_target": "g95_cw"}))
conf_path = tmp_path / "station.conf"
conf_path.write_text("SSID='ninlight-recovery'\n")
state = display.build_state(run_dir=tmp_path, conf_path=conf_path, service_name="x")
state = display.build_state(run_dir=tmp_path, conf_path=conf_path, service_name="x",
last_mode_path=tmp_path / "last-mode")
assert state["mode"] == "recovery"
assert "phase" not in state
@@ -930,12 +933,13 @@ def test_build_state_off_when_service_not_active(tmp_path, monkeypatch):
monkeypatch.setattr(display, "_service_active", lambda service_name: False)
# Selbst ein noch so frisches/gueltiges status.json darf "off" nicht verhindern -
# "ninlight-station off" laesst status.json bewusst unangetastet liegen.
(tmp_path / "mode").write_text("migration")
(tmp_path / "last-mode").write_text("migration")
(tmp_path / "status.json").write_text(json.dumps({"mode": "migration", "served": 5}))
state = display.build_state(run_dir=tmp_path, conf_path=tmp_path / "station.conf",
service_name="x",
imageset_path=tmp_path / "imageset")
imageset_path=tmp_path / "imageset",
last_mode_path=tmp_path / "last-mode")
# "off" und sonst NICHTS aus status.json - nur der Bildsatz, der auch im
# ausgeschalteten Zustand gilt (und gerade dann gesehen werden soll).
@@ -955,9 +959,10 @@ def test_build_state_off_carries_the_local_address(tmp_path, monkeypatch):
def test_build_state_running_carries_no_address(tmp_path, monkeypatch):
monkeypatch.setattr(display, "_service_active", lambda service_name: True)
(tmp_path / "mode").write_text("migration")
(tmp_path / "last-mode").write_text("migration")
state = display.build_state(run_dir=tmp_path, conf_path=tmp_path / "station.conf",
service_name="x", imageset_path=tmp_path / "imageset")
service_name="x", imageset_path=tmp_path / "imageset",
last_mode_path=tmp_path / "last-mode")
assert "ip" not in state
@@ -1022,26 +1027,28 @@ def test_served_line_without_an_activity_field_claims_nothing():
def test_build_state_carries_activity_in_recovery_and_migration(tmp_path, monkeypatch):
monkeypatch.setattr(display, "_service_active", lambda service_name: True)
(tmp_path / "mode").write_text("recovery")
(tmp_path / "last-mode").write_text("recovery")
(tmp_path / "status.json").write_text(json.dumps({"last_request_ts": None}))
conf = tmp_path / "station.conf"
conf.write_text("")
state = display.build_state(run_dir=tmp_path, conf_path=conf, service_name="x",
imageset_path=tmp_path / "imageset")
imageset_path=tmp_path / "imageset",
last_mode_path=tmp_path / "last-mode")
assert state["activity"] is False
def test_build_state_mode_comes_from_mode_file_not_status_json(tmp_path, monkeypatch):
"""Der Modus-Vorrang liegt bei RUN_DIR/mode, nicht bei status.json's "mode"-Feld -
status.json kann veraltet sein (Fixrunde 1, Kleinigkeit: der Vorrang war zuerst
umgekehrt)."""
"""Der Modus-Vorrang liegt bei der dauerhaften Modusdatei (last_mode_path), nicht
bei status.json's "mode"-Feld - status.json kann veraltet sein (Fixrunde 1,
Kleinigkeit: der Vorrang war zuerst umgekehrt)."""
monkeypatch.setattr(display, "_service_active", lambda service_name: True)
(tmp_path / "mode").write_text("recovery")
(tmp_path / "last-mode").write_text("recovery")
(tmp_path / "status.json").write_text(json.dumps({"mode": "migration", "served": 1}))
conf_path = tmp_path / "station.conf"
conf_path.write_text("SSID='ninlight-recovery'\n")
state = display.build_state(run_dir=tmp_path, conf_path=conf_path, service_name="x")
state = display.build_state(run_dir=tmp_path, conf_path=conf_path, service_name="x",
last_mode_path=tmp_path / "last-mode")
assert state["mode"] == "recovery"
assert state["served"] == 1
@@ -1050,10 +1057,36 @@ def test_build_state_mode_comes_from_mode_file_not_status_json(tmp_path, monkeyp
def test_build_state_falls_back_to_recovery_without_mode_file(tmp_path, monkeypatch):
monkeypatch.setattr(display, "_service_active", lambda service_name: True)
state = display.build_state(run_dir=tmp_path, conf_path=tmp_path / "station.conf",
service_name="x")
service_name="x", last_mode_path=tmp_path / "last-mode")
assert state["mode"] == "recovery"
def test_build_state_reads_the_durable_mode_file_even_after_a_real_stop(tmp_path, monkeypatch):
"""Fixrunde 1, Befund 3: RUN_DIR/mode ueberlebt nur einen "systemctl restart", NICHT
das "systemctl stop", das "ninlight-station off" ausloest (die Unit setzt
RuntimeDirectoryPreserve=restart, nicht "yes") - ein frueherer Docstring behauptete
das Gegenteil. Dieser Test bildet GENAU diesen Ablauf nach: "migrate on" schreibt
den Modus, RUN_DIR wird dann (wie bei einem echten Stop) GELEERT, und build_state()
muss den Modus trotzdem noch aus der dauerhaften Datei kennen - nicht auf "recovery"
zurueckfallen, waehrend tatsaechlich eine Migration ansteht."""
last_mode = tmp_path / "var" / "last-mode"
run_dir = tmp_path / "run"
run_dir.mkdir()
(run_dir / "mode").write_text("migration")
last_mode.parent.mkdir(parents=True)
last_mode.write_text("migration")
# "systemctl stop" leert das RuntimeDirectory - hier nachgestellt, ohne RUN_DIR
# selbst zu entfernen (das legt server.py/das CLI bei Bedarf neu an).
(run_dir / "mode").unlink()
monkeypatch.setattr(display, "_service_active", lambda service_name: True)
state = display.build_state(run_dir=run_dir, conf_path=tmp_path / "station.conf",
service_name="x", imageset_path=tmp_path / "imageset",
last_mode_path=last_mode)
assert state["mode"] == "migration"
def test_read_conf_accepts_export_prefix(tmp_path):
"""setup_pi.sh prueft beim Nachtragen fehlender Schluessel ausdruecklich auf ein
optionales "export "-Praefix (^(export[[:space:]]+)?KEY=) - eine bereits vorhandene
@@ -1990,7 +2023,8 @@ def test_the_image_set_line_survives_the_fullest_state():
def test_build_state_reads_the_image_set_switch(tmp_path, monkeypatch):
"""Dieselbe Datei, die das CLI schreibt und server.py liest - kein zweiter Weg."""
monkeypatch.setattr(display, "_service_active", lambda service_name: True)
(tmp_path / "mode").write_text("migration")
last_mode = tmp_path / "last-mode"
last_mode.write_text("migration")
images = tmp_path / "images"
(images / "migrate-nocreds").mkdir(parents=True)
(images / "migrate-nocreds" / "g95_cw_dev.img").write_bytes(b"IMG")
@@ -2002,7 +2036,7 @@ def test_build_state_reads_the_image_set_switch(tmp_path, monkeypatch):
def state():
return display.build_state(run_dir=tmp_path, conf_path=conf, service_name="x",
imageset_path=switch)
imageset_path=switch, last_mode_path=last_mode)
# Ohne Datei: die sichere Vorgabe.
assert state()["imageset"] == "ohne-wlan"
@@ -2017,11 +2051,14 @@ def test_build_state_reports_a_missing_image_set(tmp_path, monkeypatch):
""""Vorhanden" heisst: Verzeichnis da UND mindestens ein Image darin - dieselbe Regel
wie in server.py, sonst sagt das Panel "in Ordnung" und die Lampe bekommt nichts.
Ausdruecklich im Migrationsmodus (RUN_DIR/mode) - das ist es, was die migrate-*-
Verzeichnisse betrifft. Der Rettungsmodus prueft ein eigenes Paar (rescue-*), siehe
test_build_state_reports_a_missing_rescue_image_set unten."""
Ausdruecklich im Migrationsmodus (last_mode_path explizit auf "migration" gesetzt,
nicht dem Vorgabewert ueberlassen - Fixrunde 1, Befund 3: eine fehlende Datei faellt
auf "recovery", und dieser Test soll die MIGRATION pruefen) - das ist es, was die
migrate-*-Verzeichnisse betrifft. Der Rettungsmodus prueft ein eigenes Paar
(rescue-*), siehe test_build_state_reports_a_missing_rescue_image_set unten."""
monkeypatch.setattr(display, "_service_active", lambda service_name: True)
(tmp_path / "mode").write_text("migration")
last_mode = tmp_path / "last-mode"
last_mode.write_text("migration")
images = tmp_path / "images"
(images / "migrate-nocreds").mkdir(parents=True)
(images / "migrate-nocreds" / "g95_cw_dev.img").write_bytes(b"IMG")
@@ -2031,25 +2068,33 @@ def test_build_state_reports_a_missing_image_set(tmp_path, monkeypatch):
switch.write_text("ohne-wlan")
assert display.build_state(run_dir=tmp_path, conf_path=conf, service_name="x",
imageset_path=switch)["imageset_ok"] is True
imageset_path=switch,
last_mode_path=last_mode)["imageset_ok"] is True
switch.write_text("mit-wlan") # migrate-wlan/ gibt es nicht
state = display.build_state(run_dir=tmp_path, conf_path=conf, service_name="x",
imageset_path=switch)
imageset_path=switch, last_mode_path=last_mode)
assert state["imageset_ok"] is False
assert "MISSING" in _imageset_line(state)
# Und ein LEERES Verzeichnis zaehlt genauso als fehlend.
(images / "migrate-wlan").mkdir()
assert display.build_state(run_dir=tmp_path, conf_path=conf, service_name="x",
imageset_path=switch)["imageset_ok"] is False
imageset_path=switch,
last_mode_path=last_mode)["imageset_ok"] is False
def test_build_state_reports_a_missing_rescue_image_set(tmp_path, monkeypatch):
"""Derselbe Umschalter gilt jetzt auch fuer die Rettung, mit einem EIGENEN
Verzeichnispaar (rescue-*, nicht migrate-*) - server.py begruendet die Trennung.
Ohne Modusdatei gilt "recovery" (dieselbe Vorgabe wie resolve_mode() in server.py),
das Panel muss also hier die rescue-*-Verzeichnisse pruefen, nicht die migrate-*."""
last_mode_path wird ausdruecklich auf "recovery" gesetzt, statt den Vorgabewert zu
nutzen (Fixrunde 1, Befund 3 - ein vorheriger Zustand dieses Tests liess die Datei
ganz weg und pruefte damit nur den ZUFAELLIG gleich lautenden Rueckfallwert, nicht
die Wirklichkeit: mit einer fehlenden Datei haette dieser Test nicht bemerkt, wenn
"recovery" faelschlich auch fuer eine echte Migration zurueckkaeme)."""
monkeypatch.setattr(display, "_service_active", lambda service_name: True)
last_mode = tmp_path / "last-mode"
last_mode.write_text("recovery")
images = tmp_path / "images"
(images / "rescue-nocreds").mkdir(parents=True)
(images / "rescue-nocreds" / "ninlight_g95_cw_ota.img").write_bytes(b"IMG")
@@ -2062,17 +2107,19 @@ def test_build_state_reports_a_missing_rescue_image_set(tmp_path, monkeypatch):
switch.write_text("ohne-wlan")
assert display.build_state(run_dir=tmp_path, conf_path=conf, service_name="x",
imageset_path=switch)["imageset_ok"] is True
imageset_path=switch,
last_mode_path=last_mode)["imageset_ok"] is True
switch.write_text("mit-wlan") # rescue-wlan/ gibt es nicht
state = display.build_state(run_dir=tmp_path, conf_path=conf, service_name="x",
imageset_path=switch)
imageset_path=switch, last_mode_path=last_mode)
assert state["imageset_ok"] is False
assert "MISSING" in _imageset_line(state)
# Und ein LEERES Verzeichnis zaehlt genauso als fehlend.
(images / "rescue-wlan").mkdir()
assert display.build_state(run_dir=tmp_path, conf_path=conf, service_name="x",
imageset_path=switch)["imageset_ok"] is False
imageset_path=switch,
last_mode_path=last_mode)["imageset_ok"] is False
def test_the_display_and_the_server_agree_on_the_set_names():