-
-
Notifications
You must be signed in to change notification settings - Fork 428
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
adapted AutoUpdateDelegate to the new ESH auto update infrastructure #390
Conversation
Related to eclipse-archived/smarthome#5011 Signed-off-by: Kai Kreuzer <[email protected]>
Note to reviewers: This code will only compile / can only be merged once eclipse-archived/smarthome#5011 is merged and a new ESH stable has been built. |
FTR, eclipse-archived/smarthome#5011 is merged. I'd do a new ESH Stable once one of @openhab/2-x-add-ons-maintainers approves (but not yet merges!) this PR. |
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.
Some small comments nothing big
synchronized (itemUpdateVetos) { | ||
itemUpdateVetos.clear(); | ||
for (Item item : itemRegistry.getAll()) { | ||
for (org.openhab.core.autoupdate.AutoUpdateBindingProvider provider : providers) { |
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.
Can this be imported?
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.
Yes, indeed, now that it is removed from ESH :-)
itemUpdateVetos.clear(); | ||
for (Item item : itemRegistry.getAll()) { | ||
for (org.openhab.core.autoupdate.AutoUpdateBindingProvider provider : providers) { | ||
Boolean autoUpdate = provider.autoUpdate(item.getName()); |
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.
Does it really return a Boolean
with potential null's?
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.
Yes. A provider returns null if it does not want to do any statement.
synchronized (itemUpdateVetos) { | ||
boolean removed = itemUpdateVetos.remove(itemName); | ||
for (org.openhab.core.autoupdate.AutoUpdateBindingProvider provider : providers) { | ||
Object autoUpdate = provider.autoUpdate(itemName); |
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.
In other places we at least use a Boolean
...
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.
true
|
||
@Reference(cardinality = ReferenceCardinality.MULTIPLE) | ||
public void addAutoUpdateBindingProvider(org.openhab.core.autoupdate.AutoUpdateBindingProvider provider) { |
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.
It seems we can import these now, given the one from ESH is not imported anymore.
Signed-off-by: Kai Kreuzer <[email protected]>
Thanks - I have updated the PR. |
Retriggering PR builds... |
Signed-off-by: Kai Kreuzer <[email protected]>
Nice, DS annotations actually caught a bug in the unset method. Fixed it with 8f3dea0. |
@martinvw All green now, so I think we can merge! |
…penhab#390) * adapted AutoUpdateDelegate to the new ESH auto update infrastructure Related to eclipse-archived/smarthome#5011 Signed-off-by: Kai Kreuzer <[email protected]>
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/openhab2-mysql-persistence-setup/15829/67 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/vera-mios-bindings-for-beginner/23884/13 |
…penhab#390) * adapted AutoUpdateDelegate to the new ESH auto update infrastructure Related to eclipse-archived/smarthome#5011 Signed-off-by: Kai Kreuzer <[email protected]> GitOrigin-RevId: 47c2e33
Related to eclipse-archived/smarthome#5011
While at it, I also changed the DS components to use annotations instead of XMLs.
Signed-off-by: Kai Kreuzer [email protected]