Update the HUE binding to work with non continuous device numbers on the... #2514
Conversation
…the bridge (speed up the initial loading) Due the new option to delete bulb on the Hue bridge the old assumption of continuous device numbers is no longer true. The for loop in HueBinding.java crashed with a NPE if it hits a deleted bulb. The second fix in the update is a speed up of updates/initial reads by removing multiple call to the settings on the bridge. The code now fetches the settings once per refresh from the bridge and hands the settings over the the bulb construction.
|
openhab » openhab #2857 SUCCESS |
| @@ -223,6 +223,10 @@ protected int count() { | |||
| return dataMap.size(); | |||
| } | |||
|
|
|||
| protected Set<String> getKeys(){ | |||
| return dataMap.keySet(); | |||
| } | |||
There was a problem hiding this comment.
are there any other Key types then Integer? If not i would suggest to transform the Strings into Integer once (probably here?)
There was a problem hiding this comment.
Actually the Philips API is not clear about a type for the ID (but in the Java SDK they do use String as ID type for the lights). Since the usage of the ID in the rest API is always in a string context (and the integer ID is alway converted into string), my next suggestion for a refactoring would have been a complete usage of String for the ID type.
In the ESH the lights ID is also a String.
|
Hi @mstolt, thanks for this contribution! Please find my review comment inline. Also it would be great if you could have a look at the ESH version of the Hue Binding. It was rewritten from scratch but probably that piece of code has been copied? Could be worth having a look there. Best, Thomas E.-E. |
|
Hi Thomas, I only looked at the ESH code. It seems to be fine. The light IDs are Strings and the init loop/update loop in the method "run" of HueBridgeHandler (line 91) works with an iterator over the lights key set. |
[Hue] Update the HUE binding to work with non continuous device numbers
|
thanks, @mstolt! |
|
I have to thank you (and the other contributors) for this project. Btw. do you close the associated issues #2504 and And do you think the suggested refactoring (bulb id from int to String) as in ESH is ok? If yes, I will prepare another pull request with it. CU |
|
since you are the creator of both issues you can close them yourself. Thanks, Thomas E.-E. |
... bridge (and speed up the initial loading)
Due the new option to delete bulb on the Hue bridge the old assumption
of continuous device numbers is no longer true. The for loop in
HueBinding.java crashed with a NPE if it hits a deleted bulb.
The second fix in the update is a speed up of updates/initial reads by
removing multiple call to the settings on the bridge. The code now
fetches the settings once per refresh from the bridge and hands the
settings over the the bulb construction.
This fixes the
issue #2504 and
issue #2505