Wiederverwendung von eigenen Subs in anderen Modulen

Begonnen von Wzut, 07 Februar 2019, 15:02:56

Vorheriges Thema - Nächstes Thema

Wzut

Die Tage wollte ich im Quelltext von WeekdayTimer etwas nachschauen und in gleich am Anfang beim Initialize über folgenden Block gestolpert :

if(!$modules{Twilight}{LOADED} && -f "$attr{global}{modpath}/FHEM/59_Twilight.pm") {
    my $ret = CommandReload(undef, "59_Twilight");


Der erste Gedanke war "was hat Twilight mit WeekdayTimer zu tun" ?
Der nächste Schritt mit grep 59_Twilight *.pm ergab noch zwei weitere Vetter :
98_RandomTimer.pm  & 98_WOL.pm
Nun war es klar, alle vier Module stammen ursprünglich aus der Feder eines Autors und im Twilight Modul wurde eine Sub erfunden die ein Problem mit mehr als einem interen Timer löste und das wohl einige Zeit bevor Markus Bloch  sein 71_YAMAHA_AVR gebaut hat.
Zum Glück hat er das Timer Problem nicht auch mit mittels Reload Twilight gelöst sondern via https://forum.fhem.de/index.php/topic,50265.msg420162.html#msg420162 ,
d.h inzwischen kann InternalTimer genau das wofür myInternalTimer mal erfunden wurde.

Wäre es nicht langsam sinnvoll in allen vier Modulen diese alte Zwangsehe endgültig aufzulösen ?
Oder ist das unter der Rubrik "Furz im Wind" zu behandeln ? 
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

herrmannj

Hmmm..... Wenn es läuft dann läuft es. Oder?   ::) Vielleicht wird dein Patch ja angenommen...  ;)

Markus M.

Das ist eher die Rubrik "Gütiger Himmel, warum?"...
Ein Modul sollte meiner Meinung nach keine für den Nutzer nicht ersichtlichen Abhängigkeiten zu wahllosen anderen Modulen haben.
Spätestens bei asynchronen Updates einzelner Module ist Chaos sonst vorprogrammiert.
FHEM dev + HomeBridge + Lenovo Flex15 + HM-CFG-USB + RFXtrx433 + Fritz!Box 7590/7580/546E

HM Aktor/Sensor/Winmatic/Keymatic/Thermostat, HUE, Netatmo Weather/Security/Heating, Xiaomi AirPurifier/Vacuum, Withings Aura/BPM/Cardio/Go/Pulse/Thermo, VSX828, Harmony, Siro ERB15LE
https://paypal.me/mm0

CoolTux

Ich würde versuchen die aktuellen Autoren zu bitten die entsprechenden Funktionen um zu stellen.
Dietmar hatte wohl zur damaligen Zeit, wie bereits erwähnt, keine andere Wahl. Und weil er keine Lust hatte alles doppelt zu schreiben hat er es halt so gemacht. Ich denke wir sollten es richten.


Grüße
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://www.paypal.com/paypalme/MOldenburg
My FHEM Git: https://git.cooltux.net/FHEM/
Das TuxNet Wiki:
https://www.cooltux.net

KernSani

Zitat von: Markus M. am 07 Februar 2019, 15:31:29
Das ist eher die Rubrik "Gütiger Himmel, warum?"...
Ein Modul sollte meiner Meinung nach keine für den Nutzer nicht ersichtlichen Abhängigkeiten zu wahllosen anderen Modulen haben.
Spätestens bei asynchronen Updates einzelner Module ist Chaos sonst vorprogrammiert.
Dem schließe ich mich an. Gibt es nicht einen neuen Maintainer für Twilight? Was passiert, wenn da mal aufgeräumt wird? Dann verabschieden sich plötzlich reihenweise weekdayTimer?
RasPi: RFXTRX, HM, zigbee2mqtt, mySensors, JeeLink, miLight, squeezbox, Alexa, Siri, ...

KernSani

