[erledigt] MQTT_GENERIC_BRIDGE - Optimierungen für MQTT2-IO-Module

Begonnen von Beta-User, 13 Januar 2021, 13:50:14

Vorheriges Thema - Nächstes Thema

hexenmeister

Zitat von: Beta-User am 18 Januar 2021, 21:28:46
Diese (gesamte) Fassung läuft jedenfalls bei mir hier im Hauptsystem seit ein paar Minuten, bringt aber dann noch zusätzlich diverse perlcritic-Änderungen und den 1. cref-Vorschlag (noch ohne Hinweis auf das mit dem 2-er-IO-Attribut).

Die wesentlichen Ideen:
- IO-TYPE-Bestimmung etwas optimiert;
- Die Zählung ist jetzt abhängig vom IO-TYPE. Sollte für die neuen IO's die Event-Flut eindämmen und für 00_MQTT.pm 100% kompatibel sein;
- Die Schleife zum Durchlaufen aller MGB-Instanzen ist umgebaut, es sollte jetzt immer NEXT zurückkommen. Da bin ich aber nicht sicher, ob das noch kompatibel zu 00_MQTT.pm ist, weil da das GP_Catch verwendet wird und ggf. jede MGB-Instanz separat aufgerufen wurde (?). Unklar ist mir aber warum dann für 00_MQTT.pm überhaupt das Array verwendet wurde, da hätte eigentlich dann nur die eine Instanz "durchlaufen" werden dürfen und dann macht es ggf. auch Sinn, das return "" zwischendurch zu lassen, aber eben dann mit der Prüfung, ob es 00_MQTT.pm war, das die ParseFn aufgerufen hat.
Den NEXT-Teil habe ich bisher nicht getestet...

- Ob die Optimierung für die IO-TYPE-Bestimmung so wirklich viel bringt - bin ich nicht überzeugt. Sind ja auch nur Zugriffe auf ("etwas weiter liegende") Hashes.
Bin da aber kein Profi. Wenn das wirklich was bringt - übernehme ich die Änderung. In jedem Fall, wenn man diese cached, dann sollte aber auch Änderungen des iodev-Attributen verarbeitet werden.

- Die Änderung zum Eindämmen von "Nahrichten Flut" schaue ich mir noch genauer an und werde sie übernehmen.

- Die Änderung der Rückgabe in onmessage wird vermutlich MQTT-Kompatibilität brechen.

Werde mir morgen weiter ansehen, sonst wird meine Nacht zu kurz ;D

Beta-User

Na ja, das mit der IO-Frage sind dann halt jedes Mal noch ein paar zusätzliche Vergleiche, und @MQTT2-IO eben für jede Nachricht... _Glaube_, es würde Sinn machen, das soweit als möglich zu optimieren, ggf. bis hin zum nummerischen Vercoden (?) des TYPE und/oder der Weiterreichung der bekannten Info zwischen den Funktionen. (@Rudi: Deine Meinung zu dem Punkt würde mich interessieren).
Der Helper-Hash wird direkt in AttrFn() gefüllt, das sollte eigentlich (bis auf gelöscht) passen, ab Neuvergabe ist alles wieder i.O.. Ein Problem könnte es darstellen, wenn da Unsinn drin steht, sowas könnte man ggf. für den $init_done-Fall abfangen? (ggf. auch in einer gesonderten Funktion später, dann aber mit LOG-Eintrag?)

Was "onmessage" angeht, könnte man die Rückgabe ja wieder abhängig machen vom IO-TYPE. Das wäre dann nur eben wieder nochmal einer dieser Vergleiche mehr.



Was mehrere MGB für ein IO angeht, könnte ich mir zumindest einen Anwendungfall vorstellen (aber eher für publish): Hat man z.B. ein "ESP-Display", dann schickt man darauf ggf. parallel dasselbe Reading. (Klar, das geht eigentlich besser via !display, und damit ist es evtl. eher eine Darstellungsfrage).
Server: HP-elitedesk@Debian 12, aktuelles FHEM@ConfigDB | CUL_HM (VCCU) | MQTT2: ZigBee2mqtt, MiLight@ESP-GW, BT@OpenMQTTGw | ZWave | SIGNALduino | MapleCUN | RHASSPY
svn: u.a Weekday-&RandomTimer, Twilight,  div. attrTemplate-files, MySensors

