Dispatch und readingsBeginUpdate - Bug oder Mißbrauch meinerseits

[olwaldi]

Danke für den Tip.

Aber ich glaube, daß X_Attr ABSICHTLICH den $hash von fhem nicht bekommt. Letztendlich kann (und soll?) man das Ändern eines Attributs quasi als "globales" Notify "auffassen", und so funktionierts ja auch prima & logisch konsistent.

Beim Debuggen habe ich auch gesehen, daß jede Änderung eines Readings in DENON_AVR als notify in DENON_AVR_Notify vorbeikommt, und das sind ganz schön viele (trotz event-on-changed-reading). Könnte man auch als überflüssig empfinden, ist aber m.E. auch "richtig" im Sinne von fhem. Ich könnte das ja durch  readingsSingleUpdate(..., 0) vermeiden, aber dann fehlen die Events etwa beim Update in FHEMWEB.

Letztendlich ist das eine Stilfrage, da gibt's kein richtig oder falsch (daher meine Anführungszeichen).

Grüßle, Michael

[Beta-User]

Aber ich glaube, daß X_Attr ABSICHTLICH den $hash von fhem nicht bekommt. Letztendlich kann (und soll?) man das Ändern eines Attributs quasi als "globales" Notify "auffassen", und so funktionierts ja auch prima & logisch konsistent.

Das ist für Querschnitts-Module wie MQTT_GENERIC_BRIDGE (oder readingsWatcher etc.) vielleicht korrekt, aber für die Zwecke des hier in Rede stehenden Modul ist es "gefühlter Unsinn".

Primär ist AttrFn() dafür da, Attributänderungen im betreffenden Modul selbst zu verarbeiten, oder welchen Zweck sollte das sonst haben? Dass da aus irgendwelchen Gründen nicht direkt der Instanz-HASH referenziert wird, ist imo schlicht ein historischer Zufall, dem man keine überhöhte Beachtung zukommen lassen sollte.

Und nein, dass das Modul sich selbst triggert, ist nicht "richtig", sondern zu vermeidender overhead! Das Modul braucht weder eine NotifyFn(), noch sollte es sie haben!

Immer noch meine 2ct.

(Und auch bei dem anderen Thema mit dem ReadyFn() würde ich dafür plädieren, das Problem primär darüber zu lösen, und z.B. den anderen Timer dann darin auch immer wieder zu erneuern bzw. den auf eine längere Periode anzusetzen als die ReadyFn()).
Man sollte immer primär die vom framework bereitgestellten (Spezial-) Funktionen nutzen, statt selbst workarounds einzubauen.

[olwaldi]

Das ist für Querschnitts-Module wie MQTT_GENERIC_BRIDGE (oder readingsWatcher etc.) vielleicht korrekt, aber für die Zwecke des hier in Rede stehenden Modul ist es "gefühlter Unsinn".
...
(Und auch bei dem anderen Thema mit dem ReadyFn() würde ich dafür plädieren, das Problem primär darüber zu lösen, und z.B. den anderen Timer dann darin auch immer wieder zu erneuern bzw. den auf eine längere Periode anzusetzen als die ReadyFn()).
Man sollte immer primär die vom framework bereitgestellten (Spezial-) Funktionen nutzen, statt selbst workarounds einzubauen.

Ich bin auch grundsätzlich Deiner Meinung, eher die vom framework bereitgestellten Funktionen zu nutzen statt eigene zu erstellen. Aber ich bin ja auch nicht der Modul-Autor, sondern will mit möglichst wenigen Änderungen ein sehr gutes Modul einen Tick wartbarer machen und ein paar Minibugs fixen.

Aus meiner Sicht (so lese ich das fhem-Wiki bzgl. Modulentwicklung) heißt das aber auch, daß AttrFn nur zum Checken gedacht ist, nicht zum Reagieren. Und der "selbstgebaute" ConnectionCheck funktioniert anders als ReadyFn "aktiv" und erkennt Verbindungsabbrüche direkt, was ich gerade vorgestern konkret beobachten konnte, als ich meinen Denon resetten mußte. Dann gibt es minutenlang kein TCP/IP, aber mit ReadyFn blieb der On-Button grün. Genau das hat mich veranlaßt, genauer nach den Checks zu gucken.

