-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
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
Fix Intellifire UDP timeout #80204
Fix Intellifire UDP timeout #80204
Conversation
a84d1d6
to
54427b7
Compare
54427b7
to
f94bc50
Compare
It looks like the library is doing blocking Ideally this would get converted to an asyncio.Protocol and you can use |
@bdraco - you are a wealth of knowledge... I'll look into how to do that :) |
That being said - in the short term -> Think we could still get this patch in to the core - so I can help a user who can't install the integration because their network is doing something to UDP traffic? |
I think there is a much larger risk of blocking the event loop since if the Here are few examples you can borrow https://github.com/bdraco/discovery30303/blob/master/discovery30303/__init__.py or something more complex: |
The screen logic and unifi-discovery libraries have more examples as well |
@bdraco - Thanks for the pointers. I believe I have it using asyncio fully |
356a38f
to
e58be42
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 👍
@jeeftor Unrelated to this PR, but you can generally switch out |
Proposed change
Update the backing library to version 2.2.1 which uses a correct UDP timeout value for socket receive. It will return an empty discovery list upon timeout - which will then let the user enter an IP of their choice:
All changes can be seen here:
jeeftor/intellifire4py@2.0.1...2.2.1
I've rewritten the AsyncUDPFireplaceFinder class to utilize
asyncio.create_datagram_endpoint
which hopefully will fix the existing bug we ran into.Timeout for fireplace discovery is now changed to 12 seconds.
Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: