home-assistant-libs / pychromecast

Library for Python 3 to communicate with the Google Chromecast.
MIT License
2.54k stars 379 forks source link

Getting error: module 'select' has no attribute 'poll' #681

Open KnutssonDevelopment opened 1 year ago

KnutssonDevelopment commented 1 year ago

With pychromecast >= 13.0.2 I am getting an error:

Traceback (most recent call last): File "C:\Users\bfk\PycharmProjects\SmartImage\venv\lib\site-packages\pychromecast\socket_client.py", line 538, in run if self.run_once(timeout=POLL_TIME_BLOCKING) == 1: File "C:\Users\bfk\PycharmProjects\SmartImage\venv\lib\site-packages\pychromecast\socket_client.py", line 572, in run_once poll_obj = select.poll() AttributeError: module 'select' has no attribute 'poll'

pip freeze absl-py==1.3.0 astunparse==1.6.3 async-timeout==4.0.2 cachetools==5.2.0 casttube==0.2.1 certifi==2022.12.7 chardet==5.1.0 charset-normalizer==3.0.1 docutils==0.19 flatbuffers==22.12.6 gast==0.5.3 google-auth==2.15.0 google-auth-oauthlib==0.4.6 google-pasta==0.2.0 grpcio==1.51.1 h5py==3.7.0 idna==3.4 ifaddr==0.2.0 keras==2.11.0 Keras-Preprocessing==1.1.2 Kivy==2.1.0 kivy-deps.angle==0.3.3 kivy-deps.glew==0.3.1 kivy-deps.sdl2==0.5.1 Kivy-Garden==0.1.5 kivymd==1.1.1 libclang==14.0.6 Markdown==3.4.1 MarkupSafe==2.1.1 numpy==1.23.5 oauthlib==3.2.2 opencv-python==4.6.0.66 opt-einsum==3.3.0 packaging==22.0 Pillow==9.3.0 protobuf==4.21.11 pyasn1==0.4.8 pyasn1-modules==0.2.8 PyChromecast==13.0.2 Pygments==2.13.0 pyparsing==3.0.9 pypiwin32==223 python-engineio==4.3.4 pywin32==305 requests==2.28.1 requests-oauthlib==1.3.1 rsa==4.9 screeninfo==0.8.1 six==1.16.0 tensorboard==2.10.1 tensorboard-data-server==0.6.1 tensorboard-plugin-wit==1.8.1 tensorflow==2.10.0 tensorflow-estimator==2.11.0 tensorflow-io-gcs-filesystem==0.28.0 termcolor==2.1.1 typing_extensions==4.4.0 urllib3==1.26.13 Werkzeug==2.2.2 wrapt==1.14.1 zeroconf==0.39.4

Python version 3.10.8

The error starts on repeat after the line: "cast.wait()" in the example code.

strunker commented 1 year ago

Running into the same problem on 13.0.2

Sucked for me because I haven't updated this lib since Jan in my own project, and randomly now ran into this. 13.0.1 doesn't have the problem. And as @KnutssonDevelopment alluded to there is some type of infinite loop scenario that occurs here and it never seems to end.

@emontnemery @logan893 looks like this was broken in the below commit. Not entirely sure what is going on in this particular method but this is when select.poll() was introduced back in to the code. A bit comical because the method is called run_once() yet is stuck in some type of infinite loop scenario.

https://github.com/home-assistant-libs/pychromecast/commit/34611d2a5eebd55d5f7fc9556752fbf8166f0636

logan893 commented 1 year ago

Select.poll is according to the python documentation not available for all operating systems. I'd guess it's not available for Windows.

For compatibility, this code probably needs to check whether select.poll is present and if not fall back on the select.select function call.

As for the infinite loop, I think this is what I experienced also when select.select was failing. There's not sufficient graceful failure and retry handling here.

strunker commented 1 year ago

Select.poll is according to the python documentation not available for all operating systems. I'd guess it's not available for Windows.

For compatibility, this code probably needs to check whether select.poll is present and if not fall back on the select.select function call.

As for the infinite loop, I think this is what I experienced also when select.select was failing. There's not sufficient graceful failure and retry handling here.

So what you are saying is this method was broken for linux, so it was patched for linux, which then broke it for windows. We all know windows is the master race, soooo.

I can take a crack at fixing this if one of you dont get to it first.. While Im at it ill patch the annoying warnings out of dial.py :)

strunker commented 1 year ago

@logan893 fixed this in this pull https://github.com/home-assistant-libs/pychromecast/pull/690 Didnt fix ths crazy loop scenario because I dont really understand where thats coming from. But I added OS detection in as discussed here...

strunker commented 1 year ago

@logan893 @emontnemery this should probably be fixed with some type of priority. Not everyone runs HA, nor this library for that matter, from Linux or the HA yellow/blue, etc. This is broken in the live version for literally anyone running this on Windows. And broken in a pretty bad way with the loop problem.

Can someone please review my PR? Even in the short term, until something more robust can be implemented, it would likely be helpful to use it and it does work to solve this problem.

logan893 commented 1 year ago

@strunker While Windows may or may not be the only platform where exceptions need to be made (I really don't know), I'd still prefer to avoid OS name check like that and instead have a global check for whether the select module has "poll".

import select _has_poll = hasattr(select, "poll")

Then when to choose which to use, just refer to the _has_poll variable.

if _has_poll:

use select.poll

else:

use select.select

Does this work for you?

strunker commented 1 year ago

@strunker While Windows may or may not be the only platform where exceptions need to be made (I really don't know), I'd still prefer to avoid OS name check like that and instead have a global check for whether the select module has "poll".

import select _has_poll = hasattr(select, "poll")

Then when to choose which to use, just refer to the _has_poll variable.

if _has_poll: # use select.poll else: # use select.select

Does this work for you?

@logan893 I fixed in the same PR in this edit seen below. Let me know what you think. I agree though with your suggestion makes sense to not bother with OS check when we can just check for the method.

https://github.com/home-assistant-libs/pychromecast/pull/690/commits/94440df273e95d98a4b3fa277cfcdc5f51d9181f

logan893 commented 1 year ago

@strunker I'd prefer something like my example where the result is stored once and only the variable with the result is checked each time run_once is executed. The result will not change, so going through the check every time is an unnecessary waste of CPU resources.

Checking for "poll" in select using dir() or hasattr() will likely result in the same outcome in this scenario, though my understanding is that dir() is intended for interactive prompt use and not for this type of application. dir() is not necessarily checking the same attributes, the result could change between releases (according to the documentation, but unlikely in this case), and is not as fast.

strunker commented 1 year ago

Nev

@strunker I'd prefer something like my example where the result is stored once and only the variable with the result is checked each time run_once is executed. The result will not change, so going through the check every time is an unnecessary waste of CPU resources.

Checking for "poll" in select using dir() or hasattr() will likely result in the same outcome in this scenario, though my understanding is that dir() is intended for interactive prompt use and not for this type of application. dir() is not necessarily checking the same attributes, the result could change between releases (according to the documentation, but unlikely in this case), and is not as fast.

Understood on the suggestion and it being more optimized. I added the "hasattr" check into the class constructor. I suppose this could also be some type of global as well, and likely be determined outside of the class if you wanted. I know a lot of people shy away from globals but may work well for this purpose. As it stands in the below edit, it will do the evaluation once per object of the class.

https://github.com/home-assistant-libs/pychromecast/commit/f0801cee753b4cc84638a5a6a0032d3869736edc

I also implemented as a global here, so just let me know the preference. This last edit here is likely the most efficient. It doesnt really need to be checked each time the class is instantiated, as you had said.

https://github.com/home-assistant-libs/pychromecast/commit/32b7b454fc73619570d4fb9217c8dc6877d343f5

Thanks!

logan893 commented 1 year ago

@strunker Great work! I'll leave it up to @emontnemery to pick the desired solution.

KnutssonDevelopment commented 1 year ago

Any news as to when this could be fixed?

strunker commented 1 year ago

Any news as to when this could be fixed?

All up to @emontnemery now. To approve the PR or not. Only he can answer your question.

elibroftw commented 1 year ago

Getting this issue too

Untit1ed commented 1 year ago

13.0.4, still seeing the issue. Is there a workaround for the library users using the latest version?

strunker commented 1 year ago

13.0.4, still seeing the issue. Is there a workaround for the library users using the latest version?

This was merged as of yest I believe so latest version should have the fix.