Vermutlich werden wir geide weiterhin unterschiedlicher Meinung sein, ist ja voll OK.


Grüßle, Michael

[Beta-User]

Aus meiner Sicht (so lese ich das fhem-Wiki bzgl. Modulentwicklung) heißt das aber auch, daß AttrFn nur zum Checken gedacht ist, nicht zum Reagieren.

Könnte man als Punkt für dich werten .

Das "Problem": Das Wiki hat auch jemand geschrieben, und dort sein (ziemlich sicher mit Rudi abgestimmtes) Verständnis nachlesbar gemacht. Das bedeutet aber nicht, dass das abschließend und/oder vollständig ist. Na ja, vielleicht liest Rudi mit und wir ergänzen dann im Wiki z.B. sowas:
"Falls im Zusammenhang mit der Änderung von Attributen Aktionen auszulösen sind (z.B. InternalTimer zu setzen oder zu löschen), kann und sollte dies hier erfolgen."

Jedenfalls finde ich es nicht logisch, wenn man erst via AttrFn() nur die Gültigkeit prüft, um dann (ausschließlich zu diesem Zweck) per NotifyFn() zu checken, ob man denn jetzt noch irgendwas veranlassen muss (und dabei nochmal einen (denselben...) Attribut-if-elsif-Bandwurm kreiert).
Letztlich finde ich das auch sehr viel schwieriger zu warten.

Und man riskiert nicht, dass sich andere Module in der Eventverarbeitung weiter vorne einklinken und dann Situationen entstehen, die nicht zueinander passen (sowas ähnliches "durfte" ich mal rund um CUL_HM fixen, das man sicher zu diesem Zeitpunkt bereits als sehr ausgereift ansehen durfte!).

Und der "selbstgebaute" ConnectionCheck funktioniert anders als ReadyFn "aktiv" und erkennt Verbindungsabbrüche direkt, was ich gerade vorgestern konkret beobachten konnte, als ich meinen Denon resetten mußte. Dann gibt es minutenlang kein TCP/IP, aber mit ReadyFn blieb der On-Button grün. Genau das hat mich veranlaßt, genauer nach den Checks zu gucken.

Das leitet unmittelbar zu dem hier über:

Aber ich bin ja auch nicht der Modul-Autor, sondern will mit möglichst wenigen Änderungen ein sehr gutes Modul einen Tick wartbarer machen und ein paar Minibugs fixen.

Im Moment bist du vermutlich sehr viel besser im Code eingearbeitet als der Autor bzw. Maintainer. Mir geht es jedenfalls so: Wenn ich ein Modul mal 6 Monate nicht angefasst habe, beginne ich fast von vorne. Von daher habe ich bei fast allen Modulen, die ich in der Pflege habe in der Regel bei der Übernahme jeweils erst mal damit begonnen, alles "für mich" lesbarer zu machen - als Folge "heftiger Schläge" (v.a. seitens RichardCZ) zum vorgefundenen Code...

Es spricht m.E. sehr viel dafür, die Gelegenheit zu nutzen, nicht um ein sehr gutes Modul wesentlich zu verbessern, sondern schlicht, um das vorhandene Modulverständnis in (noch) besser wartbaren Code zu überführen. (Willige) Tester und Mitmacher scheint es ja zu geben, also why not?!? 

Ich würde z.B. darauf tippen, dass das mit der ReadyFn() und dem internen Check eigentlich auf ein Logikproblem hinweist, das nicht optimal gelöst ist. Aber so tief bin ich nicht im Code, wie geschrieben: ich habe nicht mehr gemacht, wie 2-3 Mal die svn überflogen...

[JoWiemann]

Hallo,

das Wiki ist nicht die Bibel, sondern eine Handreichung. Vieles hat sich Laufe der Zeit entwickelt, oft das Wiki nicht, das von Benutzern für Benutzer ist. Auch ich pflege als Maintainer nicht das Wiki, sondern nur die commadRef.

In vielen Modulen, auch in meinen, wird das Ändern von Attributen nicht nur validiert, sondern es werden auch Aktionen angestoßen.

Beispiel ist das Setzen des boxUser im FritzBox Modul. Erst mit setzen des Users und vorhandenen Passwort wird im Hash hinterlegt, dass ein Verbindungsversuch zur FritzBox gestartet wird.

Grüße Jörg