-
Notifications
You must be signed in to change notification settings - Fork 15
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
Update for select.comfoairq_balance_mode fails #33
Comments
As I see, this issue relates to
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? |
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 The 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. |
So you have the soluton, great to hear that. Can't wait to tried it out. |
In the log I frequently see this message:
E.g. action in my automation:
Is this related to this issue? |
Hi, i think it is an another one. I haven't seen yet this issue in my log. |
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. this is related to this #33, issue #36 a probably as well same issue is causing issue described in #40.
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:
It comes from bridge.py
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 (
home-assistant-comfoconnect/init.py
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:
|
@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:
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 |
Some random remarks and notes while going over the code real quick.
|
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:
there are two ways, either do it here for each and every function for any sensors/select/fan/switch state and attribute like this:
or better putting this functionality ONLY once into bridge.py either all higher functions like i this example Again |
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: michaelarnauts/aiocomfoconnect#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 #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
The text was updated successfully, but these errors were encountered: