Hex/Dec-Problematik bei parse in 10_ZWave.pm (->Events/Readings)

Begonnen von krikan, 15 August 2016, 15:10:54

Vorheriges Thema - Nächstes Thema

krikan

Hallo!

Bin gestern (wieder) über Hex/dec-Probleme bei parse gestolpert. Obwohl die Eingaben für get/set alle dec-Werte sein müssen, ist das in der Ausgabe von parse noch je nach Nachricht/CC unterschiedlich.
Ich würde das gerne auf dec ändern, was aber zu Anpassungsbedarf beim User führt.
Beispiele, die ich auf dec umstellen möchte:
"..2003(.*)"=> '"basicReport:$1"
"042b01(..)(..)"  => '"scene_$1:$2"'
"048614(..)(..)"             => '"versionClass_$1:$2"'

Beim genaueren nachsehen finde ich evtl. noch mehr.

Was haltet ihr davon?

Gruß, Christian

rudolfkoenig

Viel, wenn du eine Liste erstellst, stelle ich um.
Weiss nur nicht, wie man das den Leuten klarmacht, die ihren notify/Plot/etc darauf abgestimmt haben.

krikan

Zitat von: rudolfkoenig am 15 August 2016, 15:55:44
Viel, wenn du eine Liste erstellst, stelle ich um.
Liste in Form eines ungetesteten Patches haengt an. Habe noch einige Anmerkungen als Kommentare hinzugefügt, die hoffentlich verstaendlich sind.
Bei versionClass habe ich "Prot" auf 2stellig umgestellt.

Zitat
Weiss nur nicht, wie man das den Leuten klarmacht, die ihren notify/Plot/etc darauf abgestimmt haben.
Deshalb die Anfrage. Kann Dir nur beistehen eventuelle Beschwerden zu überstehen.  :)
Ich finde es aber auch nicht ideal alte Zoepfe aus Ruecksicht beizubehalten. Andreas extendedAlarmReadings für CC ALARM geht vermutlich auch unter, obwohl mMn für alle aktuellen Geraete sicherlich mit Wert 1 die bessere Loesung.

A.Harrenberg

Hi,

ui, unschön...
Ich bin ja eigentlich auch dafür einen "Standard" (= Dezimaldarstellung) einheitlich umzusetzen, aber im Fall der BASIC Klasse wird es sicherlich einige Aufschreie geben da hier wahrscheinlich wirklich viele Notifies entsprechend gesetzt sind.

An dieser Stelle würde ich dafür pladieren das auf jeden Fall anzukündigen (dann kann keiner Meckern).
Manchmal wünschte ich es wäre eine Bestätigung beim Update eines Moduls nötig um zuzustimmen das man die Änderungen akzeptiert... Dann könnte man soetwas gelassener umstellen.

Gruß,
Andreas.
FB 7360, Homematic und ZWave
Support for ZWave-SECURITY

krikan

Und genau Event basicReport war der Auslöser des Threads: Ein Dimmer, der auf 80 gestellt wird, sollte auch 80 im zugehörigen Report-Event liefern und nicht eine Hex-Wert.

krikan

ZitatManchmal wünschte ich es wäre eine Bestätigung beim Update eines Moduls nötig um zuzustimmen das man die Änderungen akzeptiert... Dann könnte man soetwas gelassener umstellen.
So etwas in der Art "Zwangslesen" gibt es: http://fhem.de/commandref#notice
Meine Begeisterung löst es derzeit nicht aus. Ob der Befehl auch modulbezogen (nur bei ZWave-Usern anzeigen) arbeitet, ist mir unbekannt.

Ansonsten haben wir ja noch:
- Ankündigung im Unterforum "Ankündigungen"
- CHANGED (wird aber bei selten Updatenden irgendwann "geskipped")

A.Harrenberg

Hallo Christian,

ja, ich stimme Dir da auch völlig zu, aber die Mehrzahl der User wird einfach nur auf 00/FF geachtet haben und das so eingerichtet haben... (Ging ja auch nicht anders...)
Ich bin ja auch für das Umstellen, irgendwie sollten die User aber die Möglichkeit haben das bewusst umzustellen.

Ich denke sogar das da teilweise noch mehr hex/dez Sachen versteckt sind... Bei einigen Klassen werden zusätzliche Werte als "arg" hexadezimal mit ausgegeben, wobei es dort meist auch Sinn macht. Ich habe das bei ALARM/NOTIFICATION auch so aus der bisherigen Implementation übernommen.

Gruß,
Andreas.

