Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hue plugin - Async fix #394

Merged
merged 14 commits into from
May 3, 2019
Merged

Hue plugin - Async fix #394

merged 14 commits into from
May 3, 2019

Conversation

f-kessler
Copy link
Contributor

@f-kessler f-kessler commented Mar 3, 2019

Plugin für Philips Hue hinzugefügt und Fehler in der asynchronen Verarbeitung der Alarme behoben.

Betrifft folgende Issues
#393
#375

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
includes/alarmHandler.py Outdated Show resolved Hide resolved
@Schrolli91 Schrolli91 mentioned this pull request Mar 3, 2019
@Schrolli91 Schrolli91 changed the title Hue plugin Hue plugin - Async fix Mar 3, 2019
@@ -36,20 +37,20 @@ def processAlarmHandler(typ, freq, data):
@return: nothing
@exception: Exception if starting a Thread failed
"""
#copy objects to avoid issues if the objects will be changed by plugin's or during asynch/threaded processing
dctyp = deepcopy(typ)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich glaube wenn wir an dieser Stelle kopieren, besteht das Problem weiterhin.
Denn alle Plugins greifen ja auf EXAKT die selbe Kopie zu - oder täusche ich mich?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Muss ich mir morgen mal ansehen.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zumindest an dieser Stelle bringt es nichts wenn wir verhindern wollen, dass die Plugins sich gegenseitig die Daten verändern könnten.

Passenderweise müsste es dann wohl in die Schleife, die die Plugins aufruft, sodass jeder Plugin Aufruf eine eigene Kopie der Daten erhält.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guter Punkt. Habe das mal umgebaut. Ich muss das aber noch auf meinem System testen.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@f-kessler Läuft stabil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Irgendwas läuft da noch nicht richtig. Die Kopien werden offenbar auch überschrieben. Zumindest landen in der Datenbank viele Alarme mit der selben RIC. Ich habe zwei Vermutungen. Entweder es liegt daran, dass in Zeile 44 bereits der Thread erzeugt wird, aber noch mit original Objekten an dieser Stelle gearbeitet wird. Oder es liegt daran, dass die Kopien immer den selben Namen haben und das Kopieren die Kopie überschreibt.
Hast du vielleicht eine Idee dazu?

Copy link
Owner

@Schrolli91 Schrolli91 Mar 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So wie sich mir das darstellt, besteht dieses Problem nur beim data-Dict.

Schlimmer ist es jedoch, wenn Seiteneffekte auftreten, mit denen man nicht gerechnet hat, und dadurch Fehler verursacht werden. Dies kann auch in Python passieren. Dies kann dann geschehen, wenn Listen oder Dictionaries als Parameter übergeben werden.

Die anderen DeepCopys sind also vermutlich gar nicht notwendig.
Evtl reicht es schon das deepCopy direkt in den Funktionsaufruf zu setzen:
plugin.run(typ, freq, deepcopy(data)) ?
Somit würde dann wahrscheinlich die kopierte Variable nicht überschreiben werden, da es keine gibt.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Das ist eine gute Idee. Werde das mal am Wochenende bei mir umschreiben. Hatte inzwischen auch gute Ergebnisse mit einem deepcopy vor dem Aufruf des Threads und einem weiteren deepcopy in der for-Schleife erzielt. Dein Vorschlag ist aber deutlich eleganter als stumpf neue Objekte anzulegen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bin leider erst neulich dazu gekommen deinen Vorschlag zu testen. Bin mit dem Ergebnis sehr zufrieden.

plugins/hue/hue.py Outdated Show resolved Hide resolved
@ghost ghost assigned Schrolli91 Mar 15, 2019
@Schrolli91
Copy link
Owner

@f-kessler dann warte ich auf dein Abschließendes Testergebnis

@Schrolli91
Copy link
Owner

@f-kessler kann man also jetzt als erledigt ansehen?

@f-kessler
Copy link
Contributor Author

@f-kessler kann man also jetzt als erledigt ansehen?

Ja, kann man.

@Schrolli91 Schrolli91 merged commit 8612533 into Schrolli91:develop May 3, 2019
@ghost ghost removed the Review needed label May 3, 2019
@maeweee
Copy link

maeweee commented Aug 8, 2019

hallo nach langer zeit frage ioch nochmals :S

ist es möglich auch andere farben einzustellen ?? Danke !!

@Schrolli91
Copy link
Owner

@f-kessler kannst du das was zu sagen?

@f-kessler
Copy link
Contributor Author

hallo nach langer zeit frage ioch nochmals :S

ist es möglich auch andere farben einzustellen ?? Danke !!

Das sollte prinzipiell kein Problem sein. In der API gibt es dazu passende Optionen. Ich habe aber leider kurzfristig nicht die Zeit den Code anzupassen. Werde mir das aber mal ansehen, sobald ich wieder mehr Zeit habe.

@maeweee
Copy link

maeweee commented Aug 13, 2019

vielen lieben dank!!
leider bin ich dazu zu blöd. :D

@maeweee
Copy link

maeweee commented Oct 2, 2019

darf ich kurz nachhaken, ob es etwas neues gibt bzw. du zeit hattest?? lgg

@f-kessler
Copy link
Contributor Author

Ich muss dich leider enttäuschen. Bin leider bisher nicht dazu gekommen.

@maeweee
Copy link

maeweee commented Oct 2, 2019

danke für deine rückmeldung!! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants