search

mezz64 / pyEight

Python library to interface with the Eight Sleep API
MIT License
59 stars 15 forks source link

Some cleanup #18

Closed raman325 closed 3 years ago

raman325 commented 3 years ago

Made a couple of changes:

  1. There's no need for the 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.
  2. Added support for getting the client session from HA instead of generating it on our own
  3. Removed the async_timeout dependency because you can pass the timeout directly to the get/put/post with current versions of aiohttp.
  4. Removed 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.
  5. Made it so that if the client session is created internally by the library, it will get closed automatically on shutdown. I wrote this so that you can safely call stop() multiple times.
  6. Removed api_key since it is not used anywhere
  7. Added aiohttp as a dependency so that users outside of HA can use the package

I 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 https://github.com/home-assistant/core/pull/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

mezz64 commented 3 years ago

Thanks for the PRs! I'll try to find some time to test this weekend, all looks good on a quick glance though.

mezz64 commented 3 years ago

All looks well in testing thus far. Few questions:

raman325 commented 3 years ago

All looks well in testing thus far. Few questions:

  • Since the password encoding fix has been merged can you update this PR to resolve the conflicts

Resolved

  • Have you been able to test as a single bed user? I know some others had potentially odd behavior in the past when setting things up to use both sides of the mattress as a single sleeper. I think your changes will handle this ok, it's just not something I'm able to test.

Yes at the time when I submitted this I had a single user bed and the fixes worked for me.

  • Can you update the README example and bump the python and package requirements appropriately.

Yep, will do that shortly

raman325 commented 3 years ago

Sorry, what part of the README needs to be updated? I removed the async_timeout dependency since we aren't using that, and I had already updated the example call, am I missing something?

mezz64 commented 3 years ago

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.

raman325 commented 3 years ago

Done! I also specified a python_requires so pip doesn't install the package on older installations of python

mezz64 commented 3 years ago

@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.

raman325 commented 3 years ago

Great thanks!