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

Add missing yeelight models mapping #24963

Merged
merged 1 commit into from
Jul 8, 2019

Conversation

cadavre
Copy link
Contributor

@cadavre cadavre commented Jul 5, 2019

Breaking Change:

No breaking changes.

Description:

Adds support for missing yeelight models.

Related issue (if applicable):

Fixes #24962

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox.
  • There is no commented out code in this PR.
  • I have followed the development checklist

@cadavre cadavre requested a review from rytilahti as a code owner July 5, 2019 07:40
@ghost ghost assigned rytilahti Jul 5, 2019
@ghost
Copy link

ghost commented Jul 5, 2019

Hey there @rytilahti, @zewelor, mind taking a look at this pull request as its been labeled with a integration (yeelight) you are listed as a codeowner for? Thanks!

This is a automatic comment generated by codeowners-mention to help ensure issues and pull requests are seen by the right people.

@zewelor
Copy link
Contributor

zewelor commented Jul 5, 2019

lamp1 is whitetemp for sure, not mono ? Its possible to set_mode to nightlight on it ?

@cadavre
Copy link
Contributor Author

cadavre commented Jul 5, 2019

lamp1 is WhiteTemp for sure, I have one.

It is not possible to yeelight.set_mode to nightlight on it, because this service have no nightlight mode allowed to set as mode:

Parameter Description
entity_id Name of the light entity.
mode Operation mode. Valid values are 'last', 'normal', 'rgb', 'hsv', 'color_flow', 'moonlight'.

PS On the other hand flows work perfectly.

@zewelor
Copy link
Contributor

zewelor commented Jul 5, 2019

Sorry does mode moonlight works on it. For effects I've recently whitelisted effets, that are supported on non color light, does they work on this lamp ?

@zewelor
Copy link
Contributor

zewelor commented Jul 5, 2019

For me it looks good.

@aidbish
Copy link
Contributor

aidbish commented Jul 5, 2019

can you update the issue number is fixes, as it points to a different one (missing 2 at the end by looks of it)

@cadavre
Copy link
Contributor Author

cadavre commented Jul 5, 2019

moonlight works ok, however binary_sensor..._nightlight is not updated – just as before refactor.

By whitelisted effects you mean just "Slow temp"? No, it does nothing or I noticed nothing.

@aidbish thanks for spotting, somehow 2 got eaten.

@zewelor
Copy link
Contributor

zewelor commented Jul 5, 2019

Its not updated because it reads model specs from https://gitlab.com/stavros/python-yeelight/blob/master/yeelight/main.py#L37 . It would require to add model also there and update python-yeelight.

@cadavre
Copy link
Contributor Author

cadavre commented Jul 5, 2019

So I'm leaving this to you.

I'm bringing yeelight functionality to point where it was before refactor on HA site.

@MartinHjelmare MartinHjelmare changed the title Added missing yeelight models mapping Add missing yeelight models mapping Jul 7, 2019
@rytilahti
Copy link
Member

So, if I'm reading it correctly, this fixes a known issue without breaking anything, so I think this should get merged. Not updating the nightlight state is another issue, which can be solved in a separate PR.

Thanks for the PR @cadavre and thanks @zewelor for your insight on how to fix that another issue!

@rytilahti rytilahti merged commit 31d7b70 into home-assistant:dev Jul 8, 2019
@lock lock bot locked and limited conversation to collaborators Jul 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

yeelight refactor broke backward compatibility for some lamp models
5 participants