Habe mir gerade WOL angesehen... Als Maintainer steht da noch Dietmar drin... Wenn es sonst niemand haben haben will, würde ich das nehmen...
RasPi: RFXTRX, HM, zigbee2mqtt, mySensors, JeeLink, miLight, squeezbox, Alexa, Siri, ...

Christoph Morrison

Zitat von: KernSani am 07 Februar 2019, 15:42:51
Dem schließe ich mich an. Gibt es nicht einen neuen Maintainer für Twilight? Was passiert, wenn da mal aufgeräumt wird? Dann verabschieden sich plötzlich reihenweise weekdayTimer?

Ja, den gibt es und es wäre vermutlich genau so gekommen, weil mein aktueller Rewrite von Twilight nicht mehr so funktionieren wird (exakt das mit den Timern hat mich so abgenervt, dass ich das neu schreibe).

CoolTux

Fassen wir mal kurz zusammen, da es doch recht ernst ist wenn Christoph die neue Twilight Version eincheckt.
Aktuell bekannte betroffende Module
- Twilight selbst
- RandomTimer
- WOL
- WeekdayTimer

Dietmar hatte als Module
98_WOL.pm, 98_Heating_Control.pm,   98_WeekdayTimer.pm, 98_RandomTimer.pm, 59_Twilight.pm

Wäre also noch Heating_Control zu prüfen.

Neue Maintainer haben meines Wissens Twilight, eventuell nun WOL.
Bleiben noch
98_WeekdayTimer.pm und 98_RandomTimer.pm.

Mein Vorschlag wäre das die aktuellen Maintainer die Module entsprechend an passen und sich bei Vollzug hier wieder melden. Eventuell könnte Wzut sich WeekdayTimer anschauen und ich schaue mir RandomTimer an.


Was haltet Ihr davon? Andere Ideen im Haus?



Grüße
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://www.paypal.com/paypalme/MOldenburg
My FHEM Git: https://git.cooltux.net/FHEM/
Das TuxNet Wiki:
https://www.cooltux.net

KernSani

Für weekday- und randomTimer steht igami in der maintainer.txt


Kurz, weil mobil
RasPi: RFXTRX, HM, zigbee2mqtt, mySensors, JeeLink, miLight, squeezbox, Alexa, Siri, ...

herrmannj

wenn noch was offen ist übernehme ich auch eins, ich hatte die Situation mit Dietmar vergessen.

CoolTux

Zitat von: KernSani am 07 Februar 2019, 17:05:34
Für weekday- und randomTimer steht igami in der maintainer.txt
Kurz, weil mobil

Das gebe ich Igami kurz Bescheid. Danke Dir fürs schauen.

Als Idee wer mag, vielleicht ein kurzer 2 Zeiler als Nachruf in die Module. So ne Art Dank, kein großer Text.


Grüße
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://www.paypal.com/paypalme/MOldenburg
My FHEM Git: https://git.cooltux.net/FHEM/
Das TuxNet Wiki:
https://www.cooltux.net

KernSani

bei mir lokal läuft schon ein WOL ohne twilight...
RasPi: RFXTRX, HM, zigbee2mqtt, mySensors, JeeLink, miLight, squeezbox, Alexa, Siri, ...

KernSani

Zitat von: CoolTux am 07 Februar 2019, 17:43:39
Als Idee wer mag, vielleicht ein kurzer 2 Zeiler als Nachruf in die Module. So ne Art Dank, kein großer Text.
Guter Gedanke!
RasPi: RFXTRX, HM, zigbee2mqtt, mySensors, JeeLink, miLight, squeezbox, Alexa, Siri, ...

Christoph Morrison

Ich nehme für mich mit, dass
a) ich nichts in Subversion einchecken werde, bis nicht die anderen Module aktualisiert wurden, und
b) ich schauen werde, dass ich meine Tests des Moduls um Integrationstests mit anderen Module erweitere

