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

[avmfritz] Add light blub color temperature support #14373

Merged
merged 12 commits into from
May 4, 2023

Conversation

vich-667
Copy link
Contributor

@vich-667 vich-667 commented Feb 9, 2023

Added an new channel to HAN_FUN_COLOR_BULB to conrol color temperature. This can be used to set the white color for the FRITZ!DECT 500 or to set it back to white mode if it is in color mode.

  • adds a color tempeature channel
  • fixes some on off behavior, what was somehow async between OH and the real world Thing

Should fix: #14287

Thanks to @sommer for posting the base of this code to #12427

@vich-667 vich-667 marked this pull request as ready for review February 9, 2023 21:03
@jlaur jlaur added the enhancement An enhancement or new feature for an existing add-on label Feb 12, 2023
@lolodomo
Copy link
Contributor

lolodomo commented Mar 11, 2023

@ccutrer you are our specialist for color temperature ;) Didn't you implement something that should avoid the hardcoded conversion of values proposed in that PR ?

@ccutrer
Copy link
Contributor

ccutrer commented Mar 11, 2023

I would highly recommend that you add both colorTemperature (as a dimmer), and colorTemperatureAbs (as a Number:Temperature, and system:color-temperature-abs type). You will need to use QuantityType (in Kelvin) when posting updates of your channel's state, and toInvertibleUnit (converting to Kelvin) when accepting commands to make sure automatic unit conversions work (some people will prefer to work in mired/mirek, and some will prefer to work in Kelvin).

There's nothing to help you scale between percent and temperature in Kelvin as in this PR. It looks like your device supports setting a color temperature directly in Kelvin. Does your device not support arbitrary color temperatures, only the few given in your long if/else if structure? If it supports arbitrary, you should definitely just use math and scale the percent value. You may want to do the scaling in mireds instead of Kelvin, since mireds have the nice property that any given interval of say 50 mireds will be the same perceptible change in color, whereas in Kelvin the higher you go in the scale, the larger the interval to achieve the same perceptible change (i.e. 2700K to 3000K is about the same perceptible difference as 5000K to 6000K).

@vich-667
Copy link
Contributor Author

I would highly recommend that you add both colorTemperature (as a dimmer), and colorTemperatureAbs (as a Number:Temperature, and system:color-temperature-abs type). You will need to use QuantityType (in Kelvin) when posting updates of your channel's state, and toInvertibleUnit (converting to Kelvin) when accepting commands to make sure automatic unit conversions work (some people will prefer to work in mired/mirek, and some will prefer to work in Kelvin).

There's nothing to help you scale between percent and temperature in Kelvin as in this PR. It looks like your device supports setting a color temperature directly in Kelvin. Does your device not support arbitrary color temperatures, only the few given in your long if/else if structure? If it supports arbitrary, you should definitely just use math and scale the percent value. You may want to do the scaling in mireds instead of Kelvin, since mireds have the nice property that any given interval of say 50 mireds will be the same perceptible change in color, whereas in Kelvin the higher you go in the scale, the larger the interval to achieve the same perceptible change (i.e. 2700K to 3000K is about the same perceptible difference as 5000K to 6000K).

I will have a look here. I need to test this out if other color temps are working and math can be used. This might take me some days, because currently I'm fighting with some issue that I can not debug.

@vich-667
Copy link
Contributor Author

vich-667 commented Mar 14, 2023

I would highly recommend that you add both colorTemperature (as a dimmer), and colorTemperatureAbs (as a Number:Temperature, and system:color-temperature-abs type). You will need to use QuantityType (in Kelvin) when posting updates of your channel's state, and toInvertibleUnit (converting to Kelvin) when accepting commands to make sure automatic unit conversions work (some people will prefer to work in mired/mirek, and some will prefer to work in Kelvin).

I had some look to the hue binding and tried to add that additional channel. At to me it worked fine.

There's nothing to help you scale between percent and temperature in Kelvin as in this PR. It looks like your device supports setting a color temperature directly in Kelvin. Does your device not support arbitrary color temperatures, only the few given in your long if/else if structure? If it supports arbitrary, you should definitely just use math and scale the percent value. You may want to do the scaling in mireds instead of Kelvin, since mireds have the nice property that any given interval of say 50 mireds will be the same perceptible change in color, whereas in Kelvin the higher you go in the scale, the larger the interval to achieve the same perceptible change (i.e. 2700K to 3000K is about the same perceptible difference as 5000K to 6000K).

The light blub type I have only supports dedicated numbers. Bacause I can configure any value and it jumps to a supported one, I changed to math.
scaling
Because the original steps are in between the scaling in Kelvin and Mired, I used the Mired as you reccomended.

@vich-667 vich-667 requested a review from lolodomo March 14, 2023 00:08
@ccutrer
Copy link
Contributor

ccutrer commented Mar 14, 2023

Color temp stuff all looks good to me, except perhaps using some consts for min/max mireds

@vich-667
Copy link
Contributor Author

Color temp stuff all looks good to me, except perhaps using some consts for min/max mireds

As I put to the inline comment the AHA-HTTP-Inteface docu only gave me that 2700K and 6500K, therefore I didn't introduced any additional magic numbers byside of that ones.

Of course It migt not make sense to calculate the min/max mireds dynamically. If you prefer I also can replace them by const values.

@wborn wborn changed the title [avmfritz] added light blub color temperature support [avmfritz] Add light blub color temperature support Mar 17, 2023
@jlaur
Copy link
Contributor

jlaur commented Mar 26, 2023

For the new channels added, please add update instructions. For reference, see #14572 and #14587. I believe you can keep target version 1.

@vich-667
Copy link
Contributor Author

@jlaur: I added. can you please check if it's OK?

@jlaur
Copy link
Contributor

jlaur commented Mar 28, 2023

I added. can you please check if it's OK?

See https://github.com/openhab/openhab-docs/blob/7bd66d601e0d1421dd18b22e0132ee406127158b/developers/bindings/thing-xml.md#updating-thing-types and https://www.openhab.org/schemas/update-description-1.0.0.xsd. You will need to use add-channel for a new channel. Also, you need to set the thingTypeVersion property.

@vich-667
Copy link
Contributor Author

@jlaur: Thanks for your hint, I think now I got it.

Tobias Lange added 10 commits March 29, 2023 20:50
Signed-off-by: Tobias Lange <vich-667@gmx.de>
Signed-off-by: Tobias Lange <vich-667@gmx.de>
Signed-off-by: Tobias Lange <vich-667@gmx.de>
Signed-off-by: Tobias Lange <vich-667@gmx.de>
Signed-off-by: Tobias Lange <vich-667@gmx.de>
Signed-off-by: Tobias Lange <vich-667@gmx.de>
Signed-off-by: Tobias Lange <vich-667@gmx.de>
Signed-off-by: Tobias Lange <vich-667@gmx.de>
Signed-off-by: Tobias Lange <vich-667@gmx.de>
Signed-off-by: Tobias Lange <vich-667@gmx.de>
Tobias Lange added 2 commits March 29, 2023 21:00
Signed-off-by: Tobias Lange <vich-667@gmx.de>
fix
Signed-off-by: Tobias Lange <vich-667@gmx.de>
@vich-667
Copy link
Contributor Author

vich-667 commented Apr 9, 2023

@lolodomo or @cweitkamp: can you please give some feedback here. I think all given comments are fixed.

Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

Just one remark about naming conventions (channel id).

Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM, thank upu

@lolodomo lolodomo merged commit 891da2c into openhab:main May 4, 2023
@lolodomo lolodomo added this to the 4.0 milestone May 4, 2023
@vich-667 vich-667 deleted the task/colortemp branch May 5, 2023 18:09
tb4jc pushed a commit to tb4jc/openhab-addons that referenced this pull request Jun 19, 2023
* added light blub color temperature support
* align valiable nameing
* fixed on off behavior of lights
* change to math scaleing
* add abs color temperature channel

---------

Signed-off-by: Tobias Lange <vich-667@gmx.de>
Signed-off-by: Thomas Burri <thomas.burri@alstomgroup.com>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Aug 9, 2023
* added light blub color temperature support
* align valiable nameing
* fixed on off behavior of lights
* change to math scaleing
* add abs color temperature channel

---------

Signed-off-by: Tobias Lange <vich-667@gmx.de>
Signed-off-by: Matt Myers <mmyers75@icloud.com>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
* added light blub color temperature support
* align valiable nameing
* fixed on off behavior of lights
* change to math scaleing
* add abs color temperature channel

---------

Signed-off-by: Tobias Lange <vich-667@gmx.de>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[avmfritz] Fritz DECT 500 lightbulb - not possible to change from color to white mode
4 participants