WIFILED.pm heißt jetzt LW12.pm

[Kuzl]

Stimmt du hast recht so einfach hab ich gar nicht gedacht
mach ich sobald ich dazukomme

[betateilchen]

Wir müssen auch die Attribut-Geschichte generell noch umbauen, die ist im Moment sehr "unschön" gelöst. Das führt momentan z.B. dazu, dass die erste Statusaktualisierung immer erst nach 60 Sekunden stattfindet, egal, welches Intervall der Anwender selbst festgelegt hat.

Dazu müssen wir in das Modul eine AttrFn() einbauen. Ich werde die Tage mal einen entsprechenden Vorschlag machen.
Generell sollte man komplett auf das Setzen von Default-Werten verzichten.

[Kuzl]

ja das stimmt, da ja noch der standard-Timer abläuft und dann erst der Timer mit dem Attribut-Wert verwendet wird.
Bin gespannt auf deinen Vorschlag, ich kenne AttrFn() nämlich gar nicht

[betateilchen]

Hier kannst Du Dir die Änderungen anschauen.
Das Modul habe ich gerade mit diesen eingebauten Änderungen nach contrib eingecheckt, da kannst Du es Dir direkt abholen.

Code Auswählen Erweitern



Subject: [PATCH] 98_LW12: added AttrFn

added: AttrFn
added: attribute disable
changed: dim minimum 1 instead 0
changed: commandref
---
fhem/contrib/98_LW12.pm | 81 ++++++++++++++++++++++++++++++-------------------
1 file changed, 49 insertions(+), 32 deletions(-)

diff --git a/fhem/contrib/98_LW12.pm b/fhem/contrib/98_LW12.pm
index 5053fa5..12cd031 100644
--- a/fhem/contrib/98_LW12.pm
+++ b/fhem/contrib/98_LW12.pm
@@ -38,6 +38,7 @@
package main;
use strict;
use warnings;
+use feature qw/say switch/;

use IO::Socket;
# include this for the self-calling timer we use later on
@@ -67,10 +68,13 @@ sub LW12_Initialize( $ ) {
   $hash->{UndefFn}    = "LW12_Undefine";
   $hash->{SetFn}      = "LW12_Set";
   $hash->{GetFn}      = "LW12_Get";
+  $hash->{AttrFn}     = "LW12_Attr";

   # the attributes we have. Space separated list of attribute values in
   # the form name:default1,default2
-  $hash->{AttrList}  = "timeout updateInterval verbose:0,1,2,3,4,5 " . $readingFnAttributes;
+  $hash->{AttrList}  = "disable:0,1 timeout updateInterval ".
+                       "verbose:0,1,2,3,4,5 ".
+                       $readingFnAttributes;

   # initialize the color picker
   FHEM_colorpickerInit();
@@ -103,11 +107,9 @@ sub LW12_Define( $$ ) {
         channels => [(255) x 3],
     };

- $attr{$name}{timeout} = 2;
- $attr{$name}{updateInterval} = 60;
- $attr{$name}{verbose} = 3;
-
- LW12_updateStatus($hash);
+# Erstes Update verzögern, falls beim fhem Start
+# ein Attribut gesetzt wird, damit nicht zwei Timer laufen.
+ InternalTimer(gettimeofday()+ 10, "LW12_updateStatus", $hash, 0);

return undef;
}
@@ -116,8 +118,7 @@ sub LW12_Define( $$ ) {
#  Undefinition of a module instance
#  called when undefining an element via fhem.cfg
# ----------------------------------------------------------------------------
-sub LW12_Undefine($$)
-{
+sub LW12_Undefine($$) {
   my ($hash,$arg) = @_;

   RemoveInternalTimer($hash);
@@ -136,16 +137,14 @@ sub LW12_Set( $@ ) {
       return( "$name: set needs at least one parameter" );
   }
 
-  if(!$hash->{LOCAL}) {
-      RemoveInternalTimer($hash);
-    if ( ($attr{$name}{updateInterval}) != 0){
- InternalTimer(gettimeofday()+ $attr{$name}{updateInterval} , "LW12_updateStatus", $hash, 0);
-     }
-  }
+ RemoveInternalTimer($hash);
+ if ( AttrVal($name,'updateInterval',60) != 0) {
+ InternalTimer(gettimeofday()+ AttrVal($name,'updateInterval',60) , "LW12_updateStatus", $hash, 0);
+ }
 
   my $cmdList = "" .
  "on off next:noArg prev:noArg animation mode speed run:noArg stop:noArg " .
-   "color dim:slider,0,1,100 " .
+   "color dim:slider,1,1,100 " .
  "rgb:colorpicker,rgb hsv";
 
   # now parse the commands
@@ -344,7 +343,7 @@ sub LW12_Set( $@ ) {
  Log3 $name, 3, $msg ;
  return( $msg );
       } else {
-
+ return 'Dim value 0 not allowed!';
         $hash->{".dim"}->{bri} = $arg[0];
         @{$hash->{".bri"}->{channels}} = Color::BrightnessToChannels($hash->{".dim"});
  LW12_Write( $hash, "\x{56}" .
@@ -382,6 +381,31 @@ sub LW12_Get( $@ ) {
   } 
}

+sub LW12_Attr(@) {
+ my @a = @_;
+ my ($command, $name, $attrName, $attrValue) = @a;
+ my $hash = $defs{$name};
+
+ given($attrName) {
+ when("updateInterval") {
+ RemoveInternalTimer($hash);
+ if ($command eq 'set') {
+ $attr{$name}{$attrName} = $attrValue;
+ } else {
+ CommandDeleteAttr($name,$attrName);
+ }
+ InternalTimer(gettimeofday()+ $attrValue, "LW12_updateStatus", $hash, 0);
+ }
+ default {
+ if ($command eq 'set') {
+ $attr{$name}{$attrName} = $attrValue;
+ } else {
+ CommandDeleteAttr($name,$attrName);
+ }
+ }
+ }
+ return undef;
+}

# ----------------------------------------------------------------------------
#  write something to the WIFI LED
@@ -393,7 +417,7 @@ sub LW12_Write( $$ ) {
     my $s = new IO::Socket::INET( PeerAddr => $hash->{IP},
  PeerPort => 5577,
  Proto => 'tcp',
-   Timeout => int( $attr{$name}{timeout} ) );
+   Timeout => int( AttrVal($name,'timeout',2) ) );

     if( defined $s ) {
my $res = "";
@@ -413,7 +437,6 @@ sub LW12_Write( $$ ) {
}
}

-
# ----------------------------------------------------------------------------
#  Update the RGB Readings for the color picker
# ----------------------------------------------------------------------------
@@ -443,12 +466,12 @@ sub LW12_updateStatus( $ ) {
     my ( $hash, @rest )  = @_;
     my $name = $hash->{NAME};

- if(!$hash->{LOCAL}) {
-    RemoveInternalTimer($hash);
-    if ( ($attr{$name}{updateInterval}) != 0){
- InternalTimer(gettimeofday()+ $attr{$name}{updateInterval} , "LW12_updateStatus", $hash, 0);
-    }
- }
+ RemoveInternalTimer($hash);
+ if ( AttrVal($name,'updateInterval',60) != 0) {
+ InternalTimer(gettimeofday()+ AttrVal($name,'updateInterval',60), "LW12_updateStatus", $hash, 0);
+ }
+
+ return if IsDisabled($name);

my $res = LW12_Write( $hash, "\x{EF}\x{01}\x{77}" );
$res = uc($res);
@@ -476,14 +499,6 @@ sub LW12_updateStatus( $ ) {
# DO NOT WRITE BEYOND THIS LINE
1;

-=pod
-=begin html
-
-
-
-
-=end html
-=cut

=pod
=begin html
@@ -566,6 +581,8 @@ sub LW12_updateStatus( $ ) {
   <a name="LW12attr"></a>
   <b>Attributes</b>
     <ul>
+      <li>disabel<br>
+        Disable tcp connection and update process. Internal Timer remains active!</li>
       <li>updateInterval<br>
         The Interval of the Statusupdates in seconds. If &lt;updateInterval&gt = 0, the automatic updates are not active. Default is 60.</li>
       <li>Timeout<br>
--
1.9.3 (Apple Git-50)



[Kuzl]

Wunderbar versteh ich sogar zum Großteil

Das Einzigste was nicht ganz klar ist: Wofür ist das Disable da?
Wenn ich es richtig gesehen habe überspringt es das automatische Update, aber welchen Zweck hat das?

[betateilchen]

das Attribut "disable" ist quasi ein fhem-Standard, der dafür sorgt, dass ein Modul keine events mehr erzeugt, ohne dass man das device löschen muss. Ich habe es einfach mit eingebaut, weil es noch gefehlt hat. Wenn Du mal in der commandref nach "disable" suchst, wirst Du sehen, dass es in vielen Modulen schon implementiert ist.

Im Normalbetrieb braucht man es nicht, aber bei Fehlersuche oder Testen von beispielsweise Logging kann es schon recht nützlich sein.

[Kuzl]

Achso, also nur die periodischen events werden unterdrückt. Die, die durch das "set" kommen sind dann ja immer noch da

[betateilchen]

Man könnte das IsDisable() prinzipiell auch noch zusätzlich in _Set() einbauen, aber erfahrungsgemäß ist es ausreichend, die vom Modul timergesteuerten Abläufe unterbrechen.zu können.

[betateilchen]

Kannst Du mal bitte versuchen rauszufinden, warum bei einem manuellen "set <name> dim 100" eine Fehlermeldung kommt, dass dim 0 (!) nicht erlaubt sei?

[Kuzl]

überprüf ich am Sonntag, davor bin ich leider nicht daheim 

[betateilchen]

kein Problem

[Kuzl]

Hab mir mal den Code angesehen da ist bei dim folgendes:

Code Auswählen Erweitern


  } elsif( $cmd eq "dim" ) {
      if( @arg != 1 ) {
  my $msg = "LW12_Set: wrong number of arguments for set brightness";
  Log3 $name, 3, $msg ;
  return( $msg );
      } else {
return 'Dim value 0 not allowed!';
        $hash->{".dim"}->{bri} = $arg[0];
        @{$hash->{".bri"}->{channels}} = Color::BrightnessToChannels($hash->{".dim"});
  LW12_Write( $hash, "\x{56}" .
    chr( @{$hash->{".bri"}->{channels}}[0] ) .
    chr( @{$hash->{".bri"}->{channels}}[1] ) .
    chr( @{$hash->{".bri"}->{channels}}[2] ) .
    "\x{AA}" );
       Log3 $name, 4, "$name set to @{$hash->{'.bri'}->{channels}}";
    }
  }


Das würde ja bedeuten, dass immer mit der Fehlermeldung rausgesprungen wird. Es müsste in etwa so heißen:

Code Auswählen Erweitern


  } elsif( $cmd eq "dim" ) {
      if( @arg != 1 ) {
  my $msg = "LW12_Set: wrong number of arguments for set brightness";
  Log3 $name, 3, $msg ;
  return( $msg );
      } else {
return 'Dim value 0 not allowed!' if( $arg[0] == 0);
        $hash->{".dim"}->{bri} = $arg[0];
        @{$hash->{".bri"}->{channels}} = Color::BrightnessToChannels($hash->{".dim"});
  LW12_Write( $hash, "\x{56}" .
    chr( @{$hash->{".bri"}->{channels}}[0] ) .
    chr( @{$hash->{".bri"}->{channels}}[1] ) .
    chr( @{$hash->{".bri"}->{channels}}[2] ) .
    "\x{AA}" );
       Log3 $name, 4, "$name set to @{$hash->{'.bri'}->{channels}}";
    }
  }


[betateilchen]

Stimmt, eigentlich hatte ich das bei mir auch so eingebaut.
Vermutlich ist der if() Teil irgendwie beim Editieren verlorengegangen

Hast Du Dir schon ein Benutzerkonto bei sourceforge angelegt?

https://sourceforge.net/user/registration

[betateilchen]

Die Änderung in der dim0-Message habe ich eingecheckt.


Mir ist nochwas aufgefallen:

Solche Konstrukte:

Code Auswählen Erweitern

$hash->{READINGS}{mode}{VAL}

solltest Du in Deinen Berechnungen besser durch

Code Auswählen Erweitern

ReadingsVal($name,'mode',<defaultwert>)

ersetzen.

[betateilchen]

Bei mir funktioniert das Modul nicht mehr - es gibt keine Aktualisierungen mehr. Und ich komme nicht dahinter, wieso das so ist.