FHEM - Entwicklung > Perl Ecke

72_UBUS.pm Code Review / Feedback

(1/11) > >>

xenos1984:
Ich arbeite gerade an einem neuen Modul, das ich - nach kritischer Prüfung - gerne unter FHEM einchecken und damit auch betreuen möchte. Es dient dem Zugriff auf uBus:

http://openwrt.org/docs/guide-developer/ubus

Neben der dort angegebenen Kommandozeile und HTTP als Zugriffsmethode habe ich auch Websocket implementiert, wie es u.a. von JUCI benutzt wird:

https://github.com/mkschreder/juci

Zum selbst testen habe ich aber leider nur letzteres auf meinem Router zur Verfügung.

Vielleicht mag jemand einen Blick auf den Code werfen und Kritik daran üben, die ich gerne entgegennehme und verbessere. Falls jemand Hardware zum testen hat, ist das natürlich noch besser. An der Dokumentation arbeite ich noch; zumindest die Definition und die notwendigen Attribut / Set Werte sind drin, falls jemand vorab testen möchte.

Das Modul fragt in regelmäßigen Abständen verschiedene Daten ab (Systeminfo, darunter Speicher und CPU Auslastung; verbundene Clients; Netzwerk-Ports und deren Datenvolumen; Wifi-Schnittstellen; Netzwerk-Interfaces; Telefonleitungen) und erzeugt entsprechende Readings. Ich arbeite noch an weiteren Set-Befehlen, z.B. zum Reboot und zum setzen von Einstellungen, aktivieren / deaktivieren von WAN und W-LAN etc.

Die 72_ habe ich in Anlehnung an 72_FRITZBOX.pm genommen - die Funktion sollte ähnlich sein.

Beta-User:
Einen vollständigen review habe ich nicht gemacht, hier mal eine Version mit ein paar (teils exemplarischen) Änderungen, die mir sinnvoll erschienen. Bitte ggf. nachfragen.

Hardware habe ich keine, hatte ein websocket-Testgerät definiert, und das schreibt ziemlich häufig fehlgeschlagene Verbindungsversuche ins log...

xenos1984:

--- Zitat von: Beta-User am 31 August 2021, 18:12:29 ---Einen vollständigen review habe ich nicht gemacht, hier mal eine Version mit ein paar (teils exemplarischen) Änderungen, die mir sinnvoll erschienen. Bitte ggf. nachfragen.

--- Ende Zitat ---

Danke für das Feedback! Ein paar Nachfragen habe ich bestimmt, wenn ich die Änderungen im Detail durchgegangen bin.


--- Zitat ---Hardware habe ich keine, hatte ein websocket-Testgerät definiert, und das schreibt ziemlich häufig fehlgeschlagene Verbindungsversuche ins log...

--- Ende Zitat ---

Hm... Ja, ich habe ein paar Meldungen mit verbose=1 ("error messages or unknown packets") drin, wenn etwas fehlschlägt, insbesondere der Verbindungsaufbau. Ich hatte mich da im Wesentlichen am Wiki-Beispiel zu DevIo orientiert, aber vielleicht kann man die Anzahl der Verbindungsversuche oder die Häufigkeit reduzieren, wenn keine Antwort kommt.

Beta-User:

--- Zitat von: xenos1984 am 31 August 2021, 22:11:03 ---Danke für das Feedback! Ein paar Nachfragen habe ich bestimmt, wenn ich die Änderungen im Detail durchgegangen bin.

--- Ende Zitat ---
Gerne.

Ergänzend vielleicht noch:

--- Code: ---return DevIo_OpenDev($hash, 0, "FHEM::UBUS::Init", "FHEM::UBUS::Callback") if !DevIo_IsOpen($hash);
--- Ende Code ---
müßte eigentlich auch funktionieren, wenn man Referenzen übergibt:

--- Code: ---return DevIo_OpenDev($hash, 0, \&Init, \&Callback) if !DevIo_IsOpen($hash);
--- Ende Code ---

Weiter unten kommen dann ja ziemlich viele (teilweise noch auskommentierte) readingsBulkUpdate-Zeilen. Bis auf wenige Ausnahmen entsprechen da die Teilstrings für die Readingnamen dem, was aus decode_json an keys kommt. Die Teile könnte man in eine Schleife packen.


--- Zitat ---Hm... Ja, ich habe ein paar Meldungen mit verbose=1 ("error messages or unknown packets") drin, wenn etwas fehlschlägt, insbesondere der Verbindungsaufbau. Ich hatte mich da im Wesentlichen am Wiki-Beispiel zu DevIo orientiert, aber vielleicht kann man die Anzahl der Verbindungsversuche oder die Häufigkeit reduzieren, wenn keine Antwort kommt.

--- Ende Zitat ---
Na ja, ich hatte pro Sekunde einige Zeilen nach dem Muster

--- Code: ---<timestamp> 3: Opening <UBUS-Instanz-Name> device ws:<ip>
--- Ende Code ---
Habe aber nicht untersucht, wo das im Detail herkommt; u.A. war das der Hintergrund, warum ich den default für $interval von 0 auf 60 geändert hatte, hat aber nicht geholfen...

Generell fehlt gefühlt bei der Initialisierung eine Option, das Gerät (vorübergehend) zu deaktivieren - "none" als Interface, attr disabled, set inactive, IsDisabled() usw.. Evtl. übersehe ich da aber auch was.

xenos1984:

--- Zitat von: Beta-User am 01 September 2021, 10:02:34 ---Na ja, ich hatte pro Sekunde einige Zeilen nach dem Muster

--- Code: ---<timestamp> 3: Opening <UBUS-Instanz-Name> device ws:<ip>
--- Ende Code ---
Habe aber nicht untersucht, wo das im Detail herkommt; u.A. war das der Hintergrund, warum ich den default für $interval von 0 auf 60 geändert hatte, hat aber nicht geholfen...

--- Ende Zitat ---
Ah... Ich denke, ich habe es gefunden. Das macht DevIo.pm:


--- Code: ---427   my $l = $hash->{devioLoglevel}; # Forum #61970
428   Log3 $name, ($l ? $l:3), ($hash->{DevioText} ? $hash->{DevioText} : "Opening").
429        " $name device ". (AttrVal($name,"privacy",0) ? "(private)" : $dev)
430        if(!$reopen);

--- Ende Code ---

Üblicherweise also auf verbose=3. Das kommt mir doch etwas arg vor, ich würde es eher auf 4 setzen (indem ich $hash->{devioLoglevel} = 4 im Modul einbaue).


--- Zitat ---Generell fehlt gefühlt bei der Initialisierung eine Option, das Gerät (vorübergehend) zu deaktivieren - "none" als Interface, attr disabled, set inactive, IsDisabled() usw.. Evtl. übersehe ich da aber auch was.

--- Ende Zitat ---

Ja, gute Idee, das habe ich in der Tat nicht bedacht - das baue ich auf jeden Fall ein.

Navigation

[0] Themen-Index

[#] Nächste Seite

Zur normalen Ansicht wechseln