search

AnthonMS / my-cards

Bundle of my custom Lovelace cards for Home Assistant. Includes: my-slider, my-slider-v2, my-button
Other
104 stars 27 forks source link

[Bug]: colorMode: "temperature" doesn't work when the light is colored with rgb #33

Closed usernein closed 1 year ago

usernein commented 2 years ago

Current Behavior

The slider-v2 works really great with colorMode==temperature... until i set any color (which sets the bulb into color_mode==rgb), then the temperature slide stops working.

It will only work again after i manually call the service to change it back to color_temp mode.

https://user-images.githubusercontent.com/29507335/184257821-77286fdf-36d8-45ae-9243-f1573b3b2706.mp4

Expected Behavior

The temperature slider should set the wanted temperature even if the current color_mode is rgb or anything else

Steps To Reproduce

No response

Environment

- OS:
- Node:
- Version:
- Hassio:

Anything else?

I hope this info helps: (as you can see in the video i'm uploading) the attribute color_temp is None/undefined/null when color_mode==rgb. I saw that you fetch at some point of your code that attribute, which is null, and do some math with it. That may be causing the error (or not) Btw, i don't get anything on JS console

AnthonMS commented 2 years ago

Yeah I see, it should of course just set the color mode to be color_temp when setting the color_temp with slider and the light is in another mode. Nothing is in the console because it is actually something I coded into it. I saw it fast when I last made some quick changes and thought to myself that it would come back and bite me in my ass. Because of course it's the wrong behavior.

So this will absolutely be fixed in the next update.

AnthonMS commented 2 years ago

I have made some debug messages on the fly, because I know this is a bug that I would like to address, sooner rather than later.

I can not test it myself at the moment, since I do not have any color_temp sliders set up in my own setup at the moment. But if you can grab a screenshot of the debug messages. It should contain the values of the slider, the current color_temp value before updating, the value for step threshold and whether or not the step is above it. And it will print a debug saying it did not set the color_temp because it did not meet the criteria. Apparently I did not know the criteria.

Edit: You know what, I just looked at the old slider, and I have decided that for now to just take it out of the if/else. So it should work now. I am guessing it has something to do with the attribute color_temp being undefined or null when using it the calculation, but I am not certain.

It should work now, but the minimum step functionality will not work with temperature sliders for the moment.

SmartLiving-Rocks commented 2 years ago

I can not test it myself at the moment, since I do not have any color_temp sliders set up in my own setup at the moment. But if you can grab a screenshot of the debug messages. It should contain the values of the slider, the current color_temp value before updating, the value for step threshold and whether or not the step is above it. And it will print a debug saying it did not set the color_temp because it did not meet the criteria. Apparently I did not know the criteria.

I am using the demo:integration for developing. https://www.home-assistant.io/integrations/demo/

If you just need the light (also including temperature)

# To load for a specific integration:
light:
  - platform: demo

Hope it helps - just a quick tip! Happy coding, and thanks for your beautiful Lovelace card. It is a premium view and makes the dashboard look more exclusive!

smartqasa commented 1 year ago

This is a problem I am experiencing as well.

basbruss commented 1 year ago

@AnthonMS With debugging bugs in Minimalist UI I encountered this bug. Just to verify to you; the attribute color_temp does not exist when a light is not in temperature mode. As I am not a TypeScript dev I can only guess how to fix it. To make debugging easier for you I will provide the attribute values from a light both in color and temperature mode.

Color mode:

min_color_temp_kelvin: 2000 
max_color_temp_kelvin: 6535 
min_mireds: 153 
max_mireds: 500 
effect_list: None, candle, fire 
supported_color_modes: color_temp, xy 
color_mode: xy 
brightness: 107 
hs_color: 43.953, 33.725 
rgb_color: 255, 232, 169 
xy_color: 0.3968, 0.3931 
effect: None 
mode: normal 
dynamics: none

Temperature mode:

min_color_temp_kelvin: 2000 
max_color_temp_kelvin: 6535 
min_mireds: 153
max_mireds: 500 
effect_list: None, candle, fire 
supported_color_modes: color_temp, xy 
mode: normal 
dynamics: none 
supported_features: 44 
color_mode: color_temp 
brightness: 255 
hs_color: 54.768, 1.6 
rgb_color: 255, 254, 250 
xy_color: 0.326, 0.333 
effect: None 
color_temp_kelvin: 6535 
color_temp: 153
jonrupell commented 1 year ago

I am experiencing the exact same problem.

adabelleleiram commented 1 year ago

I'm also experiencing this issue. Any updates? @AnthonMS

SmartLiving-Rocks commented 1 year ago

I'm also experiencing this issue. Any updates? @AnthonMS

no 😢

adabelleleiram commented 1 year ago

I'm not a typescript dev but is there any chance that this incorrect indentation could be the issue? https://github.com/AnthonMS/my-cards/blob/599b616cbc18dd822e400c9558dc16d02d68a448/src/cards/my-slider.ts#L573

Are you sure this function is being called at all?

adabelleleiram commented 1 year ago

Ignore my previous comment. Just stepped through the code with the debugger. It's what @basbruss wrote, color_temp is NaN, so Math.abs((value - oldVal)) > this.step will be false.

As I mentioned, not a typescript dev so can't verify this, but maybe the fix could be something like: if (entity.state === 'off' || isNaN(oldVal) || Math.abs((value - oldVal)) > this.step) {

I see in the repo that you moved the call to HA out of the if statement, but the compiled js doesn't seem to reflect that change...?

@AnthonMS hope this info can help!

AnthonMS commented 1 year ago

Thank you guys for all the debugging. I technically made a temporary fix for this a long time ago, I just never ran the release flow on Github. Thought there were automatic flows for dev and prod branches and all that. But I never got around to setting those up.

The latest release has implemented your fix @adabelleleiram. And it seems to work locally for me fine now. If I set a light to be RGB color I can still set the temperature of the light with the slider.

I am marking this as closed. I'm sorry I have been inactive for so long. I saw all the effort you guys were putting in to figuring it out, that I had to get it done now.

Thank you for using the card even though the developer lack motivation to update it regularly. :heart:

adabelleleiram commented 1 year ago

Thank you guys for all the debugging. I technically made a temporary fix for this a long time ago, I just never ran the release flow on Github. Thought there were automatic flows for dev and prod branches and all that. But I never got around to setting those up.

The latest release has implemented your fix @adabelleleiram. And it seems to work locally for me fine now. If I set a light to be RGB color I can still set the temperature of the light with the slider.

I am marking this as closed. I'm sorry I have been inactive for so long. I saw all the effort you guys were putting in to figuring it out, that I had to get it done now.

Thank you for using the card even though the developer lack motivation to update it regularly. ❤️

Awesome! Thanks a lot! Just glad to be able to help! Thanks for making an awesome card and no worries about the delay, we can't expect more IMO considering you make this for free. Thanks again <3