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
Finde ich super!
Vielleicht könnte man dann ja über Guidelines zum Attribut naming nachdenken. Groß/Kleinschreibung etc.
Finde ich auch gut, nur... was passiert, mit eigenen Abfragen des users? Ist bei Attrubuten wahrscheinlich eher selten, aber kommt vor.
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
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"
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.
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
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.
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.
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.
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.
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).
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.
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.
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};
Hallo Rudi,
vielen Dank dafür. Mir ist soeben beim testen aufgefallen, dass die Save-Historie beim setzen eines alten Attributnamen nachwievor den alten Namen anzeigt, anstatt dem neuen. Grund hierfür ist der Aufruf von addStructChange() am Ende von CommandAttr() die Eingangsparameter verwendet und nicht den evtl. konvertierten Attributnamen:
Index: fhem.pl
===================================================================
--- fhem.pl (revision 15945)
+++ fhem.pl (working copy)
@@ -2824,7 +2824,7 @@
return $err if($err);
evalStateFormat($hash);
}
- addStructChange("attr", $sdev, $param) if(!defined($oVal) || $oVal ne $val);
+ addStructChange("attr", $sdev, "$attrName $val") if(!defined($oVal) || $oVal ne $val);
DoTrigger("global", "ATTR $sdev $attrName $val", 1) if($init_done);
}
Viele Grüße
Markus
Danke, habs eingecheckt.
Nochmal das gleiche für dem Aufruf der AttrFn:
Index: fhem.pl
===================================================================
--- fhem.pl (revision 16042)
+++ fhem.pl (working copy)
@@ -2808,7 +2808,7 @@
$attrVal = "-" if($attrName eq "logfile");
$attrVal = 5 if($attrName eq "verbose");
}
- $ret = CallFn($sdev, "AttrFn", "set", @a);
+ $ret = CallFn($sdev, "AttrFn", "set", $sdev, $attrName, $attrVal);
if($ret) {
push @rets, $ret;
next;
Danke, habs nachgezogen im Rahmen der anderen Umbau:
https://forum.fhem.de/index.php/topic,83541.msg758051.html#msg758051