-
-
Notifications
You must be signed in to change notification settings - Fork 59
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
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
2b89d49
Merge pull request #3 from Schrolli91/develop
f-kessler 1eae309
Add hue plugin
f-kessler 04f98f2
fix for #375
f-kessler 9432141
settings for hue added
f-kessler 5fc8f92
Update CHANGELOG.md
f-kessler 6332bd3
description for multicastAlarm RIC updated
f-kessler a360b1c
Update CHANGELOG.md
f-kessler f86aeb9
Update alarmHandler.py
f-kessler ea68fab
logging fixed
f-kessler b3e63af
deepcopy of alarm data moved to processAlarm
f-kessler 07a69d5
comment updated
f-kessler b117066
moved copy into for loop
f-kessler 9c0ed13
Merge branch 'develop' into hue-plugin
af887c2
enhancement for deepcopy
f-kessler File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.