FB 7360, Homematic und ZWave
Support for ZWave-SECURITY

rudolfkoenig

Vorschlag:
- Hex/Dec ist Attributgesteuert
- Attribut default ist fuer featurelevel < 5.8 hex (wie bisher), ab 5.8 dec.
- Attribut wird bei 5.9 oder 6.0 ausgebaut.
- Aenderung wird in CHANGED dokumentiert, und beim Wechsel auf 5.8 in der grossen Ankuendigung erwaehnt.

A.Harrenberg

Hallo Rudi,

soetwas hatte ich auch kurz überlegt mich aber nicht getraut vorzuschlagen... ;-)

Schwebt Dir denn da ein globales Attribut oder ein gerätespezifisches Attribut vor? Und soll das dann nur für die jetzigen "Problemfälle" gelten oder global? (Temperturen in hex machen wenig Sinn...)

Gruß,
Andreas.
FB 7360, Homematic und ZWave
Support for ZWave-SECURITY

krikan

Zitat von: rudolfkoenig am 16 August 2016, 12:10:26
Vorschlag:
- Hex/Dec ist Attributgesteuert
- Attribut default ist fuer featurelevel < 5.8 hex (wie bisher), ab 5.8 dec.
- Attribut wird bei 5.9 oder 6.0 ausgebaut.
- Aenderung wird in CHANGED dokumentiert, und beim Wechsel auf 5.8 in der grossen Ankuendigung erwaehnt.
Für mich ok, da ich nur gerne eine Vereinheitlichung hätte. Würde dann aber eventuell noch ein paar Kandidaten heraussuchen und bspw. bei basicReport 0x00h auf off, 0xFF auf on und Rest auf dim % mappen.

Aber:
Das Attribut featurelevel verschiebt das Problem nur in die Zukunft. Neueinsteiger werden die alte Variante nutzen und damit die betroffene Usergruppe von der zwangsweisen Anpassung beim Ausbau noch größer.
Es bedeutet Mehraufwand bei der Einbindung/Pflege des Codes und auch laufend beim Support im Forum für Alt- und Neuvariante.
Persönlich passe ich lieber laufend ein wenig an, als beim "großen" Update alles anpassen zu müssen. Erhöht das nicht auch den Updatewiderstand auf neue Version?
Noch ein persönliches Beispiel zu einem Attribut mit featurelevel: perlSyntaxCheck finde ich grundsätzlich gut und sinnvoll, habe es aber ausgeschaltet, nachdem ich Probleme hatte und ich nicht suchen wollte  :-[ und nie mehr eingeschaltet. Wieviele haben es derzeit überhaupt noch eingeschaltet? Werden die Funktionen laufend ausreichend getestet? Oder kommt eine Supportlawine beim Update, weil es Spezialfälle gibt, die dann geklärt werden müssen. Ok, perlSyntaxCheck ist deutlich komplexer als nur die hex/dec-Thematik.
Soweit meine Bedenken.  :)

rudolfkoenig

ZitatSchwebt Dir denn da ein globales Attribut oder ein gerätespezifisches Attribut vor?
Wir braeuchten hier ein Modulspezifisches, das haben wir aber nicht. Mein Favorit ist z.Zt. ein im ZWave Modul gesetztes globales Attribut. Da ich aber nicht wirklich an der Idee haenge, und Christians Argumente valide sind, nehme ich erstmal Abstand davon.

Falls kein Veto kommt, dann werde ich die Aenderungen (ohne Attribut) einbauen, und in der Ankuendigung es extra melden (+ CHANGED)

ZitatWieviele haben es derzeit überhaupt noch eingeschaltet?
Keine Ahnung.

ZitatWerden die Funktionen laufend ausreichend getestet?
Ausreichend ist Definitionssache. :)
Ich mache das, da mein test fhem.cfg mit "attr global featurelevel 5.8" laeuft.

ZitatOder kommt eine Supportlawine beim Update, weil es Spezialfälle gibt, die dann geklärt werden müssen.
Das kommt sicher, allerdings hoffe ich, dass andere aktive FHEM-Benutzer es auch aktivieren, und damit Probleme vorher auffallen. Weiterhin ermoeglicht featurelevel Anwendern bei Bugs erstmal auf die alte Version zu schalten, waehrend der Modulautor nach Loesung sucht. Problemmeldungen wg. beliebige Aenderungen kommen gerne nach einem Jahr, mit dem klassischen "nach update laeuft FHEM nicht" Betreff, und ich habe keine Idee, wie man das fuer alle befriedigend loesen kann.

