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

Added Model-Information for YLDP01YL #2377

Closed
wants to merge 1 commit into from
Closed

Added Model-Information for YLDP01YL #2377

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 3, 2017

Music mode does not work with Yeelight model YLDP01YL

Description:
Using music mode on this model will throw an exception, Yeelight returns "unknown command".

Pull request in home-assistant (if applicable): home-assistant/home-assistant#

Music mode does not work with Yeelight model YLDP01YL
@mention-bot
Copy link

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

@rytilahti
Copy link
Member

rytilahti commented Apr 3, 2017

Maybe related to home-assistant/core#6624 (where some RGB bulbs do not support the mode). YLDP01YL is the white-only.

@rytilahti
Copy link
Member

I think it's wrong way to fix this through documentation. This should be fixed in code by reverting transparently to the normal mode when MM is not available. How to do this nicely is being discussed in upstream: https://gitlab.com/stavros/python-yeelight/issues/10

@Landrash
Copy link
Contributor

Landrash commented Apr 5, 2017

I agree with @rytilahti in this case. Does anything break as is or does the function just not work?

@ghost
Copy link
Author

ghost commented Apr 5, 2017

Hi there,
yes, its breaking something :D

I got two bulbs in a group. When i try to switch the group on and one of the bulbs has music mode activated, only this bulb goes on. Then the error occours and the second bulb will not light on.

tail -f home-assistant.log | grep yeelight         
17-04-05 17:35:54 INFO (MainThread) [homeassistant.loader] Loaded light.yeelight from homeassistant.components.light.yeelight
17-04-05 17:35:58 INFO (MainThread) [homeassistant.components.light] Setting up light.yeelight
...
17-04-05 17:36:27 INFO (MainThread) [homeassistant.components.discovery] Found new service: yeelight {'hostname': 'yeelink-light-mono1_miio53868729.local.', 'device_type': 'white', 'host': '192.168.1.225', 'properties': {'epoch': '1', 'mac': '286c07b17a2f'}, 'port': 54321}
17-04-05 17:36:27 INFO (MainThread) [homeassistant.core] Bus:Handling <Event platform_discovered[L]: platform=yeelight, service=load_platform.light, discovered=hostname=yeelink-light-mono1_miio53868729.local., device_type=white, host=192.168.1.225, properties=epoch=1, mac=286c07b17a2f, port=54321>
17-04-05 17:36:27 INFO (MainThread) [homeassistant.components.discovery] Found new service: yeelight {'hostname': 'yeelink-light-mono1_miio53875463.local.', 'device_type': 'white', 'host': '192.168.1.103', 'properties': {'epoch': '1', 'mac': '286c07b1947d'}, 'port': 54321}
17-04-05 17:36:27 INFO (MainThread) [homeassistant.core] Bus:Handling <Event platform_discovered[L]: platform=yeelight, service=load_platform.light, discovered=hostname=yeelink-light-mono1_miio53875463.local., device_type=white, host=192.168.1.103, properties=epoch=1, mac=286c07b1947d, port=54321>
17-04-05 17:36:27 INFO (MainThread) [homeassistant.components.light] Setting up light.yeelight
17-04-05 17:36:27 INFO (MainThread) [homeassistant.components.light] Setting up light.yeelight
  File "/srv/homeassistant/lib/python3.4/site-packages/homeassistant/components/light/yeelight.py", line 298, in turn_on
  File "/srv/homeassistant/lib/python3.4/site-packages/homeassistant/components/light/yeelight.py", line 196, in set_music_mode
  File "/home/osmc/.homeassistant/deps/yeelight/main.py", line 486, in start_music
  File "/home/osmc/.homeassistant/deps/yeelight/main.py", line 313, in send_command
yeelight.main.BulbException: {'message': 'method not supported', 'code': -1}

It is in deed possible to change the bulb order in groups, but i dont think thats the way.

I understand your concerns and i think that fixing this error is the best way too.
But i believe that adding some informations about the different models would be nice in any way. Regardless of the error.

@rytilahti
Copy link
Member

I just made a PR for fixing the issue itself: home-assistant/core#6952 .

However, I could say the documentation change is okay, but could you please add a short description for the model number such as "YLDP01YL (white bulb)" and add a dot in the end?

What you also could add is a second note discouraging the use of music mode when it's not really needed, as it has its drawbacks (not possible to receive updates from the bulb, which may lead to inconsistent states even when it shouldn't due to caching)?

@balloob
Copy link
Member

balloob commented Apr 21, 2017

This PR seems to have gone stale. Closing it. You can reopen it when you're ready to finish it.

@balloob balloob closed this Apr 21, 2017
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.

4 participants