I/O blocking in the event loop #3
Comments
|
Unfortunately not. I'm a user, not a developer. @bdraco has been the driving push behind this, he may have some pointers for you. |
|
The best way to fix it would be to break apart the lib a bit more to avoid the circular imports and make those top level imports so its all loaded at import time which automatically runs in the executor |
|
I believe it should have - not sure how to test / confirm to close this one out |
|
Well something has changed, not necessarily for the better though: |
|
Not seeing anything like that in my logs and I have 3 instances running against my controller (dev/test/prod) There are some inconsistencies between how the different models work so I can only test with the Airbase models Slow updates doesn't answer if IO Blocking and circular imports are still an issue though - how did you initially identify that as an issue? |
|
I didn't. bdraco did (see first post). |
@tomlut Which version of Home Assistant did you use to test? AFAIK the changes which have been made for this issue will only be released with Home Assistant 2024.8 |
|
Ah, I see. I'm still on 2024.7. Was going to have a go at the beta but did not get time. |
|
Just updated to 2024.8. Now it fails to connect: Failed setup, will retry: Cannot connect to host 10.1.1.18:443 ssl:default [[SSL: UNSAFE_LEGACY_RENEGOTIATION_DISABLED] unsafe legacy renegotiation disabled (_ssl.c:1000 Can connect fine in the Daikin phone app. Tried removing the integration and it was discovered before I got around to adding it again, however when I click the configure button and submit:
|
Similar fix for another lib assuming you need to turn on legacy: |
|
It's like it is trying to connect using SSL when it is not a secure connection: Logger: pydaikin.daikin_base Exception in TaskGroup: Cannot connect to host 10.1.1.18:80 ssl:default [Connect call failed ('10.1.1.18', 80)] |
For what it's worth, I'm seeing the same issue, sadly. |
|
Could you please create a new issue for that, maybe both here and in the Home Assistant repository? That new issue has nothing to do with this one here. edit: There is already an issue in the Home Assistant repository: home-assistant/core#123160 |
@fredrike can we implement this and release a new version that supports fallback to http |
|
FWIW I see that some work has been done to add ssl context that sets some things in the past. pydaikin/pydaikin/daikin_brp072c.py Lines 24 to 28 in cda9a0f However I do no see anywhere in the code this context being referenced/used: # git --no-pager grep _ssl_context
pydaikin/daikin_brp072c.py: self._ssl_context = ssl.create_default_context(ssl.Purpose.SERVER_AUTH)
pydaikin/daikin_brp072c.py: self._ssl_context.options |= 0x4
pydaikin/daikin_brp072c.py: self._ssl_context.check_hostname = False
pydaikin/daikin_brp072c.py: self._ssl_context.verify_mode = ssl.CERT_NONEWhen patching the HA side of things like this: session = async_get_clientsession(hass)
session._connector._ssl.options |= 0x4
session._connector._ssl.check_hostname = False
session._connector._ssl.verify_mode = ssl.CERT_NONEThe connection succeeds again, however I'm now getting a 403 forbidden error. I currently miss to see how these things could be related but people with more experience on the daikin local API might know more about it. Just for completion the 403 happens when the UPDATE The 403 is coming from the fact that the # git --no-pager grep _headers
pydaikin/daikin_brp072c.py: self._headers = {"X-Daikin-uuid": self._uuid}Dirty patching the async with self.session.get(
f'{self.base_url}/{path}', params=params, headers=getattr(self, '_headers', None)
) as response:Long story short:
|
|
I tried this fix but get the following error in the logs
|
The solution is mentioned in the error, you need to import the ssl module. Put |
|
Yup, that worked. That 3rd step made everything work. For anyone trying this, be careful not to change anything else including indentation of other lines. Not being a python native, it wasn't obvious. |
|
If anyone has the knowledge to build the package locally and test the above, then submit a PR for a fix that would be great. I can patch with the above steps but I don't have this model to test, and won't be near a PC for another couple of days either. Only @fredrike can package for pip and does not have a unit for testing of these fixes either. I suspect that the units could also have SSL enabled as a fix as whoever added the forced SSL assumably was using SSL. |
|
There is already a PR: #13 |
That's right I actually don't have any Daikin devices at all anymore. I would be happy to add more maintainers to
I've merged and pushed a new version now. |
|
Looks like that bump / PR has broken more devices (including ones that were already working) in 2024.8.1 Haven't looked at the content of the PR - but probably just need to set a default as not all controllers work the same way. EDIT: Happy to take a look at fixing but not near my machine for another 12-18 hours minimum
|
|
All my daikin units seemed to have stopped working this morning (I also updated to 2024.8.1, not sure if time coincides exactly though). |
I got the same here. But this started when I went from 2024.8.0 to 2024.8.1 |
|
Rolling back to 2024.8.0 fixed it for me too. |
|
It all depends on the controller you have, they all have different api requirements.
2024.8.0 saw a dependancy version bump after considerable changes to the upstream library, so any downgrade 2024.7.x will drop the dependency to the old library |
|
I loaded up the changed integration as a custom component and it has been working well enough for both my ducted and split system. I say "well enough" as while they operate as expected I do see this in the logs: |
|
@tomlut are these errors new or have you seen them before? This is the changes that seems to have introduced the shit-storm with errors: v2.11.1...v2.13.4 |
|
The exception is new. I've had the update taking over... since forever. Though not quite this bad. |
Ok, please create a new issue with the exception here and I'll look into it. Contact me on discord or in the HA-forum so we can test this before releasing another version. |
I have been ashed to post this here. See home-assistant/core#114345
As per private discussion with bdraco about relatively long Daikin integration start up times, he discovered that the Daikin integration does I/O blocking in the event loop:
... it does blocking I/O in the event loop
https://bitbucket.org/mustang51/pydaikin/src/45f5ead619d861727626a7ab0522a0c18f00e0ba/pydaikin/daikin_base.py#lines-59
these imports do blocking I/O in the event loop
creating an ssl context does blocking I/O in the event loop https://bitbucket.org/mustang51/pydaikin/src/45f5ead619d861727626a7ab0522a0c18f00e0ba/pydaikin/daikin_brp072c.py#lines-24 (edited)
The text was updated successfully, but these errors were encountered: