[PATCH] - fhem.pl - Attribute umbenennen mit AttrRenameMap in Modulen

Begonnen von Markus Bloch, 07 Januar 2018, 11:52:35

Vorheriges Thema - Nächstes Thema

Markus Bloch

Hallo zusammen,

anbei ein Patch um Modulautoren das umbenennen von Attributen zu ermöglichen, ohne dass dies bei Usern zu riesigen Problemen führt. Dazu wird in der Initialize-Funktion eine AttrRenameMap in Form eines Hash übergeben. Sobald ein User in seiner Umgebung nachwievor die alte Attribut-Bezeichnung verwendet, wird diese automatisch anhand dieser Rename-Map in die neue Attributbezeichnung konvertiert und in %attr gesetzt. Der User bekommt davon nichts mit. Bei dem nächsten "save" wird der neue Attributname in der Konfiguration gespeichert.

Dadurch wird es Modulautoren ermöglicht, Attribute die im Nachhinein vlt. unglücklich gewählt sind in einen treffenderen Namen zu ändern ohne zu befürchten, dass es bei allen Usern nach einem Update zu Problemen kommt weil das alte Attribut nicht mehr akzeptiert wird.

Bsp:

Die aktuelle Attribut-Liste von PRESENCE aus der InitializeFn:

$hash->{AttrList} = "do_not_notify:0,1 ".
                       "disable:0,1 ".
                       "disabledForIntervals ".
                       "fritzbox_speed:0,1 ".
                       "ping_count:1,2,3,4,5,6,7,8,9,10 ".
                       "bluetooth_hci_device ".
                       "absenceThreshold ".
                       "presenceThreshold ".
                       "absenceTimeout ".
                       "presenceTimeout ".
                       "powerCmd ".$readingFnAttributes;


Nun möchte ich bspw. alle Attribute mit einem "_" in lowerCamelCase ändern (oder einen gänzlich anderen Namen geben). Ich führe dazu im gesamten Modul ein Suchen/Ersetzen durch und definiere dann eine AttrRenameMap:

   $hash->{AttrList} = "do_not_notify:0,1 ".
                       "disable:0,1 ".
                       "disabledForIntervals ".
                       "fritzboxSpeed:0,1 ".
                       "pingCount:1,2,3,4,5,6,7,8,9,10 ".
                       "bluetoothHciDevice ".
                       "absenceThreshold ".
                       "presenceThreshold ".
                       "absenceTimeout ".
                       "presenceTimeout ".
                       "powerCmd ".$readingFnAttributes;

# convert outdated attribute names to new ones
$hash->{AttrRenameMap} = { "fritzbox_speed" => "fritzboxSpeed",
                           "ping_count" => "pingCount",
                           "bluetooth_hci_device" => "bluetoothHciDevice"
                         };



Wenn nun bei einem User in der Konfiguration nachwievor "ping_count" verwendet wird, wird das Attribut beim ausführen des attr-Befehls (manuel, als auch bei FHEM start) automatisch auf die neue Attribut-Bezeichnung geändert. Das Attribut funktioniert damit weiterhin unter dem neuen Namen ohne Probleme. Der User merkt nichts davon.

Viele Grüße

Markus
Developer für Module: YAMAHA_AVR, YAMAHA_BD, FB_CALLMONITOR, FB_CALLLIST, PRESENCE, Pushsafer, LGTV_IP12, version

aktives Mitglied des FHEM e.V. (Technik)

fhainz

Finde ich super!
Vielleicht könnte man dann ja über Guidelines zum Attribut naming nachdenken. Groß/Kleinschreibung etc.

KernSani

Finde ich auch gut, nur... was passiert, mit eigenen Abfragen des users? Ist bei Attrubuten wahrscheinlich eher selten, aber kommt vor.
RasPi: RFXTRX, HM, zigbee2mqtt, mySensors, JeeLink, miLight, squeezbox, Alexa, Siri, ...

Markus Bloch

Anbei ein Patch wo auch AttrVal() die AttrRenameMap berücksichtigt, sobald man einen alten Attributbezeichner in eigenen Skripten verwendet. Wenn ein User auf %attr direkt zugreift ist er mMn selber Schuld, weil er dort nichts zu suchen hat und das auch nirgendwo dokumentiert ist.

Viele Grüße

Markus
Developer für Module: YAMAHA_AVR, YAMAHA_BD, FB_CALLMONITOR, FB_CALLLIST, PRESENCE, Pushsafer, LGTV_IP12, version

aktives Mitglied des FHEM e.V. (Technik)

betateilchen

Ich finde die Idee nicht besonders gut.

Zitat von: Markus Bloch am 07 Januar 2018, 11:52:35
Dadurch wird es Modulautoren ermöglicht, Attribute die im Nachhinein vlt. unglücklich gewählt sind in einen treffenderen Namen zu ändern

Darüber sollten sich Modulautoren im Vorfeld Gedanken machen. Und wenn sich tatsächlich Bedarf an Umbenennung von Attributen gibt, sollte der Modulautor einen entsprechenden Mechanismus bereitstellen (wie das z.B. bei HTTPMOD irgendwann war) und auch die Anwender im Forum darüber informieren.

Von einem automatischen Umbenennen ohne Zutun des Anwenders halte ich gar nichts.

Zitat von: Markus Bloch am 07 Januar 2018, 11:52:35
beim ausführen des attr-Befehls (manuel, als auch bei FHEM start) automatisch auf die neue Attribut-Bezeichnung geändert. Das Attribut funktioniert damit weiterhin unter dem neuen Namen ohne Probleme. Der User merkt nichts davon.

Genau das ist das Problem. Der User muss aber wissen, dass sich Attributnamen geändert haben.

Mit einem automatischen Mapping wird der Supportaufwand unnötig erhöht, weil eigentlich schon absehbar im Forum gefragt werden wird "Mein Attribut xy funktioniert plötzlich nicht mehr?" oder "Fehlermeldung beim Setzen von Attribut xy"
-----------------------
Formuliere die Aufgabe möglichst einfach und
setze die Lösung richtig um - dann wird es auch funktionieren.
-----------------------
Lesen gefährdet die Unwissenheit!

CoolTux

Wurde nicht irgendwann einmal gesagt und es immer wieder runter gebetet das Attribute ausschließlich dem User gehören? Ich denke auch das eine vernünftige Informationspolitik des Modulautors hundert mal besser ist als hinten rum ohne Userinfo ein Mapping zu machen. Irgendwann haben wir da hunderte von Mappings und der User weiß immer noch von nichts.
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://www.paypal.com/paypalme/MOldenburg
My FHEM Git: https://git.cooltux.net/FHEM/
Das TuxNet Wiki:
https://www.cooltux.net

Markus Bloch

#6
Hallo zusammen,

die Änderung von Attributnamen zu kommunizieren steht für mich außer Frage (CHANGED + Ankündigung im Forum). Nur glaube ich nicht, dass das 100% der User die regelmäßig updaten erreicht.

Ich würde zusätzlich noch anbieten eine Logmeldung im Loglevel 2 zu produzieren wenn $init_done = 0 (z.B. "converted attribute identifier X to new identifier Y").

Dennoch glaube ich wird es ohne einen solchen Mechanismus immer User geben die dann besagte Threads im Forum eröffnen und jeder erstmal auf den Ankündigungsthread etc. hinweisen muss. Das möchte ich gerne vermeiden. Dieses Szenario ist es, was mich momentan davon abhält sowas zu machen.

Gruß
Markus
Developer für Module: YAMAHA_AVR, YAMAHA_BD, FB_CALLMONITOR, FB_CALLLIST, PRESENCE, Pushsafer, LGTV_IP12, version

aktives Mitglied des FHEM e.V. (Technik)

betateilchen

Zitat von: CoolTux am 07 Januar 2018, 12:41:35
Wurde nicht irgendwann einmal gesagt und es immer wieder runter gebetet das Attribute ausschließlich dem User gehören?

Genau. Und daran ändert auch eine Logmeldung nichts.
-----------------------
Formuliere die Aufgabe möglichst einfach und
setze die Lösung richtig um - dann wird es auch funktionieren.
-----------------------
Lesen gefährdet die Unwissenheit!

rudolfkoenig

Bin unentschlossen.

Contra:
- Falls der Modulautor ein Attribut umbenennt, und der Benutzer es statisch (d.h. in fhem.cfg) setzt, dann kriegt er das Problem nach dem Neustart in motd mit. Mit etwas Suche kann man das Problem loesen.
- AttrVal wird sehr haeufig verwendet, da tut mir jedes zusaetzliche Lookup weh.

Pro:
- es erleichtert eine einheitliche Umbenennung aller Modulattribute. Dazu muessten wir aber ein einheitliches  Namensschema haben, und idealerweise ein Mechanismus, was den Modulautor bei Verletzung am Einchecken hindert.

betateilchen

Zitat von: rudolfkoenig am 07 Januar 2018, 13:36:01
Pro:
- es erleichtert eine einheitliche Umbenennung aller Modulattribute. Dazu muessten wir aber ein einheitliches  Namensschema haben

dann lasst uns die vorgeschlagene Änderung bitte verschieben, bis es ein solches einheitliches Namensschema gibt.