(nota bene:
Ich wollte sowieso so vorgehen, dass nur getaggte, stabile Versionen (stable) von Github ins Subversion wandern, jeder der eine absolut aktuelle (latest) oder eine Testversion installieren möchte, holt die sich dann von Github.)

CoolTux

Wir können ja hier im Thread entsprechende Updates geben. Also so ne Art Vollzug Meldung.
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://www.paypal.com/paypalme/MOldenburg
My FHEM Git: https://git.cooltux.net/FHEM/
Das TuxNet Wiki:
https://www.cooltux.net

KernSani

Melde Teil-Vollzug: https://forum.fhem.de/index.php/topic,97064.0.html Im SVN ist natürlich noch nichts, bin ja noch nichtmal Maintainer (aber Einspruch hat auch noch niemand erhoben, also sprecht jetzt oder schweigt für immer ;-))
RasPi: RFXTRX, HM, zigbee2mqtt, mySensors, JeeLink, miLight, squeezbox, Alexa, Siri, ...

Markus M.

Vielleicht wäre es am sinnvollsten, wenn jemand erst mal einen Checkin über alle betroffenen Module macht, der den Code sauber trennt.
Alles weitere kann dann ja später geklärt werden und es ist erst mal niemand blockiert oder unter Zugzwang.
FHEM dev + HomeBridge + Lenovo Flex15 + HM-CFG-USB + RFXtrx433 + Fritz!Box 7590/7580/546E

HM Aktor/Sensor/Winmatic/Keymatic/Thermostat, HUE, Netatmo Weather/Security/Heating, Xiaomi AirPurifier/Vacuum, Withings Aura/BPM/Cardio/Go/Pulse/Thermo, VSX828, Harmony, Siro ERB15LE
https://paypal.me/mm0

KernSani

Zitat von: Markus M. am 07 Februar 2019, 23:06:39
Vielleicht wäre es am sinnvollsten, wenn jemand erst mal einen Checkin über alle betroffenen Module macht, der den Code sauber trennt.
Alles weitere kann dann ja später geklärt werden und es ist erst mal niemand blockiert oder unter Zugzwang.
Ich würde igami mal ein paar Tage Zeit geben sich das anzusehen, die Änderungen sind relativ trivial. Ich habe das Gefühl ChristophMorrison hat es jetzt nicht soooo eilig, ein neues Twilight einzuchecken (richtig?).
RasPi: RFXTRX, HM, zigbee2mqtt, mySensors, JeeLink, miLight, squeezbox, Alexa, Siri, ...

Christoph Morrison

Zitat von: KernSani am 07 Februar 2019, 23:48:47
Ich würde igami mal ein paar Tage Zeit geben sich das anzusehen, die Änderungen sind relativ trivial. Ich habe das Gefühl ChristophMorrison hat es jetzt nicht soooo eilig, ein neues Twilight einzuchecken (richtig?).

Nein, ich bin aktuell eh nicht so weit, dass ich eine neue Version einchecken könnte. Ich sehe hier aktuell keine Gefahr; igami hat mir auch gesagt, dass er am Wochenende danach schauen wird und vorher werde ich sicher nicht fertig. Mit WOL hat er ja nun auch eine gute Vorlage.

Es ist nur gut, dass ich es weiß.

btw: Christoph reicht.

nils_

ist heating_control  überhaupt was anderes als weekday_timer ??

ich nutze beide nicht, aber hab eben nur mal kurz in heating_control reingeschaut...
viele Wege in FHEM es gibt!

CoolTux

Zitat von: nils_ am 08 Februar 2019, 09:50:50
ist heating_control  überhaupt was anderes als weekday_timer ??

ich nutze beide nicht, aber hab eben nur mal kurz in heating_control reingeschaut...

Nein ist es nicht.
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://www.paypal.com/paypalme/MOldenburg
My FHEM Git: https://git.cooltux.net/FHEM/
Das TuxNet Wiki:
https://www.cooltux.net

Wzut

