search

smarthomeNG / smarthome

Device integration platform for your smart home
https://www.smarthomeNG.de
GNU General Public License v3.0
122 stars 91 forks source link

Vorschlag zum besseren Handling von Items mit Datentyp 'dict' oder 'list' #582

Closed msinn closed 9 months ago

msinn commented 1 year ago

Die komplexen Datentpyen dict und list werden bisher nur genau so unterstützt, wie simple Datentypen. Eine Wertzuweisung ist nur für den gesamten Item Wert (also das gesamte dict bzw. die gesamte Liste) möglich.

Es wäre zum Umgang mit diesen Datentypen hilfreich, wenn man Zuweisungen zu einzelnen Elementen dieser Typen vornehmen könnte.

Was haltet ihr davon? Habt ihr alternative Vorschläge?

Aktueller Vorschlag zur Implementierung

für Listen:

Listentry lesen: myvar = myitem(index=4)

Listentry schreiben: myitem(myvalue, index=4)

für Dictionaries:

dict Eintrag lesen: myvar = myitem(key='k1')

dict Eintrag schreiben: myitem(myvalue, key='k1')

ALTERNATIV:

1.

oder:

2.

Eventuell: Löschen eines key/value Parres erfolgt über den Parameter deletekey (myitem(deletekey='k1'))

onkelandy commented 1 year ago

Klingt sehr vernünftig. Ich würde die value, key bzw item(key) bevorzugen. Ist kompakter und auch schön schlüssig

aschwith commented 1 year ago

Finde die Idee auch sehr gut. Hätte jede Menge Anwendungsfällt wie zum Beispiel Farben (Hue, Saturation, Level) in einem Item zu vereinen. @wvhn : Super wäre hier auch eine Erweiterung, um aus der smartVisu bei list Items direkt eine Indexposition setzen zu können. Beispiel

Item hsv vom Typ list mit drei Einträgen (hue, saturation, level).

basic.slider bekommt die Möglichkeit im list item nur den index von level zu ändern.

wvhn commented 1 year ago

Zuerst mal finde ich die Idee super, dicts und lists in shNG einfacher zugänglich zu machen. Bei den lists wäre IMHO noch zu überlegen, wie man mit mehreren Dimensionen umgeht. Das ist z.B. für die list-items bei den Plots wichtig, wo jedes Listenelement wieder aus einem Wertepaar für X und Y besteht [[x1, y1], …[xn, yn]].

In wie weit ich das in der Visu noch in bestehende Widgets einbauen kann, muss ich sehen. Evtl. wäre es hilfreich, wenn man den Index eines list-items schon per Websocket abrufen könnte, wie schon die item properties. Dann kann der Anwender dies im Widget Aufruf konfigurieren: basic.slider('', 'bad.licht.rgb.index(2)'). Das wäre auch für die Kompatibilität mit den anderen Backends gut.

basic.color kann heute schon mit einem list-item umgehen.

Edit: es ist natürlich wichtig, dass abonnierte Listenelemente per Websocket Updates erhalten, wie die regulären items.. Das ist bei den item properties heute nicht gegeben.

msinn commented 1 year ago

Ich denke gerade auch dem Fehlerhandling herum. Wenn man in einem eval bei einem list Item auf einen Eintrag zugreift, den es nicht gibt, erhält das Ziel Item bisher (völlig unsinnigerweise) den index Wert.

Beispiel:

list:
    var_list1:
        type: list
        initial_value: '["12", "34", "56", "78"]'

    var_index:
        type: num

    var_str1:
        type: str
        eval: sh...var_list1()[sh...var_index()]
        eval_trigger:
          - ..var_list1
          - ..var_index

    var_str1n:
        type: str
        eval: sh...var_list1(index=sh...var_index())
        eval_trigger:
          - ..var_list1
          - ..var_index

Über einen Eintrag in var_index kann ich den gewünschten EIntrag aus der Liste wählen. Normalerweise erhält var_str1 den jeweiligen Eintrag aus var_list1.

Wenn ich var_index nun einen zu großen Wert zuweise (z.B. 13), erhält var_str1 den Wert 13. Der eval-Syntaxchecker meldet einen Fehler, in der eval Zuweisung zu Items werden diese Fehler bisher aber einfach weg gebügelt.

Die Frage ist, was sollte bei der neuen Methode (also bei var_str1n) passieren?

Für mich naheliegend wäre None zurück zu liefern. Das führt bei der Zuweisung zum Zielitem aber zu einem eher merkwürdigen Verhalten: Ein eval, welches None zurück liefert, bewirkt ja qua Definition, dass keine Zuweisung zum Zielitem erfolgen soll. Es behält also den letzten Wert.

