Closed zollak closed 4 months ago
As I see, this issue relates to aiocomfoconnect.bridge
which has newer version available in its repo. Now, I updated it to the aiocomfoconnect==0.1.9 version on my system with the following steps:
Now, I'm still checking the logs. If it works, I'm going to make a pull request with this change.
Is issue #32 related?
Is issue #32 related?
Hi, as I can see, I had a similar issue. Sensor values not updated consistently over a long period of time. Also, Update for select.comfoairq_balance_mode fails very often. This was described in detail by @JohnyDeejay on his comment here as well. It seems all 3 cases are the same.
Since there are no released versions in the home-assistant-comfoconnect
github repository, I managed to solve the update in the way described above.
The aiocomfoconnect
python module is used in many places in the home-assistant-comfoconnect
repo, and the aiocomfoconnect
0.1.8 version has some modifications/fixes in the 0.1.9 release, so I have decided to force the new version in HA. That's why I've redownloaded the modified repo again, where I have changed the dependency.
The last reboot (not just a restart, but a reboot) in the HA was at 10:00 a.m. today, and I don't have any error messages in the log since then. If it works till tomorrow, I'm going to make a pull request with this change.
I'm just trying to solve the error, like everyone else, and if I can help for @michaelarnauts with this, we'll all be happy :)
The 'ComfoAirQ Airflow Constraint' sensor.comfoairq_airflow_constraint value is flapped in both mobile app and HA. I could not set a specific value for the speed here. Solution: I switched off this sensor and restarted Zehnder. When I restarted it (while it was in shutdown state) I got the same error message as before, but after restarting it went away. Everything seems to be working now.
Is issue #32 related?
Hi, as I can see, I had a similar issue. Sensor values not updated consistently over a long period of time. Also, Update for select.comfoairq_balance_mode fails very often. This was described in detail by @JohnyDeejay on his comment here as well. It seems all 3 cases are the same.
Since there are no released versions in the
home-assistant-comfoconnect
github repository, I managed to solve the update in the way described above.The
aiocomfoconnect
python module is used in many places in thehome-assistant-comfoconnect
repo, and theaiocomfoconnect
0.1.8 version has some modifications/fixes in the 0.1.9 release, so I have decided to force the new version in HA. That's why I've redownloaded the modified repo again, where I have changed the dependency.The last reboot (not just a restart, but a reboot) in the HA was at 10:00 a.m. today, and I don't have any error messages in the log since then. If it works till tomorrow, I'm going to make a pull request with this change.
I'm just trying to solve the error, like everyone else, and if I can help for @michaelarnauts with this, we'll all be happy :)
So you have the soluton, great to hear that. Can't wait to tried it out.
In the log I frequently see this message:
WARNING (MainThread) [homeassistant.components.automation.xxxxxx] xxxxxx-medium: Already running
E.g. action in my automation:
- service: fan.set_percentage
target:
entity_id: fan.comfoairq
data:
percentage: 66.66
mode: single
Is this related to this issue?
In the log I frequently see this message:
WARNING (MainThread) [homeassistant.components.automation.xxxxxx] xxxxxx-medium: Already running
E.g. action in my automation:
- service: fan.set_percentage target: entity_id: fan.comfoairq data: percentage: 66.66 mode: single
Is this related to this issue?
Hi, i think it is an another one. I haven't seen yet this issue in my log.
Is issue #32 related?
Hi, as I can see, I had a similar issue. Sensor values not updated consistently over a long period of time. Also, Update for select.comfoairq_balance_mode fails very often. This was described in detail by @JohnyDeejay on his comment here as well. It seems all 3 cases are the same.
Since there are no released versions in the
home-assistant-comfoconnect
github repository, I managed to solve the update in the way described above.The
aiocomfoconnect
python module is used in many places in thehome-assistant-comfoconnect
repo, and theaiocomfoconnect
0.1.8 version has some modifications/fixes in the 0.1.9 release, so I have decided to force the new version in HA. That's why I've redownloaded the modified repo again, where I have changed the dependency.The last reboot (not just a restart, but a reboot) in the HA was at 10:00 a.m. today, and I don't have any error messages in the log since then. If it works till tomorrow, I'm going to make a pull request with this change.
I'm just trying to solve the error, like everyone else, and if I can help for @michaelarnauts with this, we'll all be happy :)
When will your fix be available for everyone? What do we need to do?
I have same issue, repeating pattern of 4 messages each 5 minutes:
WARNING: The connection was closed. WARNING: Timeout while waiting for response from bridge ERROR: Update for select.comfoairq_balance_mode fails WARNING: Timeout while waiting for response from bridge
this is related to this #33, issue #36 a probably as well same issue is causing issue described in #40.
2024-03-26 14:59:55.054 INFO (MainThread) [aiocomfoconnect.bridge] The connection was closed.
2024-03-26 15:00:12.577 WARNING (MainThread) [aiocomfoconnect.bridge] Timeout while waiting for response from bridge
2024-03-26 15:00:12.578 ERROR (MainThread) [homeassistant.helpers.entity] Update for select.comfoairq_balance_mode fails
Traceback (most recent call last):
File "/usr/local/lib/python3.12/asyncio/tasks.py", line 520, in wait_for
return await fut
^^^^^^^^^
asyncio.exceptions.CancelledError
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/local/lib/python3.12/site-packages/aiocomfoconnect/bridge.py", line 168, in _send
return await asyncio.wait_for(fut, TIMEOUT)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.12/asyncio/tasks.py", line 519, in wait_for
async with timeouts.timeout(timeout):
File "/usr/local/lib/python3.12/asyncio/timeouts.py", line 115, in __aexit__
raise TimeoutError from exc_val
TimeoutError
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 945, in async_update_ha_state
await self.async_device_update()
File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 1262, in async_device_update
await self.async_update()
File "/config/custom_components/comfoconnect/select.py", line 199, in async_update
self._attr_current_option = await self.entity_description.get_value_fn(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.12/site-packages/aiocomfoconnect/comfoconnect.py", line 244, in get_balance_mode
result_06 = await self.cmd_rmi_request(bytes([0x83, UNIT_SCHEDULE, SUBUNIT_06, 0x01]))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.12/site-packages/aiocomfoconnect/bridge.py", line 171, in _send
raise AioComfoConnectTimeout from exc
aiocomfoconnect.exceptions.AioComfoConnectTimeout
2024-03-26 15:00:12.585 WARNING (MainThread) [aiocomfoconnect.bridge] Timeout while waiting for response from bridge
Do we have any information why is it happening. Is something needed to help to troubleshoot it more?
Based on above, it seems there every 5 minutes there is no active connection to the gateway. So either connection is constantly being reseted/droped by gateway, maybe keepalive function doesn't work at all?
I tried to dig through library code. May I please ask @michaelarnauts for validation of my thoughts?
First message which I see in all my logs before exceptions are triggered is something like:
2024-03-23 19:34:28.269 INFO (MainThread) [aiocomfoconnect.bridge] The connection was closed.
It comes from bridge.py
async def _read_messages(self):
....
except IncompleteReadError:
_LOGGER.info("The connection was closed.")
return # Stop the background task
There are also other cases, where reply message is not received from gateway and code is exiting _read_messages() function using return call, terminating async wait in process.
What was your original idea to handle such state correctly? I would assume, that connection should be marked as stalled (self.is_connected()
to return False
) to trigger reconnect automatically at least on the next keepalive:
async def _send(self, request, request_type, params: dict = None, reply: bool = True) -> Message:
"""Sends a command and wait for a response if the request is known to return a result."""
# Check if we are actually connected
if not self.is_connected():
raise AioComfoConnectNotConnected()
home-assistant-comfoconnect/init.py
async def send_keepalive(now) -> None:
"""Send keepalive to the bridge."""
_LOGGER.debug("Sending keepalive...")
try:
# Use cmd_time_request as a keepalive since cmd_keepalive doesn't send back a reply we can wait for
await bridge.cmd_time_request()
# TODO: Mark sensors as available
except (AioComfoConnectNotConnected, AioComfoConnectTimeout):
# Reconnect when connection has been dropped
try:
await bridge.connect(entry.data[CONF_LOCAL_UUID])
except AioComfoConnectTimeout:
_LOGGER.debug("Connection timed out. Retrying later...")
# TODO: Mark all sensors as unavailable
Also please note, that I did't found similar connection checking as exists for keepalive before any other message is send to the gateway.
Different proposal to fix this would be to move connection maintenance to aiocomfoconnect/bridge.py (to check and if failed automatically reestablish connection if possible)
All problems we observe, are IMHO because even when _read_messages exits on error, nothing is done to either mark connection as stale, neither to reestablish it immediately. It seems, that gateway time to time closes or corrupts existing connection, but aiocomfoconnect is not able catch this situation and handle it transparently for home-assistant-comfoconnect without raising bunch of exceptions.
Is there anything I missed (I am not Python expert and even less HA one)? If so, please let me know. Thank you.
So just to put my thoughts into something more material, I was thinking about something like this:
diff --git a/aiocomfoconnect/bridge.py b/aiocomfoconnect/bridge.py
index 5f38432..6f84109 100644
--- a/aiocomfoconnect/bridge.py
+++ b/aiocomfoconnect/bridge.py
@@ -135,7 +135,12 @@ class Bridge:
"""Sends a command and wait for a response if the request is known to return a result."""
# Check if we are actually connected
if not self.is_connected():
- raise AioComfoConnectNotConnected()
+ # Reconnect when connection has been dropped
+ try:
+ await self.connect(self._local_uuid)
+ except:
+ _LOGGER.debug("Connection could not be reestablished")
+ raise AioComfoConnectNotConnected()
# Construct the message
cmd = zehnder_pb2.GatewayOperation() # pylint: disable=no-member
@@ -247,6 +252,7 @@ class Bridge:
except IncompleteReadError:
_LOGGER.info("The connection was closed.")
+ if self._writer:
+ self._writer.close()
return # Stop the background task
except ComfoConnectError as exc:
@michaelarnauts I know, that you are probably busy with other topics, but I would welcome at your guidance. This bug is causing several reported issue #33, #36, #40 and affecting multiple users. Would you like me to provide pull request? Will you be able to merge it (there are some other 2 months old pending and not resolved pull request)? As the issue require to patch your aiocomfoconnect library available trough PyPi, I can't provide simple fork of you comfoconnect repo to be used by HACS to deliver fix.
Please let me know what do you think is the best way to fix the situation. I would prefer to first discuss your original intentions as mentioned and provide fix to you code if possible. Thank you.
I'm terrible sorry for not replying sooner, or following up on the issues. I have a busy daytime job (as a developer), and a busy evening job (as parents).
I do remember I had separated the reconnect logic in one place, bit the details are foggy. I have to go through the code...
I really appreciate the time you take in debugging the issue. I can't look into details right now (makeing food as we speak), but I hope to do so next week...
Hello,
no problem and thank you for reply.
I only found one place where you handle re connection - when sending keepalive in home-assistant-comfoconnect/init.py:
async def send_keepalive(now) -> None:
"""Send keepalive to the bridge."""
_LOGGER.debug("Sending keepalive...")
try:
# Use cmd_time_request as a keepalive since cmd_keepalive doesn't send back a reply we can wait for
await bridge.cmd_time_request()
# TODO: Mark sensors as available
except (AioComfoConnectNotConnected, AioComfoConnectTimeout):
# Reconnect when connection has been dropped
try:
await bridge.connect(entry.data[CONF_LOCAL_UUID])
except AioComfoConnectTimeout:
_LOGGER.debug("Connection timed out. Retrying later...")
# TODO: Mark all sensors as unavailable
The problem is, that it is not covering all cases. If you would like to keep connection handling in the comfoconnect integration and not move it into lover level (library), than you need to to at least:
This is more complex, than implementing reconnect/connection handling/persistence in the library. In such case I would also move keepalive functionality to the library, to keep connection handling at one place.
In addition, I would also check on keepalive function execution. I would suppose, that reconnect function in the send_keepalive
should reestablish closed connection before it will affect reading sensors, but it doesn't seem to be the case. The issue seems to be, that even you are seeing as connection closed in library, asyncio _writer handle is let open, as you are not closing it in the library on the IncompleteReadError
error. So this shall be added as well.
Some random remarks and notes while going over the code real quick.
Initially, I think the comfoconnect class was supposed to do the reconnectiong logic. I can remember adding the _sensors variable to keep track of all sensors that were previously registered, and the connect method should re-register those sensors when it reconnects, otherwise, you won't get any sensor updates when a reconnection happens. It's not implemented like this, I forgot why. Looking at this now, it does seem a better place to do this.
My naming is confusing. I have a bridge and a comfoconnect class. The comfoconnect is supposed to be a higher abstraction above the bridge. The bridge should do low level commands, and the comfoconnect class can provide nice methods to set the speed for example. In the HA integration, i'm using the comfoconnect class, but I've named it bridge.
Hi, no problem I see logic from the traces as well, so no worry about the naming.
The problem is, that there is no connection checking/re-connection implemented in the comfoconnect class of aiocomfoconnect library neither. for example:
async def get_mode(self):
"""Get the current mode."""
result = await self.cmd_rmi_request(bytes([0x83, UNIT_SCHEDULE, SUBUNIT_08, 0x01]))
# 0000000000ffffffff0000000001 = auto
# 0100000000ffffffffffffffff01 = manual
mode = result.message[0]
return VentilationMode.MANUAL if mode == 1 else VentilationMode.AUTO
there are two ways, either do it here for each and every function for any sensors/select/fan/switch state and attribute like this:
async def get_mode(self):
"""Get the current mode."""
+ if not self.is_connected():
+ # Reconnect when connection has been dropped
+ try:
+ await self.connect(self._local_uuid)
+ except:
+ _LOGGER.debug("Connection could not be reestablished")
+ self._writer.close()
+ raise AioComfoConnectNotConnected()
+ try:
+ result = await self.cmd_rmi_request(bytes([0x83, UNIT_SCHEDULE, SUBUNIT_08, 0x01]))
+ except (AioComfoConnectNotConnected, AioComfoConnectTimeout):
+ # Reconnect when connection has been dropped
+ try:
+ await bridge.connect(entry.data[CONF_LOCAL_UUID])
+ except AioComfoConnectTimeout:
+ _LOGGER.debug("Connection timed out. Retrying later...")
+ self._writer.close()
+ return
# 0000000000ffffffff0000000001 = auto
# 0100000000ffffffffffffffff01 = manual
mode = result.message[0]
return VentilationMode.MANUAL if mode == 1 else VentilationMode.AUTO
or better putting this functionality ONLY once into bridge.py either all higher functions like i this example cmd_rmi_request
OR as I originally proposed into lower level function as _send
or _read
. Anyway such functionality is currently non existent neither in bridge neither in comfoconnect class of aiocomfoconnect library.
Again is_connected()
checks shall be either improved or write.close()
has to be implemented in case any command failed - to as least try to reconnect - as seen in my snippet as above. Due to hign level code duplication if implemented in all comfoconnect class functions I would either propose dedicated function or to implement it in the bridge class.
One thing to add. Sensors are reregistered and as well reconnection is triggered. The issue is, that this is only done when due to missing checks and missing exceptions handling HA async call fails, bringing all of this nasty errors into HA log. And also always first call which triggers this issue is NOT executed. (therefore many users are having problem with specific HA entity.
So my proposal is to handle any potential exceptions triggered by the error in communication with bridge (i.e. gateway) transparently for HA - catching and handling exceptions appropriately.
@litinoveweedle I've made a PR here with some changes: https://github.com/michaelarnauts/aiocomfoconnect/pull/23
I'll still need to verify them and test all scenarios, but I think this is the right direction.
Reconnection should happen in the library, but keepalives are still being sent from outside the library. When a keepalive isn't replied to, an exception is returned to the caller, and the library will reconnect itself.
Feel free to take a look!
@michaelarnauts Thank you very much for preparing the PR in such short time. I did uick review and I see some potential edge cases where it IMHO could produce some race condition. Especially when _send() could trigger _disconnect() when _connect() could be already called from _reconnect_loop(). I am also not sure about _send() connection checking.
Closing since I want to migrate all these issues in one issue. See https://github.com/michaelarnauts/home-assistant-comfoconnect/issues/43 for a possible solution and follow-up.
Couple of new issues have appeared in the Logs:
Logger: homeassistant.helpers.entity Source: helpers/entity.py:898 First occurred: January 28, 2024 at 13:13:31 (246 occurrences) Last logged: 11:25:33
Update for select.comfoairq_balance_mode fails
Logger: homeassistant Source: custom_components/comfoconnect/init.py:158 Integration: Zehnder ComfoAir Q First occurred: 09:23:33 (245 occurrences) Last logged: 11:25:33
Error doing job: Task exception was never retrieved
This error originated from a custom integration.
Logger: homeassistant Source: custom_components/comfoconnect/init.py:158 Integration: Zehnder ComfoAir Q First occurred: 09:23:08 (1 occurrences) Last logged: 09:23:08
Error doing job: Task exception was never retrieved