Zitat von: nils_ am 08 Februar 2019, 09:50:50
ist heating_control  überhaupt was anderes als weekday_timer ??
viel lustiger, satt 59_Twilight lädt es 98_WeekdayTimer :) daher ist es mir beim grep auch durch die Lappen gegangen.
Zitat von: KernSani am 07 Februar 2019, 19:59:14
also sprecht jetzt oder schweigt für immer
aber nur wenn du beim 98_WOL Umbau und testen auch 70_ENIGMA2 im Auge behälst (bzw. mit Loredo redest) denn das lädt 98_WOL
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

KernSani

#22
Zitat von: Wzut am 08 Februar 2019, 11:22:54
viel lustiger, satt 59_Twilight lädt es 98_WeekdayTimer :) daher ist es mir beim grep auch durch die Lappen gegangen.aber nur wenn du beim 98_WOL Umbau und testen auch 70_ENIGMA2 im Auge behälst (bzw. mit Loredo redest) denn das lädt 98_WOL
OMG... Enigma lädt WOL um die Sub die WOL aus Twilight nutzt zu nutzen? Jetzt wird's aber wirr 8-) Ok, ich warte (und Christoph wartet eh schon ;-))

EDIT: Habe gerade mal kurz geschaut, was Enigma macht - das klaut sich ja tatsächlich nur die Wake-Up-Funktionalität, da sollte es keine Probleme geben (aber danke für den Hinweis - jetzt weiß ich zumindest, dass ich mit Loredo reden muss, wenn ich an der Stelle was ändere)
RasPi: RFXTRX, HM, zigbee2mqtt, mySensors, JeeLink, miLight, squeezbox, Alexa, Siri, ...

Christoph Morrison

Vielleicht können wir ja mal anregen, dass geteilter Code auch wirklich in eine Bibliothek kommt und nicht in Modulen schlummert. Beim Timer ist das nun ja so, der Wakup wäre vielleicht auch ein Kandidat.

Zum Beispiel gibt es nun drei (Teil-)Implementierungen des Sonnenaufgangs/-untergangs, die dann wiederum von verschiedenen Modulen genutzt wird (YAAHM holt sich den aus Astro, Homemode möchte gerne Twilight, Twilight holt sich Teile aus SUNRISE_EL). Sowas tut eigentlich nicht Not und macht Teilupdates nur unnötig schwer (und man möchte Teilupdates machen können, z.B. wenn CUL_HM spinnt). Mit Weather + den API-Modulen geht das schon in die richtige Richtung.

Beta-User

Zitat von: Christoph Morrison am 08 Februar 2019, 11:52:58
Vielleicht können wir ja mal anregen, dass geteilter Code auch wirklich in eine Bibliothek kommt und nicht in Modulen schlummert. Beim Timer ist das nun ja so, der Wakup wäre vielleicht auch ein Kandidat.
Ist als Idee echt gut, scheitert aber manchmal an was auch immer...

Zwei Beispiele:
- Es gab bei ASC den Bedarf, bei der Erstellung der timer zu wissen, ob morgen WE ist. Also hat Cooltux was gebastelt. Das findet sich jetzt (vermutlich) 1:1 auch in DOIF, die Variable wäre aber vermutlich eigentlich für alle auch im Rahmen von myUtils interessant. Gehört also m.E. irgendwo zentral hin; nach fhem.pl wollte Rudi das nicht einzubauen.

- im MQTT-Bereich ist es häufig so, dass z.B. Helligkeitswerte nicht FHEM-konform mit 0-100 angegeben werden, sondern der Bereich 0-255 genutzt wird. Die passende devStateIcon-Funktion findet sich jetzt als zigbee2mqtt-spezifische Funktion in MQTT2_DEVICE, gehört aber m.E. besser nach Color.pm (ich nutze das auch für MiLight@MQTT). Vermutlich wird da bald jemand nach Farbe rufen...

