Einfache Angabe von Reading-Values in notify Commands

[Loredo]

Sehr enttäuschend 


Mir ist nicht klar, nach welchen Kriterien ich dir einen guten Patch zukommen lassen soll. Bei AnalyzeDevice() war er dir offensichtlich zu wartbar ("optimierungsbedürftig"), hier ist er dir zu kompakt (und dabei habe ich mich nur am vorher existierenden Code orientiert, was auch nicht einfach war).


Sei es drum, ich schreibe dir auf Basis deines Patches gerne nochmals einen neuen, da es mir wichtig ist die Mehrfunktionen drin zu haben.


Die Änderungen in ReadingsNum() sind IMHO auf jeden Fall sinnvoll, die neuen Funktionen InternalsNum() und AttrNum() sind eine logische und konsequente Ergänzung, die bisher gefehlt hat.

[DeeSPe]

Ich bin erschlagen vom Patch, sowohl vom vorne/mitten/hinten Platzierbarkeit der Optionen, wie von den verschiedenen Rundungsparameter (:i, :d, :r0). Dass man & _und_ i: verwenden kann, finde ich verwirrend. Ich habe die Befuerchtung, dass ich das so nicht supporten kann.

Ich habe jetzt eine kleinere Version eingecheckt, die genauso wie der devspec Filter arbeitet, also mit optionalen a:, r: oder i:, liefert aber den erstbesten zureck, falls kein Praefix spezifiziert ist. Immerhin ist damit die urspruengliche Forderung auch erfuellt.

Danke fuer die Doku fuer ReadingsTimestamp und ReadingsAge, habs uebernommen.

Ich denke in der deutschen commandref hat sich ein Fehler eingeschlichen. Zeile 1791:

Code Auswählen Erweitern

      <li>ReadingsTimestamp(&lt;devicename&gt;)<br>
Sollte aber wie im Englischen sein:

Code Auswählen Erweitern

      <li>ReadingsTimestamp(&lt;devicename&gt;,&lt;reading&gt;,&lt;defaultvalue&gt;)<br>

Gruß
Dan

[rudolfkoenig]

@DeeSPe: danke, habs geaendert.

@Loredo: Mein Problem ist nicht, dass es "zu kompakt" ist, sondern dass ich die Vielfalt von & und i: bzw. :r, :i, :d nicht gut finde.
Ich muss die Notwendigkeit von einem Patch erkennen, bevor ich es einbaue, und dann kann ich notfalls, falls es mir nicht gefaellt, auch selbst implementieren. Hier habe ich das Gefuehl, dass wir schleichend eine zweite Programmiersprache wie DOIF einfuehren, und ich will irgendwo die Grenze ziehen.

Ich lasse mich aber ueberstimmen.

[Loredo]

Ich hoffe, dass beiliegender Patch verständlicher ist.

Es ist keine neue Programmiersprache oder ein zweites DOIF, es soll lediglich die Kompatibilität da sein auch &INTERNAL verwenden zu können.
Außerdem sollte sich das Extraktionsergebnis von DOIF und ReplaceSetMagic() nicht unterscheiden, was bei der Verwendung von :d derzeit ganz klar der Fall ist.

Ich habe ReplaceSetMagic() in den msg-Befehl eingebaut und darüber werden Nachrichten verschickt. Ich hätte gerne, dass man in Nachrichten in einfacher Weise auf alle Daten eines Devices zugreifen kann. Dabei wollte ich das Rad nicht neu erfinden und nicht ReplaceSetMagic() kopieren sondern es direkt erweitern. Das ist für Benutzer nicht logisch, warum sie bei dem einen Befehl etwas verwenden können und bei dem anderen nicht.

DOIF hat neben :d noch :sec (entspricht im Grunde ReadingsAge()), welches ich auch eingebaut habe. Außerdem erschien es mir sinnvoll die Palette wie folgt zu komplettieren:

   
• :t für den Zeitstempel (analog zu ReadingsTimestamp())
• :i um den Zahlenwert als Integer zurück zu erhalten
• :r um den Zahlenwert auf die 1. Kommastelle zu runden
• :rX um den Zahlenwert auf die X. Kommastelle zu runden

Ich halte das alles für sinnvolle und überschaubare Ergänzungen. Die von mir oben schon verfasste CommandRef erklärt das genauso einfach wie die bisherigen Optionen auch beschrieben sind.

Anschließend wäre es konsequent, wenn die Funktion ReadingsNum() die Integer-/Rundungsfunktion ebenfalls bekäme (diese Funktion wird in ReplaceSetMagic() ja nun nicht mehr verwendet).
Zu guter letzt erscheint es mir auch logisch die Lücke der Funktionen durch AttrNum() und InternalNum() zu schließen.

Über etwas Unterstützung dabei würde ich mich freuen.



Gruß
Julian

[justme1968]

das ReadingsNum nur die erste zahl aus dem reading holt statt alles was nicht \d ist zu entfernen finde ich gut. das aktuelle verhalten liefert bei manchen readings unerwartete ergebnisse.

die anderen flags finde ich nützlich. wenn sie tatsächlich einheitlich (ReplaceSetMagic, list, FILTER,...) verwendet werden würde ich sie auch in readingsGroup einbauen. vor allem das runden würde einige valueFormat attribute sparen. auch die i:,r: und a: präfixe wären einfacher als die + und ? variante die es dort aktuell gibt.

AttrNum() und InternalNum() wären tatsächlich konsequent. vor allem das das handling von attributen/readings/internals zwischen modulen nicht konsistent ist und man in die verlegnheit kommen kann alle drei zu verwenden.

[rudolfkoenig]

Na dann. Ich habe:
- die ReplaceSetMagic Syntax mit den Suffixen :t, :sec, :r und :r\d erweitert
- getestet, und die neuen Suffixe dokumentiert
- ReadingsNum umgestellt auf Zahl-Extrahieren, und einen optionalen vierten Parameter round eingefuehrt.
- AttrNum und InternalNum eingefuehrt, und alles getestet und dokumentiert.

Was ich nicht uebernomen habe:
- a:, r:, i: wahlweise vor oder nach dem Geraetenamen: es ist weiterhin nur davor zulaessig. Selbs so gibt es Probleme, wenn man ein Geraet a, r oder i nennt, und ein Suffix spezifiziert.
- optional & (es bleibt bei i:)
- :r\d+ habe ich auf :r\d eingeschraenkt.

Über etwas Unterstützung dabei würde ich mich freuen.

Ich habe jetzt mit diesem Thema mehrere Stunden verbracht, fuer mich ist das mehr als "etwas". Da ich die Verantwortung fuer fhem.pl habe, muss ich das, was ich uebernehme, verstehen und vertreten koennen.

[Loredo]

Vielen Dank, Rudi. Ich verstehe deinen Aufwand durchaus. Es ist nur auch von meiner Seite aus anstrengend, denn ich habe ebenfalls mehrere Stunden damit zugebracht und mir Mühe gegeben es bestmöglich vorzubereiten. Deine erste Reaktion erschien mir deshalb etwas zu schnell; daher nochmals VIELEN Dank, dass du dir die Zeit dafür genommen hast. Das weiß ich zu schätzen und damit sehe ich auch meinen eigenen Aufwand wieder als wertgeschätzt.

Also offenbarer Beschluss: Wir leben mit den Inkonsistenzen zu DOIF und damit, dass es zwar ähnlich aussieht, aber Texte nicht miteinander per Copy&Paste austauschbar sind.
Vielmehr liegt der Ball dann bei DOIF die nun allgemein definierte Notation zu unterstützen, um interoperabel zu sein/werden.



---
EDIT:

Was mir auffällt: Wir haben jetzt ein ganz anders Verhalten als zuvor bei :d, da es intern durch :r0 ersetzt wird. Das ist aber nicht das, was erreicht werden soll mit :d.
:d soll die unveränderte Zahl so extrahieren, wie sie im Reading steht. Diese Option gibt es jetzt nicht mehr und außerdem ist es ein anderes Verhalten als :d bei DOIF.

Nur auf explizites Verlangen des Users soll mittels :r auf die erste Stelle gerundet werden (da es in FHEM anderswo auch der Standard ist Zahlen auf die 1. Stelle zu runden) oder optional auf die X. Stelle bei :rX. Du hast jetzt :i weg gelassen, was explizit zur Unterscheidung zwischen :d (also dem Originalwert) und :i (also dem Integerwert des Originalwertes) gedacht war. Grundsätzlich wollte ich für :i auch explizit int() statt rand() verwenden, denn wer eine genaue Rundung will, kann explizit :r0 verwenden.

Kannst du dieses Verhalten bitte noch anpassen? Aktuell ist es leider weder Fisch noch Fleisch.

Ich hatte mir da also schon sehr viele Gedanken gemacht und nichts dem Zufall überlassen. Ich hätte es bevorzugt, wenn du hier ggf. nochmals nachgefragt hättest, um es besser zu verstehen.

[rudolfkoenig]

Habs angepasst.

[Loredo]

Danke!

Mir ist gerade noch aufgefallen, dass die neuen *Num() Funktionen gar keinen Wert nicht zurückgeben, wenn keine Zahl gefunden werden konnte. Stattdessen wird "" zurückgegeben bzw. wenn $round gesetzt ist 0.0.
Das erwartete Verhalten ist, dass der komplette Text zurückgegeben wird, wenn keine Zahl gefunden wird.

Außerdem muss der Default-Wert derzeit eine Zahl sein und kann keinen Text beinhalten, da round() daraus dann 0.0 macht.


Patch Vorschlag:

Code Auswählen Erweitern


sub
InternalNum($$$;$)
{
  my ($d,$n,$default,$r) = @_;
  my $val = InternalVal($d,$n,$default);
  return $val !~ /(-?\d(\.\d)?)/ ? $val :
    !defined($r) ? $1 :
    $r eq "i" ? int($1) :
    round($1,$r);
}


   
• Steigt möglichst früh aus
• Berücksichtigt den Default Wert
• Kann mit Text umgehen
• Kann zusätzlich wie in ReplaceSetMagic auch die Nummer nur kürzen statt runden

[rudolfkoenig]

Das ist aber jetzt nicht neu, beim ReadingsNum war es doch genauso.

Das erwartete Verhalten ist, dass der komplette Text zurückgegeben wird, wenn keine Zahl gefunden wird.

Das mag bei ReplaceSetMagic so sein, bei ReadingsNum & co hat das niemand je versprochen.
Bin nichtmal sicher, ob das sinnvoll ist, und bei ReadingsNum waere es nicht kompatibel.

Außerdem muss der Default-Wert derzeit eine Zahl sein und kann keinen Text beinhalten, da round() daraus dann 0.0 macht.

Hier apelliere ich an dem Verstand des Benutzers.

[Loredo]

Mein gestriger Patch hatte das alles berücksichtigt. Ich finde hier braucht es keine Rückwärtskompatibilität.
Ich kann *Num() so nicht flexibel einsetzen. Wozu wird darin intern *Val() angewendet, wenn es dann nicht die selbe Rückgabe liefern kann?

[justme1968]

wie wäre es eplaceSetMagic auf auf stateFormat anzuwenden?

das würde einige sprintf zum runden einsparen.

für userReadings wäre es auch nützlich.

in beiden fällen wäre es gut mit $name:<reading> auch das aktuelle device verwenden zu können.

[rudolfkoenig]

wie wäre es eplaceSetMagic auf auf stateFormat anzuwenden?

Habe es mit etwas Bauchschmerzen eingebaut: eigentlich muesste die alte Ersetzung durch replaceMagic abgeloest werden, andererseits will ich kompatibel bleiben.

[Loredo]

Ich würde noch vorschlagen, dass der Regex von ReplaceSetMagic() übereinstimmend mit dem für die Readingnamen aus CommandSetstate() ist.
Außerdem sollte er wohl case sensitive sein, da rsmVal() es auch ist.

Code Auswählen Erweitern



Index: fhem.pl
===================================================================
--- fhem.pl (revision 13929)
+++ fhem.pl (working copy)
@@ -1682,8 +1682,8 @@
     return $val;
   }

-  $a =~ s/(\[([ari]:)?([a-z0-9._]+):([a-z0-9._-]+)(:(t|sec|i|d|r|r\d))?\])/
-         rsmVal($1,$2,$3,$4,$5)/egi;
+  $a =~ s/(\[([ari]:)?([A-Za-z\d_\.]+):([A-Za-z\d_\.\-\/]+)(:(t|sec|i|d|r|r\d))?\])/
+         rsmVal($1,$2,$3,$4,$5)/eg;

   $evalSpecials->{'%DEV'} = $hash->{NAME};
   $a =~ s/{\((.*?)\)}/AnalyzePerlCommand($hash->{CL},$1,1)/egs;


[rudolfkoenig]

Soweit ich es sehe: setstate erlaubt zusaetzlich /, das habe ich uebernommen, die Pruefung ist jetzt wie gefordert, auch case-sensitive.
Define erlaubt (noch) Doppelpunkt im Namen, das wuerde ich gerne abschaffen. Was sagt ihr dazu: per featurelevel oder rabiat/direkt?