VCLIENT - Modul sollte allgemein gültig bleiben

[Xaneu]

Hallo Andreas Löffler,

ich finde das Modul VCLIENT super, vor allem weil es im Vergleich zu den anderen Vissmann-FHEM-Lösungen am ergiebigsten ist und stabil funktioniert. Nachteil ist, dass der FHEM-User dazu erst einmal den steinigen Weg der vcontrold-Installation gehen muss.

Allerdings möchte ich hier einen (kleinen) Kritikpunkt/Verbesserungsvorschlag anbringen.

Du gehst an verschiedenen Stellen im Modul "89_vclient.pm" von bestimmten Voraussetzungen bzgl. der Namensgebung von vcontrold-Befehlen aus, was aber schlecht ist.
Das Modul sollte nach Möglichkeit allgemein bleiben. Die Verarbeitung von eingelesenen Werten sollte dann in FHEM erfolgen (z.B. Rundungen, Darstellung von Werten per readingsgroup etc.).
Das  System vcontrold bietet ansonsten alle Möglichkeiten über die Dateien "vcontrol.xml" und "vito.xml" auf bestimmte Ausgaben und Umrechnungen von Rohwerten einzugehen. Das zerstörst Du im Prinzip, in dem Du von Konventionen ausgehst, die es aber gar nicht gibt.

Beispiel:
Ich wollte die Jahresarbeitszahlen für meine Wärmepumpen einlesen.
Die Rohwerte von Vissmann müssen durch 10 geteilt werden (ähnlich wie bei den Temperaturwerten), damit der Wert stimmt. Ich hab dann in der vcontrol.xml (ähnlich wie bei der Temperatur (unit ,,UT") die neue Einheit unit ,,UJ" eingeführt. Das VCLIENT-Modul reagierte darauf so, dass nur der ganzzahlige Teil der Jahresarbeitszahl angezeigt wird.
Das hängt damit zusammen, das Du im Modul "89_vclient.pm" (Zeile 423 bis 428) nur für Temperaturwerte Nachkommastellen zulässt.

Aus folgenden Gründen ist diese Vorgehensweise schlecht:
1.
Die Namensgebung der get und set Befehle ist abhängig, von dem der die Datei "vito.xml" erstellt hat. In der Datei "vito.xml-Datei wird ja nur zu den Adressen u.a. ein Befehl zugeordnet. Der Befehlsname kann dabei willkürlich festgelegt werden.
Hier gehst Du aber davon aus, das die Temperaturbefehle ein "Temp" oder "temp" enthalten.

2.
Auch andere ,,Nicht-Temperatur"-Befehle können Werte mit Nachkommastellen liefern

3.
Du rundest dann auf eine Stelle hinter dem Komma.
Es könnten aber auch Werte geben, bei denen mehrere Stellen hinter dem Komma noch interessant sein könnten.

Ich würde hier empfehlen keine derartigen Manipulationen/Formatierungen im VCLIENT-Modul zu implementieren und stattdessen einfach den Originalwert zu übertragen.

Falls Du hier anderer Meinung bist, hätte ich zumindest den Wunsch, dass neben den Befehlen, die "Temp" oder "temp" enhalten, auch Befehle die "JAZ" (Jahresarbeitszahl) enthalten wie die Temperaturwerte darzustellen (-> sprintf("%.1f", ....  ).

Gruß
Harald Schmitz

[andies]

Hallo Harald,

Sorry erstmal für die lange Wartezeit, ich habe das erst jetzt entdeckt. Ich finde die Vorschläge alle sehr gut, und dass ich diese Fehler gemacht habe, hat einen einfachen Grund: Ich bin kein Programmierer und kannte mich zudem in Perl nicht aus.

Ich finde die Vorschläge auch gut, habe aber folgendes Problem. Ich bin noch ein paar Wochen nicht lange genug zu Hause, um das vor Ort zu testen (und zu allem Unglück ist mir FHEM abgestürzt, eine MySQL Sache). Am besten ist es, wenn du in dem anderen Thread eine neue Version einstellst, so dass alle Nutzer das bemerken? Und müssen denn die config-Dateien umgestellt werden? Dann sollten wir das entsprechend ankündigen. Wie macht man so etwas am besten? Ich kann da gern helfen, würde aber ungern den lead übernehmen - als noob, der ich eigentlich im programmieren bin.

[Xaneu]

Hallo Andreas,

Sorry erstmal für die lange Wartezeit, ich habe das erst jetzt entdeckt.

Das ist überhaupt kein Problem, schließlich arbeiten hier ja alle ehrenamtlich.

Ich finde die Vorschläge alle sehr gut, und dass ich diese Fehler gemacht habe, hat einen einfachen Grund: Ich bin kein Programmierer und kannte mich zudem in Perl nicht aus.

Wow, dann ist es aber erstaunlich, dass Du überhaupt ein Modul geschrieben hast. Respekt!

Ich finde die Vorschläge auch gut, habe aber folgendes Problem. Ich bin noch ein paar Wochen nicht lange genug zu Hause, um das vor Ort zu testen

Meine Anregungen haben Zeit. Ich habe es ja bei mir so gelöst, indem ich die Zeile 423 von

Code Auswählen Erweitern

if (($last_cmd !~ /(T|t)emp/) and ($last_cmd !~ /Neigung/))

in

Code Auswählen Erweitern

if (($last_cmd !~ /(T|t)emp/) and ($last_cmd !~ /Neigung/) and ($last_cmd !~ /JAZ/))

geändert habe.

(und zu allem Unglück ist mir FHEM abgestürzt, eine MySQL Sache).

Obwohl ich mich mit SQL-Datenbanken auskenne, habe ich bisher gezögert, die Log-Geschichten auf DbLog umzustellen. Zum einen kann man die Logs dann nicht mehr so einfach in den log-Dateien im Klartext lesen. Zum anderen wächst die Komplexität weiter und im Falle eines Crashs steht man dann da. Momentan läst meine FHEM-Perfomance es noch zu ohne Datenbank zu arbeiten.

Am besten ist es, wenn du in dem anderen Thread eine neue Version einstellst, so dass alle Nutzer das bemerken?

Ich habe das Gefühl, dass der andere Thread, der auch auf der vclient-Wiki-Seite verlinkt, ist eher so ein Sammelthread ist bzw. es eigentlich um das Modul vcontrol.pm geht!? Am besten wäre es wenn man einen eigenen vclient-Tread hätte.

Und müssen denn die config-Dateien umgestellt werden? Dann sollten wir das entsprechend ankündigen. Wie macht man so etwas am besten?

Die vlient.cfg, die ja zum vclient-Modul gehört, muss nicht verändert werden. Wenn man es sauber machen will, müsste man im Modul alles entfernen, wo die Daten verändert werden.
Im vcontrol-System kann man ohnehin Manipulationen der Daten vornehmen.
in der vcontrol.xml kann man die Einheiten bzw. Datentypen festlegen. Ich habe hier die JAZ (Jahresarbeitszahl)  z.B. hinzugefügt:

<units>
    <unit name='Plain'>
      <abbrev>PL</abbrev>
      <calc get='V' set='V'/>
      <type>short</type>
    </unit>
    <unit name='Temperatur'>
      <abbrev>UT</abbrev>
      <calc get='V/10' set='V*10'/>
      <type>short</type>
      <entity>Degrees Celsius</entity>
    </unit>
    <unit name='Neigung'>
      <abbrev>UN</abbrev>
      <calc get='V/10' set='V*10'/>
      <type>short</type>
      <entity></entity>
    </unit>
      <unit name='JAZ'>
      <abbrev>UJ</abbrev>
      <calc get='V/10' set='V*10'/>
      <type>short</type>
      <entity></entity>
    </unit>
    ...
</units>

und in der vito.xml kann man die Einheiten/Datentypen den Adressen dann zuordnen (in meinem Beispiel):

<command name="getJAZ" protocmd="getaddr">
   <addr>1680</addr>
   <len>1</len>
   <unit>UJ</unit>
   <description>Statistik - Energiebilanz: Jahresarbeitszahl (0..10)</description>
</command>
<command name="getJAZHeiz" protocmd="getaddr">
   <addr>1681</addr>
   <len>1</len>
   <unit>UJ</unit>
   <description>Statistik - Energiebilanz: Jahresarbeitszahl Heizen (0..10)</description>
</command>
<command name="getJAZWW" protocmd="getaddr">
  <addr>1682</addr>
  <len>1</len>
  <unit>UJ</unit>
  <description>Statistik - Energiebilanz: Jahresarbeitszahl WW (0..10)</description>
</command>

Ich kann da gern helfen, würde aber ungern den lead übernehmen - als noob, der ich eigentlich im programmieren bin.

Ganz tief stecke ich in dem vlient.pm-Quelltext auch noch nicht d'rin.
Ich würde aber vermuten, dass der komplette Block ab Zeile 423:

Code Auswählen Erweitern



if (looks_like_number($results[0])){
  #if ( $last_cmd =~ /(S|s)tatus/ || $last_cmd =~ /BetriebSpar/ || $last_cmd =~ /BetriebParty/ )
  # Wenn vcontrold-command "Temp" oder "Neigung" (Heizkurve!) enthaelt, Runden auf 1 , sonst Runden auf 0 (=Statuswert)
  if (($last_cmd !~ /(T|t)emp/) and ($last_cmd !~ /Neigung/)) {
    $value = sprintf("%.0f", $results[0]); #round to integer, if status value
    } else {
    $value = sprintf("%.1f", $results[0]); #round to, for example, 16.6
    }
} else {
  $value = $zeilen[0]; #Buchstaben fuer Betriebsart u.Ae.
}


einfach nur in

Code Auswählen Erweitern


$value = $zeilen[0];


geändert werden müßte. Das müßte allerdings getestet werden und wäre somit etwas aufwendiger.
Danach dürften dann die Werte so auftauchen, wie man diese aus dem vcontrol-System bekommt bzw. wie man diese in den xml-Dateien definiert hat.
Rundungen und Weiterverarbeitung der Readings etc. kann man dann im FHEM außerhalb des Moduls machen.

Ich möchte hier aber auch kein zu großes Fass aufmachen. Vielleicht wäre es am einfachsten, wenn Du wie oben beschrieben
die Zeile 423 von

Code Auswählen Erweitern

if (($last_cmd !~ /(T|t)emp/) and ($last_cmd !~ /Neigung/))

in

Code Auswählen Erweitern

if (($last_cmd !~ /(T|t)emp/) and ($last_cmd !~ /Neigung/) and ($last_cmd !~ /JAZ/))

änderst.
Das kannst Du blind ändern ohne, dass irgendjemand davon betroffen wäre.

Gruß
Harald

[Maui]

Moin,

Finde die Anmerkungen von Harald alle richtig,
Würde aber dazu tendieren, falls das einer von euch umsetzt, es in eine längere beta-Phase zu schieben, ohne relativ schnell die Version im svn hochzuziehen.
Ich zb nutze das Modul komplett für meine WW und Heizungssteuerung und deswegen wäre ein Sommer weitaus geeigneter für ein Release als ein Winter.

Da es aktuell ja auch läuft, steckt ja auch kein Zeitdruck hinter.

Gruß
Maui

[andies]

Gute Idee. Zudem ich gerade sowieso Probleme habe, mein Speicher in FHEM läuft ständig voll und ich weiß nicht warum (habe dazu einen eigenen Thread und dachte, ich hätte den Verursacher gefunden; dem war aber doch nicht so - alle fünf Stunden ein Neustart von Fhem...).

[Maui]

Klingt nicht so schön.
Aber mit voll laufendem Speicher ist das Forum hier ja schon gut bestückt mit Threads.
Viel Erfolg.

[betateilchen]

dass ich diese Fehler gemacht habe, hat einen einfachen Grund: Ich bin kein Programmierer und kannte mich zudem in Perl nicht aus.

Moin,

wenn Du wirklich darüber nachdenkst, das Modul zu optimieren, könntest Du noch zwei Dinge in Betracht ziehen:

• Such Dir eventuell einen FHEM Developer, der Dich bei der Überarbeitung unterstützt.
• Versuche, mit DevIO.pm als "FHEM-Bordmittel" zu arbeiten, anstatt die komplette IO-Verbindung über telnet selbst zu erstellen.

[fiedel]

• Versuche, mit DevIO.pm als "FHEM-Bordmittel" zu arbeiten, anstatt die komplette IO-Verbindung über telnet selbst zu erstellen.

Sehr schön,

ich bastle auch gerade an einem ähnlichen Modul und hatte mich gefragt ob DevIO.pm noch "state of the art" ist, oder die Lösung
aus dem VCLIENT moderner ist. DevIO.pm hat doch so viele "verbotene" Funktionen laut Wiki...

[andies]

Wir hatten ja vereinbart, in der Sommerzeit noch einmal auf das Modul zurückzukommen. Das würde ich jetzt gern tun. Es gab ein paar Ideen zur Veränderung. Einmal DevIO.pm, da brauche ich Nachhilfe - aber mal schauen, das ist momentan weniger dringend. Die wichtigere Frage war, ob die Ausgaben von vcontrold vom Modul direkt durchgereicht werden (und daher deren Behandlung innerhalb von FHEM erfolgen soll), das scheint mE der programmiertheoretisch saubere Weg zu sein, oder ob innerhalb vom Modul die Angaben vorab sortiert werden. Solche eine Sortierung findet beispielhaft für diejenigen, die das nicht sehen, hier statt

Code Auswählen Erweitern

sub VCLIENT_integrity_check($)
{
    my $value = shift;
    my $integrity = 1;
    #Temperaturen muessen unter 110 Grad Celsius sein (vorher testen ob value Zahl ist - vermeidet warnings bei 'Unkown buffer')
    if (($last_cmd =~ /(T|t)emp/) and ($value =~ /^\d+.\d*$/))
    {
        $integrity &&= ($value < 110);
    }
    #Status darf nur 0 oder 1 sein
    if (($last_cmd =~ /(S|s)tatus/) and ($value =~ /^\d$/))
    {
        $integrity &&= ($value =~ /(0|1)/);
    }
    # Unkown buffer format ist nicht korrekt
    $integrity &&= ($value !~ /Unkown buffer format/);

    return $integrity;
}Nun habe ich auch Rückmeldungen von Anwendern, die genau einen solchen Check einfordern: siehe beispielsweise
https://forum.fhem.de/index.php?msg=869360

Ich weiß jetzt nicht, wie man hier weitermacht. Einerseits könnte ich die gesamten Integrity-Checks entfernen (da gibt es ja weitere, siehe beispielsweise Zeile 415 ff. in dem Modul), das bedeutet dann Arbeit für diejenigen, die das bisher nutzen und sich da nicht auskennen. Andererseits scheint das Modul aber so sauberer und vielleicht auch langfristig von anderen pflegbar zu werden.

Wie entscheidet man so was? Wie nimmt man Rücksicht auf Wünsche mehrerer? Ich bin da etwas ratlos. Eine Umfrage kann ich machen, aber reicht das?

[Maui]

Also ich würde im Sinne der Wartbarkeit ganz klar die checks rausnehmen und ggf auf fhem Ebene machen mit zb userreadings oä.
Es mag ja auch Fälle geben, wo der User gerne sehen würde, wenn andauernd falsche Werte kommen. Und wenn sie im Modul schon weggefiltert werden sieht er das nicht mehr (und kann nicht mehr ggf. Darauf reagieren)

[andies]

Wie würde man denn zB dem oben genannten Plausibilitätscheck auf FHEM-Ebene machen können? Es geht ja unterm Anderem auch darum, dass unsinnige Werte nicht ins Log kommen etc. Ich denke, hier müssen wir auch denjenigen Hilfestellungen geben, die das haben wollen.

[betateilchen]

So ganz habe ich das Problem noch nicht verstanden.
Wenn Du als Entwickler entscheidest, diesen Check einzubauen, dann mach das - dafür musst Du Dich nicht rechtfertigen. Den Anwendern später zu überlassen, mit userReadings rumzufrickeln hat nix mehr mit usability zu tun. Ob der Check innerhalb des Moduls durchgeführt wird oder nicht, könntest Du für den Benutzer beispielsweise per Attribut wählbar machen (wenn das wirklich Sinn macht).

Aber Dein Check ist für mich viel zu kompliziert aufgebaut, es scheitert wohl an der einfachen Formulierung der zu lösenden Aufgabe.

Du möchtest gerne 3 Dinge prüfen:

• Der check ist "false", wenn $value keine Zahl ist
• Der check ist "false", wenn der Befehl Temp kommt und $value > 110 ist
• Der check ist "false", wenn der Befehl Status kommt und $value nicht 0 oder 1 ist

Diese Bedingungen brauchst Du gar nicht miteinander zu verknüpfen, denn Du kannst die Verarbeitung beenden, wenn eine (!) der Bedingungen "false" ist. Bei mir würde das beispielsweise so aussehen - ganz ohne die vielen unübersichtlichen regex:

Code Auswählen Erweitern

sub VCLIENT_integrity_check {
    my $value = shift;
    return 0 unless looks_like_number($value);
    return 0 if (uc($last_cmd) eq "TEMP" && $value > 110);
    return 0 if (uc($last_cmd) eq "STATUS" && $value > 1);
    return 1;
}


[betateilchen]

Es mag ja auch Fälle geben, wo der User gerne sehen würde, wenn andauernd falsche Werte kommen.

Das kann man auch einfach mit einer Logmeldung erreichen, die im Fehlerfall (wenn der Check negativ war) ausgegeben wird.

[andies]

Vielen Dank nochmal an betateilchen für die Codeschnipsel, man lernt nie aus. Ich bin jetzt mal in mich gegangen und habe darüber nachgedacht, ob ich das Modul anfasse. Ich neige dazu, das eher nicht zu tun:

• Ich bin kein professioneller Programmierer. Das heißt: Jede Veränderung bei mir birgt Risiken, dass sich Fehler einschleichen. Das würde ich dann auf mich nehmen, wenn es sinnvollere Verbesserungen gibt. Das sehe ich aber nicht. Denn
• Die Grundidee der Veränderung war ja, dass man das Modul zukunftsfähiger macht. Das mag aus programmiertechnischer Sicht sinnvoll sein, ich habe leider nicht die Fähigkeiten, das mal so nebenbei zu tun. Daher schaffe ich das nicht, ohne andere Dinge (dir mir gerade wichtiger sind) aufzuschieben - und das will ich nicht tun.
• Die Idee, dass man je nach Nutzereingaben mal alles direkt ausgibt und mal "bearbeitet" klingt ja gut, ist aber nicht das, was ursprünglich gewollt war. Da ging es ja darum, einer anderen Programmierphilosophie zu folgen. Und auch da gilt: Das mache ich leider nicht nebenbei.

Also kurz gesagt: Wenn jemand das machen möchte, versuche ich gern zu helfen. Aber ich kann den Umbau des Moduls in die moderne Richtung nicht vornehmen bzw mache das erst dann, wenn FHEM anders nicht mehr funktioniert. Ich kann da nur auf Verständnis hoffen.