(Mindestens) für MQTT (alt) und MySensors gibt es GPUtils, da sind aber nur ein paar wenige (ungeliebte!) Funktionen drin. Letzte Aktualisierung: 2014, Maintainer: ntruchsess. Der war hier aber soweit ersichtlich schon länger nicht mehr aktiv...
Vielleicht böte sich das an? (Notfalls mache ich dafür den neuen "Maintainer", habe aber weder eine Ahnung, wie diese pm eigentlich funktioniert und kann auch keine Funktionsgarantie für "fremden Code" übernehmen...)

Aber: Wie liefe dann der Abstimmungsprozess für eventuelle Änderungen?!?

Gruß, Beta-User
Server: HP-elitedesk@Debian 12, aktuelles FHEM@ConfigDB | CUL_HM (VCCU) | MQTT2: MiLight@ESP-GW, BT@OpenMQTTGw | MySensors: seriell, v.a. 2.3.1@RS485 | ZWave | ZigBee@deCONZ | SIGNALduino | MapleCUN | RHASSPY
svn: u.a MySensors, Weekday-&RandomTimer, Twilight,  div. attrTemplate-files

Wzut

[OT on]
Abstimmung ???? Hat FHEM mit Gründung des eV sich auch vom Königreich zur Demokratie gewandelt ?  8)
[OT off]
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

rudolfkoenig

Die eV ist nicht fuer die Entwicklung zustaendig, das bleibt weiterhin in den Haenden der Maintainer.
Und Demokratie finde ich doof, z.Bsp. wenn die Mehrheit eine Minderheit zu etwas zwingt.
Aber ich lasse mich haeufig ueberreden, wenn eine Mehrheit was will, insb. wenn ich alleine mit meiner "Gegenmeinung" liege.

Beta-User

Hmm, bisher galt doch die Devise: nicht in fremdem Code rumpfuschen!
Bei gemeinsam genutztem Code ist das aber ein ziemliches Problem: Wer entscheidet wie über Änderungen? (Sicher nicht demokratisch; jeder Maintainer ist schließlich derjenige, den die User jeweils ansehen, wenn was nicht will!)

Aber ernsthaft: Die GPUtils importieren - soweit ich das nachvollziehen kann, wie gesagt, dazu fehlen mir eigentlich Grundlagen - alles mögliche/vorhandene aus dem "lib"-Verzeichnis. Da könnte also scheinbar jeder "seine" Codestücke reinpacken, die ggf. für mehrere interessant sind. Wer da was braucht, müßte dann dur die GPUtils vorneweg laden und hätte das dann verfügbar. Auch das Firmata-Modul scheint das zu nutzen (früher auch ntruchsess)...

Ob gerade diese Konstruktion aber gut oder schlecht ist? (es gab einige grundlegende Kritik an der Konstruktion der MQTT-Module, die auch an dieser Architektur liegen könnte, letztlich hat Rudi dann für MQTT entschieden, das anders zu lösen...)
Server: HP-elitedesk@Debian 12, aktuelles FHEM@ConfigDB | CUL_HM (VCCU) | MQTT2: MiLight@ESP-GW, BT@OpenMQTTGw | MySensors: seriell, v.a. 2.3.1@RS485 | ZWave | ZigBee@deCONZ | SIGNALduino | MapleCUN | RHASSPY
svn: u.a MySensors, Weekday-&RandomTimer, Twilight,  div. attrTemplate-files

nils_

Zitat von: Wzut am 08 Februar 2019, 11:22:54
viel lustiger, satt 59_Twilight lädt es 98_WeekdayTimer :) daher ist es mir beim grep auch durch die Lappen gegangen.
ja das ist mir auch aufgefallen  :o

bisher/früher war es vermutlich kein problem, da ja alle module den gleichen maintainer hatten. momentan wird es wohl eher krachen, wenn da was umgebaut wird und das importierte modul gar nichts davon weiß, das es seine funktionen an anderer stelle genutzt werden.

eine lösung hab ich so auch nicht parat.
zumindest eine dokumentation wer wen und was importiert und dann nutzt, würde schonmal weiterhelfen.
viele Wege in FHEM es gibt!

igami

