Skip to content

Conversation

@lmagyar
Copy link
Contributor

@lmagyar lmagyar commented Jun 17, 2024

Without this, an unsubscribe() followed by a disconnect() always causes an exception: unsubscribe() can't even send the unsubscribe message, OK it's not critical before a close/disconnect to send it (and even wait for the reply), but not causing an unnecessary exception I think is better:

DEBUG:hass_client:Closing client connection
DEBUG:hass_client:Listen completed. Cleaning up
ERROR:asyncio:Task exception was never retrieved
future: <Task finished name='Task-6' coro=<HomeAssistantClient._send_json_message() done, defined at Z:\xxx\.venv\Lib\site-packages\hass_client\client.py:390> exception=NotConnected()>
Traceback (most recent call last):
  File "Z:\xxx\.venv\Lib\site-packages\hass_client\client.py", line 396, in _send_json_message
    raise NotConnected
hass_client.exceptions.NotConnected

without this unsubscribe + disconnect causes an exception
'''
DEBUG:hass_client:Closing client connection
DEBUG:hass_client:Listen completed. Cleaning up
ERROR:asyncio:Task exception was never retrieved
future: <Task finished name='Task-6' coro=<HomeAssistantClient._send_json_message() done, defined at Z:\xxx\.venv\Lib\site-packages\hass_client\client.py:390> exception=NotConnected()>
Traceback (most recent call last):
  File "Z:\xxx\.venv\Lib\site-packages\hass_client\client.py", line 396, in _send_json_message
    raise NotConnected
hass_client.exceptions.NotConnected
'''
@marcelveldt
Copy link
Member

I'm not a fan of needing a coroutine to unsubscribe, especially because in 99% of the cases this is done when we want to cleanup. At the same time I wonder if the unsubscribe is even needed because HA already cleans up the sub when the client disconnects.

I'd really like to keep this a simple callback method and not a couroutine

@marcelveldt marcelveldt reopened this Jun 17, 2024
@lmagyar
Copy link
Contributor Author

lmagyar commented Jun 18, 2024

It's your lib. It was just an idea, even myselft wasn't sure it is really needed.

I use unsubscribe, because currently I need only to capture few event's But even in my case when I close the connection, the unsubscribe is already happened.

So you can close or merge, your decision. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants