Add support for HSV encoded color lights #90

postlund · 2020-10-17T21:15:57Z

This PR adds support for light bulbs with colors encoded in HSV format. Brightness is supported as well (since it's embedded in the color data).

@rospogrigio This PR is ready for review, but it should not be merged before #92 and a final test by user.

SmartM-ui · 2020-10-17T21:30:05Z

Here I am,
I follow the development with interest.
As soon as possible I try and send the log.
If I can be useful for anything, don't hesitate to ask

postlund · 2020-10-18T11:34:20Z

Would be great with a tester for this 👍 Do you have bulbs configured via RGB as well?

ultratoto14 · 2020-10-18T17:20:00Z

Hi i do have multiple lights with different settings:

HSV on 4 bytes (0-1000) range
RGB_HSV (0-255) range

I have something almost fully working here https://github.com/ultratoto14/localtuya-homeassistant/tree/rgbw_update. Be kind, i'm not a python developer, many things can be done differently
You can have a check to implement and merge what you need in yours.
As a high number of DP is used, i tried to suggest one for each entry in case i can found it during the config

What is not working in mine:

I have RGBW GU10 from different manufacturer which have different white range. So mired should be also configurable
I need to detect when HA sends white or color commands to adapt, as it is separated. When a scene is saved via the editor it stores all the information, meaning brightness, color_temp, color and everything is sent to the bulb once the scene is triggered. Need to manually update the yaml to ensure only the needed command is sent.

postlund · 2020-10-18T17:34:32Z

Thanks @ultratoto14, I will a greater look. I think I have implemented most things in a similar manner, not RGB though. But I can take some inspiration from your code for that. One change I'm planning is to use a single option for color values, as HSV or RGB can be deduced from length of the value.

Color temperature is a different matter, related of course. Should be simple enough to implement (there is also an issue about it: #91). I intend to add two options to allow custom values for lower and upper range, specified in Kelvin and converted to mired internally.

ultratoto14 · 2020-10-18T18:19:52Z

I used the set_dps you introduced, i fixed the unneeded value and put the corresponding in common.py

My full list of devices:

Led Strip HSV mode
GU10 RGBW RGB mode
GU10 RGBW HSV mode
Color Bulb HSV mode

They are from different manufacturers and all the one in HSV are using this 1000 range where the RGB_HSV are using the 255 range.
Do you know HSV lights using the 255 range ?

postlund · 2020-10-18T20:49:00Z

I would not expect HSV to be limited to 255 like that. This picture explains HSV pretty well:

ultratoto14 · 2020-10-19T05:08:58Z

;-) my question was not clear. Do you know tuya lights that uses HSV coding for the color and 255 range for brightness and color_temp ? Because if you deduce the kind of color from the length, you can then also deduce the max value for brightness and color_temp.
As many things should be setup for a simple light, if the configurator can help the user in choosing the configuration values, it's a big addition.

postlund · 2020-10-19T07:16:47Z

Ah, yes, then I understand. No, I don't know about that and I would expect that to not exist either. They however seem to use the same brightness range for white and color, so no distinction needs to be made there. Color temperature uses a different range based on mirad/kelvin range of the lamp and has to be entered separately.

postlund · 2020-10-19T10:19:47Z

Tested and only needs review before merging!

ultratoto14 · 2020-10-19T16:00:07Z

Just tested it, two main remarks:

Color temp range is not aligned but perhaps it's not the purpose of this PR (cold white not accessible 255-> 1000)
When going from colour to white mode, the bulb icon do not switch to the white and keep the old color setup

postlund · 2020-10-19T16:33:40Z

@ultratoto14 Thanks for testing! Color temperature will be dealt with in #91. Regarding the icon, not sure how that works. Will have to look into that, thanks for reporting!

postlund · 2020-10-19T17:04:14Z

@ultratoto14 Pushed an update now, can you try if icon works as expected?

ultratoto14 · 2020-10-19T17:29:04Z

@postlund Icon is now working as expected.

postlund · 2020-10-19T17:30:02Z

@ultratoto14 Brilliant, thanks! 🎉

ultratoto14 · 2020-10-20T12:41:17Z

@postlund, i found a remaining small bug about the consistency of the brightness setting.
If you go from white to colour, the brightness is conserved. I think it's the desired behavior.
Once in color, i you change the brightness, it's still working. But when you go back to white, the brightness is the same as before going to color.
Steps to reproduce:

Put in white - OK
Set brightness to 50% - OK
Choose a color - OK brightness still at 50%, color is OK too
Change brightness to 100% - OK
Change color_temp to get back to white: KO brightness is back to 50%

postlund · 2020-10-21T20:14:46Z

@ultratoto14 Ok, that's interesting. Maybe I should set white and color brightness at the same time to keep them in sync. I guess we could try that.

postlund · 2020-10-21T20:37:29Z

@ultratoto14 Loong-shot here, but please try the latest commit and see if it works as expected (or at all).

ultratoto14 · 2020-10-22T06:38:47Z

Hi @postlund, sorry for the delay. I still have a couple of hours to test before being unavailable for 3 days.
Unfortunately, the fix does not work. During the test , at the third step, changing brightness when in color, the bulb switch back to white.
Setting the brightness dp when in color mode is switching the bulb to white, it should not be set every time.

lapy · 2020-10-23T09:59:45Z

Hi guys,

I hadn't noticed this pull request so I added this feature by myself on my fork.
I tested it on a tuya compatible E14 bulb.

Working:

Color selection
Color brightness
Improved white temp mapping (for me the original code only covered 1/4 of the range)

Please take a look if that helps! (another set of eyes/ideas).

lapy@7d76d91
Please ignore the workspace.xml file :)

Thank you,
Vince

ultratoto14 · 2020-10-24T19:01:38Z

Hi @postlund, tried to find the problem and it seems that the update and turn on are competing.
You can update the brightness when changing the color_temp between line 171 and 172.

          if not self._is_white_mode:
            await self._device.set_dp(self._brightness, self._config.get(CONF_BRIGHTNESS))
            await self._device.set_dp("white", self._config.get(CONF_COLOR_MODE))

You need to apply the brightness first because of the status updated.
When you will use a single call to apply multiple DPs, the problem may be solved.

postlund · 2020-10-24T20:07:35Z

@ultratoto14 Seems like @lapy has come a longer way than I have (especially since I haven't prioritized this recently), maybe you can try his commit and see it if works better? At the end of the day I would love to see someone else implement this so I can focus on core stuff instead 🙃

postlund · 2020-10-24T20:08:22Z

@lapy Send a PR, I can review it for inclusion here and @ultratoto14 can probably test it.

SmartM-ui · 2020-10-24T23:13:55Z

@ultratoto14 Seems like @lapy has come a longer way than I have (especially since I haven't prioritized this recently), maybe you can try his commit and see it if works better? At the end of the day I would love to see someone else implement this so I can focus on core stuff instead 🙃

Hi @postlund ,
if I can help I am available to carry out some tests.

Which version are you testing?

lapy · 2020-10-24T23:17:20Z

Hi,
This is my pull request #115 if you wanna test it.

Thank you,
Vince

SmartM-ui · 2020-10-24T23:25:42Z

Hi,
This is my pull request #115 if you wanna test it.

Thank you,
Vince

OK! Thanks

SmartM-ui · 2020-10-24T23:59:54Z

Hi @postlund ,
I tried the pull # 115 and I must say that as far as the color temperature is concerned it manages to align correctly with the smartlife app.

I was also testing this version # 90, I configured a light bulb via config flow and I could also test the colors and they work correctly and in real time. GREAT!

You just have to fix the color temperature and then you have reached perfection :-)

Thanks

postlund · 2020-10-26T08:56:29Z

@ultratoto14 Got your email, will respond here so we can decide on how to move forward.

From my point-of-view, it doesn't really matter if we go with my solution or the one by @lapy. I do feel that it'a a bit problematic to not have a steady maintainer behind the code we merge. So I need your input, based on what we have. Should we merge this and continue adding RGB support from @ultratoto14 as well as what else is missing in additional steps? Or should we go with @lapy? I'm up for anything, we just need to increase the pace a bit and get something merged that we can extend upon.

ultratoto14 · 2020-10-26T10:43:47Z

@postlund, I totally agree with you. I just want to have this included to ensure i can work with it without implementing RGB each time because it's not yet supported. @lapy said he returned his bulb, so he will not maintain the code.
The branch I posted just fixed the brightness pb of your PR, add RGB bulb support and fixed the color_temp.
If it's Ok, I open a PR. As it is based on this one, you close it and link to mine.

postlund · 2020-10-26T10:49:13Z

Sounds good to me 👍 Let's merge this and continue with additional PRs and writing issues for bugs.

@rospogrigio You can review this now.

rospogrigio

Just one comment, for the rest I think it's ok.

rospogrigio · 2020-10-26T10:55:22Z

custom_components/localtuya/common.py

@@ -219,6 +219,8 @@ async def async_added_to_hass(self):
        """Subscribe localtuya events."""
        await super().async_added_to_hass()

+        _LOGGER.debug("Adding %s with configuration: %s", self.entity_id, self._config)


Do we really want to keep this debug message?

I do like this debug print as it will tell us if a certain option is configured or not (it helps when a user thinks a he/she configured an option, but actually didn't).

postlund · 2020-10-26T11:10:07Z

@ultratoto14 This is merged now. You can rebase your changes on top of master and open a new PR with your changes now.

ultratoto14 · 2020-10-26T13:30:59Z

@postlund, @rospogrigio, @SmartM-ui the PR120 is created

SmartM-ui · 2020-10-26T13:44:11Z

@postlund, @rospogrigio, @SmartM-ui the PR120 is created

Thanks, I'll try now

postlund added the enhancement New feature or request label Oct 17, 2020

postlund self-assigned this Oct 17, 2020

postlund mentioned this pull request Oct 17, 2020

Control Tuya RGB (W or C or CW) from localtuya #33

Closed

postlund force-pushed the light_hsv branch 2 times, most recently from 5d510a4 to 722aa70 Compare October 19, 2020 08:25

postlund changed the title ~~wip: testing with light hsv color support~~ Add support for HSV encoded color lights Oct 19, 2020

postlund requested a review from rospogrigio October 19, 2020 08:27

postlund force-pushed the light_hsv branch from 722aa70 to b83c30a Compare October 19, 2020 08:47

postlund force-pushed the light_hsv branch from f732878 to 3fa2201 Compare October 19, 2020 17:03

postlund mentioned this pull request Oct 19, 2020

Only forward config entry to each platform once #100

Merged

postlund mentioned this pull request Oct 20, 2020

Problem with setting up LED light and cover #105

Closed

postlund added 3 commits October 21, 2020 22:33

Add support for HSV encoded color light

0ceb479

Add debug logs for entity config

ae1bd35

Unset color when going to white mode

eb53a83

postlund force-pushed the light_hsv branch from e64a048 to a533e4f Compare October 21, 2020 20:36

postlund force-pushed the light_hsv branch from a533e4f to eb53a83 Compare October 22, 2020 07:31

Merge branch 'master' into light_hsv

0a89696

rospogrigio approved these changes Oct 26, 2020

View reviewed changes

postlund merged commit d783293 into master Oct 26, 2020

postlund deleted the light_hsv branch October 26, 2020 11:09

Add support for HSV encoded color lights #90

Add support for HSV encoded color lights #90

Conversation

postlund commented Oct 17, 2020 • edited Loading

SmartM-ui commented Oct 17, 2020

postlund commented Oct 18, 2020

ultratoto14 commented Oct 18, 2020

postlund commented Oct 18, 2020

ultratoto14 commented Oct 18, 2020

postlund commented Oct 18, 2020

ultratoto14 commented Oct 19, 2020

postlund commented Oct 19, 2020

postlund commented Oct 19, 2020

ultratoto14 commented Oct 19, 2020 • edited Loading

postlund commented Oct 19, 2020

postlund commented Oct 19, 2020

ultratoto14 commented Oct 19, 2020

postlund commented Oct 19, 2020

ultratoto14 commented Oct 20, 2020

postlund commented Oct 21, 2020

postlund commented Oct 21, 2020

ultratoto14 commented Oct 22, 2020 • edited Loading

lapy commented Oct 23, 2020

ultratoto14 commented Oct 24, 2020

postlund commented Oct 24, 2020

postlund commented Oct 24, 2020

SmartM-ui commented Oct 24, 2020

lapy commented Oct 24, 2020

SmartM-ui commented Oct 24, 2020

SmartM-ui commented Oct 24, 2020 • edited Loading

postlund commented Oct 26, 2020

ultratoto14 commented Oct 26, 2020

postlund commented Oct 26, 2020

rospogrigio left a comment

Choose a reason for hiding this comment

rospogrigio Oct 26, 2020

Choose a reason for hiding this comment

postlund Oct 26, 2020

Choose a reason for hiding this comment

postlund commented Oct 26, 2020

ultratoto14 commented Oct 26, 2020

SmartM-ui commented Oct 26, 2020

postlund commented Oct 17, 2020 •

edited

Loading

ultratoto14 commented Oct 19, 2020 •

edited

Loading

ultratoto14 commented Oct 22, 2020 •

edited

Loading

SmartM-ui commented Oct 24, 2020 •

edited

Loading