Beta-User

Sorry, aber ich habe in meinem Vorschlag auf alle Fälle auch noch einen bug drin:
Zitat1 : ERROR: >ARRAY(0x5574eb7a44f8)< returned by the MQTT_GENERIC_BRIDGE ParseFn is invalid, notify the module maintainer
Mache grade den Test mit
if( ref($fret) eq 'ARRAY' ) {
      push @ret, @$fret;
    }

Das sieht - jedenfalls was das Log angeht und auf den ersten Blick - besser aus...
Server: HP-elitedesk@Debian 12, aktuelles FHEM@ConfigDB | CUL_HM (VCCU) | MQTT2: ZigBee2mqtt, MiLight@ESP-GW, BT@OpenMQTTGw | ZWave | SIGNALduino | MapleCUN | RHASSPY
svn: u.a Weekday-&RandomTimer, Twilight,  div. attrTemplate-files, MySensors

Beta-User

Zitat von: hexenmeister am 19 Januar 2021, 00:31:00
- Die Änderung der Rückgabe in onmessage wird vermutlich MQTT-Kompatibilität brechen.
Habe versucht, die Befürchtung im Code nachzuvollziehen. Hier mal meine Überlegungen:

- MQTT ruft direkt onmessage auf:
Eingehende Nachrichten sind "MQTT_PUBLISH", daher ist das die Verarbeitung ab Zeile 542 in 00_MQTT.pm?
Dann wird für alle Clients via GP_ForallClients (diese von Rudi ungeliebte Parse-Funktion) deren "OnMessageFn" aufgerufen, wenn es eine solche gibt.

- Für MGB ist das der Fall (~Zeile 411):
  $hash->{OnMessageFn} = "MQTT::GENERIC_BRIDGE::onmessage";

- Die Rückgabe interessiert 00_MQTT.pm gar nicht, oder?
Die "onmessage" wird (#551) über CallFn() aufgerufen, aber deren Rückgabe wird nicht in eine Variable gepackt oä.. Problematisch könnte also nur "wantarray" sein, aber da keine Rückgabevariable genannt ist, dürfte das auch schlicht "false" sein...?

(- "Parse" ist - soweit ich das erkennen kann für 00_MQTT.pm eigentlich gar nie Thema.)

Ergo sollte es unproblematisch sein, dass 00_MQTT.pm da nun ein Array präsentiert bekommt (oder wie bisher nichts)...

M.E. ist es aber vermutlich einfacher, onmessage() einen weiteren (optionalen, boolschen) Parameter zu spendieren, um die Art der Ansteuerung zu kennzeichnen, ein Vorschlag dazu wäre unter https://github.com/rejoe2/fhem_MQTT_GENERIC_BRIDGE/commit/ad030d274ed2bb084f9aeb66b4c416b92a878ee1 zu finden.

[OT]
CallFn() kannte ich noch gar nicht, ist aber ggf. hilfreich, um ein kleines Problem im Zusammenspiel zwischen WeekdayTimer und weekprofile zu beheben.
[/OT]
Server: HP-elitedesk@Debian 12, aktuelles FHEM@ConfigDB | CUL_HM (VCCU) | MQTT2: ZigBee2mqtt, MiLight@ESP-GW, BT@OpenMQTTGw | ZWave | SIGNALduino | MapleCUN | RHASSPY
svn: u.a Weekday-&RandomTimer, Twilight,  div. attrTemplate-files, MySensors

Beta-User

Server: HP-elitedesk@Debian 12, aktuelles FHEM@ConfigDB | CUL_HM (VCCU) | MQTT2: ZigBee2mqtt, MiLight@ESP-GW, BT@OpenMQTTGw | ZWave | SIGNALduino | MapleCUN | RHASSPY
svn: u.a Weekday-&RandomTimer, Twilight,  div. attrTemplate-files, MySensors

hexenmeister

Zitat von: Beta-User am 19 Januar 2021, 13:31:16
(Hier OT) betr. die Unterstützung von AttrTemplate:
'n Abend!
Habe jetzt folgendes (mehr oder weniger abgewandelt) übernommen:
- increment 'incoming-count' only if at least one device is affected => gegen "Nachrichtenflut"
- Schleife über die Instanzen in Parse => damit alle Instanzen zum Zuge kommen
- Optimierung für die Ermittlung von IODev-Type

und Deine Erweiterung für AttrTemplate.

Kurz getestet, scheint alles noch zu funktionieren :)

Danke für die Patches!

Beta-User

Danke auch für's übernehmen!

Was AttrTemplate angeht, scheinst du die Methode also als gute Idee zu empfinden, dann wird sich jetzt wohl "jemand" um die damit zusammenhängenden Fragen der "Standardisierung" von Topics, Readings und Werten befassen dürfen...
Werde dazu daher den einen oder anderen Thread eröffnen, mal schauen.

Im Nachgang ist mir noch eingefallen, dass man vermutlich AttrTemplate_Initialize() aufrufen sollte, wenn eine MGB nach $init_done definiert wird. Zum Hintergrund: da man die betreffenden attrTemplates nur braucht, wenn eine MGB vorhanden ist, werden die auch nur in diesem Fall geladen. Das würde also Rückfragen vermeiden, warum keine templates zu sehen sind.



Eine Sache betr. Optimierung ist mir noch aufgefallen: MGB verwendet derzeit kein NOTIFYDEV. Kostet nicht nur ~30% Performance, es wäre v.a. auch transparenter, wenn dieser Mechanismus genutzt würde. Kann gerne versuchen, dazu was (auf Basis der aktuellen svn-Version ::) ) liefern, wenn das gewünscht ist.




Ansonsten wären wir m.E. - abgesehen von Doku-Fragen zur Reihenfolge (clientOrder) - mit dem eigentlichen Thema dieses Threads soweit durch, oder?
Server: HP-elitedesk@Debian 12, aktuelles FHEM@ConfigDB | CUL_HM (VCCU) | MQTT2: ZigBee2mqtt, MiLight@ESP-GW, BT@OpenMQTTGw | ZWave | SIGNALduino | MapleCUN | RHASSPY
svn: u.a Weekday-&RandomTimer, Twilight,  div. attrTemplate-files, MySensors

rudolfkoenig

ZitatMGB verwendet derzeit kein NOTIFYDEV. Kostet nicht nur ~30% Performance
Die "~30% Performance"-Kosten sind stark davon abhaengig, wie die Konfiguration ist. Wenn alle Events per MGB weitergeleitet werden, dann wird NOTIFYDEV eher Zusatzaufwand bei der Neuberechnung der NotifyFn-Warteschlangen bedeuten. Und weil normalerweise nur eine MGB-Instanz definiert ist, kostet das Fehlen von NOTIFYDEV pro Event "nur" einen zusaetzlichen, potentiell ueberfluessigen NotifyFn Aufruf. Ich weiss allerdings (noch?) nicht, wie teuer ein MGB-NotifyFn Aufruf ist.

Ich will damit nicht sagen, dass man NOTIFYDEV nicht setzen soll, nur dass man Aufwand/Nutzen vorher abwaegen sollte.

Beta-User

Na ja, es wird - soweit ich das verstehe - kurz gecheckt, ob es "global" ist und ansonsten mehr oder weniger direkt geschaut, ob das betr. Device in der internen Device-Liste ist (#2165ff in der aktellen svn-Version). Von daher würde ich annehmen, dass es MGB-intern ziemlich effizient ist bzw. es sich kaum unterscheidet von dem, was zur Auswertung von NOTIFYDEV erforderlich wäre.

Meine Tendenz betr. der "Beispiel-Implementierung" (auch via attrTemplate-Vorschläge) ist eher in Richtung selektiver Weiterleitung einzelner Readings aus tendenziell wenigen Geräten. Von daher dürfte es in diesem gedanklichen Modell so sein, dass z.B. bei der Implementierung eines vollen Bedieninterfaces auf MQTT-Basis vielleicht 30-40% der Devices sich dann auch im Bedieninterface wiederfinden. Dabei bin ich mal davon ausgegangen, dass z.B. bei CUL_HM alle Kanäle via Bulk-update aktualisiert werden, wenn eine Nachricht empfangen wird und habe das einschl. der Kanäle dann als ein Device gezählt.

Ob sich das unter diesen Gegebenheiten lohnt, kannst du vermutlich besser beurteilen. ATM macht das dann eher keinen großen Sinn (oder vielleicht optional, falls jemand das nur dazu nutzen will, ein paar Infos an ein ESP-Display zu schicken?), oder?

Die überwachten Geräte kann man via list sehen, von daher ist das mit der Transparenz auch kein echtes Argument.
Server: HP-elitedesk@Debian 12, aktuelles FHEM@ConfigDB | CUL_HM (VCCU) | MQTT2: ZigBee2mqtt, MiLight@ESP-GW, BT@OpenMQTTGw | ZWave | SIGNALduino | MapleCUN | RHASSPY
svn: u.a Weekday-&RandomTimer, Twilight,  div. attrTemplate-files, MySensors

hexenmeister

Zitat von: Beta-User am 20 Januar 2021, 07:53:31
Was AttrTemplate angeht, scheinst du die Methode also als gute Idee zu empfinden, dann wird sich jetzt wohl "jemand" um die damit zusammenhängenden Fragen der "Standardisierung" von Topics, Readings und Werten befassen dürfen...
Werde dazu daher den einen oder anderen Thread eröffnen, mal schauen.

Im Nachgang ist mir noch eingefallen, dass man vermutlich AttrTemplate_Initialize() aufrufen sollte, wenn eine MGB nach $init_done definiert wird. Zum Hintergrund: da man die betreffenden attrTemplates nur braucht, wenn eine MGB vorhanden ist, werden die auch nur in diesem Fall geladen. Das würde also Rückfragen vermeiden, warum keine templates zu sehen sind.

Die Idee mit den Templates finde ich gut und hatte frühen auch schon mit dem Gedanken gespielt, noch bevor es diesen Mechanismuf gab. Blieb aber auch bei Ideen. Ich muss mir das Verfahren noch genauer angucken, weiß noch nicht, ob ich mir das richtig vorstelle. Was ich gerne hätte, wäre eine Möglichkeit, quasi per Knopfdruck ein bestimmten Device/DeviceTyp gebrauchsfertig einzurichten, in diesem Fall mit Attribute wie "mqttDefaults", "mqttPublish"; "mqttSubscribe" (bzw. können diese je nach Prefix auch anders heißen).

Wäre es schädlich, wenn die AttrTemplate_Initialize() unter Umständen mehrfach aufgerufen wird? Wenn nicht, könne ich einfach in firstInit stecken, dieser wird ggf. bei Setzen von IODev nochmals aufgerufen.


Zitat von: Beta-User am 20 Januar 2021, 07:53:31
Eine Sache betr. Optimierung ist mir noch aufgefallen: MGB verwendet derzeit kein NOTIFYDEV. Kostet nicht nur ~30% Performance, es wäre v.a. auch transparenter, wenn dieser Mechanismus genutzt würde. Kann gerne versuchen, dazu was (auf Basis der aktuellen svn-Version ::) ) liefern, wenn das gewünscht ist.

Ich weiß nicht recht... Die MQTT_GENERIC_BRIDGE braucht potintiell alle Events von allen Geräten mit den entsprechenden Attributen. Es wirf ein Lookup gemacht und wenn das Gerät nicht dabei ist, geht es schon wieder raus. Ich verspreche mir nicht viel davon.

hexenmeister

