Added effects to Yeelight bulbs by Mister-Espria · Pull Request #7152 · home-assistant/core

Mister-Espria · 2017-04-17T16:00:57Z

Description:

This PR is now reopened, because there are now effects implemented in the yeelight module version 3.0.0.
I did update the original code of this PR to support the newly implemented effects.

Any suggetions/improvements are welcome of course.

PR text before closing:

Added some basic effects to Yeelight bulbs. Deleted requirement for the bulb to be in a specific colormode in order to use flash parameter.

There are three different type of effects added:

Predefined colors and brightness;

Predefined colors with current brightness;

Random colors with current brightness.

I also did a "stop" effect which stops the effect and returns to the previous state.
I did add this to the effectlist i don't know if this is the best approach as it is not really an effect.

This is my first PR ever. So if i did something wrong please tell me. So i can improve it.
Also i would like to thank @armills for all his help!

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for `configuration.yaml` (if applicable):

Checklist:

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.github.io

If the code communicates with devices, web services, or third-party tools:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
New dependencies have been added to the REQUIREMENTS variable (example).
New dependencies are only imported inside functions that use them (example).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

If the code does not interact with devices:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
Tests have been added to verify that the new code works.

mention-bot · 2017-04-17T16:00:58Z

@Mister-Espria, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rytilahti, @jjensn and @HydrelioxGitHub to be potential reviewers.

homeassistant · 2017-04-17T16:00:59Z

Hi @Mister-Espria,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

homeassistant · 2017-04-17T21:02:24Z

Hi @Mister-Espria,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

amelchio · 2017-04-17T21:21:11Z

I think you mean duration=2000 here, not 200

Yes typo i will fix it!

amelchio · 2017-04-17T21:25:15Z

You want randrange(0, 256) or randint(0, 255)

random.randrange(0, 255) is used in the flux component. i did't give it any thought. So i should change it into random.randint(0, 255) right?

Yes, if you want 255 as a possible outcome. Not that anyone will be able to tell the difference 😃

Nope but then it is as random as it can get 😄 . So i will change it.

fabaff · 2017-04-22T11:02:11Z

I think that the logic for the effects should be located in the yeelight module itself. This way the effects can be maintained at one place.

Thanks for your response. I think you are right, that the best place for it is in the yeelight module. I have tried to make a start with adding it there. But with trial and error i'm getting nowhere. I have no clue on how to start the implementation there. I will still be trying, but i'm not certain i will get it done 😃
What do you want me to do with this PR?

This is being discussed here: https://gitlab.com/stavros/python-yeelight/issues/11 so I'd propose closing this PR for now and reopening/creating a new one after the implementation hits the upstream :-)

Ok. I will do that now. Thank you.

Mister-Espria · 2017-05-14T16:57:40Z

I'm re-opening this PR, because there are now effects implemented in the yeelight module.
Will be adding changes shortly!
Edit: Done!

rytilahti · 2017-05-14T18:13:26Z

+EFFECT_TWITTER = "Twitter"
+EFFECT_STOP = "Stop"
+
+YEE_EFFECT_LIST = [


YEELIGHT_EFFECT_LIST to be consistent?

Thanks for your quick review. This is something i thought of and then forgot it of course.
Thank you i will change it.

rytilahti · 2017-05-14T18:13:49Z

+                                              strobe_color, alarm, police,
+                                              police2, christmas, rgb,
+                                              randomloop, slowdown)
+            # transition = int(self.config[CONF_TRANSITION])


Remove commented out lines.

rytilahti · 2017-05-14T18:14:45Z

+                                              randomloop, slowdown)
+            # transition = int(self.config[CONF_TRANSITION])
+            if effect == EFFECT_DISCO:
+                flow = Flow(count=0, transitions=disco(),)


Any reason these are passed as tuples? A list is expected? Maybe it'd make sense to do [disco()] to make it clear transitions expects an iterable? (Or in even better case, it should also allow single transitions...)

Probably my incompetence. The reason i have this is because this is how i did it to get it to work. the disco function returns a list with the transitions. I just tried you example [disco()] but i can't get it to work. maybe i need to do something other then replace disco() with [disco()] ?

Hmm, and passing the list directly does not work? To my understanding it should..

Yes just passing the list with transitions does work, but that is what i originally had.

@fabaff commented:

I think that the logic for the effects should be located in the yeelight module itself. This way the effects can be maintained at one place.

So this way we don't have to maintain the list for every transition, in case of changes in the yeelight module

Ah, I mean passing "disco()" as a parameter to transitions like:

Flow(count=0, transitions=disco())

should work. @fabaff is correct by saying that the implementation details belong to the module itself, where they are now thanks to you! :-)

Ah i see, That works! thank you i have changed it. And i completely agree with having the effects over there.

rytilahti · 2017-05-14T18:15:11Z

+            if effect == EFFECT_TWITTER:
+                flow = Flow(count=2, transitions=pulse(0, 172, 237),)
+
+            if effect == EFFECT_STOP:


Should this be before other checks?

Yes i think that will make more sense. Changing it now.

balloob

Looks good. Sorry for this taking so long. 🐬

bokub · 2017-08-21T18:36:28Z

Neat effects!
I just discovered them a few minutes ago by chance

Thanks for your work @Mister-Espria 👍

homeassistant added platform: light.yeelight cla-needed labels Apr 17, 2017

robbiet480 added the cla-recheck label Apr 17, 2017

homeassistant added cla-needed cla-signed and removed cla-recheck cla-needed labels Apr 17, 2017

amelchio reviewed Apr 17, 2017

View reviewed changes

homeassistant added the cla-signed label Apr 17, 2017

balloobbot added the platform label Apr 17, 2017

Mister-Espria mentioned this pull request Apr 17, 2017

Flux_led: small fix random effects #7156

Merged

8 tasks

fabaff reviewed Apr 22, 2017

View reviewed changes

Mister-Espria closed this Apr 26, 2017

Mister-Espria added 4 commits May 14, 2017 16:12

Added effects to Yeelight bulbs

73eb0f4

Fix Typo and Use randint instead of randrange

0d8584f

Added Effects

aa841d4

updated requirements_all.txt

40f7ec5

Mister-Espria reopened this May 14, 2017

Mister-Espria changed the title ~~Added effects to Yeelight bulbs~~ WIP:Added effects to Yeelight bulbs May 14, 2017

homeassistant added the cla-signed label May 14, 2017

balloobbot added the requirement label May 14, 2017

fix empty line

1b66e09

homeassistant added the cla-signed label May 14, 2017

Mister-Espria changed the title ~~WIP:Added effects to Yeelight bulbs~~ Added effects to Yeelight bulbs May 14, 2017

rytilahti reviewed May 14, 2017

View reviewed changes

minor fixes

b9a70ed

homeassistant added the cla-signed label May 14, 2017

fix passing effects as parameter

fac0542

homeassistant added the cla-signed label May 17, 2017

balloob approved these changes Jun 3, 2017

View reviewed changes

balloob merged commit 7d24efc into home-assistant:dev Jun 3, 2017

balloob mentioned this pull request Jun 16, 2017

0.47 #8055

Merged

home-assistant locked and limited conversation to collaborators Dec 11, 2017

Mister-Espria deleted the yeelight-adding-effects branch September 24, 2018 14:24

Mister-Espria restored the yeelight-adding-effects branch September 24, 2018 14:35

ghost added integration: yeelight and removed platform: light.yeelight labels Mar 21, 2019

Uh oh!

Conversation

Mister-Espria commented Apr 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

mention-bot commented Apr 17, 2017

Uh oh!

homeassistant commented Apr 17, 2017

Uh oh!

homeassistant commented Apr 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mister-Espria Apr 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mister-Espria commented May 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balloob left a comment

Choose a reason for hiding this comment

Uh oh!

bokub commented Aug 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Mister-Espria commented Apr 17, 2017 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):

Mister-Espria Apr 17, 2017 •

edited

Loading

Mister-Espria commented May 14, 2017 •

edited

Loading

bokub commented Aug 21, 2017 •

edited

Loading