70_Klafs.pm Code Review

[xasher]

Hallo zusammen,

ich habe für meine Saunasteuerung ein Modul gebaut und habe inzwischen einen Stand erreicht, wo ich einen erfahrenen Programmierer bitten möchte drüberzuschauen.
Aktuell ist die Version nicht als Package konzipiert. Da kenne ich mich (derzeit) noch nicht aus.
Ich habe das ganze schon mal im Forum bekannt gegeben: https://forum.fhem.de/index.php?topic=127701.msg1222018#msg1222018 (ok, ist nicht so verbreitet die Sauna)
Mit Beta-User konnte ich schon viele Verbesserungen durchführen - vielen Dank an dich an dieser Stelle.

So konnte das Passwort in das set überführt werden. Danke an CoolTux!
Außerdems konnten die Readings stark vereinfacht werden.

An dieser Stelle würde ich das gerne öffentlich diskutieren.

Besten Dank schon mal vorab

Viele Grüße
Alex

[Beta-User]

Hi  ,

vorab mal Danke, dass du die Anregung aufgegriffen hast!

Anbei mal eine erste "Rückmeldung" (nochmal der Hinweis: ich kann das nur dahingehend checken, ob es ohne Fehler lädt!). Vorschlag wäre, du machst dazu mal ein "diff -u" zu deiner Fassung, dann können wir die Unterschiede vielleicht am einfachsten durchgehen und es ist dann auch für andere leichter, den entsprechenden Code-Schnipsel zu verstehen und ggf. auch ihre Meinung dazu zu sagen. Bitte erforderlichenfalls dann die betreffende Funktion komplett in Code-Tags zeigen.

Sachen, die (jetzt und für dich) selbstverständlich/selbsterklärend sind, kannst du gerne einfach umsetzen/übernehmen, das ist im diff nicht mehr so interessant. Was "Quotes" angeht, sollten wir diese nach und nach bereinigen. Was nicht extrapoliert werden muss, kann in einfache Quotes oder ein "q", siehe (u.A.) https://www.perlmonks.org/?node_id=401006...

Da ist jedenfalls ein "bunter Strauß" von Sachen drin, von denen ich als nicht soooo erfahrener Perl-Anwender annehme, dass die auch für andere User und Gelegenheits-Maintainer interessant sind .

Grüße, Beta-User

[xasher]

Hallo Beta-User

vielen Dank für deine Antwort. Ich habe schon ein ersten Blick drübergeworfen. Sind mal wieder interessante Dinge dabei.
Das Wochenende ist leider etwas voll - bin aber dran - alsbald möglich.

Viele Grüße
Alex

[Beta-User]

Moin,

lass dir Zeit...

Vielleicht noch ein Hinweis wegen des Vorschlags, die Nach-Initialisierung nicht per NotifyFn() zu machen, sondern timer-basiert: In https://forum.fhem.de/index.php/topic,24658.msg1219913.html#msg1219913 findest du meine Überarbeitung von VALVES, und im ersten Beitrag die Vergleichsversion. Da ist u.A. auch dieser Punkt geändert...

Grüße und ein angenehmes WE,

Beta-User

[xasher]

Guten Morgen,

ich habe jetzt die Teile übernommen und übernommen, soweit es mir klar war.
Im Detail bedeutet das:

Zeilennummern bezieht sich auf die Version von Beta-User. Bei der aktuell angehängten Version kann es abweichen:

144: Raum vorbelegen, fand ich nett, da Unsorted schon recht voll ist. Kann man aber auch lassen. Es bringt der Sache nichts
152: Vorbelegung der Hashwerte im Define: Dachte, ich hätte mal Perl Fehlermeldungen bekommen nach dem Define. Seitdem ist es drin
431: RemainTime formatiert eingefügt, ging verloren

Darüberhinaus:
Log3 mit Klammern versehen
Klafs_connected mit Bulk übernommen