Analog stellt sich die Frage natürlich auch für dicts, beim Zugriff mit einem nicht existierenden Key.

msinn commented 1 year ago

Eine Zuweisung zu einem nicht existierenden Listenelement führt ja zu einem Fehler und zu keiner Veränderung des Items. Bei einem dict, würde die Zuweisung zu einem nicht existierenden Key dazu führen, dass ein weitees Element in das dict eingefügt wird. Das wirft weitere Fragen auf:

Nächste Frage: Soll es möglich sein auf dicts auch über index zuzugreifen, also auf das n-te Element eines dicts?

wvhn commented 1 year ago

Ein lesender Zugriff sollte gar keine Zuweisungen ändern. Hier wäre ein „None“ als Rückmeldung sinnvoll. Ebenso sollte ein Schreibversuch auf ein nicht existentes Element mit Fehler quittiert werden. Neue Elemente sollten mit extra dafür vorgesehenen Kommandos erzeugt werden, z.B. prepend und append, um Elemente am Beginn oder am Ende der Liste einzufügen. Auch für die Dicts würde ich ein spezielles Kommando für sinnvoll erachten, z.B. „add“.

Löschen sollte auch möglich sein.

Der Vorteil von dicts ist es ja gerade, dass die Eigenschaften benannt sind und deshalb in beliebiger Reihenfolge angeordnet sein können. Das sieht man deutlich an den UZSU dicts. Deshalb sollte aus meiner Sicht auf einen Zugriff per Index verzichtet werden.

msinn commented 1 year ago

Ein lesender Zugriff sollte gar keine Zuweisungen ändern.

Worauf beziehst Du das? Davon war doch nie die Rede.

Ebenso sollte ein Schreibversuch auf ein nicht existentes Element mit Fehler quittiert werden.

Das tun evals aus Item Attributen bisher generell nicht.

Auch für die Dicts würde ich ein spezielles Kommando für sinnvoll erachten, z.B. „add“.

Du würdest das Python implizite Hinzufügen bei dicts also bewusst aktiv unterbunden haben wollen?

Deshalb sollte aus meiner Sicht auf einen Zugriff per Index verzichtet werden.

Auf die Idee auf dicts auch über den Index zuzugreifen kam ich als Analogie zu named Tuples. Bei denen geht auch beides (der Zigriff über index und key). Und da seit Python 3.6 sich die Reihenfolge von Einträgen in einem dict nicht mehr ändert, wäre es analog zu den named Tuples handhabbar.

wvhn commented 1 year ago

Wegen des lesenden Zugriffs: dann habe ich Dein Beispiel mit dem Index 13 falsch verstanden.

Generell sind dies ja Funktionen, die Du allen Anwendern zur Verfügung stellen willst. Deshalb würde ich eher auf Sicherheit setzen und Methoden unterbinden, die zu unerwarteten Ergebnissen führen können und dann im Forum betreut werden müssen.

Meine Erfahrungen mit dicts leiten sich aus den intensiven Tests der UZSU ab. Dort hat sich die Reihenfolge immer wieder geändert. Soweit ich noch weiß, war die Python Version v3.7. Daher der Vorschlag, auf den Indexzugriff als eine mögliche zusätzliche Fehlerquelle zu verzichten.

Dies ist aber nur eine von vielen möglichen Meinungen und ich bin da leidenschaftslos.

msinn commented 1 year ago

Soweit ich noch weiß, war die Python Version v3.7

Das muss Python 3.5 oder davor gewesen sein oder der Autor der UZSU hat durch Änderungen an der Programmierung die Reihenfolge verändert. Seit Python 3.6 bleibt die Reihenfolge der Einträge eines dicts konstant und neue key-value Paare werden hinten angehängt.

Dies ist aber nur eine von vielen möglichen Meinungen und ich bin da leidenschaftslos.

Genau diese Meinungen sammle ich und würde mich über weiteres Feedback (z.B. von @onkelandy , @aschwith , @Morg42 , @bmxp und anderen) freuen.

aschwith commented 1 year ago

Den use case, dict items auch über indices zu adressieren, sehe ich wie @wvhn auch gefährlich an. Ich hätte dafür bisher auch keinen Bedarf.

Dein Hinweis auf das bisherige Verhalten bei einer Bereichsüberschreitung ist gut. War mir nicht bewusst, dass es dort dieses Fehlverhalten gut. Die Lösung über None gefällt mir gut. Dass dabei das Zielitem unverändert bleibt, ist in der smarthomeNG Logik konsequent. Unabhängig von dieser Diskussion wünsche ich mir manchmal schon, ein Item explizit auf None setzten zu können, um es damit als invalid zu kennzeichnen.

bmxp commented 1 year ago

