piekstra / tplink-cloud-api

A Python library to remotely control TP-Link smart home devices using their cloud service - no need to be on the same network as your devices
GNU General Public License v3.0
41 stars 11 forks source link

Fix Jupyter (and other library) compatibility #50

Closed ShayBox closed 2 years ago

ShayBox commented 2 years ago

Hard to explain, but it fixes Jupyter compatibility, and compatibility with any other library that uses asyncio, you can still do the manual disable listed in the readme but this should automatically fix most situations.

ShayBox commented 2 years ago

I would recommend a squash merge if you merge this, I used the web editor so there's a lot of commits

EDIT: I also want to say, that workaround in the readme only fixes the error when loading the library, other functions still use asyncio.run and don't work, this fixes everything.

ShayBox commented 2 years ago

Forgot device_manager.py, now the pr works

piekstra commented 2 years ago

I've not had time to investigate this PR in more detail yet. I want to do some testing myself locally with a jupyter environment to verify. I also am still deciding if this change makes sense from a design standpoint. Wrapping an existing library (asyncio) with custom code to handle a particular edge case when using a separate platform (jupyter) feels dangerous and prone to causing bugs when updating the asyncio dependency.

The provided workaround in the README should work for any functions in this library. E.g.

function_thread = threading.Thread(target=device_manager.any_function_example, args=[some_arg, another_arg])
function_thread.start()
result = function_thread.join()
ShayBox commented 2 years ago

The problem is this doesn't just effect Jupiter, only one library can use asyncio at one time, so if I'm using this with flask, fastapi, discord.py, etc, this one doesn't work.

I'm not sure something this simple really needs asyncio in the first place, but this is the only way I know of to fix this problem.

The right way to fix this would be to remove the sync functions and all uses of asyncio.run, since you're kinda hacking sync functions out of an async library, similar to how discord.py let's you fix it with other libraries

piekstra commented 2 years ago

Evaluating this PR as a solution https://github.com/piekstra/tplink-cloud-api/pull/53

piekstra commented 2 years ago

closed in favor of https://github.com/piekstra/tplink-cloud-api/pull/53

will cut a release shortly

piekstra commented 2 years ago

Compatibility with async libraries is published and available :)

https://github.com/piekstra/tplink-cloud-api/releases/tag/v4.0.0