-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
[zigbee] Add a color temperature absolute channel #868
Conversation
Signed-off-by: AndrewFG <[email protected]>
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.
Just a few initial comments...
I know this is still WIP but I was following the other thread and thought I'd take a look.
...n/java/org/openhab/binding/zigbee/internal/converter/ZigBeeConverterColorTemperatureAbs.java
Outdated
Show resolved
Hide resolved
...n/java/org/openhab/binding/zigbee/internal/converter/ZigBeeConverterColorTemperatureAbs.java
Outdated
Show resolved
Hide resolved
...n/java/org/openhab/binding/zigbee/internal/converter/ZigBeeConverterColorTemperatureAbs.java
Outdated
Show resolved
Hide resolved
...nhab.binding.zigbee/src/main/java/org/openhab/binding/zigbee/handler/ZigBeeThingHandler.java
Outdated
Show resolved
Hide resolved
Signed-off-by: AndrewFG <[email protected]>
Signed-off-by: AndrewFG <[email protected]>
@cdjackson I think the code is quite clean now. I will set up my test rig and run real tests tomorrow. |
Signed-off-by: AndrewFG <[email protected]>
@cdjackson I did some refactoring. I tested this version and can confirm that it all seems to be working fine. Note however, that I wanted to add the new channel to the documentation but I discovered that in the ChannelUID table many of the other IDs are in fact wrong -- so it is not only an issue of adding one line to that table. I can easily make the corrections if you want, but perhaps you have some thoughts on this? => WDYT? EDIT: I committed a minor fix to the channel type ids, and added some documentation changes and extensions |
Signed-off-by: AndrewFG <[email protected]>
Thanks - I’ll try and take a look in the next day or so. Please feel free to update the doc - that table has probably (well - obviously it seems!) been badly maintained and overlooked.
The test is unstable - I’ve noticed it myself. I’ve just made a change to it so we’ll see if it is better.
Thanks
… On 31/10/2024, at 1:27 AM, Andrew Fiddian-Green ***@***.***> wrote:
@cdjackson <https://github.com/cdjackson> I did some refactoring. I tested this version and can confirm that it all seems to be working fine. Note however, that I wanted to add the new channel to the documentation <https://www.openhab.org/addons/bindings/zigbee/#channel-types> but I discovered that in the ChannelUID table many of the other IDs are in fact wrong -- so it is not only an issue of adding one line to that table. I can easily make the corrections if you want, but perhaps you have some thoughts on this? => WDYT?
PS I notice also that the CI build fails a test in org.openhab.binding.zigbee.slzb06.internal.Slzb06NetworkPortTest -- "(expected: <0> but was: <-1>)" -- which I think has nothing to do with me. Particularly since my local Maven build succeeds without error.
—
Reply to this email directly, view it on GitHub <#868 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAH6IQ35ACA4CACCCCSFXE3Z6DGCNAVCNFSM6AAAAABQX2SHMKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBWHE3DONZXGE>.
You are receiving this because you were mentioned.
|
Thanks @andrewfg LGTM. |
This is a draft PR as proof of concept for potentially adding a color temperature absolute channel in addition to the already existing color temperature percent channel. Initiall for discussion purposes only.
Resolves #867
See openhab/openhab-core#3891
Signed-off-by: AndrewFG [email protected]