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

Merge Yeelight Music Mode (#329) #331

Merged
merged 19 commits into from
Mar 31, 2020
Merged

Merge Yeelight Music Mode (#329) #331

merged 19 commits into from
Mar 31, 2020

Conversation

alexyao2015
Copy link
Member

  • Implementation of Yeelight Music Mode
    Yeelight now supports Philips Entertainment API
  • Changed entertainment service method to pass Host IP argument;
  • Changed sendLightRequest method to pass entertainment Host IP to turn on music mode on yeelight devices before sending them any commands;
  • Implemented YeelightConnection class which is now used to communicate with Yeelight devices and to maintain Yeelight Music Mode connections;
  • Implemented dictionary logic to support dedicated yeelight connections;
  • Fixed small string format error

This will serve as the pull request for #329
Fixes #327

SilveIT and others added 3 commits January 17, 2020 00:35
Yeelight now supports Philips Entertainment API
* Changed entertainment service method to pass Host IP argument;
* Changed sendLightRequest method to pass entertainment Host IP to turn on music mode on yeelight devices before sending them any commands;
+ Implemented YeelightConnection class which is now used to communicate with Yeelight devices and to maintain Yeelight Music Mode connections;
+ Implemented dictionary logic to support dedicated yeelight connections;
* Implementation of Yeelight Music Mode
Yeelight now supports Philips Entertainment API
* Changed entertainment service method to pass Host IP argument;
* Changed sendLightRequest method to pass entertainment Host IP to turn on music mode on yeelight devices before sending them any commands;
+ Implemented YeelightConnection class which is now used to communicate with Yeelight devices and to maintain Yeelight Music Mode connections;
+ Implemented dictionary logic to support dedicated yeelight connections;

* Fixed small string format error
@alexyao2015
Copy link
Member Author

I happen to have 3 Yeelight bulbs and can confirm that the lights do change color, however, there is very noticeable latency with color changes. If memory serves correctly, I believe yeelight music mode is a reverse connection to the server to obtain the light data, which makes the latency confusing.

@alexyao2015
Copy link
Member Author

Also, it appears to be stuck in light sync mode... Not sure if thats related to this commit or not...

@SilveIT
Copy link
Contributor

SilveIT commented Jan 18, 2020

Yeah, I have noticed that latency too. Dunno where the problem might actually be, it might even be on Yeelight's side.

Mine sync mode works fine actually. There are some problems with syncWithLights (the bulb is online and connected in music mode, but has unreachable state), but hue sync sends commands properly even when the bulb is offline :D. Once I got my Yeelight to always "timed out" state, fixed it by switching off/on.

Also, you forgot to take one more commit b4061f5
Pulled it with one more in #332

* Implementation of Yeelight Music Mode
Yeelight now supports Philips Entertainment API
* Changed entertainment service method to pass Host IP argument;
* Changed sendLightRequest method to pass entertainment Host IP to turn on music mode on yeelight devices before sending them any commands;
+ Implemented YeelightConnection class which is now used to communicate with Yeelight devices and to maintain Yeelight Music Mode connections;
+ Implemented dictionary logic to support dedicated yeelight connections;

* Fixed small string format error

* Small music mode listener improvement
@alexyao2015
Copy link
Member Author

I haven't gotten a chance to look very closely at the code, but I have don't more testing and have noticed the bulbs being stuck in light sync mode does appear to be related to this commit.

@SilveIT
Copy link
Contributor

SilveIT commented Jan 18, 2020

Some funny stuff. The first one isn't my fault, but the other is probably because of my code. Maybe I'll try to fix it later.
image
image

@SilveIT
Copy link
Contributor

SilveIT commented Jan 18, 2020

syncWithLights's and sendLightRequest's Connections dictionaries are same. Will fix it tomorrow...

@SilveIT
Copy link
Contributor

SilveIT commented Jan 20, 2020

This is insane... Merge #334 by the way..
image

SilveIT and others added 9 commits January 20, 2020 14:04
* Implementation of Yeelight Music Mode
Yeelight now supports Philips Entertainment API
* Changed entertainment service method to pass Host IP argument;
* Changed sendLightRequest method to pass entertainment Host IP to turn on music mode on yeelight devices before sending them any commands;
+ Implemented YeelightConnection class which is now used to communicate with Yeelight devices and to maintain Yeelight Music Mode connections;
+ Implemented dictionary logic to support dedicated yeelight connections;

* Fixed small string format error

* Small music mode listener improvement

* syncWithLights timedelta fix and Yeelight's get_state rollback

* Handle an exception in getIpAddress method
+ Added RGB argument to sendLightRequest and most of set_light methods to avoid double RGB conversion;
* Fixed possibly incorrect brightness value in MiBox, ESPHome protocols;
* Transition time in entertainment module is little bit dynamic now;
…-master

# Conflicts:
#	BridgeEmulator/functions/entertainment.py
Resolving conflict
Fixed weird stuff with tabulation
Color brightness fixes + merged commits from master
@mariusmotea
Copy link
Member

Is this tested, can be merged?

@SilveIT
Copy link
Contributor

SilveIT commented Jan 23, 2020

Not really, I can only test Yeelight protocol. Someone has to check the new logic of color brightness for Domoticz (2872804#diff-386e842c566c501e0c945a31f177beccR78-R81), ESPHome (2872804#diff-7227138d2fb9abccfe0f7c8e327e5989R199-R202), MiBox (2872804#diff-c6176cfbe026837caa0d86ebaead9f76R21-R24)

@alexyao2015
Copy link
Member Author

I'll test it this weekend.

@alexyao2015
Copy link
Member Author

I did some testing. This version seems to be improved from before, but there is still significant amounts of latency observed. Personally, I have not reviewed the code at all, so I am not sure if this is possible to get this to the speed that ESPHome bulbs can react. But in the current state, it is still pretty far away latency-wise.

@SilveIT
Copy link
Contributor

SilveIT commented Jan 25, 2020

Have you tested new color logic of Domoticz, ESPHome, MiBox? It should be ok, but I can't test. Also I don't quite understand what's the reason of this latency. Native Yeelight works perfect and zero-latency.

@alexyao2015
Copy link
Member Author

I haven't, I only tested Yeelight but I did notice when entertainment mode was supposed to flash white, it flashed a more off-white, pinkish color in testing. I have 3 Yeelight bulbs and there is latency, so I'm not quite sure what you mean by zero-latency.

@SilveIT
Copy link
Contributor

SilveIT commented Jan 25, 2020

Yes, I noticed that too, Yeelight's ~255;255;255 can be pinkish or greenish, I guess it might depend on previous commands.
Also I already mentioned it before #329 (comment)

@SilveIT
Copy link
Contributor

SilveIT commented Jan 25, 2020

I guess, I was wrong about no-delay in Yeelight app. It seems delayed too. It also doesn't seem like Yeelight has more or less white color in set_rgb. So, I don't think I can do anything about those problems at the moment.
I think it can be merged safely if the new colors on Domoticz, ESPHome, MiBox are all right.

@alexyao2015
Copy link
Member Author

Alright I'll take a look at the code and make sure the colors work properly. Once I do that, I'll merge it.

@SilveIT
Copy link
Contributor

SilveIT commented Jan 25, 2020

LOL, I have a really stupid idea. We can check if r=g=b with really small tolerance and use CT instead of RGB to get a better white color, because they are using different leds for white. Will check it later.

@alexyao2015
Copy link
Member Author

alexyao2015 commented Jan 26, 2020

Small problem with that is the white LEDs might have a very different brightness range so you'll have to scale that accordingly.

@SilveIT
Copy link
Contributor

SilveIT commented Jan 26, 2020

It actually works! Yeah, I knew it would have a different brightness. In my case white leds are ~3 times brighter, so I just devided brightness by 3. I hope it's same with other Yeelights... The only small problem is that brightness command (set_bright) on CT to RGB transition causes weird blinking. May be it's not that nasty, may be I'm wrong, I'll commit it anyway.

UPD: That blinking between CT and RGB seems much more annoying than incorrect white, so I reverted that commit.

* Small changes to disconnect logic

* Controversial changes relative to white
* Using CT instead of RGB where color is nearly white;

* Revert "Controversial changes relative to white"

This reverts commit 1768f7f.
@stale
Copy link

stale bot commented Feb 16, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 2 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale This issue is stale, and will be closed label Feb 16, 2020
@stale stale bot removed the stale This issue is stale, and will be closed label Feb 16, 2020
@mariusmotea
Copy link
Member

Will be an update to this commit? The stale boot want to close it.

@SilveIT
Copy link
Contributor

SilveIT commented Mar 30, 2020

I don't know what to update. It works fine for me. I haven't tested other protocols than Yeelight.

@mariusmotea
Copy link
Member

I see the file esphome.py is entirely replaced but with identical content so i believe you change only the end of the line from unix to windows format. Can you undo this so i can merge your work?

Thanks.

@SilveIT
Copy link
Contributor

SilveIT commented Mar 30, 2020

I tried dos2unix on this file, but it hasn't changed. I also don't see a commit where I could have ruined it. I made changes to it before, but it didn't replace all the lines. I tried dos2unix on a version of the file from the master branch of your repository and it replaces all lines there. I think my commit is fixing it.

@SilveIT
Copy link
Contributor

SilveIT commented Mar 30, 2020

Before that, a couple of changes were made to this file here: 2872804#diff-7227138d2fb9abccfe0f7c8e327e5989L12-L200

@mariusmotea mariusmotea merged commit d52e8b2 into master Mar 31, 2020
@alexyao2015 alexyao2015 deleted the master-yeelightmusic branch May 23, 2020 07:38
@SilveIT SilveIT mentioned this pull request Feb 14, 2022
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.

Yeelight Music Mode
4 participants