72_UBUS.pm Code Review / Feedback

[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]

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.

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

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

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]

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

Gerne.

Ergänzend vielleicht noch:

Code Auswählen Erweitern

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

Code Auswählen Erweitern

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

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.

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.

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

Code Auswählen Erweitern

<timestamp> 3: Opening <UBUS-Instanz-Name> device ws:<ip>
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]

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

Code Auswählen Erweitern

<timestamp> 3: Opening <UBUS-Instanz-Name> device ws:<ip>
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...

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

Code Auswählen Erweitern


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);


Ü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).

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.

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

[Beta-User]

Bzgl. DevIo: eigentlich sind dort für neue Anläufe 60 Sekunden hinterlegt, wenn ich den Code richtig interpretiere. Evtl. ist das auch ein spezielles Wind.*-Thema - das Testsystem läuft auf win32-strawberry-Perl (5.32)....

Verbose 3 finde ich dagegen ok, man kann das ja einstellen; mir kommen eher die vielen Versuche komisch vor.

[xenos1984]

Ich habe mal etwas nachgeforscht und probiert, was bei mir (Ubuntu 20.04) passiert, wenn ich eine nicht vorhandene Websocket-Adresse eingebe. Gleiches Ergebnis, Verbindungsversuch einmal pro Sekunde. Und ich konnte auch den Grund finden. So häufig wird nämlich Ready aufgerufen, und das versucht dann, mittels Connect die Verbindung wieder aufzubauen. Dabei habe ich mich nur an das Schema aus dem Beispiel gehalten...

http://wiki.fhem.de/wiki/DevIo#TCP.2FIP-Verbindung

Code Auswählen Erweitern


# called repeatedly if device disappeared
sub MY_MODULE_Ready($)
{
  my ($hash) = @_;
 
  # try to reopen the connection in case the connection is lost
  return DevIo_OpenDev($hash, 1, "MY_MODULE_Init", "MY_MODULE_Callback");
}


Ich werde mal sehen, was andere Module da machen. Eine Möglichkeit, Verbindungsversuche komplett abzuschalten, baue ich aber auf jeden Fall auch noch ein.

Inzwischen bin ich auch deine anderen Kommentare durchgegangen und habe sie im wesentlichen umgesetzt. Meine ursprünglichen Nachfragen haben sich beim Nachschlagen in PBP auch geklärt

parseParams werde ich mir auch noch ansehen. Für den Anfang hatte ich mich noch nicht darin vertieft, weil es kaum Parameter zu parsen gab, aber inzwischen sind es bei define und set doch ein paar mehr geworden, da erscheint es sinnvoll.

Eine neue Version kommt, sobald ich o.g. eingebaut habe.

[xenos1984]

Hier ist erst einmal der aktuelle Stand. Ein paar Funktionen habe ich umstrukturiert, unnötige Wiederholungen durch Schleifen ersetzt und ein paar mehr Perl-Konventionen beachtet. Außerdem habe ich jetzt attr disable und set disable / enable eingebaut, um das Modul zu deaktivieren.

Zum Problem mit den häufigen Verbindungsversuchen habe ich noch keine schlüssige Lösung. So weit ich sehe, benutzt sonst nur 69_SoftliqCloud.pm WebSocket mit DevIo, und da macht Ready nichts - die Logik zum Verbinden sitzt anderswo. Wenn ich es aber ganz aus Ready rausnehme, passiert nach einem fehlgeschlagenen Verbindungsversuch nichts mehr, auch nicht nach 60 Sekunden. Vielleicht kann Rudi etwas dazu sagen, wie hier die beabsichtigte Vorgehensweise ist für Module, die DevIo benutzen?

parseParams sehe ich mir noch im Detail an und baue es dann ein - noch ist es nicht drin.

[Beta-User]

...bislang ist mir beim auf den Code starren auch nichts aufgefallen, an was es ggf. hängt...

Evtl. gibt es (allerdings nicht mit ws) einen reconnect-loop-"Zwilling": https://forum.fhem.de/index.php/topic,122767.msg1173072.html#msg1173072 (müßte dafür aber auch erst mal ein passendes Testsystem aufsetzen, und eigentlich ist MQTT-old auch nicht meine bevorzugte Baustelle...)

(Fast) ganz aus ready raus klingt aber uU. doch nach einem Ansatzpunkt, die Frage ist dann aber, wer nach einem fehlgeschlagenen Versuch dann (wann?) den nächsten initiieren sollte. So beim Überfliegen (!) hatte ich den Eindruck, dass die ganze Timer-Logik eigentlich auf DevIo-Seite liegen soll, aber das kann auch täuschen...

Die reconnects sind jedenfalls bei mir im Testsystem ca. Faktor 8-12 häufiger, an "sekündlich" glaube ich also noch nicht als Frequenz.

Falls du ein Implementierungsbeispiel für parseParams (an "allen Ecken") suchst: RHASSPY (contrib)

Ansonsten wäre "perltidy" noch ein Stichwort. Tabulatoren sind grausam...

[xenos1984]

...bislang ist mir beim auf den Code starren auch nichts aufgefallen, an was es ggf. hängt...

Evtl. gibt es (allerdings nicht mit ws) einen reconnect-loop-"Zwilling": https://forum.fhem.de/index.php/topic,122767.msg1173072.html#msg1173072 (müßte dafür aber auch erst mal ein passendes Testsystem aufsetzen, und eigentlich ist MQTT-old auch nicht meine bevorzugte Baustelle...)

(Fast) ganz aus ready raus klingt aber uU. doch nach einem Ansatzpunkt, die Frage ist dann aber, wer nach einem fehlgeschlagenen Versuch dann (wann?) den nächsten initiieren sollte. So beim Überfliegen (!) hatte ich den Eindruck, dass die ganze Timer-Logik eigentlich auf DevIo-Seite liegen soll, aber das kann auch täuschen...

Eine Möglichkeit wäre, bei jedem Verbindungsversuch einen Zeitstempel zu merken, und in Ready nur dann einen neuen Versuch zu unternehmen, wenn eine (setzbare) Wartezeit abgelaufen ist. Ich werde das mal versuchen.

Die reconnects sind jedenfalls bei mir im Testsystem ca. Faktor 8-12 häufiger, an "sekündlich" glaube ich also noch nicht als Frequenz.

Mit was versuchst du denn zu verbinden? Ich habe ws://<nicht vorhandene IP> genommen - vielleicht braucht es dann einfach eine Sekunde, bis mein Router meldet, dass es die IP nicht gibt. Ich werde das aber auch noch mal mit Wireshark o.ä. anschauen, ob das die Frequenz erklärt.

Falls du ein Implementierungsbeispiel für parseParams (an "allen Ecken") suchst: RHASSPY (contrib)

Danke! Das sieht ja recht überschaubar aus. Ich werde mal schauen, wie gut es mit der Struktur von define und set bei meinem Modul zusammenpasst. Benannte Parameter habe ich bislang nicht, aber unbenannte Parameter trennen ist schon praktisch (wenn es nicht an ungewollten Stellen passiert). Aber so weit ich sehe, würde es wohl auch einen JSON-Block intakt lassen (den übergebe ich derzeit mit set <name> rpc <...> zum Testen neuer Funktionsaufrufe).

Ansonsten wäre "perltidy" noch ein Stichwort. Tabulatoren sind grausam...

Das ist vermutlich eine viel diskutierte Frage Und wohl eine, auf die jeder Programmierer (auch wenn ich mich nicht so nennen würde) seine eigene Antwort hat. Ja, PBP und perltidy empfehlen Leerzeichen, weil man dann immer die gleiche Einrückung bekommt. Anderenorts bevorzugt man Tabs, und überlässt die angezeigte Breite der Einrückung dem Editor bzw. dem dort vom Benutzer eingestellten Wert... Meine Prämisse ist, konsistent vorzugehen, eine Einrückungsebene immer gleich zu formatieren und nicht mit anderen zu mischen (d.h. entweder nur Tabulatoren oder nur Leerzeichen, nie beides), und bevorzugt "eine Ebene = ein Zeichen". Aber wie gesagt, ich denke, das ist eine Frage der Philosophie, und wenn ich mir die existierenden Module ansehe, kommt wohl beides vor (teilweise nicht konsistent und gemischt - das finde ich grausam...).

[Beta-User]

Eine Möglichkeit wäre, bei jedem Verbindungsversuch einen Zeitstempel zu merken, und in Ready nur dann einen neuen Versuch zu unternehmen, wenn eine (setzbare) Wartezeit abgelaufen ist. Ich werde das mal versuchen.

Hmm, setzt halt voraus, dass der "flow control" von UBUS aus gemacht wird; eigentlich war ich davon ausgegangen, dass das DevIo machen sollte und nur die timeout-Parameter ggf. beizusteuern wären.

Habe gestern noch ein wenig mit 00_MQTT gespielt und dabei teilweise die return-Werte von $callback (\&Ready) und \&Start geändert, z.B. von

Code Auswählen Erweitern

  DevIo_CloseDev($hash);
  return DevIo_OpenDev($hash, 0, "MQTT::Init");
}
nach

Code Auswählen Erweitern

  DevIo_CloseDev($hash);
  DevIo_OpenDev($hash, 0, &Init);
  return;
}
Damit scheint sich zumindest das Fehlerbild teils geändert zu haben...

ws://<nicht vorhandene IP>

Ja, dto. hier, ich kann aber nicht sagen, wie schnell da die Rückmeldung erfolgt.

Danke! Das sieht ja recht überschaubar aus. Ich werde mal schauen, wie gut es mit der Struktur von define und set bei meinem Modul zusammenpasst. Benannte Parameter habe ich bislang nicht, aber unbenannte Parameter trennen ist schon praktisch (wenn es nicht an ungewollten Stellen passiert). Aber so weit ich sehe, würde es wohl auch einen JSON-Block intakt lassen (den übergebe ich derzeit mit set <name> rpc <...> zum Testen neuer Funktionsaufrufe).

Ist es. Für RHASSPY war mehr "Text-Block" und weniger das JSON-Geklammere relevant, aber z.B. MQTT_GENERIC_BRIDGE nutzt das auch für das Erkennen von Perl-Kommandos .

wenn ich mir die existierenden Module ansehe, kommt wohl beides vor (teilweise nicht konsistent und gemischt - das finde ich grausam...).

fully agreed .

[xenos1984]

Hmm, setzt halt voraus, dass der "flow control" von UBUS aus gemacht wird; eigentlich war ich davon ausgegangen, dass das DevIo machen sollte und nur die timeout-Parameter ggf. beizusteuern wären.

Letzteres habe ich auch gedacht, wobei ich die derzeitige Funktionsweise so verstehe, dass DevIo das Gerät bzw. dessen Hash einfach nur in die %readyfnlist einträgt, wenn die Verbindung nicht vorhanden ist, und bei vorhandener Verbindung entfernt. Die ReadyFn wird dann periodisch von fhem.pl für alle Geräte auf der Liste aufgerufen. Darauf, wie oft das geschieht, scheint DevIo aber keinen Einfluss zu nehmen, insbesondere auch keinen Timeout / Begrenzung der Zahl oder Häufigkeit der Versuche. Das scheint auch gar nicht vorgesehen zu sein, es wird eben immer die ganze Liste in einem bestimmten Intervall abgearbeitet (s.u.).

Habe gestern noch ein wenig mit 00_MQTT gespielt und dabei teilweise die return-Werte von $callback (\&Ready) und \&Start geändert

So weit ich sehe, hängt davon ab, ob anschließend von fhem.pl ReadFn aufgerufen wird oder nicht:

Code Auswählen Erweitern


833   foreach my $p (keys %readyfnlist) {
834     next if(!$readyfnlist{$p});                 # due to rereadcfg / delete
835
836     if(CallFn($readyfnlist{$p}{NAME}, "ReadyFn", $readyfnlist{$p})) {
837       if($readyfnlist{$p}) {                    # delete itself inside ReadyFn
838         CallFn($readyfnlist{$p}{NAME}, "ReadFn", $readyfnlist{$p});
839       }
840
841     }
842   }


[Beta-User]

...das deckt sich mit dem, wie es in https://wiki.fhem.de/wiki/DevIo#Allgemeine_Funktionsweise beschrieben ist. Stellt sich die Frage, ob eine "disabled" Verbindung dann überhaupt auf diese Weise detektiert werden kann...

Na ja, ein Blick in MQTT2_CLIENT zeigt: Rudi setzt anscheinend den timeout in der readyfn (MQTT2_CLIENT_connect). Da MQTT2_CLIENT das neueste der von Rudi selbst entworfenden Module ist, sollten wir (ich habe grade ein halbes Auge auf MYSENSORS) uns ggf. dann da mal näher umschauen.

[xenos1984]

Scheint, als hätte ich den Schuldigen gefunden - der Hinweis hat geholfen, wenn auch indirekt. nextOpenDelay wird von DevIo als 60 Sekunden angenommen, wenn es nicht gesetzt ist - das scheint es also nicht zu sein. Aber beim nochmaligen Lesen des Wiki ist mir aufgefallen, dass ich vergessen habe, beim erneuten Verbindungsversuch das reopen-Flag in DevIo_OpenDev zu setzen. Jetzt habe ich Ready so geändert:

Code Auswählen Erweitern


sub Ready
{
my ($hash) = @_;
my $name = $hash->{NAME};

return if IsDisabled($name) || $hash->{method} ne "websocket" || DevIo_IsOpen($hash);

Log3($name, 5, "UBUS ($name) - reconnect");

return DevIo_OpenDev($hash, 1, \&Init, \&Callback);
}


(DevIo wird ohnehin nur bei WebSocket URL benutzt.) Jetzt wird Ready zwar immer noch häufig aufgerufen, aber führt nicht mehr jedes Mal zu einem Verbindungsversuch, sondern brav einmal pro Minute:

Code Auswählen Erweitern


2021.09.07 20:09:19 5: HttpUtils url=http://192.168.1.2:80/ NonBlocking via http
2021.09.07 20:09:19 4: IP: 192.168.1.2 -> 192.168.1.2
2021.09.07 20:09:22 1: UBUS (juci) - error while connecting: connect to http://192.168.1.2:80 timed out
2021.09.07 20:09:27 5: UBUS (juci) - reconnect
2021.09.07 20:09:32 5: UBUS (juci) - reconnect
2021.09.07 20:09:37 5: UBUS (juci) - reconnect
2021.09.07 20:09:42 5: UBUS (juci) - reconnect
2021.09.07 20:09:47 5: UBUS (juci) - reconnect
2021.09.07 20:09:52 5: UBUS (juci) - reconnect
2021.09.07 20:09:57 5: UBUS (juci) - reconnect
2021.09.07 20:10:02 5: UBUS (juci) - reconnect
2021.09.07 20:10:07 5: UBUS (juci) - reconnect
2021.09.07 20:10:12 5: UBUS (juci) - reconnect
2021.09.07 20:10:13 5: UBUS (juci) - reconnect
2021.09.07 20:10:18 5: UBUS (juci) - reconnect
2021.09.07 20:10:23 5: UBUS (juci) - reconnect
2021.09.07 20:10:23 5: HttpUtils url=http://192.168.1.2:80/ NonBlocking via http
2021.09.07 20:10:23 4: IP: 192.168.1.2 -> 192.168.1.2
2021.09.07 20:10:26 1: UBUS (juci) - error while connecting: connect to http://192.168.1.2:80 timed out


Übrigens habe ich auch noch eine Vermutung, was die unterschiedliche Frequenz der Ready-Aufrufe verursachen könnte. Hat dein Testsystem noch andere TCP-Verbindungen am Laufen? Bei mir ist es ein leeres System mit nur dem UBUS und FHEMWEB, das alle 5 Sekunden aktualisiert.

http://svn.fhem.de/trac/browser/trunk/fhem/FHEM/DevIo.pm#L487

Code Auswählen Erweitern


487     # This part is called every time the timeout (5sec) is expired _OR_
488     # somebody is communicating over another TCP connection. As the connect
489     # for non-existent devices has a delay of 3 sec, we are sitting all the
490     # time in this connect. NEXT_OPEN tries to avoid this problem.


Aktueller Code ist angehängt.

[Beta-User]

OK, so langsam macht das auch für mich Sinn...

Wenn "Ready" so oft aufgerufen wird, sollte sie möglichst effizient sein. Daher sollte man m.E.:
- die Abfragen für das direkte return umdrehen (IsOpen, websocket, IsDisabled)
- Log3 auskommentieren (wir wissen ja jetzt (hoffentlich), dass die einfach in der Main-loop - optimalerweise mehrfach sekündlich - mitläuft).

[rudolfkoenig]

Vielleicht kann Rudi etwas dazu sagen, wie hier die beabsichtigte Vorgehensweise ist für Module, die DevIo benutzen?

Wenn ich das richtig sehe, habt Ihr das in der Zwischenzeit selbst herausgefunden.
ReadyFn wird ziemlich haeufig aufgerufen. Mit $hash->{nextOpenDelay} (relative Zeitangabe) bzw. $hash->{NEXT_OPEN} (absolute Zeitangabe) kann man steuern, wie haeufig ein Verbindungsversuch in DevIo_OpenDev unternommen wird.

[xenos1984]

Sehr gut, dann stimmen wir ja überein. Ich habe noch die Vorschläge von Beta-User umgesetzt. Ich gehe am Wochenende noch mal durch, aber dann kann ich es wohl ins SVN einchecken und mich als Maintainer eintragen, wenn ich so weit alles dafür nötige beachtet habe.

[Beta-User]

Habe beim Überfliegen noch folgende Anmerkungen:
- die Kontrolle, ob (erforderliche) Parameter auch an die Funktionen übergeben werden, ist eher unvollständig. Da viel intern ist, ist das nicht so dramatisch, aber eben auch nicht "vorbildlich";
- bei unveränderlichen Texten/Vergleichen usw. sind für meinen Geschmack zu viele Doppel-Quotes drin;
- Sicher, dass du den Start/rereadcfg-Vorgang nicht timer-basiert lösen willst und NotifyFn wirklich Sinn macht?

@Rudi:
OT betr. DevIo noch: Hast du eine Idee, ob irgendwo im framework eine Funktionalität verborgen ist, die dazu führt, dass HMLAN seinen "Clients"-Eintrag verliert? (https://forum.fhem.de/index.php/topic,122848.0.html)
EDIT: Hat sich vermutlich geklärt, sorry, 1m-Problem...

[xenos1984]

- die Kontrolle, ob (erforderliche) Parameter auch an die Funktionen übergeben werden, ist eher unvollständig. Da viel intern ist, ist das nicht so dramatisch, aber eben auch nicht "vorbildlich";

Stimmt, da hatte ich mich im Wesentlichen am Wiki orientiert. Ich werde mal sehen, ob ich ein "vorbildliches" Modul finde, an dem ich mich orientieren kann, wie man fehlende Parameter in den diversen Funktionen am besten abfangen kann.

- bei unveränderlichen Texten/Vergleichen usw. sind für meinen Geschmack zu viele Doppel-Quotes drin;

Richtig, das hatte ich bisher nur sporadisch geändert und es für das "noch mal Durchsehen" eingeplant Inzwischen habe ich alle Zeichenketten umgestellt und Doppel-Quotes nur noch da, wo Variablen expandiert werden.

- Sicher, dass du den Start/rereadcfg-Vorgang nicht timer-basiert lösen willst und NotifyFn wirklich Sinn macht?

Sicher - nein. Auch da bin ich nach Wiki vorgegangen, aber falls du einen Vorschlag oder ein Beispiel für eine andere Implementierung hast, schaue ich es mir mal an. Ich wüsste spontan nicht, wie sich z.B. auf ein Neuladen der Konfiguration am besten reagieren sollte.

[Beta-User]

Stimmt, da hatte ich mich im Wesentlichen am Wiki orientiert. Ich werde mal sehen, ob ich ein "vorbildliches" Modul finde, an dem ich mich orientieren kann, wie man fehlende Parameter in den diversen Funktionen am besten abfangen kann.

Komplettes Modul? Du darfst  gerne RHASSPY einem peer-review unterziehen, das ist -afaik- nicht nur eines der wenigen (derzeit: das einzige?), das parseParams im Initialize-Hash verwendet, sondern auch der Code sollte weitestgehend qualifizierte Rückmeldungen geben, wenn was schief läuft.
Das betrifft btw. auch Angaben in Attributen usw. (einschließlich der Rückmeldung, in welcher Zeile genau, wenn man mehrzeilig setzen kann).
Im Wiki ist das gar nicht Thema, weil man da davon ausgeht, dass Prototypen schon das meiste verhindern... (lustiges Beispiel, über das ich die Tage gestolpert bin: https://forum.fhem.de/index.php/topic,122816.msg1174175.html#msg1174175 - vermutlich wird der Prototype da übrigens gebraucht, um durch diese Konstruktion zu ermöglichen, dass man (neben den Instanz-Hash) beliebig viele Hashes (keine Scalare! (was suggeriert der Prototype?!?)) übergeben kann, die dann alle in einem (!!) HASH landen - wobei witzigerweise die Mindestzahl der Argumente ist: Ein (!) HASH, bestehend aus einem Wertepaar (oder vielleicht auch ganz ohne Wertepaar, solange der "ref" stimmt ...?!?))

Das Ziel, sowas wie eine "aktualisierte" Diskussionsgrundlage zu Perl + FHEM anzubieten, war btw. auch der Hintergrund für "peer review" - myUtils im "package"-Format (für Fortgeschrittene) - da findet sich also hoffentlich auch das eine oder andere.

Sicher - nein. Auch da bin ich nach Wiki vorgegangen, aber falls du einen Vorschlag oder ein Beispiel für eine andere Implementierung hast, schaue ich es mir mal an. Ich wüsste spontan nicht, wie sich z.B. auf ein Neuladen der Konfiguration am besten reagieren sollte.

Ähm, ...RHASSPY...? Oder jüngst https://svn.fhem.de/trac/changeset/24882/trunk/fhem/FHEM für 11_OWDevice.
Auf das rereadcfg sollte man so oder so nicht unbedingt Rücksicht nehmen, das erscheint mir (inoffiziell) deprecated. Aber es ist auch "wurst", weil bei rereadcfg auch "DefFn" nochmal aufgerufen  wird. Ergo muss da nur die passende InternalTimer-Anweisung rein und "gut ist" .

Apropos Wiki:
Vermutlich könnte/sollte man das mal (vorsichtig) modernisieren. Falls du also Lust hast, wäre es m.E. eventuell zumindest sinnvoll, die eine oder andere Fußnote anzubringen, es muss ja nicht gleich "Revolution" gerufen werden...

[xenos1984]

Ich habe mich jetzt an RHASSPY orientiert. Jetzt sollten alle Parameter geprüft oder mit sinnvollen Vorgabewerten gefüllt sein. Ich schaue noch, ob sich bestimmte Fehlerwerte besser durch Meldungen abfangen lassen.

Die Initialisierung durch eine Wartezeit von 1 Sekunde abzuwarten klingt einfach Ich habe es jetzt auch einmal so eingebaut.

Die aktuelle Version ist wieder angehängt und läuft bislang stabil (ich lasse sie noch mal länger laufen und schaue, ob es Fehler, Verbindungsabbrüche o.ä. gibt).

Das RHASSPY Modul kann ich gerne mal im Detail reviewen - es ist ja etwas umfangreicher Die myUtils als Package habe ich mir auch schon mal angesehen, das sieht auf jeden Fall ansprechend aus. Die wollte ich auch schon mal bei Gelegenheit bei mir umsetzen und kommentiere dann gerne auch, wenn mir etwas einfällt. Selbiges auch im Wiki - ich denke, mir ist sogar der eine oder andere Fehler aufgefallen. So weit ich das sehen kann, ist $a hier falsch - "set" wird nicht als erster Wert übergeben, sondern gar nicht, oder?

http://wiki.fhem.de/wiki/DevelopmentModuleAPI#parseParams

[Beta-User]

Kurz zur parseParams-Anmerkung:
Es ist m.E. RICHTIG, wie es da steht, aber der Gesamtkontext ist m.E. etwas uneindeutig; so wie das in RHASSPY umgesetzt ist, kann man erkennen, dass das "set" nicht da sein kann. Das hat aber damit zu tun, dass fhem.pl das schon "rausgefischt" hat, um überhaupt rauszufinden, an welche Funktion der Rest nach Durchlauf durch parseParams zu übergeben ist, und für diese empfangende Funktion ist es "sowieso klar"...

Wenn man aber ALLES an parseParams übergibt, kommt genau das raus, was im Wiki steht. Man sollte es nur (aus "hygienischen Gründen") nicht in $a packen, weil $a und $b auch "speziell" sind. (Es gibt da Code-Teile in dem zugegebenermaßen längeren RHASSPY-Code, an dem man erkennen kann, was ich meine...)
Btw.: Du wolltest ein "Modul" haben, also habe ich eines genannt .

Danke auch für die Rückmeldung zu der myUtils-Demo. Würde schon wieder was "anders" machen, weil "$hash" in "Initialize" ist was anderes als in allen anderen "üblichen" Fällen wie SetFn etc.. Siehe den aktuellen Patch zu HMLAN bzgl. der Frage, wo eigentlich "Clients" sinnvollerweise stehen sollte . Sollte eigentlich besser z.B. "$codeHash" oä. heißen, wäre klarer...

Die Wartezeit kann auch "0" sein - alle (!) Timer werden so oder so erst nach $init_done abgearbeitet. Bei "unwichtigen" Modulen macht es aber uU. Sinn, "jetzt + einige Sekunden" zu nehmen, damit die "wichtigen Module" Zeit haben, direkt loszulegen - was btw. auch ein Vorteil gg. der notify-Lösung ist. Die startet unmittelbar nach $init_done (weiß nicht ob vor oder nach den "abgelaufenen" Timern), und (ohne weitere Maßnahmen) vermutlich "in order of apearance" in der cfg.

[xenos1984]

Man sollte es nur (aus "hygienischen Gründen") nicht in $a packen, weil $a und $b auch "speziell" sind. (Es gibt da Code-Teile in dem zugegebenermaßen längeren RHASSPY-Code, an dem man erkennen kann, was ich meine...)

Stimmt, an sort() hatte ich gar nicht gedacht. Die Eigenheiten von Perl Ich habe es jetzt $apar und $hpar genannt.

[Beta-User]

Kurze Durchsicht:
Sieht aufgeräumt aus .

Eines ist mir jetzt noch über die Leber gelaufen:

Code Auswählen Erweitern

my ($proto, $host, $port, $path) = ($1, $2, $3 ? $3 : ':' . ($1 eq 'wss' ? '443' : '80'), $4);
Dem Gefühl nach paßt das nicht, wenn $3 keinen match liefert: Was steht dann in $4? Dann willst du stattdessen vermutlich $3 haben?
Kurzum: mit "named captures" und einem kleinen defined-or wäre es klarer...

Ob das nach

Code Auswählen Erweitern

elsif($hash->{method} eq "cmd")
   
(ich ziehe eine Quote ) funktioniert, ist mir unklar. Sieht nach einem Versuch aus, einen Befehl an das OS abzusetzen? Dann müßte da eher ein qx stehen, oder? (Ist aber nicht mein Spezialgebiet, kann sein, dass die Anmerkung kompletter Unfug ist).
Und bei "encode_json" kann schon auch mal was schief gehen, oder? Falls ja,  gehört es m.E. in {eval}
Für decode gilt das m.E. aber vorsichtshalber immer. (Erfahrungsgemäß reißt das FHEM direkt in den Abgrund, wenn da - aus welchen Gründen auch immer - irgendwas nicht paßt).



Die weiteren "Feinheiten" (ist jetzt aber irgendwie langsam aber sicher "na ja, schon irgendwie" ...):

Irgendwo ist  ein ausdrückliches return; verlorengegangen?

"$args[1]" müßte eigentlich "$apar->[1]" entsprechen? (Weiß nicht, ob das in RHASSPY überall "mustergültig" steht, jedenfalls könnte man sich dann das Umpacken sparen...)

(Textlich) längere "or-Kaskaden" (mit ||) könnte man auch untereinander notieren, ist evtl. optisch leichter zu erfassen (ich notiere auch "gerne" gedrängt, ist aber nicht vorbildlich...)

Für sowas

Code Auswählen Erweitern

if(grep {$item eq $_} @dataitems)
gab's irgendwo in List::Utils auch eine Funktion, meine ich mich zu erinnern. Sollte auch seit längerem in corelist sein. Perlcritic mag "grep" irgendwie häufig nicht, ich bin aber noch nicht so richtig dahintergestiegen, wieso genau nicht...

In Set gibt's noch eine elsif-Kaskade und andernorts auch noch das eine oder andere "else", das ein direktes return erschlagen würde?

Ähnliches gilt für (exemplarisch)

Code Auswählen Erweitern


if($function eq 'get')
Das ist eigentlich ein "next if ... ne ...", und man spart sich da und analog im Folgenden einige Einrückungen.

Er hat "or" gesagt... (na ja,...)

[xenos1984]

So langsam nähern wir uns dem Ziel

Eines ist mir jetzt noch über die Leber gelaufen:

Code Auswählen Erweitern

my ($proto, $host, $port, $path) = ($1, $2, $3 ? $3 : ':' . ($1 eq 'wss' ? '443' : '80'), $4);
Dem Gefühl nach paßt das nicht, wenn $3 keinen match liefert: Was steht dann in $4? Dann willst du stattdessen vermutlich $3 haben?
Kurzum: mit "named captures" und einem kleinen defined-or wäre es klarer...

Die Regex hatte ich nach dem gleichen Schema gebaut wie eine Regex im DevIo Modul. So weit ich das sehe, bewirkt dass Fragezeichen nach dem dritten geklammerten Ausdruck, dass dieser immer greift und ggf. $3 leer (also "") lässt. So sieht es jedenfalls Regex101:

http://regex101.com/?regex=%5E(ws%7Cwss)://(%5B%5E/:%5D%2B)(:%5B0-9%5D%2B)?(.*?)$&testString=ws://abc.de/xyz&delimiter=@

"named captures" wären eine Alternative. Hier finde ich die numerische Schreibweise etwas lesbarer...

Ob das nach

Code Auswählen Erweitern

elsif($hash->{method} eq "cmd")
   
(ich ziehe eine Quote ) funktioniert, ist mir unklar. Sieht nach einem Versuch aus, einen Befehl an das OS abzusetzen? Dann müßte da eher ein qx stehen, oder? (Ist aber nicht mein Spezialgebiet, kann sein, dass die Anmerkung kompletter Unfug ist).

`...` sollte das gleiche tun, der Lesbarkeit halber habe ich es in qx{...} geändert. (Und die Quotes )

Und bei "encode_json" kann schon auch mal was schief gehen, oder? Falls ja,  gehört es m.E. in {eval}
Für decode gilt das m.E. aber vorsichtshalber immer. (Erfahrungsgemäß reißt das FHEM direkt in den Abgrund, wenn da - aus welchen Gründen auch immer - irgendwas nicht paßt).

Guter Punkt. Im Fehlerfall gibt es jetzt eine Meldung im Log.

Irgendwo ist  ein ausdrückliches return; verlorengegangen?

In der Tat - jetzt sollten sie aber überall sein.

"$args[1]" müßte eigentlich "$apar->[1]" entsprechen? (Weiß nicht, ob das in RHASSPY überall "mustergültig" steht, jedenfalls könnte man sich dann das Umpacken sparen...)

Stimmt, das macht die Sache einfacher. Bei der Gelegenheit habe ich gleich noch eine Fehlerprüfung eingebaut.

(Textlich) längere "or-Kaskaden" (mit ||) könnte man auch untereinander notieren, ist evtl. optisch leichter zu erfassen (ich notiere auch "gerne" gedrängt, ist aber nicht vorbildlich...)

Im Prinzip ja, wobei ich beim Einzeiler-if die einzeilige Variante praktischer finde. So weit ich sehe, taucht es nur dort auf.

Für sowas

Code Auswählen Erweitern

if(grep {$item eq $_} @dataitems)
gab's irgendwo in List::Utils auch eine Funktion, meine ich mich zu erinnern. Sollte auch seit längerem in corelist sein. Perlcritic mag "grep" irgendwie häufig nicht, ich bin aber noch nicht so richtig dahintergestiegen, wieso genau nicht...

Stimmt, wobei PBP beim einem eq Vergleich stattdessen einen Readonly Hash empfiehlt. Bei der Gelegenheit habe ich @dataitems auch gleich Readonly gemacht und den Konstanten Großbuchstaben gegeben.

In Set gibt's noch eine elsif-Kaskade und andernorts auch noch das eine oder andere "else", das ein direktes return erschlagen würde?

Die sollten jetzt auch erschlagen sein

Ähnliches gilt für (exemplarisch)

Code Auswählen Erweitern


if($function eq 'get')
Das ist eigentlich ein "next if ... ne ...", und man spart sich da und analog im Folgenden einige Einrückungen.

Im Prinzip ja, wenn da nur die eine Abfrage steht. Die ParseCall Funktion ist eher "vorausschauend" geschrieben, soll heißen, dass ich beabsichtige, noch weitere Funktionen / Unterabfragen zu implementieren. Deshalb habe ich es eher so geschrieben, statt es kompakter zu schreiben und später "aufzudröseln", wenn weitere Funktionen dazukommen.

Er hat "or" gesagt... (na ja,...)

Stimmt Jetzt gibt es "or" nur noch in der Dokumentation, und da setze ich dem Leser lieber kein "||" vor die Nase. || doch?

[Beta-User]

Hehe, spätestens ab jetzt bist du auf einem höheren Kenntnisstand wie ich, und es macht wenig Sinn, dass ich den Code nochmal durchstöbere...

Bzgl. der "next/elsif"-Frage noch:
Kann man  uU. auch dadurch lösen, dass man jeden "if"-Zweig in der Schleife mit einem next beendet, wenn danach nichts mehr kommt, ||?

(Lesbarer Code braucht doch keine Doku, ||)



Nachtrag zu "next" @Rudi:
Mags du mal #495 in MQTT2_SERVER anschauen?

[xenos1984]

Man könnte sagen, ich bin lernfähig Und beim Durchgehen habe ich noch ein "and" gefunden && ersetzt

Zum "elsif" könnte ich aber noch Rat gebrauchen. In dem Zusammenhang, d.h. außerhalb einer Schleife, habe ich next bisher nicht gesehen... Wäre es auf Funktionsebene nicht das gleiche wie return? Ansonsten wäre die if-Kaskade für mich eigentlich ein Fall für etwas wie given/when. Ein paar wenige FHEM-Module nutzen das, allerdings frage ich mich, wie "experimentell" das ist, oder ob man es bedenkenlos nutzen kann.

[Beta-User]

"lernfähig" paßt, würde ich mal sagen...

Bzgl. der "next"-Zeile in MQTT2_SERVER war das als Hinweis auf ein vermeidbares Warning an @Rudi gemeint , das ist mAn. (und aus Sicht von "use warnings;") nicht zur Nachahmung empfohlen.

Bzgl. "given" und "when" habe ich keine wirkliche Meinung, bisher konnte ich _fast_ immer die Abkürzungen "return;" bzw. "next" (oder "last") so einsetzen, dass die alten "HANDLER: if ($x == $z) and do { ..." - Konstruktionen (ehemals z.B. in MYSENSORS zu finden) ohne Schmerzen entsorgen konnte - kurz: ich sehe aktuell keinen Bedarf für Experimente in diese Richtung...
Und wo das nicht geht, hilft häufig ein "dispatch"-Hash, mit dem man einfach das passende Unterprogramm aufruft. Nur bei den sehr komplexen Abfragen innerhalb einiger Funktionen in RHASSPY ist es mir bisher nicht gelungen, auf tiefergreifende elsif-Verschachtelungen zu verzichten...

[xenos1984]

Gut, ich denke, dann werde ich die elsif-Kaskade erst einmal so belassen. An einen Dispatch-Hash hatte ich auch schon gedacht, allerdings müsste man dann gleich einige Argumente übergeben und in jeder Funktion wieder auspacken, was sich eigentlich kompakter und mit weniger Aufwand in der einen Funktion realisieren lässt (ist jedenfalls mein Eindruck). Aber ich behalte es mal als Option im Hinterkopf - jedenfalls scheinen die meisten meiner Fälle nur $hash und $result zu benutzen, mit wenigen exotischen Ausnahmen, und da wäre das Auspacken ja nur 2 * shift...

[Beta-User]

...das mit den vielen Variablen und Parametern ist der Grund, warum es in RHASSPY teilweise so grausam zugeht...

Am Ende soll es halt "lesbar" sein, und ähnlich wie das eindeutige "return;" ist halt ein "next;" oder "last;" ein ziemlich deutlicher Hinweis, dass danach nichts mehr kommt, von daher ist mir (in so einer Schleife) ein "next" (+Leerzeile?) gefolgt von einem "if" deutlich lieber wie ein "elsif". Ist vermutlich auch eine Frage der (erworbenen) Leseschwäche ...

[xenos1984]

Das ist wahrscheinlich eine Stilfrage - für mich ist ein "elsif" ein Zeichen, dass dieser Zweig nur dann ausgeführt wird, wenn der vorherige nicht ausgeführt wurde Das "return" am Ende eines vorherigen if sagt natürlich das gleiche, aber das im vorherigen Zweig zu suchen, um herauszufinden, wann eine if/elsif abgefragt wird, finde ich wiederum kontra-intuitiv...

Ich habe mir inzwischen angesehen, welche Abfragen es noch gibt, und da sind einige Dabei, bei denen ein Parameter direkt im Namen der aufgerufenen Funktion steckt (sowas wie "network.interface.wan" + "down" oder "led.broadband" + "0" etc.) und man die nicht durch einen statischen Hash aufteilen kann, wenn man nicht die Namen aller LEDs oder Netzwerk-Interfaces in dem Hash haben will. Da ist ein

Code Auswählen Erweitern


if($module =~ m/network\.interface\.(.*)/)

o.ä. dann wohl doch die bessere Wahl.

Übrigens gibt es doch nichts schöneres als eine stabile API... Letzte Nacht hat mein Router ein Update durchlaufen, und ich dachte mir, das wäre doch ein perfekter Test für die Robustheit meines Moduls. Seitdem ist ein Teil meiner Readings verschwunden, die Interfaces haben neue Namen bekommen (meine externe IP steht jetzt unter "wan" statt "internet" und mein Dynamic DNS Client hat sie nicht mehr gefunden), manche RPCs sind komplett geändert (statt "asterisk" liefert mir jetzt "voice.asterisk" den Status der Telefonleitungen - mit anders strukturierter Antwort, für die ich den Parser umschreiben muss) oder die Daten liegen woanders ("clients" zeigt mir nicht mehr, an welchem Port ein Gerät hängt - dafür listet "interfaces" jetzt die damit verbundenen Geräte auf). Kurzum, ich muss meinen Parser nicht nur anpassen, sondern das Modul wohl so umbauen, dass die RPC-Struktur konfigurierbar ist. Wer weiß, was sich noch für mysteriöse Geräte "im freien Feld" finden lassen... Da muss ich wohl mal sehen, ob und wo ich dazu Informationen finde...

[Beta-User]

Das "return" am Ende eines vorherigen if sagt natürlich das gleiche, aber das im vorherigen Zweig zu suchen, um herauszufinden, wann eine if/elsif abgefragt wird, finde ich wiederum kontra-intuitiv...

...wie angedeutet, ist vermutlich auch eine Gewohnheitsfrage...

Ich habe mir inzwischen angesehen, welche Abfragen es noch gibt, und da sind einige Dabei, bei denen ein Parameter direkt im Namen der aufgerufenen Funktion steckt (sowas wie "network.interface.wan" + "down" oder "led.broadband" + "0" etc.) und man die nicht durch einen statischen Hash aufteilen kann, [...]

Eventuell gibt es Möglichkeiten, das zu mischen. Macht -  - RHASSPY z.B. so, dass gefragt wird, ob es eine CODE-Referenz auf eine Dispatch-Funktion gibt, und wenn nicht, dann geht es eben (nach einem return für den anderen Fall ) mit der nächsten if-Abfrage weiter...

Übrigens gibt es doch nichts schöneres als eine stabile API... Letzte Nacht [...]

Arg, sowas ist ärgerlich... Du hast mein Mitgefühl!

[Beta-User]

fyi: Es gibt hier eine User-Anfrage dazu. Evtl. wäre es sinnvoll, irgendwie eine (interne) Debugging-Ausgabe zu ermöglichen, um z.B. die jeweils empfangenen JSON sichtbar zu machen?

Ergänzend: Man kann einzelne Infos auch im Geräte-Hash versteckt vorhalten, indem man den Namen mit einem Punkt beginnen läßt (das braucht dann aber Quotes...), falls da sensible Daten drin stehen sollten. Um sowas anzuzeigen gibt es dann einen Schalter in "global", und ich meine mich entsinnen zu können, dass es die Option gibt, bestimmte Dinge auch als "privat" zu markieren.

[xenos1984]

fyi: Es gibt hier eine User-Anfrage dazu.

Danke für die Info! Ich habe auch schon eine PM dazu bekommen.

Evtl. wäre es sinnvoll, irgendwie eine (interne) Debugging-Ausgabe zu ermöglichen, um z.B. die jeweils empfangenen JSON sichtbar zu machen?

An und für sich sollte mit verbose 5 genau das passieren, d.h. alle gesendeten und empfangenen Daten werden geloggt.

Ergänzend: Man kann einzelne Infos auch im Geräte-Hash versteckt vorhalten, indem man den Namen mit einem Punkt beginnen läßt (das braucht dann aber Quotes...), falls da sensible Daten drin stehen sollten. Um sowas anzuzeigen gibt es dann einen Schalter in "global", und ich meine mich entsinnen zu können, dass es die Option gibt, bestimmte Dinge auch als "privat" zu markieren.

Hast du einen Vorschlag, was man auf diese Weise versteckt vorhalten sollte? Ich hatte temporäre Daten unter $hash->{helper} oder tiefer unter $hash->{rpc} abgelegt. Das Passwort ist über ein Hilfsmodul gespeichert.

[Beta-User]

Der Link war v.a. auch, damit ich das ggf. selbst wiederfinde...

Ansonsten hatte ich nicht in den Code geschaut, was wann warum wo landet, und auch Funktionalität war nicht der Schwerpunkt meiner Durchsichten . Wenn bei Bedarf schon ausreichend geloggt wird, ist alles gut .

Meine Anmerkungen waren eher in die Richtung "falls es hilreich ist..." gemeint, und manchmal ist es eben für's helfen einfacher, man kann (für typische Stolperfallen) ein list anfordern, wie den User erst mal ins Log zu schicken. Aber auch das bezieht sich nicht auf irgendeine konkrete Stelle im Code .

[xenos1984]

Die Funktionalität ist auch etwas schwer zu prüfen, wenn man kein Testsystem / UBUS-fähiges Gerät hat

Stimmt, das ist natürlich eine gute Idee. Wenn ich gezählt hätte, wie viele Lists von "DOIF im falschen Zustand" ich schon wie die Suchbilder in der Tageszeitung betrachtet habe... Mit dem Testlauf im Devolo-Thread habe ich schon ein paar Ideen, z.B. den letzten Fehler könnte man als Internal speichern, damit man ihn im List sieht. Ich hatte bisher nur die Strategie "Fehlermeldungen ins Log, Daten zur späteren, internen Verwendung in Internals" auf dem Schirm.

Mit den aktuellen Testergebnissen und Erfahrungen, was die Vielzahl von Implementierungen / APIs angeht, die auf UBUS aufsetzen können, überlege ich, wie ich das Modul am besten flexibel gestalte. Wahrscheinlich ist es wenig sinnvoll, alle Feld, Wald und Wiesenrouter, deren Firmware-Versionen und ihre APIs zu erfassen und zu implementieren. Da bietet es sich wohl eher an, die konkreten Funktionsaufrufe und die Datenauswertung dem User zu überlassen. Das wäre so ähnlich wie beim MQTT-Topic und dessen Payload, die ja auch unterschiedlich sein können.

Im Moment überlege ich sogar, ob es sinnvoll wäre, ein zweistufiges Modul zu benutzen. Die erste Stufe würde die Verbindung als solche aufbauen und verwalten, also Transport-Protokoll (Websocket, HTTP...), Login, Kommunikation mit dem Gerät. Die zweite Stufe würde dann jeweils einen Funktionsaufruf und die Auswertung der Antwort übernehmen. Also so ähnlich wie MQTT2_CLIENT und MQTT2_DEVICE. Das erscheint mir sinnvoll, weil ein Funktionsaufruf u.U. recht komplex sein kann und diverse Parameter braucht (Name der aufgerufenen Funktion, zusätzliche Parameter, Häufigkeit des Aufrufs, Funktion zur Auswertung...), die man dann als Attribute (module, function, parameters, interval, response...) eines zweiten Moduls implementieren könnte, von dem man jeweils eine Instanz pro gewünschtem Funktionsaufruf anlegt. Das hätte auch den Vorteil, dass man nicht ein Gerät mit 100+ Readings hätte, sondern die "Last" etwas verteilt. Bei diesen Attributen müsste man wohl am besten auch Variablen parsen - sofern der User z.B. nicht die Namen aller Ethernet-Ports von Hand eingeben will und sich die zwischen Firmware-Versionen ändern, sondern diese aus Readings ersetzen möchte.

Inzwischen habe ich auch herausgefunden, dass es neben dem "call" Aufruf auch noch andere Anfragen mit anderer Semantik gibt. Darunter ist z.B. ein "subscribe", mit dem man eine Benachrichtigung für bestimmte Ereignisse anfordern kann. Damit ist die Struktur ganz anders, weil es keine einfache Anfrage-Antwort Abfolge mehr ist, sondern man ein Bestellen, viele Benachrichtigungen und ein Abbestellen hätte. Da wäre es vermutlich sogar sinnvoll, ein separates Modul als zweite Stufe zu haben, und zu schauen, wie man am besten den gemeinsamen Code-Anteil verwaltet. Also sowas wie UBUS_CLIENT für die Verbindung, UBUS_CALL und UBUS_SUBSCRIBE für die Inhalte.

Ich bin offen für Kommentare / Kritik

[Beta-User]

Noch schwieriger zu beurteilen...

Allgemeine Anmerkungen:
- viele Teil-Module => viel Erklärungsbedarf (Bauchgefühl sagt: aufsplitten ist eher für "wenn es gar nicht mehr anders geht")
- evtl. ist eine Implementierung verschiedener APIs denkbar (analog Weather, aber den eigentlichen API-Code dann nach ./lib/UBUS/API/*.pm )? Die Attributierung müßte dann halt ggf. "modellabhängig" im Define untergebracht werden. parseParams kann helfen, viele verschiedene Optionen zur Definitionszeit zu ermöglichen, ohne immer alles setzen zu müssen;
- User-Code über Attribute zu ermöglichen, ist evtl. eine gute Idee, hat aber einen Haken: Der sollte im Main-Kontext laufen => evalSpecials und via Analyze.*-Funktionen aufrufen, mappings, Filter etc. ggf. über den Geräte-Hash bereitstellen (analog jsonMap@MQTT2_DEVICE). Das muss man relativ gut vorbereiten, sonst fallen ggf. die User damit auf die Nase.

Insgesamt fühle ich mich nicht hinreichend kompetent und in den konkreten UBUS-Details, um da was "gefühlt fundiertes" sagen zu können, bin also mal gespannt, was wirklich Wissende dazu äußern.

[xenos1984]

Danke für die Einschätzung! Inzwischen habe ich mir auch die Doku im Wiki sowie die Implementierung der MQTT2-Module angesehen. Das sieht an und für sich recht überschaubar aus. Die Nachrichten sind beim zweistufigen Modulkonzept immer immer Zeichenketten, da bietet sich also an, die JSON-Nachrichten zu übergeben.

- viele Teil-Module => viel Erklärungsbedarf (Bauchgefühl sagt: aufsplitten ist eher für "wenn es gar nicht mehr anders geht")

Ja, da stimme ich zu. Ich würde auch den Erklärungsbedarf möglichst begrenzen wollen, wobei ich den Eindruck habe, dass das mit dem zweistufigen Aufbau vielleicht sogar einfacher wäre. Wenn man nur ein Modul und damit nur ein Device für ein UBUS-fähiges Gerät hat, landen alle Readings in diesem Device. Wenn der User diese konfiguriert, muss er sich also selbst um diese relativ komplexe Struktur kümmern (und z.B. identische Reading-Namen verhindern). Außerdem könnten die Attribute recht komplex werden, wenn z.B. verschiedene Abfragen unterschiedlich häufig aufgerufen werden sollen. Und für die Auswertung müsste man im Prinzip die ganze Funktionalität von ParseCall in User-Code stecken... Da erscheint es mir zumindest einfacher, pro Funktionsaufruf ein Device anzulegen, das diesen konkreten Aufruf ausführt und sich einfacher konfigurieren lässt. (Wie häufig? Welche Funktion? Welche Parameter? Wie verteilt sich die Antwort auf Readings?) Vielleicht könnte man die Konfiguration mit attrTemplate unterstützen / vereinfachen.

- evtl. ist eine Implementierung verschiedener APIs denkbar (analog Weather, aber den eigentlichen API-Code dann nach ./lib/UBUS/API/*.pm )? Die Attributierung müßte dann halt ggf. "modellabhängig" im Define untergebracht werden. parseParams kann helfen, viele verschiedene Optionen zur Definitionszeit zu ermöglichen, ohne immer alles setzen zu müssen;

Das wäre auch eine Möglichkeit, wobei ich den Eindruck habe, dass die API-Vielfalt bei UBUS-Geräten deutlich größer ist. Es handelt sich ja zunächst einmal nur um einen Bus bzw. ein Transport-Protokoll, und da kann jeder Hersteller drauf implementieren, was er möchte. Bei Weather ist es einfacher, weil einfach eine Hand voll APIs unterstützt werden, auf die im Prinzip jeder (Entwickler) zugreifen kann. Um die API eines bestimmten Routers zu implementieren, bräuchte man Informationen über diesen Router, und die ist meistens nur durch Lauschen des Datenverkehrs zu bekommen. Die Implementierung funktioniert dann u.U. nur für diesen konkreten Typ und Firmware-Version. Von daher denke ich, es wäre sinnvoller, dem User zumindest die Möglichkeit einer eigenen Konfiguration der API zu geben (und für "bekannte" Geräte eine fertige Konfiguration anzubieten, die dann idealerweise über den gleichen Mechanismus importiert werden kann -> attrTemplate).

- User-Code über Attribute zu ermöglichen, ist evtl. eine gute Idee, hat aber einen Haken: Der sollte im Main-Kontext laufen => evalSpecials und via Analyze.*-Funktionen aufrufen, mappings, Filter etc. ggf. über den Geräte-Hash bereitstellen (analog jsonMap@MQTT2_DEVICE). Das muss man relativ gut vorbereiten, sonst fallen ggf. die User damit auf die Nase.

Stimmt, wobei sich das sicher hinbekommen lässt, wenn man weiß, wie es geht.

[Beta-User]

Kurz zu AttrTemplate: SetExtensions würden das implizit mitbringen. Wenn man es isoliert haben will, wäre https://svn.fhem.de/trac/changeset/23561/trunk/fhem/FHEM/10_MQTT_GENERIC_BRIDGE.pm ein gutes Beispiel, wie das geht. Da hier vermutlich on/off nicht schaltbar sind, wäre das die m.E. sinnvolle Variante, sonst würde ich zum "Gesamtpaket" raten.

Ansonsten teile ich die Einschätzung, dass größere gerätespezifische Einrichtungsoptionen durch die User damit gut bedient werden könnten.

Wie die templates selbst dann strukturell aussehen müßten, ist vermutlich klar? (Das m.E. einfachste Beispiel wäre https://svn.fhem.de/trac/browser/trunk/fhem/FHEM/lib/AttrTemplate/max.template?order=name)

[xenos1984]

Kurz zu AttrTemplate: SetExtensions würden das implizit mitbringen. Wenn man es isoliert haben will, wäre https://svn.fhem.de/trac/changeset/23561/trunk/fhem/FHEM/10_MQTT_GENERIC_BRIDGE.pm ein gutes Beispiel, wie das geht. Da hier vermutlich on/off nicht schaltbar sind, wäre das die m.E. sinnvolle Variante, sonst würde ich zum "Gesamtpaket" raten.

Richtig, i.A. sollen die Geräte kein on/off als Befehle haben, von daher wäre SetExtensions hier fehl am Platz. Die isolierte Variante hört sich gut an. Was mir noch nicht ganz klar ist, ist der Aufruf von AttrTemplate_Set wenn die Set-Funktion 1. noch andere Befehle verarbeitet und 2. parseParams benutzt. Ersteres sehe ich u.a. bei MAX, da wird sie einfach am Ende von Set aufgerufen, wenn nichts anderes zutrifft. Letzteres finde ich bei HMCCUCHN und HMCCUDEV, aber die Struktur der übergebenen Werte ist mir nicht ganz klar. Hier soll wohl parseParams "rückgängig" gemacht werden?

Wie die templates selbst dann strukturell aussehen müßten, ist vermutlich klar? (Das m.E. einfachste Beispiel wäre https://svn.fhem.de/trac/browser/trunk/fhem/FHEM/lib/AttrTemplate/max.template?order=name)

Mehr oder weniger Angesehen habe ich es mir schon mal, weil ich eine AttrTemplate für HTTPMOD und das VLC-Player HTTP Interface erstellen wollte. Das könnte ich eigentlich mal als vorbereitende Fingerübung angehen, ich wollte da ohnehin noch ein paar Dinge implementieren

[Beta-User]

Ja, die Logik ist: Was das Modul selbst nicht versteht, gibt es an SE bzw. AttrTemplate weiter, und wenn die es auch nicht wissen, gibt's die "choose one of ..."-Meldung. Ob man das Array so übergeben kann, wie parseParams das liefert, oder ob man erst wieder ein join q{ } vorab braucht, müßte man (Sandbox-) testen...

Was die "Fingerübung" angeht, ist es vermutlich ein vergleichsweise steiler Einstieg, das aber eher weniger wegen der "Vertemplatung", sondern eher wegen HTTPMOD . Ansonsten ist es eine simple Textersetzung die im wesentlichen besteht aus DEVICE=>Name der Instanz... Alles im Prinzip GAAANZZZZ EINFACH .
Wenn es komplizierter werden sollte mit unterschiedlichen Optionen oder Ausbaustufen, gibt es dazu auch Möglichkeiten, aber vielleicht sollten wir mal ein paar Attribute haben und einen Gleitflug absolvieren, bevor wir das Ding mit Düsentriebwerken ausstatten ...

(OT: Das mit VLC hört sich interessant an).

[xenos1984]

In der Zwischenzeit habe ich mir mal die Struktur von ein paar Funktionsaufrufen und den zurückgegebenen Daten angesehen, und frage mich, wie sich diese am besten durch Geräte und Attribute abbilden lassen. Als Veranschaulichung ein Beispiel.

Abfrage der Netzwerkschnittstellen:

Code Auswählen Erweitern


{"jsonrpc":"2.0","method":"call","id":"...","params":["...","iwinfo","devices",{}]}

Alle Parameter sind konstant; als Antwort kommt z.B.

Code Auswählen Erweitern


{"jsonrpc":"2.0","id":"...","result":[0,{"devices":["wifi0","wifi1","ath1","ath0"]}]}

Als Antwort kommt die Liste der Schnittstellennamen. Wenn man nun mehr über eine Schnittstelle wissen möchte, z.B. die verbundenen Clients, braucht es diese als Parameter:

Code Auswählen Erweitern


{"jsonrpc":"2.0","method":"call","id":"...","params":["...","network.info","clients",{"device":"ath0"}]}

Als Antwort kommt die Liste der Clients.

Nun zur Frage der Umsetzung, bei der ich noch nach einer guten Lösung suche. Eine Idee wäre das zweistufige Modulkonzept zu benutzen. Also z.B. ein physikalisches Modul, mit dem man eine Schnittstelle öffnen kann:

Code Auswählen Erweitern


define ubus UBUS_CLIENT http://a.b.c.d/ubus
attr ubus username xyz
attr ubus timeout 900

Dieses Modul würde dann die Kommunikation mit dem Gerät übernehmen, d.h. Login, Verbindung mittels WebSocket / HTTP / Kommandozeile etc. sowie Sessions. Für jeden Funktionsaufruf könnte man ein Gerät definieren, das auf das logische Modul zugreift. Beim ersten Aufruf könnte das Gerät z.B. in etwa so aussehen

Code Auswählen Erweitern


define ubus_iwinfo UBUS_CALL
attr ubus_iwinfo IODev ubus
attr ubus_iwinfo module iwinfo
attr ubus_iwinfo function devices
attr ubus_iwinfo interval 3600
attr ubus_iwinfo readings {json2nameValue($EVENT)}

...und Readings der Form devices_0 = wifi0 etc. erzeugen. So lange alles statisch ist, sieht die Implementierung relativ gut umsetzbar aus. Mehr Kopfzerbrechen bereitet mir der komplexe Fall, dass man dynamische Informationen übergeben möchte. Also beim zweiten Aufruf:

Code Auswählen Erweitern


define ubus_clients UBUS_CALL
attr ubus_clients IODev ubus
attr ubus_clients module network.info
attr ubus_clients function clients
attr ubus_clients interval 60

Nun braucht es aber noch die Schnittstelle als Parameter. So lange man nur eine fest vorgegebene abfragen möchte, könnte man den Parameter als Attribut setzen:

Code Auswählen Erweitern


attr ubus_clients params {device => "ath0"}
attr ubus_clients readings {json2nameValue($EVENT, $PARAMS{device})}

Aber wenn man es nun dynamisch haben möchte, wird es trickreicher... Einen Namen abfragen geht noch gut:

Code Auswählen Erweitern


attr ubus_clients params {device => ReadingsVal("ubus_iwinfo", "devices_0", "")}

Wenn man nun mehrere haben möchte, muss das Attribut params statt eines einzelnen Parameter-Hash auch eine Liste davon verdauen können, und dann statt eines einzigen Funktionsaufrufs mehrere an das physikalische Gerät senden. Wenn man das nach dem gleichen Schema handhaben wollte, müsste man dem User schon zutrauen, Perl-Code mit map zu basteln, der die Readings ausliest.

Das obige Beispiel hat nur die Parameter als dynamische Größe. Bei meinem Router finde ich aber auch Funktionen der Form

Code Auswählen Erweitern


{"jsonrpc":"2.0","method":"call","id":"...","params":["...","network.interface.XXX","up",{}]}

Hier ist der Name XXX des Interface also an einer anderen Stelle zu finden. Demzufolge müsste man diese Attribute ähnlich dynamisch gestalten...

Ideen und Kommentare sind willkommen

[Beta-User]

Irgendwie habe ich bei zweistufig immer noch einen gedanklichen Hänger - kann aber sein, dass ich damit komplett falsch liege.

Die Schnittstellennamen scheinen generisch zu sein. Hat man also zwei UBUS-Instanzen (z.B. für zwei Router), hat man potentiell auch dieselben "devices"-Namen. Wie hält man dann auseinander, was vom jeweiligen UBUS-IO-Modul kommt. Das geht mAn. nur, wenn man die Verbindung zwischen IO und Client ähnlich "hart" vercoded, wie das bei MySensors und MQTT-classic der Fall ist - und das "gehört verboten", um unseren Chef-Architekten zu zitieren...

Eine wirkliche Lösung habe ich dafür nicht anzubieten, aber wenn ich das richtig verstanden habe, liefe zweistufig darauf hinaus, erst den JSON in UBUS auszupacken, dann zu analysieren, die einzelnen Teile wieder einzupacken (hab nicht nachgesehen, aber Dispatch braucht einen "flachen" Text, oder?), mit einem Flag zu kennzeichnen, von welcher UBUS-Instanz sie kommen und dann in den sub-Devices wieder auszupacken.

Es erscheint mir einfacher, die konkreten setter und getter dann eben erst im laufenden Betrieb einer UBUS-Instanz zu ermitteln und diesen Teil dynamisch aufzubauen, eben je nachdem, was tatsächlich gefunden wird (ähnlich, wie CUL_HM das in Abhängigkeit vom Gerätetyp macht).

Hoffe mal, dass sich dazu jemand äußert, der mehr Ahnung von zweistufig hat...

[xenos1984]

Die Schnittstellennamen scheinen generisch zu sein. Hat man also zwei UBUS-Instanzen (z.B. für zwei Router), hat man potentiell auch dieselben "devices"-Namen. Wie hält man dann auseinander, was vom jeweiligen UBUS-IO-Modul kommt. Das geht mAn. nur, wenn man die Verbindung zwischen IO und Client ähnlich "hart" vercoded, wie das bei MySensors und MQTT-classic der Fall ist - und das "gehört verboten", um unseren Chef-Architekten zu zitieren...

Mehr oder weniger, ja. (Bei mir heißen sie stattdessen "ethX".) Sollte diese Verbindung zwischen physikalischem und logischem Modul nicht mittels IODev hergestellt werden? D.h. ein logisches Gerät weiß, an welches physikalische Gerät es Anfragen schicken muss, und von wo es Antworten zu erwarten hat.

Eine wirkliche Lösung habe ich dafür nicht anzubieten, aber wenn ich das richtig verstanden habe, liefe zweistufig darauf hinaus, erst den JSON in UBUS auszupacken, dann zu analysieren, die einzelnen Teile wieder einzupacken (hab nicht nachgesehen, aber Dispatch braucht einen "flachen" Text, oder?), mit einem Flag zu kennzeichnen, von welcher UBUS-Instanz sie kommen und dann in den sub-Devices wieder auszupacken.

Ich denke, so kompliziert ist es gar nicht. Der Hash des physikalischen Gerätes wird in Parse übergeben, zusammen mit den Daten (und die sind tatsächlich nur ein Text). Also weiß das logische Modul, von welchem Gerät die Daten kommen, und kann das bei der Auswertung berücksichtigen. Also braucht es kein solches Flag und man kann einfach direkt den JSON-Text übergeben. Man könnte höchstens vorher noch per RegEx prüfen, ob es eine Antwort auf ein "call" ist oder eine mit "subscribe" bestelltes Event. Die sind recht verschieden von ihrer Funktionsweise und Auswertung her und könnten vielleicht am besten von zwei getrennten logischen Modulen verarbeitet werden.

Es erscheint mir einfacher, die konkreten setter und getter dann eben erst im laufenden Betrieb einer UBUS-Instanz zu ermitteln und diesen Teil dynamisch aufzubauen, eben je nachdem, was tatsächlich gefunden wird (ähnlich, wie CUL_HM das in Abhängigkeit vom Gerätetyp macht).

Das wäre auch eine Möglichkeit. Zu einem gewissen Grad kann man diese Information abfragen. Z.B. gibt es einen Aufruf "list", der die verfügbaren Funktionen und die Datentypen der zu übergebenden Parameter liefert. Allerdings fehlt dann noch der Zusammenhang, was genau diese Funktionen tun und welchen Inhalt diese Parameter haben sollen - das alleine aus den Namen zu schließen erscheint mir nicht möglich... Zumindest basierend auf meinen bisherigen Beobachtungen (zwei Geräte, eins davon mit alter und mit neuer Firmware). Auch die zurückgegebenen Daten unterscheiden sich zwischen Firmware-Versionen bei ansonsten identischem Funktionsaufruf. Normalerweise wird die Client-Software (z.B. als JavaScript-Anwendung in einem Web-Interface) zusammen mit der Firmware verpackt und die entsprechenden Funktionsaufrufe sind hart verdrahtet.
Hoffe mal, dass sich dazu jemand äußert, der mehr Ahnung von zweistufig hat...
[/quote]

[rudolfkoenig]

Hoffe mal, dass sich dazu jemand äußert, der mehr Ahnung von zweistufig hat...

Bin nicht sicher, ob ich das eigentliche Problem verstehe, falls ich daneben liegen sollte, bitte korrigiert mich.

Logische Module kriegen in der ParseFn den hash des physikalischen Moduls als Parameter mit ($iodev).
IOWrite (wird im logischen Modul verwendet) ruft WriteFn des physikalischen Moduls mit $hash->{IODev} des logischen Moduls auf. Im physikalischen Modul kann man die Daten mit weiteren, vom physikalischen Modul abhaengigen Parameter bereichern.
Das logische Modul muss mAn (bis auf esoterische Faelle, wie bei HM, wo FHEM fuer ein dynamisches Routing zustaendig ist) nichts ueber das physikalische ​Modul wissen.

Die Liste der moeglichen set/get Befehle und Attribute kann fuer jede Instanz des gleichen Moduls unterschiedlich sein.

Dispatch braucht einfache Strings als Parameter, da die Verteilung per Regexp realisiert wird.

[xenos1984]

Danke, Rudi, für die Erläuterungen! Das entspricht so weit auch meinem Verständnis aus der Doku im Wiki.

Bei den WebSocket und HTTP Verbindungen, die jeweils asynchron kommunizieren, erscheint mir der Ablauf halbwegs klar. Da geschieht die Zuordnung zwischen Anfrage und Antwort über eine ID, die bei beiden Richtungen identisch ist. Wenn also ein logisches Gerät eine Anfrage mittels IOWrite schickt, ist dieser Anfrage eine ID zugeordnet. Wenn eine Antwort kommt, muss Parse nur schauen, welches Gerät die Anfrage dieser ID geschickt hat. Im Prinzip sehe ich dann zwei Möglichkeiten einer Implementierung:

• Wenn ich das richtig sehe, gibt IOWrite auch den Rückgabewert der Write-Funktion des physikalischen Moduls an das logische Modul zurück. Man könnte also die Erzeugung und Verwaltung der IDs dem physikalischen Modul überlassen. Write erzeugt eine ID und führt den Aufruf aus, gibt dann die ID an das logische Modul zurück, und letzteres merkt sie sich. Wenn die Antwort kommt, ist die ID darin enthalten, und das logische Modul prüft in Parse, wer die Anfrage gestellt hat. "Bitte stellen Sie eine Frage und ziehen Sie eine Nummer. Sie werden aufgerufen."
• Andererseits übergibt IOWrite alle seine Argumente direkt an Write. Also könnte auch das logische Modul eine ID erstellen und zusammen mit der Anfrage übergeben. Die ID kann eine Zeichenkette sein, könnte also einfach den Namen des Geräts + eindeutiger Kennung enthalten. "Bitte stellen Sie eine Frage und geben Sie uns Ihre Telefonnummer und das Aktenzeichen, wir rufen zurück."

Noch bin ich etwas unschlüssig, was besser ist, da es immer noch den Sonderfall gibt, der keine ID benutzt. Wenn man UBUS nicht über eine Schnittstelle, sondern über die Kommandozeile benutzt, erfolgt der Aufruf synchron, und es gibt keine ID. Letzteres ist vermutlich das kleinere Problem, da das physikalische Modul ja bei einem synchronen Aufruf die Zuordnung zwischen Anfrage und Antwort gegeben hat, und den ID-Mechanismus für das logische Modul simulieren kann.

Etwas unklar ist mir noch, wie man in dem Fall am besten die Antwort zurück schickt. Die kommt ja in dem Fall nicht über Read. Und in Write synchron Dispatch aufzurufen, ist vermutlich keine gute Idee, wenn das logische Modul als Reaktion gleich eine weitere Anfrage mittels IOWrite schicken könnte... Das gäbe eine Endlosschleife. Außerdem würde das nicht mit dem Mechanismus 1 von oben funktionieren, weil das logische Modul die Antwort bekäme, bevor das physikalische Modul über den Rückgabewert die ID mitgeteilt hat. D.h. die Nummer würde aufgerufen, bevor sie gezogen wurde Mechanismus 2 ginge, wenn sich das logische Modul die ID merkt, bevor es IOWrite aufruft.

[rudolfkoenig]

Version 2 ist mir sympatischer, da die Abhaengigkeit zwischen den Modulen hier etwas kleiner ist.
Das Problem mit dem Antwort zurueckschicken habe ich leider nicht wirklich verstanden.

[xenos1984]

Version 2 ist mir sympatischer, da die Abhaengigkeit zwischen den Modulen hier etwas kleiner ist.

Inzwischen habe ich mir die Abfragen weiter angesehen und ich denke, dass beide Vor- und Nachteile haben, und man vielleicht die Vorteile von beiden nutzen kann. Letztlich soll die ID ja zwei Dinge erfüllen: A. Zuordnung einer Antwort zu dem logischen Device, das die Anfrage gestellt hat (mittels Dispatch / Parse), B. eindeutige Zuordnung der Antwort zu einer Anfrage innerhalb des logischen Device. Um A zu erreichen, macht es Sinn, den Namen des Device als Teil der ID zu benutzen, um diesen dann in Parse abzufragen. Dafür bräuchte es Methode 2 aus dem letzten Post, d.h. das logische Modul gibt den Device-Namen mit. Wenn man allerdings die Verwaltung von (innerhalb einer Session) eindeutigen IDs für Anfragen dem logischen Modul überlässt, muss man diese ggf. mehrfach implementieren, wenn man ein weiteres logisches Modul haben möchte, das auf das gleiche physikalische Gerät zugreift. Außerdem muss das physikalische Modul auch für seine eigenen Anfragen (Login, Session-Status, ggf. "list" als Information, welche Funktionen unterstützt werden) eine eindeutige ID erzeugen und verwalten. Das sowohl im physikalischen als auch im logischen Modul zu implementieren erscheint mir etwas nach doppeltem Aufwand.

Letzteres erscheint mir sinnvoll, weil "call" und "subscribe" zwei recht unterschiedlich strukturierte Anfrage-Antwort Modelle sind, die beide über die gleiche Schnittstelle laufen. Bei call kommt auf eine Anfrage eine Antwort mit der gleichen ID (Polling). Bei "subscribe" dagegen kommen Events ohne ID und man braucht wiederum eine andere Methode, um herauszufinden, wer die Anfrage gestellt hat...

Meine Idee wäre: Das logische Modul übergibt den Namen des aufrufenden Device mittels IOWrite, damit das physikalische Modul diesen in der ID unterbringen kann. Das physikalische Modul baut diesen in die ID ein und stellt sicher, dass IDs eindeutig sind. Write gibt die eindeutige ID an das logische Modul zurück.

Das Problem mit dem Antwort zurueckschicken habe ich leider nicht wirklich verstanden.

Vielleicht hilft etwas Pseudo-Code:

Code Auswählen Erweitern


sub Phys_Write
{
    my $id = createID();
    if(USE_ASYNC_METHOD)
    {
        sendRequest($id);
    }
    else
    {
        $ret = execProgram();
        Dispatch($id . ":" . $ret);
    }
    return $id;
}

sub Phys_Read
{
    Dispatch($ret_containing_id);
}

sub Log_Parse
{
    # Find device which asked for $id
}

sub Log_sendRequest
{
    $id = IOWrite(...)
    $some_table{id} = $hash->{NAME} # Remember who asked.
}

Wenn das physikalische Gerät Anfragen asynchron behandelt (z.B. über HTTP), gibt obiger Code kein Problem:

Anfrage stellen: Log_sendRequest - > IOWrite -> Phys_Write -> sendRequest; die $id wird an Log_sendRequest zurückgegeben und gespeichert. Wenn die Antwort kommt: Phys_Read -> Dispatch -> Log_Parse und letzteres vergleicht mit der gespeicherten ID.

Wenn die Anfrage synchron erfolgt (z.B. über ein Kommandozeilen-Programm), passiert aber folgendes:

Log_sendRequest - > IOWrite -> Phys_Write -> execProgram, Dispatch -> Parse; jetzt wird Parse aufgerufen und die ID übergeben, bevor Write die ID als Rückgabewert an das logische Modul zurückgegeben hat und letzteres sie sich merken konnte. Parse kann die ID also nicht finden.

Aber auch ohne dieses ID-Problem ist es wohl keine gute Idee, Dispatch direkt in Write aufzurufen, vermute ich mal. Wenn nun das logische Modul als Folge von Parse eine weitere Anfrage stellen möchte und erneut IOWrite aufruft, kommt es dann nicht zu einer Rekursions-Schleife, wenn sich beide ständig gegenseitig aufrufen? Ich habe den synchronen Fall jetzt so "entschärft", dass Write nicht direkt Dispatch aufruft, sondern einen InternalTimer von 1 Sekunde setzt, der dann Dispatch aufruft.

Könnte man alternativ auch direkt in Write die Antwort zurückgeben? Dann müsste aber das logische Modul unterscheiden können, ob es von IOWrite gerade die komplette Antwort bekommen hat, oder nur eine ID, mit der es später via Parse die Antwort bekommt.

[rudolfkoenig]

Der IOWrite Aufrifer bekommt doch die Antwort von WriteFn, das hast Du doch selbst festgestellt, siehe oben.
Dispatch in WriteFn aufzurufen ist auch OK, nur ParseFn muss sich benehmen, und in diesem Fall nicht wieder IOWrite aufrufen.

Viel schlimmer ist "$ret = execProgram()", weil das potentiell blockiert.
Richtigerweise ruft man das Programm mit $fd = system("programm|") auf, und man packt $fd ins selectList, damit ReadFn damit aufgerufen wird. Alternativ verwendet man BlockingCall.

[xenos1984]

Der IOWrite Aufrifer bekommt doch die Antwort von WriteFn, das hast Du doch selbst festgestellt, siehe oben.
Dispatch in WriteFn aufzurufen ist auch OK, nur ParseFn muss sich benehmen, und in diesem Fall nicht wieder IOWrite aufrufen.

Ja, aber WriteFn gibt die ID an den IOWrite Aufrufer zurück, nachdem WriteFn die Antwort des Calls über Dispatch an ParseFn gegeben hat. In dem Moment wartet ParseFn aber noch gar nicht auf diese Antwort, die zu dieser ID gehört, weil es die ID noch nicht bekommen und in seine "meine IDs auf deren Antwort ich warte" Liste eingetragen hat.

Viel schlimmer ist "$ret = execProgram()", weil das potentiell blockiert.
Richtigerweise ruft man das Programm mit $fd = system("programm|") auf, und man packt $fd ins selectList, damit ReadFn damit aufgerufen wird. Alternativ verwendet man BlockingCall.

Ah, da hast du natürlich Recht. Die Syntax $fd = system("programm|") ist mir in der Form noch nie begegnet... Hast du ein Beispiel, wo die in FHEM benutzt wird, oder einen Link, wo sie beschrieben wird? In der Perl-Doku konnte ich nur system() als blockierende Funktion finden, die den Exit-Status zurückliefert, aber keinen Dateideskriptor. Um die Programmausgabe zu lesen, verweist die Perl-Doku auf qx, was aber auch blockiert.

[rudolfkoenig]

Die Syntax $fd = system("programm|") ist mir in der Form noch nie begegnet...

Sorry, mein Fehler, ich habe es mit open verwechselt.
Siehe auch https://perldoc.perl.org/functions/open#Opening-a-filehandle-into-a-command

[xenos1984]

Nach einigem Einlesen und Probieren habe ich jetzt einmal versucht, ein zweistufiges Modul zu implementieren, von denen das physikalische Modul UBUS_CLIENT die Kommunikation mit dem uBus-Gerät verwaltet und das logische Modul UBUS_CALL einen Funktionsaufruf ausführt. Damit ist es jetzt möglich, individuell zu konfigurieren, welche Aufrufe ausgeführt werden sollen, in welchem zeitlichen Abstand und wie die Antwort in Readings umgesetzt werden soll (dafür kann Perl-Code angegeben werden).

NB! Das ist zuerst einmal nur ein Proof-of-Concept, um zu testen, ob da überhaupt Daten rauskommen und man die in Readings verarbeiten kann. Es sind noch ganz viele Sachen auf der ToDo-Liste, die ich noch nicht umgesetzt habe:

• Richtige Implementierung von enable/disable, um die Module zeitweise anzuhalten.
• Periodische "keepalive" Aufrufe im physikalischen Modul.
• Automatischer neuer Login wenn die Verbindung verloren gegangen ist.
• Timeout und Fehlerbehandlung.
• Nicht-blockierender Aufruf der Kommandozeilen-Version.
• Implementierung von subscribe / ereignisgesteuerte Kommunikation.
• Perl-Code in der Definition des logischen Geräts, um dynamische Parameter zu übergeben.
• Arrays statt einzelner Parameter-Werte, um die gleiche Funktion mehrfach aufzurufen (z.B. um den Status mehrerer Netzwerk-Schnittstellen in einem Gerät abzurufen).

Die Benutzung ist in der Commandref dokumentiert - Beispiele kommen noch.

Code Auswählen Erweitern


UBUS_CALL
[EN DE]

    The uBus IPC/RPC system is a common interconnect system used by OpenWrt. Services can connect to the bus and provide methods that can be called by other services or clients or deliver events to subscribers. This module implements the "call" type request. It is supposed to be used together with an UBUS_CLIENT device, which must be defined first.
    Define

    define <name> UBUS_CALL <module> <function> [<parameters>]

    uBus calls are grouped under separate modules or "paths". In order to call a particular function, one needs to specify this path, the function to be called and optional parameters as <key>=<value> pairs. Examples:

        define <name> UBUS_CALL system board

        define <name> UBUS_CALL iwinfo devices

        define <name> UBUS_CALL network.device status name=eth0

        define <name> UBUS_CALL file list path=/tmp

        define <name> UBUS_CALL file read path=/etc/hosts

    The supported calls highly depend on the device on which the uBus daemon is running and its firmware. To get an overview of the calls supported by your device, consult the readings of the UBUS_CLIENT device which represents the connection to the physical device.
    Set

        set <name> disable

        Sets the state of the device to inactive, disables periodic updates and disconnects a websocket connection.

        set <name> enable

        Enables the device, so that automatic updates are performed.

        set <name> update

        Performs an uBus call, updates the corresponding readings and resets any pending interval timer.
    Get

    There are no get commands defined.
    Attributes
        disable
        disabledForIntervals

        attr <name> interval <interval>

        Defines the interval (in seconds) between performing consecutive calls and updating the readings.

        attr <name> readings {<Perl-code>}

        Perl code which must return a hash of <key> => <value> pairs, where <key> is the name of the reading and <value> is its value. The following variables are available in the code:
            $NAME: name of the UBUS_CALL device.
            $MODULE: module name used in the call (see definition).
            $FUNCTION: function name used in the call (see definition).
            %PARAMS: hash of parameters used in the call (see definition).
            $RAW: raw JSON response returned by the call.
            $ERROR: reported error code, 0 means success.
            %DATA: decoded result data as Perl hash.

        If this attribute is omitted, its default value is {FHEM::UBUS_CALL::DefaultReadings($RAW)}. This function executes json2nameValue in the JSON result and turns all returned data into readings named by their position in the JSON tree. It is also possible to call this function in user-defined Perl code first, and then modify the returned hash, for example by deleting unwanted readings or adding additional, computed readings.
    Readings

    Any readings are defined by the attribute readings.

UBUS_CLIENT
[EN DE]

    The uBus IPC/RPC system is a common interconnect system used by OpenWrt. Services can connect to the bus and provide methods that can be called by other services or clients or deliver events to subscribers. This module provides different methods to connect to an uBus interface, either using its command line interface or remotely via websocket or HTTP.
    Define

    define <name> UBUS_CLIENT <method>

    Three possible connection methods for <method> are supported:
        For a websocket connection, a url of the form (ws|wss)://<host>[:port][/path] is used. Example:

        define <name> UBUS_CLIENT ws://192.168.1.1

        For a HTTP connection, a url of the form (http|https)://<host>[:port][/path] is used. Example:

        define <name> UBUS_CLIENT http://192.168.1.1/ubus

        To use the ubus command line tool (if FHEM is running on the same device as ubus), use ubus. Example:

        define <name> UBUS_CLIENT ubus

    When using the websocket or HTTP connection methods, a valid user name and password must be provided. The user name defaults to user, but can be changed with an attribute:

    attr <name> username <username>

    The password is set with the following command, which must be issued only once, and stored as an obfuscated value on disk:

    set <name> password <password>

    When a connection and login have been performed successfully, a list command is executed to obtain the available calls supported by this device, and the result is filled into the readings of the device.
    Set

        set <name> disable

        Sets the state of the device to inactive, disables periodic updates and disconnects a websocket connection.

        set <name> enable

        Enables the device, establishing a websocket connection first if necessary.

        set <name> password <password>

        Sets the password used to authenticate via websocket or HTTP and stores it on disk.
    Get

    There are no get commands defined.
    Attributes
        disable
        disabledForIntervals

        attr <name> username <username>

        Defines the username to be used for login via websocket or HTTP. The default value is user.
    Readings

    When the connection is established, the module executes a list command and creates the following readings:
        mod_<n>_name: name (path) of the n'th module in the uBus tree
        mod_<n>_func_<m>_name: name of the m'th function supported by the n'th module
        mod_<n>_func_<m>_param_<k>_name: name of the k'th parameter of the m'th function of the n'th module
        mod_<n>_func_<m>_param_<k>_type: type of the k'th parameter of the m'th function of the n'th module

    These can be used to perform calls using the UBUS_CALL module.


[xenos1984]

Ich habe wieder ein wenig an meinen UBUS-Modulen gebastelt und langsam nähern sie sich der Praxistauglichkeit.

Erledigt:

• Richtige Implementierung von enable/disable, um die Module zeitweise anzuhalten.
• Periodische "keepalive" Aufrufe im physikalischen Modul.
• Automatischer neuer Login wenn die Verbindung verloren gegangen ist.
• Perl-Code in der Definition des logischen Geräts, um dynamische Parameter zu übergeben.
• Arrays statt einzelner Parameter-Werte, um die gleiche Funktion mehrfach aufzurufen (z.B. um den Status mehrerer Netzwerk-Schnittstellen in einem Gerät abzurufen).
• attr IODev zur Wahl des physikalischen Gerätes, wenn es mehrere gibt. (Ungetestet, da ich nur eines habe.)
• Reading state bei UBUS_CALL gibt Aufschluss über den Status des Funktionsaufrufs.

ToDo:

• Timeout und Fehlerbehandlung.
• Nicht-blockierender Aufruf der Kommandozeilen-Version. (Da die scheinbar eine andere API benutzt und ich kein Gerät zum Testen habe, ist die erst einmal weiter hinten auf der Liste, und ich werde sie erst einmal deaktivieren.)
• Implementierung von subscribe / ereignisgesteuerte Kommunikation. (Erfordert ebenfalls noch mehr Input.)

Die Commandref aktualisiere ich noch, aber wer schon einmal einen Blick werfen möchte, oder bei entsprechender Hardware auch testen, ist herzlich dazu eingeladen. Beispiele (können je nach verwendeter Hardware anders aussehen) - die Readings des physikalischen Geräts (UBUS_CLIENT) geben eine Info):

Physikalisches Gerät:

Code Auswählen Erweitern


defmod juci UBUS_CLIENT ws://192.168.1.1


Status aller Netzwerk-Interfaces:

Code Auswählen Erweitern


defmod network_device_status UBUS_CALL network.device status


Code Auswählen Erweitern


...
eth0_up true
...


Vorhandene Status-LEDs (wird nur einmal ausgelesen, Komma-separierte Liste wird als Reading leds erzeugt):

Code Auswählen Erweitern


defmod router_config_leds UBUS_CALL uci get config=leds
attr router_config_leds interval 0
attr router_config_leds readings {\
my $r = FHEM::UBUS_CALL::DefaultReadings($RAW);;\
$r->{leds} = join(',', map {$r->{$_}} grep(/values_.*_\.name/, keys %{$r}));;\
$r\
}


Code Auswählen Erweitern


...
leds internet,voice,wifi
...


LED Status (Funktion erwartet als Parameter "name" eine LED. Hier wird ein Reading übergeben, das eine Komma-separierte Liste der Namen enthält. Die Funktion wird für jeden Wert einzeln aufgerufen. Bei der Erzeugung der Readings wird dieser Wert als Präfix übergeben.):

Code Auswählen Erweitern


defmod router_system_led UBUS_CALL juci.system led_status name={ReadingsVal('router_config_leds','leds','')}
attr router_system_led readings {FHEM::UBUS_CALL::DefaultReadings($RAW, $PARAMS{name} . '_')}


Code Auswählen Erweitern


...
wifi_brightness 100
wifi_state off
...


Perl-Code in der Definition kann entweder eine einzelne Zeichenkette zurückgeben oder eine Array-Referenz. Bei letzterem wird für jeden Wert des Arrays ein Funktionsaufruf an das physikalische Modul geschickt. Bei ersterem wird eine Komma-separierte Liste aufgeteilt und ebenfalls für jeden Wert ein Funktionsaufruf generiert, bei einem einzelnen Wert also nur einer. Es dürfen keine Leerzeichen enthalten sein, sonst kommt parseParams durcheinander - längerer Perl-Code sollte also besser in myUtils o.ä. ausgelagert werden.

Es kann passieren, dass UBUS_CALL nach einem Neustart disconnected anzeigt. Das passiert, wenn ein Funktionsaufruf abgesetzt wird, bevor sich der UBUS_CLIENT eingeloggt hat. Beim nächsten Update sollte es dann aber laufen.

[xenos1984]

Update zum UBUS_CLIENT: Mit

Code Auswählen Erweitern


attr <device> refresh 0


kann man nun das periodische "session list" zur Überprüfung, ob die aktuelle Session noch gültig ist, abschalten. Wenn "session list" keine Session zurückliefert, wird auf diese Möglichkeit im Log hingewiesen.

[xenos1984]

Nachdem es nach ausführlichem Testen über eine Woche stabil im Betrieb war (und ich die Guidelines noch einmal gründlich durchgegangen bin, da es mein erstes Modul ist), habe ich es jetzt im SVN eingecheckt.