Also ungefähr bis kurz nach der Eröffnung des neuen Berliner Flughafens.
-----------------------
Formuliere die Aufgabe möglichst einfach und
setze die Lösung richtig um - dann wird es auch funktionieren.
-----------------------
Lesen gefährdet die Unwissenheit!

betateilchen

Zitat von: rudolfkoenig am 07 Januar 2018, 13:36:01
- AttrVal wird sehr haeufig verwendet, da tut mir jedes zusaetzliche Lookup weh.

und es wird immer noch viel zuviel auf %attr direkt zugegriffen, da würde ein Mapping in AttrVal() nicht viel nützen.
-----------------------
Formuliere die Aufgabe möglichst einfach und
setze die Lösung richtig um - dann wird es auch funktionieren.
-----------------------
Lesen gefährdet die Unwissenheit!

Markus Bloch

Zitat von: betateilchen am 07 Januar 2018, 13:42:05
dann lasst uns die vorgeschlagene Änderung bitte verschieben, bis es ein solches einheitliches Namensschema gibt.

Vor (sehr) langer Zeit wurde das mal im Wiki festgehalten https://wiki.fhem.de/wiki/DevelopmentGuidelines#Bezeichnungen Ich weis nicht, inwieweit dieser Wiki-Artikel überhaupt noch aktuell und gültig ist. Der Artikel stammt noch von vor dem Wiki-Crash. Bis auf diesen Artikel gibt es sonst keine "Guidelines" meines Wissens.

Ich gebe zu das selber nicht konsequent befolgt zu haben, weswegen in meinen Modulen ein Wildwuchs an Schreibweisen herscht ("parameterName", parameter_name", parameter-name") und es eben Bezeichner gibt die für den User missverstanden werden (meine primäre Intention für den Patch).
Developer für Module: YAMAHA_AVR, YAMAHA_BD, FB_CALLMONITOR, FB_CALLLIST, PRESENCE, Pushsafer, LGTV_IP12, version

aktives Mitglied des FHEM e.V. (Technik)

betateilchen

#12
Die Diskussion über Konventionen bei Attributnamen hatten wir hier in den vergangenen Jahren schon mehrfach begonnen. Etwas wirklich sinnvoll umsetzbares ist bisher nicht rausgekommen. Es geht schon damit los, dass man bis heute keine Möglichkeit hat, in der Attributliste eines devices einfach zu erkennen, welche Attribute vom TYPE des Moduls stammen.

Deshalb bin ich schon lange dazu übergegangen, Attribute in meinen Modulen mit einem präfix beginnen zu lassen (z.B. bsh_.*) was zudem den Vorteil hat, dass alle Attribute in der Liste direkt hintereinander auftauchen.

Für mich gibt es bei den Attributen wirklich wichtigere offene Aufgaben als die Frage, ob und wie man die automatisch irgendwie umbennen soll/muss.
-----------------------
Formuliere die Aufgabe möglichst einfach und
setze die Lösung richtig um - dann wird es auch funktionieren.
-----------------------
Lesen gefährdet die Unwissenheit!

zap

ich finde es auch wichtig, dass Attributnamen einen Bezug zum Modul haben, zB über ein Präfix.

Es braucht aber eine praktikable Lösung, mit der man dieses Ziel erreichen kann, wenn das Kind schon in den Brunnen gefallen ist. Und das ist leider in vielen Modulen (auch meinen eigenen) der Fall.

Der Patch von Markus wäre hier sehr hilfreich. Alternative: man baut die Attribut Umbenennung in das eigene Modul ein. Man könnte in Notify als Reaktion auf das initialized Event die gesetzten Attribut umbenennen und die Config speichern. Natürlich muss der Code dann so angepasst sein, dass sowohl alte als auch neue Attribute möglich sind, zumindest bis man davon ausgehen kann, dass alle User dieses Release installiert haben.
2xCCU3, Fenster, Rollläden, Themostate, Stromzähler, Steckdosen ...)
Entwicklung: FHEM auf AMD NUC (Ubuntu)
Produktiv inzwischen auf Home Assistant gewechselt.
Maintainer: FULLY, Meteohub, HMCCU, AndroidDB

rudolfkoenig

Nach laengerem Nachdenken habe ich den Patch eingebaut.

Leichte Aenderung: statt $hash muss man resolveAttrRename mit $d aufrufen, damit man mit einem ungeschickten AttrVal Aufruf keine "Strange call for typeless X" Warnungen produziert. Bei jeder Konvertierung wird was im Log ausgegeben:
    Log 3, "WARNING: $d attribute $n was renamed to ".$m->{AttrRenameMap}{$n};