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

Bump yeelight to 0.5.4 #41524

Merged
merged 2 commits into from Oct 9, 2020
Merged

Bump yeelight to 0.5.4 #41524

merged 2 commits into from Oct 9, 2020

Conversation

shenxn
Copy link
Contributor

@shenxn shenxn commented Oct 9, 2020

Proposed change

The bug is introduced in #39542. yeelight version 0.5.3 renames transitions.randomloop to transitions.random_loop which causes AttributeError.
The author of the yeelight library uploads multiple files of version 0.5.3 with different code that causes AttributeError. Bump yeelight to 0.5.4 to fix this problem.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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

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

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@probot-home-assistant
Copy link

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

@project-bot project-bot bot moved this from Needs review to By Code Owner in Dev Oct 9, 2020
@bdraco
Copy link
Member

bdraco commented Oct 9, 2020

@bdraco
Copy link
Member

bdraco commented Oct 9, 2020

(venv) root@ha-dev:~/home-assistant# grep random venv/lib/python3.8/site-packages/yeelight/transitions.py 
import random
def random_loop(duration=750, brightness=100, count=9):

Its definitely now random_loop

It seems the CI is using an old package?

@shenxn
Copy link
Contributor Author

shenxn commented Oct 9, 2020

@bdraco Yes seems that the CI is using an old package, and that might be the reason that #39542 passed all checks.

@bessarabov
Copy link

Maybe we should also change file requirements_all.txt to bump the version to 0.5.4 ?

I think that there is problem with 0.5.3 — #41507 (comment)

@bessarabov
Copy link

bessarabov commented Oct 9, 2020

After thinking about this problem a bit I think that we definitely should change the version in requirements_all.txt to 0.5.4

I'm not sure what the author of yeelight will do. If he re-re-upload 0.5.3 with the same code as before Home Assistant yeelight integration will break again.

@bieniu
Copy link
Member

bieniu commented Oct 9, 2020

I also think we should bump yeelight library to version 0.5.4.

@shenxn shenxn changed the title Fix yeelight AttributeError Bump yeelight to 0.5.4 Oct 9, 2020
Copy link
Member

@bieniu bieniu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this change locally and everything works as it should.

@balloob balloob merged commit e6a65b8 into home-assistant:dev Oct 9, 2020
Dev automation moved this from By Code Owner to Done Oct 9, 2020
@bieniu
Copy link
Member

bieniu commented Oct 9, 2020

IMO, this PR should be tagged 0.116.2.

@springstan springstan added this to the 0.116.2 milestone Oct 9, 2020
@shenxn shenxn deleted the yeelight-name-fix branch October 9, 2020 13:35
balloob pushed a commit that referenced this pull request Oct 9, 2020
@balloob balloob mentioned this pull request Oct 9, 2020
@slydiman
Copy link
Contributor

slydiman commented Oct 10, 2020

Most of problems are fixed in 0.116.2, but something is still broken.
I got the following error: Cannot determine device type for [IP], [name]. Falling back to white only
This yeelight object is completely missing.
The device model is yeelink.light.lamp3
Probably few other yeelight models are broken too.

@MartinHjelmare
Copy link
Member

Please open an issue. Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Oct 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
Development

Successfully merging this pull request may close these issues.

Yeelight integration broken in 0.116.1
10 participants