Fixrunde 2 zu Task 4:
- N1 (wichtig): file_mode() in test_cli.sh/test_setup_pi.sh probierte
"stat -f '%Lp'" vor "stat -c '%a'" - auf GNU-coreutils (dem Pi) deutet
"-f" dort "Dateisystemstatus" und liefert Muell statt der Rechte-Zahl,
faellt aber trotzdem in den "||"-Zweig durch, der zusaetzlich "644"
anhaengt. Reihenfolge umgedreht und mit einem per gstat erzwungenen
GNU-Testlauf zusaetzlich abgesichert.
- N2 (wichtig): status zeigte nach einem Wechsel zurueck in den
Rettungsmodus weiterhin die letzte Migrationsphase aus einer liegen
gebliebenen control.json (moeglich seit RuntimeDirectoryPreserve=
restart aus Fixrunde 1). Phase wird jetzt nur noch angezeigt, wenn
Modus tatsaechlich "migration" ist.
- N3 (klein): die Schluesselerkennung in setup_pi.sh war rein
zeilenverankert ("^KEY=") und haette einen eingerueckten Bestandswert
als fehlend gewertet - beim naechsten Sourcen der Konfiguration haette
die zusaetzlich angehaengte Vorlagenzeile das eigene Passwort
ueberschrieben. Muster auf fuehrenden Leerraum/export gehaertet.
- N4 (klein): test_setup_pi.sh liess setup_pi.sh ueber den scp-Stub echte
Dateien im /tmp des Entwicklungsrechners anlegen und wieder loeschen.
Neue Variable NINLIGHT_STAGE_DIR biegt das auf eine Sandbox um.
Alle vier Punkte mit Regressionstests, N2 und N3 zusaetzlich per
Mutationsprobe (Fix zurueckgenommen, Test wird rot) verifiziert.
Fixrunde 1 zu Task 4:
- C1 (kritisch): die Unit leerte /run/ninlight-station bei jedem
systemctl restart, weil RuntimeDirectoryPreserve fehlte - genau der
Neustart, mit dem migrate/recovery on den Dienst hochfahren, wischte
die eben geschriebene Modus-/Steuerdatei wieder weg, und der Server
fiel still auf den Recovery-Default zurueck. Unit bekommt
RuntimeDirectoryPreserve=restart, und start_and_verify() prueft nach
dem Neustart zusaetzlich ausdruecklich nach, dass Modus und Phase noch
stimmen.
- I1: status las die Phase aus status.json, die arm/listen (die den
Dienst bewusst nicht neu starten) nie aktualisieren - jetzt kommt sie
aus control.json.
- I2: off pruefte nach dem Herunterfahren nichts nach und konnte
"Station aus" melden, waehrend ein Profil wegen haengendem
NetworkManager weiterhin aktiv war - ap_switch() verifiziert das jetzt
auch im Aus-Fall.
- I3: der Fehlerzweig von ap_switch (Mismatch nach dem Umschalten) war
im bisherigen Test nie erreichbar; der nmcli-Stub kann das jetzt
erzwingen.
- I4: neue Pflichtschluessel blockierten jeden Unterbefehl auf einem
bereits eingerichteten Pi. setup_pi.sh ergaenzt fehlende Schluessel in
einer bestehenden Konfiguration jetzt einzeln, statt sie unangetastet
zu lassen; dafuer musste das Skript per bash-s-Heredocs und
ueberschreibbaren Pfadvariablen erst testbar gemacht werden (neuer
test_setup_pi.sh).
Dazu vier kleinere Punkte aus derselben Review: grep -Fqx statt grep -qx
fuer Profilnamen, install-Stub durch tee+chmod ersetzt (POSIX, kein
Sonderfall mehr fuer macOS), Rechtepruefung auf control.json in den
arm/listen-Testfaellen, stderr sauber von stdout getrennt in den
Warnungs-/Fehlerfaellen.
ninlight-station bekommt recovery/migrate/arm/listen als Unterbefehle statt nur
on/off/status; "on" bleibt Kurzform fuer "recovery on". Eine gemeinsame
ap_switch()-Funktion faehrt vor jedem Wechsel erst jedes andere Stationsprofil
herunter und prueft per nmcli nach, dass auf wlan0 wirklich nur eines aktiv
ist - die beiden Zugangspunkte (Rettung/Migration) schliessen sich damit
gegenseitig aus. Der Modus wandert per /run/ninlight-station/mode an den
(ohnehin neu gestarteten) Dienst statt ueber die Konfigurationsdatei; arm/listen
schreiben die Steuerdatei aus Task 3 mit demselben atomaren 0644-Muster wie
server.py selbst. station.conf.example und die Pflichtschluesselpruefung
bekommen die neuen Migrations- und Anzeige-Schluessel dazu.
Fixrunde 1 zu Task 3 (Migrationsroute):
- C1 (Critical): read_control()/Phasenpruefung/Auslieferung/write_control("listen") liefen
ohne gemeinsame Sperre - zwei gleichzeitige Anfragen (zwei Lampen, oder Erst- und
Range-Resume-Verbindung derselben Lampe) konnten beide phase == "armed" sehen und beide
ausgeliefert bekommen. Neues Station.claim_delivery() liest und verbraucht die Freigabe
jetzt atomar unter station.lock, vor Beginn der Auslieferung; Station.restore_armed()
macht das bei einem Abbruch rueckgaengig.
- I2 (Important): eine unerwartet kurz gelesene Datei zaehlte im Sendeloop als "ok" statt
als "aborted" - relevant geworden, weil dieser Status jetzt entscheidet, ob eine armed-
Freigabe restauriert wird.
- I3 (Important): ein fehlschlagendes write_control() (z.B. schreibgeschuetztes /run)
verschluckte den Fehler und liess "armed" dauerhaft scharf. Der Fehler landet jetzt in
status.json (control_error), und claim_delivery() laesst nur die eine ausloesende
Anfrage noch durch, bis ein Schreibversuch wieder gelingt.
- I4 (Important): die neuen Server-Tests synchronisierten sich per fester Pause statt wie
Station.read_state() bis zu einem Praedikat zu pollen. status()/read_control() im
Testklienten tun das jetzt ebenfalls.
- I5 (Important): resolve_mode()/--mode-file waren ungetestet trotz bindender
Vorrangregel, auf der Task 4 aufbaut - fuenf neue reine Funktionstests.
Dazu: recent[0] -> recent[-1] in allen betroffenen Tests (Brief-Fehler), ein neuer
Rechte-Test fuer control.json (0644 unter restriktivem umask), und eine neue
Nebenlaeufigkeits-Regression fuer C1.
server.py bedient jetzt zwei Modi ueber denselben Dienst: --mode/--mode-file/--control
legen fest, ob ninlight_<target>_ota.img (Rettung) oder qcx_firmware_<pid>_..._ota_light.img
(Migration alter Klyqa-Lampen, via products.parse_request) ausgeliefert wird. Der Modus
kommt wahlweise aus einer Datei statt einem Startparameter, damit Task 4 ihn spaeter ohne
Dienst-Neustart umschalten kann. Die Steuerdatei control.json (Phase auto/listen/armed)
wird bei jeder Anfrage frisch gelesen und atomar mit 0644 geschrieben - eine fehlende oder
kaputte Steuerdatei blockiert eine Migration nie, sondern faellt auf "auto" zurueck. Nach
einer erfolgreichen Auslieferung in Phase "armed" faellt die Freigabe erst nach dem
vollstaendigen Senden auf "listen" zurueck, nie davor, damit ein abgebrochener Transfer die
Freigabe fuer den naechsten Bootversuch stehen laesst. status.json bekommt die
Migrationsfelder aus Spec 5.2 (mode, phase, armed_target, last_seen).
deploy.sh liefert products.py mit aus, sonst startet der Dienst nach dem naechsten Deploy
mit ModuleNotFoundError; test_deploy.sh zieht die Sandbox-Dummy-Datei entsprechend nach.
Neue Tests in test_server.py verwenden eigene server/migration_server-Fixtures samt
Testklient (get/status/set_control/read_control/delete_control/write_raw_control), ohne die
bestehende station-Fixture und ihre Tests anzufassen.
CLI und beide Skripte (setup_pi.sh, deploy.sh) rufen sudo nicht-interaktiv
auf; ohne passwortloses sudo fuer den SSH-Benutzer bricht der gesamte
Ablauf mit einem Passwort-Prompt ab, den nichts davon behandeln kann. Das
war bisher eine unausgesprochene Voraussetzung.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
do_POST (und PUT/DELETE/HEAD) riefen reject() auf, ohne eine Logzeile zu
schreiben, und das ueberschriebene log_message() legte damit unbeabsichtigt
auch log_error() lahm (BaseHTTPRequestHandler ruft es standardmaessig auf,
z.B. bei einer kaputten Request-Zeile oder einem Timeout, bevor ueberhaupt
ein Handler wie do_GET erreicht wird). Fuer das einzige Diagnosewerkzeug im
Rettungsfall darf keine Anfrage stumm bleiben.
do_POST loggt jetzt wie do_GET explizit, und log_error() ist separat
ueberschrieben statt implizit mit log_message() stillgelegt zu werden.
Die neuen Tests fuer den 405-Fall lesen den rohen Datei-Deskriptor des
Server-Prozesses statt ueber TextIOWrapper.readline() zu gehen - Letzteres
kann mehr Zeilen aus der Pipe in den eigenen Puffer lesen, als es
zurueckgibt, was select() auf dem naechsten Datei-Deskriptor faelschlich
"nichts da" melden liess. Der Test fuer die kaputte Request-Zeile ruft
log_error() direkt auf: ein echter kaputter Rohsocket wird in dieser
Sandbox von einer Netzwerkschicht abgefangen, bevor er den Server erreicht
(sichtbar an einem untergeschobenen "Server: aiohttp"-Header).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Die Spec (§9) verlangt beide Betriebsregeln in der Ausgabe von "on": die
Erinnerung ans Ausschalten stand schon da, die zum eigenen Passwort fehlte.
Bleibt PSK auf dem oeffentlichen Default 'ninlight-recovery' stehen, kann
jeder in Funkreichweite, der ihn kennt, jeder Lampe beliebige Firmware
unterschieben - das wird jetzt bei jedem Einschalten sichtbar gemacht.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Zwei Build-Verzeichnisse mit demselben Image-Dateinamen (z.B. build/evk und
build/evk-onb, letzteres ohne WLAN-Zugangsdaten) landeten flach im selben
Staging-Namen - die Sortierreihenfolge entschied zufaellig, welches gewinnt,
und die Trailer-Pruefung merkt nichts, weil in beiden derselbe Geraetetyp
steht. deploy.sh bricht jetzt ab und nennt beide Fundorte, statt zu raten.
Der Voll-Deploy (deploy.sh <host> ohne target - der in README.md gezeigte
Normalfall) loeschte per rsync --delete zudem die Images aller gerade nicht
gebauten Geraetetypen, ohne je zu sagen, welche. Vor dem Loeschen wird jetzt
per Namensvergleich ermittelt und ausgegeben, welche Dateien verschwinden -
sichtbar statt still, ohne den Ablauf fuer Skripte zu blockieren.
Neue Testszenarien 7-9 in tools/station/tests/test_deploy.sh decken beide
Faelle ab.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Der Testschalter zählte bisher pro Anfrage, sodass eine Lampe nach ihrem
Empfangs-Timeout einfach per Range neu verband und ein frisches Budget bekam
– am EVK reichten fünf Reconnects, um trotz --stall-after das komplette
Image zu übertragen. Station verfolgt die ausgelieferten Bytes jetzt pro
Client-IP über alle Anfragen hinweg (Station.reserve_stall_bytes, unter dem
bestehenden Lock), sodass kein erneutes Verbinden das Budget auffüllt.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
server.py legt status.json ueber mkstemp() (Modus 0600) + os.replace() an; da
os.replace() den Modus der temporaeren Datei uebernimmt, war die Datei nach jedem
Schreiben nur fuer root lesbar - der Dienst laeuft als root, das CLI liest aber als
normaler Benutzer. os.chmod(tmp, 0o644) vor dem Rename behebt das umask-fest, auch
bei wiederholten Schreibvorgaengen.
ninlight-station status hat den fehlenden Lesezugriff bisher stillschweigend wie
"noch nie ausgeliefert" behandelt (beides fiel durch denselben "-r"-Test). Jetzt wird
zwischen "Datei fehlt" (unauffaellig) und "Datei da, aber nicht lesbar" (Fehlermeldung
auf stderr) unterschieden.
Neuer Test prueft, dass status.json nach dem ersten und nach einem weiteren
Schreibvorgang world-readable bleibt, auch unter restriktivem umask.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
tools/mkimage/uv.lock ist getrackt, tools/station/uv.lock war es nicht -
ohne die Datei sind die Testlaeufe der Station nicht reproduzierbar.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Auf dem Rechner des Nutzers liegt im PATH ein ssh, das sich anders
verhaelt als das System-ssh - die Skripte sollen sich darauf nicht
verlassen. setup_pi.sh und deploy.sh riefen ssh, scp und rsync bisher
ohne vollen Pfad auf.
Beide Skripte nutzen jetzt SSH_BIN/SCP_BIN mit Default /usr/bin/ssh bzw.
/usr/bin/scp, ueberschreibbar per NINLIGHT_SSH_BIN/NINLIGHT_SCP_BIN -
dasselbe Prinzip wie NINLIGHT_STATION_ROOT weiter unten in deploy.sh, das
Tests ohne echten Pi ermoeglicht. rsync bekommt "-e $SSH_BIN", damit auch
dessen Transport ueber /usr/bin/ssh laeuft. Die Beispielaufrufe in
setup_pi.sh's Abschlussmeldung und in der README bleiben bewusst bei
schlichtem "ssh" - dort geht es um das, was der Nutzer selbst eintippt.
test: tools/station/tests/test_deploy.sh auf den neuen Override
umgestellt (NINLIGHT_SSH_BIN zeigt auf den ssh-Stub) und um ein Szenario
ergaenzt, das den /usr/bin/ssh- bzw. /usr/bin/scp-Default in beiden
Skripten absichert.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
"systemctl is-active" schreibt den Zustand ("inactive", ...) auf stdout
und liefert dabei einen Rueckgabewert ungleich 0 - der "|| echo inaktiv"-
Zweig lief also zusaetzlich mit, und beide Zeilen landeten in der
Ausgabe. Ausserdem endete "status" ueber den letzten Befehl im
case-Zweig faktisch mit dessen Exit-Code, der 1 war, sobald noch nie
ausgeliefert wurde (kein /run/ninlight-station/status.json) - das machte
"status" in jedem Skript, das den Exit-Code abfragt, unbrauchbar.
Beide Stellen behoben: der Dienstzustand wird nur noch einmal ausgegeben,
und der Zweig endet mit explizitem "exit 0", sobald die Auskunft erteilt
werden konnte.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
setup_pi.sh sourcte /etc/ninlight-station.conf (0600 root:root) als
unprivilegierter Benutzer und scheiterte damit bei jedem Lauf, bevor
das NetworkManager-Profil angelegt wurde. Die Konfiguration wird jetzt
per "sudo cat" gelesen; ninlight-station macht denselben Fallback fuer
den dokumentierten Aufruf als normaler Benutzer, ohne die Datei fuer
alle lesbar zu machen.
Das AP-Profil wird nicht mehr per "nmcli ... wifi-sec.psk $PSK"
angelegt (das WLAN-Passwort waere sonst fuer jeden lokalen Prozess in
der Kommandozeile sichtbar), sondern als NetworkManager-Keyfile
(0600 root:root) geschrieben und per "nmcli connection load"
eingelesen.
deploy.sh loeschte bei einem gezielten Deploy eines einzelnen
Geraetetyps per "rsync --delete" alle Images anderer Geraetetypen aus
dem live ausgelieferten Verzeichnis. --delete laeuft jetzt nur noch
beim Vollabgleich (kein target-Argument); ein gezielter Deploy ersetzt
nur die genannten Dateien. Dabei zusaetzlich -c (Checksumme statt
Groesse+Zeitstempel) ergaenzt, sonst uebersieht rsync ein neues Image
mit zufaellig gleicher Byte-Groesse. Ausserdem meldet deploy.sh jetzt
einen fehlgeschlagenen Dienst-Neustart als Fehler statt ihn mit "Dienst
laeuft nicht" zu verwechseln.
Stub-Tests unter tools/station/tests/test_deploy.sh (ssh/rsync im PATH
ersetzt, Installationsschritt laeuft gegen ein Sandbox-Verzeichnis)
decken beide Bugfixes sowie die bereits bestehenden Faelle ab: Image
besteht die Trailer-Pruefung nicht, keine Images gefunden.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
setup_pi.sh ist idempotent und laesst eine vorhandene Konfiguration in
Ruhe; deploy.sh prueft jedes Image mit nl_mkimage --verify, bevor es
etwas uebertraegt.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- 'on' fing bisher nur einen bereits laufenden, aber toten Dienst ab. Scheiterte
schon der Aufruf "systemctl start" selbst (z.B. start-limit-hit nach einer
Absturzserie durch Restart=always), beendete set -eu das Skript dort, bevor
die Lebendigkeitspruefung je erreicht wurde - der eben hochgefahrene
Zugangspunkt blieb offen ohne Server dahinter stehen. Der Startaufruf wird
jetzt selbst abgefangen und faehrt im Fehlerfall dieselbe Rueckabwicklung wie
die is-active-Pruefung. "systemctl reset-failed" vor dem Start verhindert,
dass eine fruehere Absturzserie den naechsten ehrlichen Versuch blockiert.
- Stirbt der Dienst nach einem erfolgreichen Start, stoppt das Skript ihn jetzt
explizit, bevor es den Zugangspunkt herunterfaehrt und "abgeschaltet" meldet -
sonst haette ein mit Restart=always weiterlaufender Dienst im Hintergrund
einen saubereren Zustand suggeriert, als tatsaechlich vorlag.
- ninlight-station.service setzt StartLimitIntervalSec/StartLimitBurst jetzt
explizit statt sich stillschweigend auf systemds Vorgabewerte zu verlassen,
mit Begruendung als Kommentar: ohne Zugangspunkt kann server.py nicht binden
und stirbt sofort, Restart=always soll das nach kurzer Zeit ehrlich aufgeben
statt das Journal zu fluten.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- 'on' prueft nach dem Start wirklich, ob der Dienst aktiv ist (Type=simple
meldet sonst schon Erfolg, wenn der Prozess nur gestartet wurde), und faehrt
den Zugangspunkt bei einem toten Server wieder herunter statt ihn offen mit
bekanntem Passwort ohne Server dahinter stehen zu lassen.
- 'off' faengt einen fehlschlagenden Wechsel zurueck ins Heim-WLAN ab statt
unter set -eu mit nmclis Rohausgabe abzubrechen.
- Fehlende Schluessel in der Konfiguration werden nach dem Sourcen explizit
geprueft und mit einem Verweis auf setup_pi.sh gemeldet statt mit
"parameter not set" abzubrechen.
- station.conf.example verwendet einfache statt doppelter Anfuehrungszeichen:
POSIX ". datei" wertet $ und ` auch in doppelten Anfuehrungszeichen aus,
ein Passwort mit "$1" waere sonst lautlos durch ein Shell-Argument ersetzt
worden. systemds EnvironmentFile= fuehrt ohnehin nie eine Shell aus und
entfernt einfache wie doppelte Anfuehrungszeichen gleichermassen.
- ninlight-station.service: BindsTo=NetworkManager.service entfernt (ein
dadurch ausgeloester Stop gilt fuer systemd als sauber und wird von keiner
Restart=-Politik erneut gestartet - ein NetworkManager-Neustart haette den
Server sonst still und dauerhaft mitgerissen); stattdessen nur
After=NetworkManager.service fuer die Startreihenfolge und Restart=always,
damit der Prozess nach einem Absturz (z.B. Bind-Fehler) automatisch
wiederkommt.
- Trailer-Reader im 'status'-Zweig nutzt jetzt errors="replace" wie
trailer_version() in server.py, statt bei nicht-ASCII-Bytes im
Versionsfeld abzustuerzen.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Die Station ist bewusst nicht dauerhaft an - 'ninlight-station on'
schaltet Zugangspunkt und Dienst gemeinsam ein, 'off' schaltet wlan0
zurueck ins Heim-WLAN. Der Dienst laeuft als root (Port 80 und die
DHCP-Leases fuer 4c) und bekommt dafuer systemd-Einschraenkungen.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fix-Runde 1 nach Review: Station.served und Station.recent wurden ohne
Lock von mehreren Handler-Threads (ein Thread pro Verbindung unter
ThreadingHTTPServer) mutiert - der Normalfall, wenn mehrere Lampen
gleichzeitig laden. Ein threading.RLock schuetzt jetzt Mutation und
Serialisierung nach status.json als eine Einheit.
Ausserdem hatte mein vorheriger Fix fuer das Status-Race einen neuen
Fehler eingefuehrt: "ok" wurde vor dem letzten wfile.write() vermerkt,
sodass ein danach scheiternder Schreibvorgang trotzdem als Erfolg in
status.json stand. record() laeuft jetzt erst, nachdem die Uebertragung
tatsaechlich abgeschlossen oder gescheitert ist; die betroffenen Tests
warten stattdessen auf den Eintrag statt sofortige Konsistenz
vorauszusetzen.
test_stall_after_stops_sending_but_keeps_the_connection prüfte bisher
nur die gesendete Bytezahl und haette einen faelschlich schliessenden
Server nicht bemerkt - ergaenzt um eine explizite Pruefung, dass ein
weiterer recv() in einen Timeout laeuft statt EOF zu liefern.
Neuer Test test_concurrent_downloads_are_all_recorded faehrt 20
gleichzeitige Downloads und prueft served/recent auf Vollstaendigkeit;
er schlaegt zuverlaessig fehl, wenn das Lock entfernt wird (lokal
gegengeprueft: 3 von 10 Laeufen ohne Lock verloren Eintraege, 10 von 10
mit Lock bestanden).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Liefert ausschliesslich Images aus, deren Name dem Muster
ninlight_<target>_ota.img entspricht, unterstuetzt Range-Anfragen fuer
den Wiederaufnahmeversuch der Firmware und schreibt seinen Zustand nach
status.json - die Schnittstelle fuer die E-Paper-Anzeige in 4c.
Behebt zwei Races aus der Referenzimplementierung des Task-Briefs: der
Server trug Erfolg/404 erst nach dem letzten gesendeten Byte in
status.json ein, wodurch ein Client, der die Antwort vollstaendig
gelesen hat, noch den alten Zustand sehen konnte. Ausserdem wurde ein
ganzer 32-KiB-Chunk auch dann komplett gesendet, wenn --stall-after
schon ueberschritten war; die Lesegroesse wird jetzt an die Stall-Grenze
gekappt, und ein Verbindungsabbruch waehrend des Stalls wird per
select/MSG_PEEK erkannt und als "aborted" vermerkt statt den Thread
endlos zu blockieren.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