Also ganz konkret fallen mir zum Thema dict/lists komplexe knx Datenpunkte ein mit RGB Werten oder sowas. Da wäre es dann tatsächlich zu überlegen, wie man das abbildet um einen Mehrwert zu erzeugen. Allerdings habe ich mir dazu noch keinen Kopf gemacht.

msinn commented 1 year ago

Ich habe im ersten Beitrag des Issues einen (zu diskutierenden) Implementierungsvorschlag, der auch noch einige Fragen enthält, erfasst.

Morg42 commented 1 year ago

Da wir mit dem Lesen/Schreiben von items über die __call__-Methode schon vertraut sind und das eine quasi-analoge Subskription nicht erlaubt, finde ich den oben skizzierten Ansatz gut. Mir ist spontan die Idee gekommen, listitem.get(index) und dictitem.get(key[, default]) analog zu Python (als Item-Property?) zu implementieren. Index könnte dann zB auch "3:-2" sein, analog zu list[3:-2]. Fehler und Rückgabewerte sollten analog zu Python erzeugt werden.

Ob man auch append (eher ja), push/pop (indifferent) oder andere list-/dict-Methoden als Property anbieter, könnte man diskutieren.

Get fände ich super, aber es sollte auch nicht zu komplex werden.

Was ist mit foo-items, die list oder dict enthalten? Verhalten nach item-Typ oder type(item())? 😉

Achso - ein iterator und/oder dict.items() wären ggf hilfreich 👍

bmxp commented 1 year ago

Ich bin neugierig: Warum ist dieses Thema bei Dir gerade jetzt aufgetaucht? Hast Du da was konkretes im Blick wofür Du das brauchst?

Und wenn in einem dict kein key vorhanden ist, könnte man einen weiteren Parameter mitgeben ``createkey=True oder sowas?

msinn commented 1 year ago

@Morg42 foo bleibt foo. Ich würde diese Erweiterungen nur für bekannte Typen list/dict implementieren.

Im einem foo Item Item kann alles stehen. Herauszufinden ob das eine gültige Liste oder in gültiges dict ist ist recht aufwendig. Wenn der Anwender weiss, dass es eine Liste oder ein dict ist, kann er den Typ ja entsprechend setzen.

msinn commented 1 year ago

@bmxp Ich bin gerade dabei das shelly Plugin zu überarbeiten und implementiere dabei (auf Wunsch) ein rgbw Device. Außer der Inflation an Items (4 Stück) stört auch, dass die Attribute nicht zur gleichen Zeit gesetzt werden können, da es 4 unterschiedliche Items sind.

Das gleiche Problem gibt es auch beim hue2 Plugin. Dort hat jemand ein dict als Attribut eingebaut, aber das Handling ist bisher suboptimal.

Morg42 commented 1 year ago

Das Problem ist ja nicht, mit dicts umzugehen, sondern dass das Ändern von dicts "in" items nicht den Wert des Items ändern. Lesezugriffe sind ja wie immer möglich mit item()[key]oder item().get(key)...

Insofern sollte beim Schreiben die Analogie so weit wie möglich erhalten bleiben. Ich fände es sehr seltsam, wenn eine Zuweisung an ein dict fehlschlägt, weil der key noch nicht vorhanden ist.

Soll die automatische Erweiterung des dicts verhindert werden (um eine Analogie zu lists herzustellen)?

Nein, s.o.

Soll alternativ bei Listen eine Möglichkeit geschaffen werden, Werte zu appenden?

Ja. Könnte ein list-Item ein Property bzw. eine Methode "append" haben? Das wäre aus meiner Sicht das Naheliegendste listitem.append(val)

Soll es für lists und dicts die Möglichkeit geben, Einträge zu löschen?

Hätte ich keinen konkreten Usecase für, aber wenn es jemand braucht, wieso nicht?

listitem.remove(index), dictitem.del(key)

Nächste Frage: Soll es möglich sein auf dicts auch über index zuzugreifen, also auf das n-te Element eines dicts?

Da sehe ich - persönlich - noch weniger Usecase für. Wenn, dann muss der Unterschied zwischen key und index eindeutig sein: dictitem.get(key) wäre identisch mit dictitem(key=foo), Indexzugriff über dictitem(index=bar)?

msinn commented 1 year ago

Soll alternativ bei Listen eine Möglichkeit geschaffen werden, Werte zu appenden?

Ja. Könnte ein list-Item ein Property bzw. eine Methode "append" haben? Das wäre aus meiner Sicht das Naheliegendste listitem.append(val)

Ist im Implementierungs Vorschlag im 1. Beitrag dieses Issues bereits enthalten/beschrieben.