Zitat von: nils_ am 08 Februar 2019, 09:50:50
ist heating_control  überhaupt was anderes als weekday_timer ??

ich nutze beide nicht, aber hab eben nur mal kurz in heating_control reingeschaut...
Das könnte man dann auch mal auf depracted stellen und in einem Jahr nach contrib verschieben
Pi3 mit fhem.cfg + DbLog/logProxy
Komm vorbei zum FHEM Treffen im Kreis Gütersloh! Das nächste Mal im April 2020.

MAINTAINER: archetype, LuftdatenInfo, monitoring, msgDialog, Nmap, powerMap
ToDo: AVScene, FluxLED

igami

Da das WeekdayTimer Modul doch eher wirr geschrieben ist habe ich nun der einfachheithalber die drei Funktionen

sub WeekdayTimer_GetHashIndirekt($$);
sub WeekdayTimer_InternalTimer($$$$$);
sub WeekdayTimer_RemoveInternalTimer($$);

eingeführt, damit keine Abhängigkeit mehr zu Twilight besteht.
Somit muss niemand auf mich warten und das Modul funktioniert genauso wie vorher.
https://github.com/Igami/FHEM-WeekdayTimer/blob/development/FHEM/98_WeekdayTimer.pm
Pi3 mit fhem.cfg + DbLog/logProxy
Komm vorbei zum FHEM Treffen im Kreis Gütersloh! Das nächste Mal im April 2020.

MAINTAINER: archetype, LuftdatenInfo, monitoring, msgDialog, Nmap, powerMap
ToDo: AVScene, FluxLED

igami

Wenn niemand Einwände hat übertrage ich die Änderungen ins SVN und dann können WOL und Twilight auch nachziehen.
Pi3 mit fhem.cfg + DbLog/logProxy
Komm vorbei zum FHEM Treffen im Kreis Gütersloh! Das nächste Mal im April 2020.

MAINTAINER: archetype, LuftdatenInfo, monitoring, msgDialog, Nmap, powerMap
ToDo: AVScene, FluxLED

CoolTux

Sieht gut aus denke ich.
Danke für Deine liebevollen Worte  :)
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://www.paypal.com/paypalme/MOldenburg
My FHEM Git: https://git.cooltux.net/FHEM/
Das TuxNet Wiki:
https://www.cooltux.net

KernSani

Zitat von: CoolTux am 11 Februar 2019, 17:58:36
Danke für Deine liebevollen Worte  :)
Hat sich da jemand die liebevollen Worte abgeguckt ;-) Ich warte noch auf Feedback eines Users, dann geht WOL auch ins SVN
RasPi: RFXTRX, HM, zigbee2mqtt, mySensors, JeeLink, miLight, squeezbox, Alexa, Siri, ...

Christoph Morrison

Oh nein, jetzt muss ich ja doch mal den Rewrite fortsetzen ;-)

igami

Zitat von: KernSani am 11 Februar 2019, 18:17:08
Hat sich da jemand die liebevollen Worte abgeguckt ;-)
Dafür ist es ja open Source ;)

WeekdayTimer wird dann mit dem morgigen Update ohne Abhängigkeit zu Twilight ausgeliefert.
Pi3 mit fhem.cfg + DbLog/logProxy
Komm vorbei zum FHEM Treffen im Kreis Gütersloh! Das nächste Mal im April 2020.

MAINTAINER: archetype, LuftdatenInfo, monitoring, msgDialog, Nmap, powerMap
ToDo: AVScene, FluxLED

CoolTux

Zitat von: KernSani am 11 Februar 2019, 18:17:08
Hat sich da jemand die liebevollen Worte abgeguckt ;-) Ich warte noch auf Feedback eines Users, dann geht WOL auch ins SVN

Dann Danke ich Dir auch ganz dolle.
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://www.paypal.com/paypalme/MOldenburg
My FHEM Git: https://git.cooltux.net/FHEM/
Das TuxNet Wiki:
https://www.cooltux.net