Zitat von: rudolfkoenig am 20 Januar 2021, 09:48:43
Die "~30% Performance"-Kosten sind stark davon abhaengig, wie die Konfiguration ist. Wenn alle Events per MGB weitergeleitet werden, dann wird NOTIFYDEV eher Zusatzaufwand bei der Neuberechnung der NotifyFn-Warteschlangen bedeuten. Und weil normalerweise nur eine MGB-Instanz definiert ist, kostet das Fehlen von NOTIFYDEV pro Event "nur" einen zusaetzlichen, potentiell ueberfluessigen NotifyFn Aufruf. Ich weiss allerdings (noch?) nicht, wie teuer ein MGB-NotifyFn Aufruf ist.

Ich will damit nicht sagen, dass man NOTIFYDEV nicht setzen soll, nur dass man Aufwand/Nutzen vorher abwaegen sollte.

Wenn das Gerät/Event nicht von Interesse ist, läuft es nur auf ein kurzes Lookup hinaus, wenn natürlich gesendet werden soll, kann man man das mit 'expression' sehr teuer machen. ;D

Beta-User

@Rudi: ich hätte noch eine Performance-Frage:
Die Überlegung wäre, ggf. AnalyzePerlCommand statt eval für "expression" aufzurufen. Das kostet etwas Performance, bietet aber dafür "gewohnte Syntax" (z.B. ReadingsNum() sollte sich direkt verwenden lassen, sonst müsste man das entweder mit "::" aufrufen oder in das Package importeren)  und (ggf.) stacktrace.
An sich glaube ich nicht, dass es mit bestehenden Installationen bzw. heutigen expression-Ausdrücken Probleme aufwerfen würde, aber deine Meinung zu diesem Aspekt der Rückwärts-Kompabilität und zur Performance-Frage wäre interessant.

Zitat von: hexenmeister am 20 Januar 2021, 22:21:20
Die Idee mit den Templates finde ich gut und hatte frühen auch schon mit dem Gedanken gespielt, noch bevor es diesen Mechanismuf gab. Blieb aber auch bei Ideen. Ich muss mir das Verfahren noch genauer angucken, weiß noch nicht, ob ich mir das richtig vorstelle. Was ich gerne hätte, wäre eine Möglichkeit, quasi per Knopfdruck ein bestimmten Device/DeviceTyp gebrauchsfertig einzurichten, in diesem Fall mit Attribute wie "mqttDefaults", "mqttPublish"; "mqttSubscribe" (bzw. können diese je nach Prefix auch anders heißen).
Hier findest du eine passende attrTemplate-Datei:
https://forum.fhem.de/index.php/topic,117423.msg1123626.html#msg1123626

Zitat
Wäre es schädlich, wenn die AttrTemplate_Initialize() unter Umständen mehrfach aufgerufen wird? Wenn nicht, könne ich einfach in firstInit stecken, dieser wird ggf. bei Setzen von IODev nochmals aufgerufen.
"Schädlich" nicht, aber unnötig. Der Aufruf macht nur Sinn, wenn man entweder
- eine neue/geänderte template-File hat (ergo: nach dem Abspeichern der oben verlinkten Datei einmalig ausführen), oder
- ein Device erstellt hat, das ggf. zusätzliche attrTemplate möglich macht, die bisher nicht geladen waren (das "prereq:"-Flag, das z.B. die zigbee2tasmota-attrTemplate nicht lädt, wenn es keine bridge (=passende Hardware) gibt.

Beim FHEM-Start wird es sowieso ausgeführt, das Thema beschränkt sich also auf diesen einen Fall...

Die vorliegende .tempate beinhaltet
- etwas englische allgemeine Info ("commandref" ist übertrieben, aber es sollte ausreichen um die Frage zu klären, wie es denn gedacht ist...)
- die Option, das MGB-Device selbst zu konfigurieren (ist derzeit im Wesentlichen halt etwas allgemeine Info sowie ein Vorschlag, wie $base festgelegt werden könnte)
- ein "Device"-Template, mit dem Thermostat-Geräte verschiedener Typen (MAX, CUL_HM und ZWave) mit passenden (einschl. der prefix-Bestimmung ;) ) Attributen versorgt werden können. (Für den "vollen Komfort" würde nur die "automatische Zwischenbestimmung" des Device-Type fehlen, was aber ggf. andere Probleme verursachen könnte).

Kurz:
Du wendest das "base_settings_to_MQTT_GENERIC_BRIDGE" an, und dann für jeden Thermostaten das "mgb_thermostat" (das dich dann nach einigen weiteren Infos fragt). Fertig...
(noch nicht perfekt, aber man hat zumindest die Option, via MQTT die Solltemperatur zu regeln, immer unter der "Flagge" "desired-temp".)

Würde vorschlagen, das Thema attrTemplate in einem separaten Thread abzuhandeln, auch, damit das ggf. mehr "Helfer" mitbekommen und wir dann auch die $base-Geschichte so geregelt bekommen, dass es für die User paßt.

Zitat
Ich weiß nicht recht... Die MQTT_GENERIC_BRIDGE braucht potintiell alle Events von allen Geräten mit den entsprechenden Attributen. Es wirf ein Lookup gemacht und wenn das Gerät nicht dabei ist, geht es schon wieder raus. Ich verspreche mir nicht viel davon.
Das wäre nicht das Problem. Via NOTIFYDEV könntest du (nur) bestimmen, von welchen Geräten du (immer alle) Events haben willst. Aber der Aufwand dürfte in den meisten Fällen nicht gerechtfertigt sein; anders als bei einem klassischen Event-Handler geht es ja bei so einem Querschnitts-Device darum, eine zentrale Stelle zu haben, über die (sowieso) alles läuft; zum Vorsortieren (durch fhem-pl) besteht daher häufig deutlich weniger Anlass...
M.E. kann/sollte man das Modul in dem Punkt lassen "as is".
Server: HP-elitedesk@Debian 12, aktuelles FHEM@ConfigDB | CUL_HM (VCCU) | MQTT2: ZigBee2mqtt, MiLight@ESP-GW, BT@OpenMQTTGw | ZWave | SIGNALduino | MapleCUN | RHASSPY
svn: u.a Weekday-&RandomTimer, Twilight,  div. attrTemplate-files, MySensors

rudolfkoenig

ZitatDie Überlegung wäre, ggf. AnalyzePerlCommand statt eval für "expression" aufzurufen. Das kostet etwas Performance, bietet aber dafür "gewohnte Syntax" (z.B. ReadingsNum() sollte sich direkt verwenden lassen, sonst müsste man das entweder mit "::" aufrufen oder in das Package importeren)  und (ggf.) stacktrace.
Bin verwirrt: welcher meiner Module braucht ein ::, um auf ReadingsNum zugreifen zu koennen?

Beta-User

Zitat von: rudolfkoenig am 21 Januar 2021, 10:54:42
Bin verwirrt: welcher meiner Module braucht ein ::, um auf ReadingsNum zugreifen zu koennen?
Bei keinem!
Es ging um MGB; da braucht man diesen Umweg (derzeit) in "expression", und die Frage war, ob es im Ergebnis "teuer" ist, das umzustellen.

Dass sich die Frage nicht wirklich allgemeingültig beantworten läßt, ist mir schon klar, ich _vermute_, dass das, was hexenmeister in diesem Beitrag gezeigt hat eher eine komplexe Variante ist, und tendiere daher dazu, das über AnalyzePerlCommand in die "FHEM-übliche" Syntax zu überführen.
Server: HP-elitedesk@Debian 12, aktuelles FHEM@ConfigDB | CUL_HM (VCCU) | MQTT2: ZigBee2mqtt, MiLight@ESP-GW, BT@OpenMQTTGw | ZWave | SIGNALduino | MapleCUN | RHASSPY
svn: u.a Weekday-&RandomTimer, Twilight,  div. attrTemplate-files, MySensors

rudolfkoenig

AnalyzePerlCommand ist nicht viel teurer in diesem Fall als eval, es werden zusaetzlich nur die $sec,$min,$hour, etc Variablen gesetzt und $we.
Das potentiell teure Authorized ist nur fuer Faelle mit Client-Verbindungen aktiv, was hier nicht der Fall ist.
Und featurelevel <= 5.6 sollte auch kaum jemand setzen.