Ich habe mich jedoch für das erste gegen eigene Methoden (wie listitem.append(val)) entschieden, sondern route alles über myitem(...), also über __call__. Da müsste sowieso alles durch (zumindest durch das daraus aufgerufene __update__ damit damit das ganze Item-typische Handling (Pflege der Update- und Change Properties) erfolgt.

Morg42 commented 1 year ago

get, append usw hätte ich auch als "Umleitung" auf __call__ verstanden, des einfacheren bzw. verständlicheren Handling wegen. Die Implementation gem. 1. Beitrag setze ich dabei immer voraus ;)

bmxp commented 1 year ago

für Dictionaries:

dict Eintrag schreiben: myitem(myvalue, key='k1')

    • Wenn kein zu key passender Eintrag gefunden wird, wird ein neuer Eintrag im dict erzeugt

Ich wäre für die erste Variante um so nah wie möglich am Python dict Standard zu bleiben.

Die Frage wäre noch zu klären, wie man in der SmartVISU auf ein Unterelement eines Items mit dict Inhalt zugreifen kann?

@wvhn die Implementation für die Properties sind IMHO noch nicht erweitert daraufhin das bei geänderten Properties auch eine Rückmeldung für die SmartVisu erfolgt. Das wäre auch nicht generell möglich da sich ja die Zeitstempel sonst immer verändern würden und damit einen riesigen Traffic über den Websocket erzeugen würden.

Also ich habe jetzt selber einen Use-Case weil ich einen Strompfad mit Sicherung, Schaltschütz und Schaltaktorkanälen für eine Heizung abbilden will.

wvhn commented 1 year ago

@bmxp: Man kann heute einzelne properties eines Items „abonnieren“. SmartViSU fragt diese wie ein Item beim Backend an. Das Problem dabei ist, dass der Wert der property von shNG nur einmal bei der Anfrage gesendet, aber nicht weiter aktualisiert wird. Hier hatte ich mit @msinn schon Lösungsmöglichkeiten diskutiert. Eine Idee war, dass shNG bei item-updates prüft, welche property des items abonniert ist und für diese dann eine Aktualisierung sendet. Das sollte den Traffic nur minimal erhöhen.

SmartVISU verarbeitet bisher nur komplette lists und dicts - siehe basic.color, device.uzsuicon und einzelne andere. Sogar mit basic.stateswitch kann man lists als Ganzes verarbeiten. Will man in einer list aber nur ein Element ansprechen, dann geht das nicht. Ich hatte oben den Vorschlag gemacht, dass man einzelne Elemente so wie item properties abonniert. Also z.B. indiziert „meinItem(3)“ oder assoziativ „meinItem.element.kanal1“ und shNG dies dann entsprechend zur Verfügung stellt. Alternativ kann das sicherlich auch im Treiber gemacht werden. Dann abonniert die Visu immer die komplette list bzw. das komplette dict und der Treiber zerlegt dies. Wir sollten das dann vor einer Umsetzung in shNG gemeinsam testen.

Morg42 commented 10 months ago

Habe in #620 mal noch ein paar Methoden ergänzt. Die laufen alle über __call__, so dass die normale Item-Mechanik komplett durchlaufen wird.

Zum bequemeren (und "sichereren") Handling werden die nur "sichtbar" eingebunden, wenn das Item vom passenden Typ ist.

Morg42 commented 9 months ago

@wvhn: schau dir mal an, was ich aktuell noch implementiert habe. Auf der Basis könnte man überlegen, ob man einen assoziativen Element-Handler einbaut. Ich bin nicht sicher, ob es günstiger ist, das im core oder im Treiber zu implementieren.

wvhn commented 9 months ago

@Morg42
ich habe mal den develop branch ausgecheckt, um den Zugang zu diesen Funktionen durch die Visu zu testen. Sie sind aber bisher nicht erreichbar.

Man muss sich sowieso Gedanken machen, welche Funktionen der Anwender über die Visu sinnvoll nutzen kann. Im Sinne der Kompatibilität mit anderen Backends ist es vielleicht sogar besser, die lists und dicts bei Bedarf in eigenen Widgets innerhalb der Visu zu verändern und immer das vollständige list- oder dict-item zu übertragen.

Die Visu ist darauf ausgelegt, dass items per "monitor"-Kommando abonniert und vom Backend selbständig aktualisiert werden, sobald Änderungen erfolgt sind. Leider klappt das schon bei den item properties nicht. Wenn man das im Treiber realisieren wollte, müsste auch das smartvisu-Protokoll in shNG erweitert werden. IMHO würde dies zu Lasten einer klaren Schnittstellendefinition (monitor/listen oder pull) gehen.

Morg42 commented 9 months ago

Dann schließe ich hier erstmal - die Erweiterungen sind ja schon implementiert.

Wenn es an der Implementation noch Änderungsbedarf gibt, können wir ja wieder aufmachen.