krikan

Zitat von: rudolfkoenig am 16 August 2016, 14:28:05
Ich mache das, da mein test fhem.cfg mit "attr global featurelevel 5.8" laeuft.
Dass Du es nutzt, denke ich mir. Nur nutzt Du bspw. Codemirror? Vmtl. nicht, oder?  ;)

ZitatDas kommt sicher, allerdings hoffe ich, dass andere aktive FHEM-Benutzer es auch aktivieren, und damit Probleme vorher auffallen.
Hätte man mir nicht die Möglichkeit des Abschaltens gegeben, hätte ich das wieder Einschalten nicht vergessen.   :-[  (featurelevel ist grds. ok und gut :) )
Zitat
Problemmeldungen wg. beliebige Aenderungen kommen gerne nach einem Jahr, mit dem klassischen "nach update laeuft FHEM nicht" Betreff, und ich habe keine Idee, wie man das fuer alle befriedigend loesen kann.
Sachlich: Vermutlich nahezu unmöglich. Jeder wird es anders sehen. Und ich bin auch evtl. nicht der typische Anwender. Vielleicht braucht es doch eines "Zwangslesens" bei wichtigen/großen Updates.

Zurück zu ZWave:
Ich sehe die Änderung als nicht so gravierend an. Tippe darauf, dass die Anpassung von basicReport gar nicht so viele trifft. Um das noch weiter zu minimieren, könnte man notfalls auch 0x00 und 0xff lassen wie sie sind und nur den Rest konvertieren (nicht mein Wunsch!). Und bei anderen parse-Dingen wird die betroffene Usergruppe mMn noch einmal deutlich geringer sein. Das Hex-Werte als (zusätzliche) Argumente noch vorhanden sind, finde ich Ok. Korrespondierende set- und get-Befehle sollten sich auf jeden Fall entsprechen. Zudem leuchtet mir nicht ein, warum bspw. Szene mit hex bezeichnet sein sollen.
Btw: Es gibt extra das sogenannte "Paranoiker"-Attribut eventForRaw. Wer also beim Update nicht liest und auch das Attribut nicht gesetzt hat, der ist es selbst Schuld und wir helfen doch bei Problemen trotzdem gerne weiter.

rudolfkoenig

Habs eingecheckt, habe nur Kommentare entfernt, und bei >80 Zeichen umformatiert.

Dein Kommentar "ZWave_sensorbinaryV2Parse... #liefert 00=idle und ff=detected; umstellen?" habe ich nicht verstanden, ich sehe 00=>unknown:00 und ff=>unknown:ff.

Fuer die Benutzer aendern sich folgende events/readings:

basicSet
basicReport
scene_*
group_*
ccStatus_*
assocGroupName_*
assocGroupCmdList_*
versionClass_*
mca_*
alarm_type_

Falls jemand einen dieser Readings in notify/DOIF/FileLog/DbLog verwendet, dann bitte erneut pruefen, und ggf. hex Zahlen durch (nicht mit 0 aufgefuellte) Dezimalzahlen ersetzen.

krikan

Zitat von: rudolfkoenig am 17 August 2016, 13:24:26
Habs eingecheckt, habe nur Kommentare entfernt, und bei >80 Zeichen umformatiert.
War ungetestet, das hattest Du hoffentlich gelesen und noch mal drübergeschaut.
Kommentare waren nur für Dich.

ZitatDein Kommentar "ZWave_sensorbinaryV2Parse... #liefert 00=idle und ff=detected; umstellen?" habe ich nicht verstanden, ich sehe 00=>unknown:00 und ff=>unknown:ff.
Bezieht sich nicht auf den Alarmtype, sondern auf den Alarmzustand:
Bisher werden "kein Alarm" mit 00 und "Alarm" mit ff gemeldet. Soll das nicht besser auf Klartext umgestellt werden (Alarm = detected und kein Alarm = "idle") ?

krikan

Zitat von: krikan am 17 August 2016, 13:32:46
Bezieht sich nicht auf den Alarmtype, sondern auf den Alarmzustand:
Bisher werden "kein Alarm" mit 00 und "Alarm" mit ff gemeldet. Soll das nicht besser auf Klartext umgestellt werden (Alarm = detected und kein Alarm = "idle") ?
Im Patch nachgeschaut und jetzt verstehe ich das Problem und meinen Fehler 8)
Der Kommentar gehört nicht zu SENSOR_BINARY sondern SENSOR_ALARM...