Improve Yeelight support (expose more properties, add support for secondary lights)

[Kirmas]

In this pull request I want to add more property to Yeelight (I looked to python-yeelight and operation specification). Base reason of this because Yeelight closing lan controle for theirs lamps that`s why I want to add all functionality from python-yeelight to this repository. This is first PR from many :)

@rytilahti due to this pull request I want to ask some question:

1. First off all Yeelight has very different lamp. Is it ok that we use one Device class to describe them all?
2. Second I use "status" command to get all type of the properties. Maybe better add additional command like "bg_status" for Background light. What is better for Home Assistant integration?

I would be grateful for any criticism

[rytilahti]

Just a very quick review, I haven't yet tested the code on my own lights but I'll try to do that soonish to make sure this doesn't break anything.

[rytilahti]

Hi @Kirmas and thanks for the PR! :-)

• First off all Yeelight has very different lamp. Is it ok that we use one Device class to describe them all?

Yes, I think that is fine. Using several different classes would make it really confusing for users at some point, something we have been seeing recently with some other integrations.

At some point in the future, I have some plans to integrate some sort of "supported features" property based on the model to make it simpler to allow the class (and the downstream) to adapt to different types of lights.

• Second I use "status" command to get all type of the properties. Maybe better add additional command like "bg_status" for Background light. What is better for Home Assistant integration?

I think the best approach wrt homeassistant integration would be to have a YeelightLight (or similar) class to represent a single controllable light. A single status() could return a regular status of the main lamp, and the status container would have a property like lights() to return a list of lights that can be controlled.

This way the homeassistant integration would just require a single update (and single light implementation) even if there are multiple light elements available. Or at least this is how I imagine it would work :-)

[Kirmas]

@rytilahti additional question: what type off data better use for flow_params and bg_flow_params:
output looks like '0,0,1000,1,16711680,100,1000,1,65280,100,1000,1,255,100' I use string but maybe its array of int or something else?

[syssi]

A string is easier to manage for now. If it's just passed around and not manipulated. Adding new flows by configuration as strings would be easy.

[rytilahti]

@Kirmas I think the yeelight's spec defines the elements of the flow, so I think it would be best to use a separate container class (similar to the status container) to parse and store this data for easier use. But having it as a string for the time being is also fine (and better for comprehension than converting it into a list).

python-yeelight does implement some parsing (and building of flows), but I'm not sure if that can be easily used here. We want no external dependencies for this, and there might be a clash in licenses between these two projects which makes direct code sharing harder..

[Kirmas]

@rytilahti what I must to do next, for this PR? Is it ok to be merged or requires additional work?

[rytilahti]

@Kirmas a test or two for the secondary light would be great to have to avoid breaking the functionality inadvertently in the future, otherwise this is good to go! 👍

[Kirmas]

@rytilahti I added new devices to the yeelight test but not sure if I did it right. Please review.
Test_status looks good in my opinion. But another test requires additional work beyond the scope of this pull request.
When adding new functions, I will add new tests.
Some tests require https://gitlab.com/stavros/python-yeelight/-/issues/68/ to understand which feature is available.

PS:
It came as a surprise to me that I can move a non-color lamp to RGB mode without any errors. I think this is dangerous. When the separate library will be created, I plan to check it in the code on our side.

[rytilahti]

@Kirmas if you want, you/we could create a YAML file containing the data from python-yeelight and use that in this library for the time being. That file (and potential wrappers) can then be extracted into a separate lib later on, if wanted.

About the color mode, hmm, ohkay, it is probably then better to use the model info data to raise an exception if the library user tries to use features that are not supported by the light.

Wrt the tests, I think there should be a single "CommonBulb" that tests all the common features, leaving only the feature specific tests to their own classes. Could you please do that and then we can merge this and continue further improvements in separate PRs?

P.S. I adjusted the title a bit to make it clearer what was improved, feel free to edit it if you wish though!

[Kirmas]

@rytilahti Hello, sorry i missed one week. There was no time. I have added a general test. Now everything is fine? For my opinion test status must be totally unique for different types of lamp.

[Kirmas]

And other question. I couldn't understand why tests is failed. I didn't change files were they failed. Could you help with this?

[rytilahti]

Hi, no worries! Yes, I think it's fine for now as it is.

Hmm, that linting error is odd and I cannot reproduce it locally on this branch. Could you try to modify the .flake8 file and add SIM119 to the end of ignore list? Those are irrelevant as we still support python 3.6 series.