sbidy / wiz_light

A WiZ Light integration for Home Assistant
MIT License
341 stars 68 forks source link

UDP async error #6

Closed sbidy closed 4 years ago

sbidy commented 4 years ago

The UDP connection creates a system timeout error in the HA environment.

sbidy commented 4 years ago

The Exception:

2020-03-16 16:39:48 ERROR (MainThread) [homeassistant.helpers.entity] Update for light.lampe fails
homeassistant    | Traceback (most recent call last):
homeassistant    |   File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 279, in async_update_ha_state
homeassistant    |     await self.async_device_update()
homeassistant    |   File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 459, in async_device_update
homeassistant    |     await self.async_update()
homeassistant    |   File "/config/custom_components/wiz_light/light.py", line 189, in async_update
homeassistant    |     self.update_color()
homeassistant    |   File "/config/custom_components/wiz_light/light.py", line 244, in update_color
homeassistant    |     if self._light.rgb is None:
homeassistant    |   File "/usr/local/lib/python3.7/site-packages/pywizlight/bulb.py", line 164, in rgb
homeassistant    |     resp = self.getState()
homeassistant    |   File "/usr/local/lib/python3.7/site-packages/pywizlight/bulb.py", line 247, in getState
homeassistant    |     return self.sendUDPMessage(message)
homeassistant    |   File "/usr/local/lib/python3.7/site-packages/pywizlight/bulb.py", line 288, in sendUDPMessage
homeassistant    |     data, addr = sock.recvfrom(1024)
homeassistant    | socket.timeout: timed out
angadsingh commented 4 years ago

The problem is that you have classified your methods with async_ (i.e. async_update(), async_turn_on(), async_turn_off()), however it is clearly mentioned in https://developers.home-assistant.io/docs/asyncio_working_with_async that if you are doing so then you async methods should not do any blocking I/O, otherwise they will block the event loop of HA core. It has to be async through and through.

An example of how async HTTP I/O is done by the alexa component: https://github.com/home-assistant/core/blob/a3e250447029388b61b8436fe86ce1a9f829922d/homeassistant/components/alexa/auth.py#L102

socket.sendto is a blocking operation, so is socket.recvfrom: https://stackoverflow.com/questions/40085318/is-it-safe-to-assume-that-socket-sendto-is-non-blocking-operation/40108035#40108035

We'll have to use DatagramTransport.sendto with awaits all the way back to async_update()

Alternatively you can declare your component as synchronous (remove the async_ prefixes) and HA will then spawn threads to do the updates.

The second problem is that there are too many redundant getPilot calls being done for each and every state when a single one could be done (e.g. even getConnection() does it).

angadsingh commented 4 years ago

Something that can be used: https://pypi.org/project/asyncio-dgram/

sbidy commented 4 years ago

Jup ... thats the problem with the async. My UDP implementation is fare away from be non-blocking. I think I'll redefine the method as synchron.

The problem with the getPilot I have to investigate.

angadsingh commented 4 years ago

I'm almost 90% done with an async version of your module. will be sending a pull request soon :)