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

Fix white only scene on RGBW lights #129

Merged
merged 2 commits into from
Nov 6, 2020

Conversation

ultratoto14
Copy link
Collaborator

When a scene is created using the HA scene editor, it stores the complete status of the light.
When the scene is applied, HA scene this complete status.

In my previous PR, I forced the color to hs [0, 0] to be sure that the color picker return to white when we use color_temp
With that, the scene editor will now store this white value. If we detect this value, we force ignore the color and only set the white related settings.

@SmartM-ui
Copy link

When a scene is created using the HA scene editor, it stores the complete status of the light.
When the scene is applied, HA scene this complete status.

In my previous PR, I forced the color to hs [0, 0] to be sure that the color picker return to white when we use color_temp
With that, the scene editor will now store this white value. If we detect this value, we force ignore the color and only set the white related settings.

Hi @ultratoto14 ,
Can you explain to me what you mean by creating a scene using HA?
If you explain the test to be performed, I'll try :-)

@ultratoto14
Copy link
Collaborator Author

Hi @SmartM-ui, i mean scene editor
The process should be:

  • choose a tuya RGBW light.
  • set it to white and choose a color_temp somewhere in the middle
  • save the scene
  • change the color_temp to anything else than in the scene
  • change to color
  • apply the scene
  • check that color_temp is ok, and bulb is respecting what has been saved.

Without this patch, we can have different behaviour.

Thanks for testing.

@SmartM-ui
Copy link

SmartM-ui commented Nov 1, 2020

Hi @SmartM-ui, i mean scene editor
The process should be:

* choose a tuya RGBW light.

* set it to white and choose a color_temp somewhere in the middle

* save the scene

* change the color_temp to anything else than in the scene

* change to color

* apply the scene

* check that color_temp is ok, and bulb is respecting what has been saved.

Without this patch, we can have different behaviour.

Thanks for testing.

Hi @ultratoto14 ,
here I am, I just installed your latest work "localtuya-light_scene_fix"!
I hope I understand the test to be carried out, because I had never used the Home Assistant "scenes" :-)

  1. I set a scene with white color and color temperature in the center of the slider and saved the scene.
  2. I turned on the bulb and set the color temperature "cold" (end of the slider) and then I set the color red
  3. I went back to the "scenes" and applied the previously saved scene

The light bulb returned to white, but the slider remained at the "cold" color temperature and did not return to the center of the slider as it was previously saved in the scene.

This is scenes.yaml:
entities:
light.lampadario_studio_localtuya:
min_mireds: 154
max_mireds: 370
brightness: 51
color_temp: 259
hs_color:
- 0
- 0
rgb_color:
- 255
- 255
- 255
xy_color:
- 0.323
- 0.329
friendly_name: Lampadario Studio LocalTuya
supported_features: 19
state: 'on'

I tried the same situation with the localtuya version prior to your work and the scenes do not bring the light bulb back to white, but change to the light "violet" color.

PS thanks to you for developing this component!

@ultratoto14
Copy link
Collaborator Author

Hi @SmartM-ui, thanks again for testing, seems that the problem comes from the changed i made to put back the color picker to white (if you remember).
Do you have non-tuya RGB+W light ? Just to test if the color_picker keeping the previously chosen color is not the default behavior ?

I mean, when we change to color, the color_temp slider stay where it is, only the bulb icon color is changed. It seems not so bad, what do you think ?
If i remove the color picker fix, the scene is stored without color arguments that prevent HA to send color_temp data to bulb.

@SmartM-ui
Copy link

Hi @SmartM-ui, thanks again for testing, seems that the problem comes from the changed i made to put back the color picker to white (if you remember).
Do you have non-tuya RGB+W light ? Just to test if the color_picker keeping the previously chosen color is not the default behavior ?

I mean, when we change to color, the color_temp slider stay where it is, only the bulb icon color is changed. It seems not so bad, what do you think ?
If i remove the color picker fix, the scene is stored without color arguments that prevent HA to send color_temp data to bulb.

Hi @ultratoto14 ,
of course I remember your change, I asked for it :-)

Unfortunately I only have TUYA bulbs, I can't try scenes with other bulbs.

However, this new version is better than the previous one.
I left this!

This change of yours made me think of a request to make ... I open a new issue.

Thanks again

@ultratoto14
Copy link
Collaborator Author

ultratoto14 commented Nov 3, 2020

Hi @postlund , @rospogrigio, @SmartM-ui need your advice on this one.
During the PR for the support of RGB, i fixed a "bug"
LightData
When we switch from color (left) to white (right), the bulb color is changed but the color picker keep the last set color.
I fixed that by forcing the color to [ 0, 0 ]. But it seems to not be the normal case. It should be None as @postlund did in his PR.

Setting to [0,0] generates an unwanted behavior, when using white only scene, the HA scene editor is storing the bulb state and stores the color_temp and this [ 0, 0 ] color. When applying the state as there is some color, HA does not send the color_temp to the light in the turn_on call.

So the normal scene behavior and internal way of managing color and color_temp is that when in white, color is None. I do not know if when in color, color_temp should be None too.

Should we remove this forcing and go back to a color picker temporary keeping the previous color as the bulb icon reflects the real setting ?
P.S: Sorry for the screenshots (i'm French)

@ultratoto14
Copy link
Collaborator Author

@SmartM-ui, @postlund , @rospogrigio just found that : home-assistant/frontend#6091

Seems that only relevant data should be returned, so in case Color, color_temp should be none and so one.
It means that the color picker behavior, keeping set on the last selected is expected.

@ultratoto14
Copy link
Collaborator Author

@rospogrigio or @postlund. This is ready for review.

@SmartM-ui
Copy link

@SmartM-ui, @postlund , @rospogrigio just found that : home-assistant/frontend#6091

Seems that only relevant data should be returned, so in case Color, color_temp should be none and so one.
It means that the color picker behavior, keeping set on the last selected is expected.

Hi,
as soon as possible I try your latest fix!

@SmartM-ui SmartM-ui mentioned this pull request Nov 4, 2020
@ultratoto14
Copy link
Collaborator Author

Hi @SmartM-ui
So the test should be same as before, you can also verify that:

  • In case we use a color in the scene editor, color_temp is not present in scenes.yaml
  • In case we use a color_temp in the scene editor, hs, rgb and xy are not present in scenes.yaml

Thanks for testing.

@SmartM-ui
Copy link

Hi @SmartM-ui
So the test should be same as before, you can also verify that:

* In case we use a color in the scene editor, color_temp is not present in scenes.yaml

* In case we use a color_temp in the scene editor, hs, rgb and xy are not present in scenes.yaml

Thanks for testing.

Hi @ultratoto14 ,
Perfect, it seems to work correctly:
When you set the scene with color it is done correctly by also reporting the chosen color in the yaml:
entities:
light.studio_localtuya chandelier:
min_mireds: 154
max_mireds: 370
brightness: 122
hs_color:
- 360
- 100
rgb_color:
- 255
- 0
- 0
xy_color:
- 0.701
- 0.299
friendly_name: Studio LocalTuya chandelier
supported_features: 19
state: 'on'

When you edit the scene previously set to color and move the color_temp, the position of the indicator remains on the previously chosen color (red), but when you execute the scene the color is set to white also respecting the set color temperature. The yaml file of the white is as follows:

  • id: '1604270296005'
    name: Try White Scene
    entities:
    light.studio_localtuya chandelier:
    min_mireds: 154
    max_mireds: 370
    brightness: 122
    color_temp: 257
    friendly_name: Studio LocalTuya chandelier
    supported_features: 19
    state: 'on'

@ultratoto14
Copy link
Collaborator Author

Hi @postlund, @SmartM-ui confirmed the behavior. It could merged.

@postlund postlund merged commit a0f4fdf into rospogrigio:master Nov 6, 2020
@ultratoto14 ultratoto14 deleted the light_scene_fix branch November 6, 2020 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants