-
Notifications
You must be signed in to change notification settings - Fork 15
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
Some cleanup #18
Some cleanup #18
Conversation
Thanks for the PRs! I'll try to find some time to test this weekend, all looks good on a quick glance though. |
All looks well in testing thus far. Few questions:
|
Resolved
Yes at the time when I submitted this I had a single user bed and the fixes worked for me.
Yep, will do that shortly |
Sorry, what part of the README needs to be updated? I removed the |
…into remove_partner * 'remove_partner' of https://github.com/raman325/pyEight: Fix bug with username and password (mezz64#17)
Main thing left in the README is the python version. I believe we need at least 3.7 now to support the asyncio.run call. |
Done! I also specified a |
@raman325 This is now published as 0.1.7 on pypi (missed updating the tarball location in the setup so had to republish). Feel free to move forward with changes on the HA side. |
Great thanks! |
Made a couple of changes:
partner
flag. We can simply check if there is a right user and then if there is a left user that isn't the same as the right user. I think the end result of this is the same... I guess you lose the ability to not include your partner as a user, but I am not sure if that's a common use case.async_timeout
dependency because you can pass the timeout directly to the get/put/post with current versions ofaiohttp
.loop
as an input. I don't think we need to set the event loop, HA will do that for us, and because of the other changes, it's not needed as an input elsewhere.api_key
since it is not used anywhereaiohttp
as a dependency so that users outside of HA can use the packageI have done some brief testing, and things appear to be working as expected. Assuming you are OK with these changes and can cut a new release, I'd be happy to make the corresponding changes in HA since this is a breaking change that I am introducing. I will also likely pick up home-assistant/core#47493 at some point to try to get that over the finish line
EDIT: Just found this comment on Discord, so it seems we are on the same page for item 1 🙂 https://discord.com/channels/330944238910963714/330990195199442944/817744252058337330