Anmerkungen zu justme1968's Änderungen bzgl. timestamp und on-change-reading

[ht]

Hallo Andre,

ich habe mir Deinen Patch aus https://forum.fhem.de/index.php/topic,52483.msg442422.html#msg442422 angesehen. Da ich kein Entwickler bin, stelle ich meine Anmerkungen in einem neuen Thread ein.

Ca. Zeile 4050:

Code Auswählen Erweitern

    my $trigger = !($attreocr || $attreour)
                  || $eour 
                  || ($eocr && ($value ne $readings->{VAL}));
    $filtered = !$trigger;
    #Log 1, "EOCR:$eocr EOUR:$eour CHANGED:$filtered";


Ich war über das "my" gestolpert, könnte ja unbeabsichtigt eine lokale Variable sein, die die "globale" verdeckt. Soweit ich das sehe ist das hier aber beabsichtigt. Von daher mein Vorschlag: entweder $filtered direkt zuweisen (und die Abfragelogik umdrehen, hmm), oder die lokale Variable anders benennen. Einfach, damit nicht später mal jemand denkt, dass my wäre ein Versehen und es wegnimmt. Und (Pingelmodus an) das CHANGED im Log noch umbenennen

Ca. Zeile 4099

Code Auswählen Erweitern

      $trigger = $ts->elapsed($now);
      $value = $ts->{$function} if($trigger);


Müsste hier nicht auch $filtered gesetzt werden, wenn !$trigger. Also

Code Auswählen Erweitern

  $filtered = 1 if( !$trigger );
Allerdings kenne ich mich mit dem Feature nicht aus, um das vollständig übersehen zu können.

Ich hoffe, die Anmerkungen helfen und nerven nicht,
Volker

[justme1968]

hallo volker,

das mit der logik umdrehen hat zwei nachteile. zum einen passt der text dann nicht mehr direkt auf den code und zum andren ist dem code dann nicht mehr genau anzusehen was die idee dahinter ist. durch das negieren würden alle || zu && und das ist auf den ersten blick nicht das was beabsichtigt ist.

das mit dem lokalem my ist absicht. das ist vermutlich geschmacksache. ich finde es sehr praktisch . man könnte es auch $should_trigger nennen um es vom anderen zu unterscheiden.


der teil mit der timeseries ist erst mal nur 1:1 aus der bisherigen version übernommen. ob es eventuell sinnvoll ist hier noch zu erweitern muss boris entscheiden. ich hab ihn schon mal danach gefragt.

gruss
  andre

[ht]

Hallo Andre,

das mit der logik umdrehen hat zwei nachteile.

Sehe ich 100% genauso. Du hast sozusagen mein "hmm" ausformuliert.

man könnte es auch $should_trigger nennen um es vom anderen zu unterscheiden.

So hätte ich es gemacht

ich hab ihn schon mal danach gefragt

Prima. Werde es verfolgen.

Grüße,
Volker