Dann zu den Schleifen. Du hast aus den foreach Schleifen for Schleifen gemacht. Kannst du nochmals erklären, warum wir das machen? Ich bin da vmtl nicht ganz firm drin. Das hatte ich mal aus einem Beispiel übernommen und es hat funktioniert. 

Dann unten hast du noch ein Kommentar:

Code Auswählen Erweitern

if ( $devtype eq 'KLAFS') { #Beta-User: KLAFS oder Klafs....?

Ja, gute Frage nun. In einer ersten Version hatte ich $hash->{TYPE} = 'KLAFS'; drinstehen. Das wurde aber mal entfernt. Wie kommt das jetzt zustande?
Vermutlich durch meine Definition in Zeile 1:

Code Auswählen Erweitern

define mySauna KLAFS
attr mySauna disable 0
attr mySauna genericDeviceType switch
attr mySauna interval 30
attr mySauna pin 0000
attr mySauna room Homekit,Klafs
attr mySauna saunaid ab0c123d-ef4g-5h67-8ij9-k0l12mn34op5
attr mySauna siriName Sauna
attr mySauna username user
attr mySauna verbose 4

Grüße
Alex

[Beta-User]

Moin zurück.

Ein paar kurze Anmerkungen:

144: Raum vorbelegen, fand ich nett, da Unsorted schon recht voll ist. Kann man aber auch lassen. Es bringt der Sache nichts

An sich ist das schon nett, hat aber ein paar Haken:
- direktes "Rummalen" in Standard-Hashes sollte man vermeiden, für Attribute nutzt man eben CommandAttr(), wenn man was setzen will. Will man aber @define-time eigentlich nicht.
- Wenn "unsorted" voll ist, ist das "dein Problem". Ist (wie manches, das wir jetzt gleich diskutieren) eine Philosophie-Frage: Ich nutze "unsorted" eher als Übergangsort und finde es eigentlich ganz ok, wenn alles dort landet, was ich nicht explizit irgendwohin geschoben habe. Das mache ich dann nämlich in der Regel mit allem, was nicht temporär ist, und nicht gebraucht wird, bekommt ein "ignore", bevor es in den Raum Steuerung->unused geht...

152: Vorbelegung der Hashwerte im Define: Dachte, ich hätte mal Perl Fehlermeldungen bekommen nach dem Define. Seitdem ist es drin

Dass damit Fehlermeldungen unterdrückt werden sollen, war mir fast klar. Die Frage ist: Warum kamen die? Antwort: Direkter Zugriff auf den Device-Hash. Lösung: Methodenzugriff verwenden (InternalVal()) und einen default mitgeben...

Du hast aus den foreach Schleifen for Schleifen gemacht. Kannst du nochmals erklären, warum wir das machen? Ich bin da vmtl nicht ganz firm drin. Das hatte ich mal aus einem Beispiel übernommen und es hat funktioniert. 

Die Frage ist auch hier weniger, ob es funktioniert (tut es beides), sondern ob man an einer Schreibweise festhält, die nach meinem Kenntnisstand einfach nur noch eine unnötige Indirektion verursacht. Wenn man weiß, dass es funktional auf's selbe rausläuft, kann man sich diese paar Zeichen sparen ...

Dann unten hast du noch ein Kommentar:

Code Auswählen Erweitern

if ( $devtype eq 'KLAFS') { #Beta-User: KLAFS oder Klafs....?

Du hast in deinem Device-Hash beide Varianten drinstehen. Der Sinn der unterschiedlichen Schreibweise hat sich mir nur noch nicht erschlossen. Das hat nichts mit der Frage zu Tun, wie man auf den Wert dann sinnvollerweise zugreift.

[xasher]

Hi Beta-User,

also,
1) Raum vorbelegen ist draußen. Das kann dann jeder selber machen
2) Vorbelegung der Hashwerte ist weitgehend draußen. in Zeile 229 bekomme ich einen Fehler. Ich habe in 228 das eingefügt "$hash->{Klafs}->{LoginFailures} //= '';"
3) Ich habe auf For-Schleifen umgestellt.
4) Das mit den Hashwerten ist mir noch nicht ganz klar. Stimmt, ich hatte mal $hash->{KLAFS} und $hash->{Klafs} gemischt im Einsatz. Aktuell aber nur diese Variante: $hash->{Klafs}

Soweit dazu. Ich habe aber gerade noch etwas Streß mit der Sub Klafs_Connected. Du hast meine Ursprungsversion um die Variable $notUseBulk erweitert:

#Beta-User

Code Auswählen Erweitern

sub Klafs_CONNECTED {
    my $hash = shift // return;
    my $set  = shift;
    my $notUseBulk = shift;

    if ($set) {
      $hash->{Klafs}->{CONNECTED} = $set;

      if ( $notUseBulk ) {
          readingsSingleUpdate($hash,'state',$set,1) if $set eq ReadingsVal($hash->{NAME},'state','');
      } else {
        readingsBulkUpdate($hash,'state',$set) if $set eq ReadingsVal($hash->{NAME},'state','');
      }
      return;
    }
    return 'disabled' if $hash->{Klafs}->{CONNECTED} eq 'disabled';
    return 1 if $hash->{Klafs}->{CONNECTED} eq 'connected';
        return 0;
}

#Xasher: Ursprüngliche Version

Code Auswählen Erweitern

sub Klafs_CONNECTED {
    my $hash = shift // return;
    my $set  = shift;
    if ($set) {
      $hash->{Klafs}->{CONNECTED} = $set;

      if (!defined($hash->{READINGS}->{state}->{VAL}) || $hash->{READINGS}->{state}->{VAL} ne $set) {
                      readingsSingleUpdate($hash,"state",$set,1);
      }
      return;
    } else {
      if ($hash->{Klafs}->{CONNECTED} eq 'disabled') {
        return 'disabled';
      }
      elsif ($hash->{Klafs}->{CONNECTED} eq 'connected')
      {
        return 1;
      } else {
        return 0;
      }
    }
}

Das bringt laut Log > 10 Refreshs in der Sekunde:

Code Auswählen Erweitern

2022.05.23 11:18:20 4: mySauna - Update with device: ab0c123d-ef4g-5h67-8ij9-k0l12mn34op5
2022.05.23 11:18:20 3: mySauna - notify DoUpdate...
2022.05.23 11:18:20 4: mySauna - Update with device: ab0c123d-ef4g-5h67-8ij9-k0l12mn34op5
2022.05.23 11:18:20 3: mySauna - notify DoUpdate...
2022.05.23 11:18:20 4: mySauna - Update with device: ab0c123d-ef4g-5h67-8ij9-k0l12mn34op5
2022.05.23 11:18:20 3: mySauna - notify DoUpdate...
2022.05.23 11:18:20 4: mySauna - Update with device: ab0c123d-ef4g-5h67-8ij9-k0l12mn34op5
2022.05.23 11:18:20 3: mySauna - notify DoUpdate...
2022.05.23 11:18:20 4: mySauna - Update with device: ab0c123d-ef4g-5h67-8ij9-k0l12mn34op5
2022.05.23 11:18:20 3: mySauna - notify DoUpdate...
2022.05.23 11:18:20 4: mySauna - Update with device: ab0c123d-ef4g-5h67-8ij9-k0l12mn34op5
2022.05.23 11:18:20 3: mySauna - notify DoUpdate...
2022.05.23 11:18:20 4: mySauna - Update with device: ab0c123d-ef4g-5h67-8ij9-k0l12mn34op5
2022.05.23 11:18:20 3: mySauna - notify DoUpdate...
2022.05.23 11:18:21 4: mySauna - Update with device: ab0c123d-ef4g-5h67-8ij9-k0l12mn34op5
2022.05.23 11:18:21 3: mySauna - notify DoUpdate...
2022.05.23 11:18:21 4: mySauna - Update with device: ab0c123d-ef4g-5h67-8ij9-k0l12mn34op5
2022.05.23 11:18:21 3: mySauna - notify DoUpdate...
2022.05.23 11:18:21 4: mySauna - Update with device: ab0c123d-ef4g-5h67-8ij9-k0l12mn34op5
2022.05.23 11:18:21 3: mySauna - notify DoUpdate...

Ich kapiers gerade nicht, warum das passiert

Ich hänge mal die aktuelle Version dran. Vielleicht sollten wir uns auf die beziehen. Deine Anmerkungen sollten drin sein - bis auf Klafs_CONNECTED. Diff ist langsam unübersichtlich.

Grüße,
Alex

[Beta-User]

Ich kapiers gerade nicht, warum das passiert

Klingt danach, als wäre deine NotifyFn() irgendwie suboptimal (wobei da TYPE noch groß drin ist, was mir seltsam vorkommt, aber der log-Eintrag liest sich so)...

Vielleicht noch ein Hinweis wegen des Vorschlags, die Nach-Initialisierung nicht per NotifyFn() zu machen, sondern timer-basiert: In https://forum.fhem.de/index.php/topic,24658.msg1219913.html#msg1219913 findest du meine Überarbeitung von VALVES, und im ersten Beitrag die Vergleichsversion. Da ist u.A. auch dieser Punkt geändert...

Als Sofortmaßnahme würde ich erst mal empfehlen, in Define das hier aufzunehmen :

Code Auswählen Erweitern

setNotifyDev($hash,'global');
Braucht aber ein ziemlich aktuelles fhem.pl!

[xasher]

Klingt danach, als wäre deine NotifyFn() irgendwie suboptimal

Ich habe den Code mal angeschaut und versuche das zu interpretieren. Du hast
1) NotifyFn rausgenommen
2)  eine Ergänzung im Define gemacht:

Code Auswählen Erweitern

    InternalTimer(gettimeofday() + AttrVal($name,'valvesInitialDelay',61), 'VALVES_GetUpdate', $hash, 0) if !$init_done;
    notifyRegexpChanged($hash, 'global',1);
    VALVES_GetUpdate($hash) if $init_done && !AttrVal($name,'disable',0);
Interpretiert heißt das in etwa: Timer setzen mit dem Attribut 'valvesInitialDelay' bzw 61 Sekunden, wenn nichts dfiniert ist und wenn die Initialisierung noch nicht abgeschlossen ist.
Oder was heißt "if !$init_done;"
Das kann ich mir nicht herleiten: notifyRegexpChanged($hash, 'global',1);
Letzte Zeile:  VALVES_GetUpdate($hash) if $init_done && !AttrVal($name,'disable',0);
Update, wenn Initalisierung abgeschlossen und das Device nicht disabled ist.

Grüße
Alex

[Beta-User]

Ich habe den Code mal angeschaut und versuche das zu interpretieren. Du hast
1) NotifyFn rausgenommen
2)  eine Ergänzung im Define gemacht:

Code Auswählen Erweitern

    InternalTimer(gettimeofday() + AttrVal($name,'valvesInitialDelay',61), 'VALVES_GetUpdate', $hash, 0) if !$init_done;
    notifyRegexpChanged($hash, 'global',1);
    VALVES_GetUpdate($hash) if $init_done && !AttrVal($name,'disable',0);

Es gibt noch eine relevante Änderung in dem Zusammenhang; die findet sich in Attr (u.A. bei disable)*.

Interpretiert heißt das in etwa: Timer setzen mit dem Attribut 'valvesInitialDelay' bzw 61 Sekunden, wenn nichts dfiniert ist und wenn die Initialisierung noch nicht abgeschlossen ist.
Oder was heißt "if !$init_done;"

Genau. Bei Einlesen der DEF bei FHEM-Start erst mal 61 Sekunden delay rein (das Attribut gibt es da noch nicht); wird später ein define/defmod angewiesen, wird der Attributwert genommen. *Der Timer wird modifiziert, wenn dann das Attribut gesetzt wird (meine ich).

Das kann ich mir nicht herleiten: notifyRegexpChanged($hash, 'global',1);

Ich mir auch nicht mehr; ist ein Rest aus den Versuchen, ob es ohne NotifyFn() klappt (die wird dadurch deaktiviert, siehe https://wiki.fhem.de/wiki/DevelopmentModuleAPI#notifyRegexpChanged).

Letzte Zeile:  VALVES_GetUpdate($hash) if $init_done && !AttrVal($name,'disable',0);
Update, wenn Initalisierung abgeschlossen und das Device nicht disabled ist.

Genau. Ist das Device disabled (oder wird es später) wird kein Timer gesetzt bzw. der Timer gelöscht. Daher wird bei Löschen von disable bzw. 0-Setzen dann auch wieder der Timer gestartet...

=> NotifyFn() nicht mehr erforderlich...

Dein Problem (und auch das von VALVES in der Ausgangsversion) war, dass bei ALLEN Events diese NotifyFn() aufgerufen wurde, also jeder Temperatur-Änderung auf dem Mond, ...
Die 10x/Sek. sind übrigens recht viel, wobei mir nicht klar ist, ob da auch noch eine Art Selbsttriggerung eine Rolle gespielt hatte. Du solltest mal sehen, wie viele Events durch den Event-Monitor durchrauschen und das ggf. auch eingrenzen.

[xasher]

Hallo,

also, ich hab notify rausgenommen und es jetzt so mal probiert.
Scheint auf den ersten Eindruck gut auszusehen, was denkst du - zumindest was du so im Quelltext siehst?

Änderungen:
Klafs_connect in deiner Version drin
NotifyFN raus und Timer ins Define gelegt.

Gruß
Alex

[Beta-User]

Scheint auf den ersten Eindruck gut auszusehen, was denkst du - zumindest was du so im Quelltext siehst?

Um sicherzugehen, dass du den Hinweis wahrgenommen hattest:
- attr disable braucht ggf. eine Ergänzung;
- was das NotifyFn() auf sich selbst sollte, hatte ich nicht geprüft, weil eigentlich dieser Zweig nicht hätte durchlaufen werden dürfen. Es kann aber sein, dass man jetzt bei bestimmten Readings (vor dem Setzen!) schauen muss, ob es eine Änderung geben wird und dann ggf. entsprechend darauf reagieren...

Das Zugrunde liegende Problem könnte uU. auch anders gelöst werden, bitte mal den UBUS-Thread auf "ReadyFn()" durchsuchen (bzw. https://wiki.fhem.de/wiki/DevelopmentModuleIntro#X_Ready). Aber Achtung bevor du da was einbaust: Die wird uU. noch deutlich häufiger aufgerufen wie NotifyFn() - du solltest vorher wissen, wie man da einen Timeout einbaut!

[xasher]

Da haben sich unsere Nachrichten überschnitten mit dem disable im AttrFn

Was ich schon ab Zeile 85 habe, ist:

Code Auswählen Erweitern

        if( $attrName eq "disable" ) {
          if( $cmd eq "set" and $attrVal eq "1" ) {
              RemoveInternalTimer($hash);
              readingsSingleUpdate ( $hash, "state", "disable", 1 );
              Log3 ($name, 3, "$name - disabled");
          }

          elsif( $cmd eq "del" ) {
              readingsSingleUpdate ( $hash, "state", "active", 1 );
              Log3 ($name, 3, "$name - enabled");
          }
         
        }


Wenn ich es in VALVES richtig sehe:

Code Auswählen Erweitern

    if ($attrName eq 'disable') {
        RemoveInternalTimer($hash) if $cmd ne 'del';
        InternalTimer(gettimeofday(), \&VALVES_GetUpdate, $hash, 0) if $cmd eq 'del' || !$attrVal && $init_done;
        return;
    }

Dann machst du bei Disable einen RemoveInternalTimer und ein GetUpdate bei aktivem Modul bzw. wenn disable gelöscht bzw. wenn Initialisierung fertig ist.
Dann mit einem return zurück.

Für mich müsste es dann doch ähnlich aussehen?!

Code Auswählen Erweitern

        if( $attrName eq "disable" ) {
          RemoveInternalTimer($hash) if $cmd ne 'del';
          InternalTimer(gettimeofday(), \&Klafs_DoUpdate, $hash, 0) if $cmd eq 'del' || !$attrVal && $init_done;
          return;
        }elsif( $attrName eq "username" ) {...


Grüße
Alex

[Beta-User]

  Sollte passen.

Anmerkungen:
- "Quotes"...? Immer doppelte eliminieren, wenn keine Evaluierung erforderlich ist und du was anfaßt...
- elsif() nach return; ?
- die "Übersetzung" ist nicht ganz korrekt:

Code Auswählen Erweitern

if $cmd eq 'del' || !$attrVal && $init_done;bedeutet eher: Mache das update, wenn das Attribut gelöscht oder nach der Initialisierung auf "0" gestellt wird.

[xasher]

- "Quotes"...? Immer doppelte eliminieren, wenn keine Evaluierung erforderlich ist und du was anfaßt...

Stimmt, habs natürlich geändert!

- elsif() nach return; ?

Habs rausgenommen, kommt ja unten.

Gibt es aus deiner Sicht noch grobe Schnitzer bzw. Einwände gegen eine Veröffentlichung?

Grüße
Alex

[Beta-User]

Na ja, ich sehe das Modul nicht in Aktion...

Auf die ärgsten Themen seit der ersten Version sind wir m.E. eingegangen.

Prinzipiell würde ich mal noch empfehlen,
- die "sehr repetitiven" Codestellen anzusehen, ob sich da nicht manches vereinfachen läßt? (20x am Stück ReadingsVal-Wertezuweisungen mit anschließender Concatenation?) Richt nach (vorläufiges! Stichwort) "encode_json" (und die Werte vorab per Schleife in einen Hash packen);
- decode_json und encode_json enthalten eine Transformation nach utf8. Kann sein, dass du das hier brauchst, aber wenn nicht, könntest du das noch auf use JSON() umstellen und z.B. sowas hier verwenden:

Code Auswählen Erweitern

if ( !eval { $decoded  = JSON->new->decode($content) ; 1 } ) {
Siehe dazu auch den "Unicode first aid"-Thread in Development.
- Die Reading-Inhalte ("3 Tage") sind teilweise auch eingedeutscht. Ich bin ein Fan von Internationalisierung... Könnte man hier mit einem "verschachtelten Hash" lösen und dabei die language-Einstellungen in "global" beachten.

- Auch das "elsif"-Gedöns bei der Sprache wäre einfacher in einem Hash untergebracht.

Nun ja, und einpacken wäre nach wie vor nicht verkehrt... Aber Rom und so .

[xasher]

Hallo Beta-User,

Sprache und Standby-Time habe ich noch vereinfacht.
Das Modul ist eingecheckt - sollte Morgen in der Verteilung drin sind.

Dir ganz herzlichen Dank für deine Zeit. Das Modul ist nun so viel besser als es ursprünglich war.

Grüße
Alex

[Beta-User]

  Danke für deine Rückmeldung.

Ich werd' vermutlich nochmal über den Code schauen, eine Sache ist mir aber auf die Schnelle aufgefallen: In der "en"-Commandref hatte ich schon "id="-Anker verteilt. Die sind aus irgendwelchen Gründen wieder rausgefallen...

Das solltest du bei Gelegenheit insgesamt umstellen.

[xasher]

Hallo zusammen,

ich muss mein Modul 70_Klafs.pm nochmals in den Review geben. Ich habe folgende Fehlersituation.

Wenn ich mein Modul definiere:

Code Auswählen Erweitern

define mySauna KLAFS

Dann funktioniert meine FritzBox Verbindung nicht mehr. Die Fritzbox Config kommt nach der Saunadefinition in fhem.cfg (Vielleicht ist das relevant)

Es liegt an Zeile 37

Code Auswählen Erweitern

use FHEM::Core::Authentication::Passwords qw(:ALL);
und Zeile 120

Code Auswählen Erweitern

$hash->{helper}->{passObj} = FHEM::Core::Authentication::Passwords->new($hash->{TYPE});

Ich verwende die Passwortroute von CoolTux, aber dann geht die Fritzbox nicht mehr. Es kommt ständig Checking API

Da mache ich also etwas falsch. Könnt ihr mir helfen?

Besten Dank und viele Grüße
Alex

[Beta-User]

Weiß nicht recht, eigentlich dürfte das nicht die tiefere Ursache sein.

Wo ich aber auf alle Fälle Probleme sehe: Du erzeugst Events beim Starten von FHEM (in define ohne init_done-Prüfung), und liest das Passwort aus dem $hash, der ggf. zu diesem Zeitpunkt noch gar nicht angelegt ist (aber in $type stehen würde). Soweit ich mich entsinne, ist die Empfehlung von CoolTux, $name zu verwenden, und dann eine RenameFn() für den Fall eines rename vorzusehen.

Die BlockingGet sind uu. auch zu überdenken.

PS: Der Rest meiner Hinweise scheint nicht so sehr auf fruchtbaren Boden gefallen zu sein, angefangen von dem id-Thema für die Commandref...

[xasher]

Hallo Beta-User,

vielen Dank für deine Antwort. Also, das mit deinen Hinweisen und fruchtbaren Boden soll nicht meine Undankbarkeit widerspiegeln. Es hat auch oft damit zu tun, dass ich es nicht richtig verstehe und man vermutlich immer weiter optimieren kann.

Aber das Problem kommt definitiv in Verbindung mit dem Modul von CoolTux und meinem Modul. Ich finde das Problem müsste man als erstes beheben.
Vorneweg, es liegt nicht an CoolTux.

Hintergrund, dass das Fritzbox Modul nicht funktioniert kommt schon bei der Einbindung hier:

Code Auswählen Erweitern

use FHEM::Core::Authentication::Passwords qw(:ALL);

Das CoolTux Hilfmodul verwenden noch zwei andere Module in FHEM. Ich habe mir das SoftliqCloud-Modul angeschaut, da ich ebenfalls den Wasserenthärter besitze.

Bei beiden Packagen folgt eine Packagedefinition

Code Auswählen Erweitern


package FHEM::Gruenbeck::SoftliqCloud;

oder

Code Auswählen Erweitern

package FHEM::UBUS_CLIENT; ## no critic "Package declaration"

Wenn ich bei mir so ein Eintrage mache

Code Auswählen Erweitern

package FHEM::Klafs;

funktioniert ebenfalls das Modul der Fritzbox wieder. Aber es kommt dann zu Folgefehler bei mir im Modul.

Da kenne ich mich definitiv derzeit nicht aus. Liegt es daran, dass dann das Modul - wie sagt man - zum Package wird?

Viele Grüße
Alex

[Beta-User]

Ja, damit nimmst du das Modul aus dem "main"-Namespace. An sich finde ich das sehr sinnvoll, du musst dann halt das "drumrum" auch sauber machen und v.a. die aus fhme.pl etc. erforderlichen Funktionen ebenfalls importieren.

Im Moment dürfte das Modul entweder schon nicht eingebunden werden (initialize kann nicht adressiert werden), oder es gibt andere "undefined subroutines".

Ob das Passwort-Helferlein auch für ungepackagte Module geht, kann ich nicht sagen.

Der Übergang ist gar nicht soooo schwer, siehe z.B. als aktuelles Beispiel:
https://svn.fhem.de/trac/changeset/25877/

Aktuell meine ich, dass es für dich vielleicht sinnvoll wäre, mal in die TvHeadend-reloaded zu schauen, das ist mAn. relativ ähnlich wie das, was du hier machst.

[xasher]

Guten Morgen zusammnen,

ich habe am Wochenende das verlinkte Beispiel angesehen und auch TVHeadend.pm. Ich möchte niemand nerven, komm aber an einer Stelle nicht weiter. Vielleicht hilft ein kleiner Fingerzeig, damit es weitergeht...

Was ich inzwischen gemacht habe

Packagedefinition eingefügt:

Code Auswählen Erweitern

package FHEM::Klafs;

Die GPUtils importiert:

Code Auswählen Erweitern

use GPUtils qw(:all);

Die Namen wie z.B. Klafs_Initialize in Initalize umbenannt und die benötigten Funktionen importiert:

Code Auswählen Erweitern

BEGIN {

  GP_Import(qw(
    readingsBeginUpdate
    readingsBulkUpdate
    readingsEndUpdate
    readingsSingleUpdate
    Log3
    defs
    init_done
    InternalTimer
    strftime
    RemoveInternalTimer
    readingFnAttributes
    AttrVal
    notifyRegexpChanged
    ReadingsVal
    HttpUtils_NonblockingGet
    HttpUtils_BlockingGet
  ))
};

Beim Start kommt noch ein Killeraufruf:

Code Auswählen Erweitern

Undefined subroutine &main::Klafs called at fhem.pl line 3486.

Ich tu mir da schwer, da der Fehler nicht auf mein Modul referenziert. Kann mir nochmals jemand eine Herangehensweise zeigen, wo ich genau suchen muss. Wo hängt es im Moment?

Ich hänge das Modul mal im aktuellen Stand an.

VG
Alex

[Beta-User]

Die Zeile gehört in den Kontext InternalTimer().

Aus dem Modul kann ich das auch nicht direkt ablesen, würde aber empfehlen, alle InternalTimer-Aufrufe auf Referenzierungen umzustellen:

War:

Code Auswählen Erweitern

InternalTimer(gettimeofday(), 'FHEM::Klafs::Klafs_DoUpdate', $hash, 0) if $cmd eq 'del' || !$attrVal && $init_done;
Soll:

Code Auswählen Erweitern

InternalTimer(gettimeofday(), \&Klafs_DoUpdate, $hash, 0) if $cmd eq 'del' || !$attrVal && $init_done;

(Da waren auch noch ein paar Aufrufe ohne Package-Adresse, die allerdings m.E. nicht für den Absturz verantwortlich sind).

EDIT: das ist noch ein InternalTimer, der auf $self referenziert. Der dürfte es sein (die se caller-Analysen kannst du m.E. rauswerfen, sehe keinen Sinn darin).

[xasher]

Ja, genau das mit $self wars.

Aktueller Stand mit dem Modul. Es läuft im Namespace von Fhem. Fritzboxmodul läuft jetzt auch wieder.
ID in der Hilfe ist auch vorhanden.

Augrund der Schwere des Fehlers würde ich morgen mal das vorhandene Modul in Fhem ersetzen. Ich teste es heute noch.
Dann ist es auf jeden Fall mal besser.

Besten Dank dir für deine Zeit.

VG
Alex

[Beta-User]

Ähm, habe grade gesehen, dass die neue Fassung mit package main; beginnt, und dann in diesem Kontext use strikt/warnings gesetzt wird... Das sollte nicht so bleiben .

[xasher]

Hallo Beta-User,

vielen Dank für deine Mühe. Wie gesagt, ich habe mich auch am Modul 69_SoftliqCloud.pm orientiert...
Ich habe mir daher nochmals TV-Headend Reloaded runtergeladen und auch das package main; gestrichen und package FHEM::Klafs; an den Anfang gesetzt.

Viele Grüße
Alex

[Beta-User]

Danke für's Aufgreifen.

Den Rest habe ich mir nicht angesehen, aber der angesprochene Teil paßt, warnings&Co sind wieder aktiv .