[PATCH] CallFn() sucht auch in %defs

Begonnen von Loredo, 13 Januar 2017, 10:53:33

Vorheriges Thema - Nächstes Thema

Loredo

Hallo,

mit Bezug auf diesen Beitrag hier habe ich anbei einen kleinen Patch für CallFn in fhem.pl angehängt.
Der Funktionsname wird darin nun auch in den INTERNALs in der Device-Definition gesucht und dann vorrangig verwendet. Ebenfalls wird berücksichtigt, dass man das INTERNAL mit einem führenden Punkt verstecken kann. Der Benutzerfreundlichkeit halber werden angezeigte INTERNALs jedoch bevorzugt.

Ich habe mit fhem.cfg.demo getestet und konnte keine Nebenwirkungen feststellen.

Es wäre prima, wenn der Patch so Einzug finden könnte.




Gruß
Julian
Hat meine Arbeit dir geholfen? ⟹ https://paypal.me/pools/c/8gDLrIWrG9

Maintainer:
FHEM-Docker Image, https://github.com/fhem, Astro(Co-Maintainer), ENIGMA2, GEOFANCY, GUEST, HP1000, Installer, LaMetric2, MSG, msgConfig, npmjs, PET, PHTV, Pushover, RESIDENTS, ROOMMATE, search, THINKINGCLEANER

rudolfkoenig

Ich will nach Moeglichkeit Funktionen im Modul halten, und Daten in $defs. Weiterhin ist CallFn eine der zentralen Funktionen, auch leichte Aenderungen haben potentiell grosse Nebenwirkungen. Dazu kommt, dass fhem.pl an etlichen Stellen direkt $modules{X}{Y_Fn} prueft, d.h. das alles muesste konsequenterweise auch angepasst werden, fuer einen fraglichen Gewinn.

Da die Anwendung fuer DbLog mAn sehr speziell ist, schlage ich vor da eine angepasste Kopie von CallFn zu verwenden.
Wenn sie an mehreren Stellen benoetigt wird, kann diese neue Funktion gerne in fhem.pl eingebaut werden.

Loredo

#2
Ich bin da leidenschaftslos, aber es ist wohl etwas Hü und Hott, da dieser Wunsch explizit von Markus kam und wir deshalb Heiko extra gebeten hatten von der (vorher schon vorhandenen und implementierten) Moduleigenen Lösung Abstand zu nehmen.

Zitat von: rudolfkoenig am 13 Januar 2017, 11:38:02Ich will nach Moeglichkeit Funktionen im Modul halten, und Daten in $defs. Weiterhin ist CallFn eine der zentralen Funktionen, auch leichte Aenderungen haben potentiell grosse Nebenwirkungen.

An diesem Grundsatz im Kern festzuhalten finde ich richtig.
Die Wirklichkeit bei Helper-Modulen sieht allerdings manchmal anders aus, wenn diese (wünschenswerter Weise) minimal-invasiv arbeiten wollen/sollen. 98_powerMap erweitert z.B. jedes FHEM Device um die Möglichkeit den Stromverbrauch zu ergänzen. Um DbLog die Einheit korrekt getrennt vom Zahlenwert mit zu übergeben, nutzt 98_powerMap eine eigene Funktion statt der Funktion in DbLog. Nun würde ich aber gerne vermeiden, dass 98_powerMap einfach $modules{$defs{$n}{TYPE}}{DbLog_splitFn} für alle definierten setzen muss und somit ggf. auch unnötigerweise das Logging Verhalten von Devices verändert, obwohl der Nutzer dort gar kein powerMap definiert hat. Diese Unterscheidung kann ich also nur in %defs vornehmen und deshalb macht es hier IMHO Sinn Funktionsnamen zusätzlich auch als INTERNAL setzen zu können, welche dann vorrangig behandelt werden.

Aus dem Vorschlag von Markus das in CallFn() zu definieren folgere ich, dass es hier durchaus wünschenswert ist diese Funktion für alle Module in FHEM verfügbar zu machen.

Zitat von: rudolfkoenig am 13 Januar 2017, 11:38:02
Dazu kommt, dass fhem.pl an etlichen Stellen direkt $modules{X}{Y_Fn} prueft, d.h. das alles muesste konsequenterweise auch angepasst werden, fuer einen fraglichen Gewinn.

Dafür besteht IMHO keine Notwendigkeit; es ist eine reine Erweiterung für Module, die mit anderen Modulen zusammenarbeiten wollen.
Hat meine Arbeit dir geholfen? ⟹ https://paypal.me/pools/c/8gDLrIWrG9

Maintainer:
FHEM-Docker Image, https://github.com/fhem, Astro(Co-Maintainer), ENIGMA2, GEOFANCY, GUEST, HP1000, Installer, LaMetric2, MSG, msgConfig, npmjs, PET, PHTV, Pushover, RESIDENTS, ROOMMATE, search, THINKINGCLEANER

rudolfkoenig

Wenn ich das richtig verstanden habe, dann ist selbst dieser Patch nicht perfekt, da das powerMap-SplitFn nur fuer die powerMap spezifische Readings relevant ist. Waere es nicht einfacher, DbLog zu erweitern? Oder gar auf Trennung von Zahl und Einheit zu verzichten? :)

Zitates ist eine reine Erweiterung für Module, die mit anderen Modulen zusammenarbeiten wollen.
Das siehst du jetzt so, aber wenn in ein paar Monaten/Jahren jemand ein Bug meldet, weil sein device-spezifischer NotifyFn unter bestimmten Umstaenden nicht aufgerufen wird, dann bin ich in Erklaerungsnot, und darf es fixen.
Ich bleibe dabei: wenn es nur einmal verwendet wird, dann gehoert ins DbLog. Sonst baue ich das in fhem.pl unter einem neuen Namen ein.

justme1968

wie wäre es CallFn zu lassen und den vorschlag hier zusätzlich als CallIInstanceFn einzubauen?

CallInstanceFn würde zuerst im hash nachschauen und dann als fallback CallFn aufrufen wenn nichts gefunden wurde.
hue, tradfri, alexa-fhem, homebridge-fhem, LightScene, readingsGroup, ...

https://github.com/sponsors/justme-1968

Markus Bloch

Ich habe diesen Wunsch so geäußert, da es meiner Meinung nach der falsche Weg ist, wenn bei sowas jedes Modul seine eigenen Brötchen bäckt. Es gibt in fhem.pl bereits eine Funktion die auf diese Weise Funktionen starten kann, warum muss das DbLog nachimplementieren?

Ich könnte auch mit Andre's Vorschlag leben.

Gruß
Markus
Developer für Module: YAMAHA_AVR, YAMAHA_BD, FB_CALLMONITOR, FB_CALLLIST, PRESENCE, Pushsafer, LGTV_IP12, version

aktives Mitglied des FHEM e.V. (Technik)

Loredo

André's Vorschlag klingt nach einem guten Kompromiss.
Hat meine Arbeit dir geholfen? ⟹ https://paypal.me/pools/c/8gDLrIWrG9

Maintainer:
FHEM-Docker Image, https://github.com/fhem, Astro(Co-Maintainer), ENIGMA2, GEOFANCY, GUEST, HP1000, Installer, LaMetric2, MSG, msgConfig, npmjs, PET, PHTV, Pushover, RESIDENTS, ROOMMATE, search, THINKINGCLEANER

rudolfkoenig

Habe Andrés Vorschlag umgesetzt, kurz getestet und eingecheckt.

Loredo

Hat meine Arbeit dir geholfen? ⟹ https://paypal.me/pools/c/8gDLrIWrG9

Maintainer:
FHEM-Docker Image, https://github.com/fhem, Astro(Co-Maintainer), ENIGMA2, GEOFANCY, GUEST, HP1000, Installer, LaMetric2, MSG, msgConfig, npmjs, PET, PHTV, Pushover, RESIDENTS, ROOMMATE, search, THINKINGCLEANER