[PATCH 1/2] nodewatcher: split up

Adrian Schmutzler mail at adrianschmutzler.de
Mo Okt 7 20:33:54 CEST 2019


Hallo,

Kommentare zu den Kommentaren unten:

> > +debug() { 
> > +    (>&2 echo "$1") 
> Möchte man das $(date) vielleicht noch mit in diese Funktion nehmen? 

Fände ich gut.

> > +IFACEBLACKLIST="lo ifb0" 
> > +IPWHITELIST="br-mesh" 
> Hat es einen Grund, warum die Listen nicht mehr in der uci-Konfiguration stehen? 
> Falls ja: -> Commitmessage 
> Ich fand die Konfigurierbarkeit eigentlich recht angenehm, so konnte man selbst entscheiden, ob man die IPs von Interfaces im Monitoring haben möchte.

Ich habe das tatsächlich nie benutzt, insofern habe ich kein Problem mit dem hardcoden. Wenn man das in uci haben möchte, kann man das aber auch ohne weiteres durch ein zusätzliches uci-default im jeweiligen Paket lösen.
Für den konkreten Fall würde ich die hardgecodeten Variablen erstmal behalten und dann mit einem separaten Patch wieder uci einführen, sonst doktern wir ewig an dem Patch rum.
BTW: Die nodewatcher config ist nicht mal updatefest, oder? Könnte man auch mal ändern.

> > +MESH_INTERFACE=br-mesh 

> Siehe oben, nicht mehr konfigurierbar. 

Hier finde ich ein uci setting nicht wirklich gerechtfertigt. Wer das ändert, baut relativ viel an der Firmware um, da kann man dann auch den Wert im Skript hier ändern.

Wie auch oben festgestellt würde ich das aber ohnehin für diesen Patch belassen.

> > -if [ -f /etc/config/nodewatcher ];then 
> > -    SCRIPT_ERROR_LEVEL=$(uci get nodewatcher. at script[0].error_level) 
> > -    SCRIPT_LOGFILE=$(uci get nodewatcher. at script[0].logfile) 
> > -    SCRIPT_DATA_FILE=$(uci get nodewatcher. at script[0].data_file) 
> > -    MESH_INTERFACE=$(uci get nodewatcher. at network[0].mesh_interface) 
> > -    IFACEBLACKLIST=$(uci get nodewatcher. at network[0].iface_blacklist) 
> > -    IPWHITELIST=$(uci get nodewatcher. at network[0].ip_whitelist) 
> > -    SCRIPT_STATUS_FILE=$(uci get nodewatcher. at script[0].status_text_file) 
> > -else 
> > -    . "$(dirname "$0")/nodewatcher_config" 
> > -fi 
> Es gab mal Bestrebungen den nodewatcher auch unter nicht-OpenWRT lauffähig zu machen. 
> Daher kommt vermutlich auch dieser Test. 
> Möchte man das weiter verfolgen? Gute Frage eigentlich, wie man das gut machen kann, da der Nodewatcher jetzt über mehrere Packages verteilt ist..

Nun ja, die Frage ist, ob dieser alternative Nodewatcher mit in die Firmware eingebaut werden muss. Ich finde: Nein.

Ich denke, hierfür muss es eigene _separate_ Skripte geben, die dann auch besser auf die jeweiligen Bedürfnisse abgestimmt sind (so wie es ja auch im Moment ist).
Ich war eigentlich froh, dass dieser Sonderfall gleich mit rausgeflogen ist.

Für meinen Geschmack wurde hier für die aktuelle Situation genau an den richtigen Stellen geändert oder belassen.

Grüße

Adrian
-------------- nächster Teil --------------
Ein Dateianhang mit Binärdaten wurde abgetrennt...
Dateiname   : openpgp-digital-signature.asc
Dateityp    : application/pgp-signature
Dateigröße  : 834 bytes
Beschreibung: nicht verfügbar
URL         : <https://lists.freifunk.net/pipermail/franken-dev-freifunk.net/attachments/20191007/40df7eeb/attachment.sig>


Mehr Informationen über die Mailingliste franken-dev