Closed gpayer closed 10 months ago
I did some digging and I assume the problem is that async_set_hvac_mode
does not have a protection against running two times in parallel.
There should be some hvac_mode_change_in_progress
flag. Until the total system state settles more calls to async_set_hvac_mode
should simply wait.
E.g. method _async_climate_changed
in thermostat_climate.py
can handle the detection when the underlying thermostats are in sync with VTherm. I assume there are similar methods in the other implementations.
Hello @gpayer ,
I had already this issue in the past and have done some improvements to avoid it. See https://github.com/jmcollin78/versatile_thermostat/issues/121, https://github.com/jmcollin78/versatile_thermostat/issues/95.
So I'm very surprise you ran into this case once more.
This is a severe issue than make HA unusable and I will need your help to understand how it is possible.
Are your 3 underlying devices the same brand and type ? underlying_climate_0: climate.thermostat_wohnzimmer_fenster_links underlying_climate_1: climate.thermostat_wohnzimmer_fenster_rechts underlying_climate_2: climate.thermostat_wohnzimmer_mitte
or have you different climate of different brands with different behavior ?
In there are not the same, I guess this is possible to ran into this case and I would recommend to build 3 VTherm and not only one with 3 devices. I know it is annoying but before I (with your help) find the case, this could takes a long time.
I will have more investigation this week end so be patient please (and maybe build 3 VTherm while waiting for a complete fix).
I see you are in 5.2.0. Can you do the further test in 5.2.1 ? This will be easier to map the code with your logs.
As soon as HACS offer the new version, I will upgrade. The thermostats are all the same brand and type and firmware version (Bosch BTH-RA). Thanks for the idea to use 3 VTherms, I might try that! VTherms with only one underlying device seem to be unaffected by this bug.
For joining the debugging and development effort, do you have any good ideas how to that? Like, are there mockup climate devices I can integrate into a test setup?
5.2.1 have been released this week-end. It should definitively be available now.
For joining the debugging and development effort, do you have any good ideas how to that? Like, are there mockup climate devices I can integrate into a test setup?
Yes. There is full unitary tests framework you can have a look. It is in the dir /tests.
If you want to start slowy, try to look at test_start.py
which is basic start of VTherm with mocks.
Mocks are in the tests/commons.py.
Of course the first thing you have to do is to fork/clone the repo on the main branch, start devcontainer and run all the tests with the test explorator:
All should be green before you start something.
Feel free to ask more questions if needed.
I solved the 5.2.0/5.2.1 mystery, in github there is a 5.2.1 tag, but the manfest.json included in this commit still has version 5.2.0 in it.
I solved the 5.2.0/5.2.1 mystery, in github there is a 5.2.1 tag, but the manfest.json included in this commit still has version 5.2.0 in it.
And HACS don't see the change ? So nobody have installed the 5.2.1 👎
I will fix that
Should be a 5.2.2 visible.
After looking more in details of your case, here is what I saw:
2024-01-11 06:33:16.073
Thermostat Wohnzimmer - Set hvac mode: heat
Thermostat Wohnzimmer - Calling ThermostatClimate._send_regulated_temperature force=False
Thermostat Wohnzimmer - regulation calculation will be done
Thermostat Wohnzimmer - Regulated temp have changed to 23.0. Resend it to underlyings
Thermostat Wohnzimmer - Sending event EventType.HVAC_MODE_EVENT with data: {'hvac_mode': <HVACMode.HEAT: 'heat'>}
Thermostat Wohnzimmer - Underlying climate changed. Event.new_hvac_mode is off, current_hvac_mode=heat, new_hvac_action=idle, old_hvac_action=idle
Thermostat Wohnzimmer - underlying event is received less than 10 sec after command. Forget it to avoid loop
Then Vtherm should switch to off to follow the underlying state change. But because hvac_mode have been manually set to heat just before, this return is ignored (this is the throttle feature I speak above).
So what is weird here is that VTherm send hvac_mode heat and the underlying says "ok I'm off". This is the first point (but correctly managed by the throttling feature).
One second after, you send an hvac_off off command (I guess you change your mind or something like that):
2024-01-11 06:33:17.741 INFO (MainThread) [custom_components.versatile_thermostat.base_thermostat] VersatileThermostat-V Thermostat Wohnzimmer - Set hvac mode: off
and then it the mess because some all of the underlying are not synchronized (some are managing the first hvac_heat command I guess):
2024-01-11 06:33:35.710 INFO (MainThread) [custom_components.versatile_thermostat.thermostat_climate] Thermostat Wohnzimmer - Underlying climate changed. Event.new_hvac_mode is heat
VersatileThermostat-V Thermostat Wohnzimmer - Underlying climate changed. Event.new_hvac_mode is heat,
VersatileThermostat-V Thermostat Wohnzimmer - Underlying climate changed. Event.new_hvac_mode is off,
VersatileThermostat-V Thermostat Wohnzimmer - Underlying climate changed. Event.new_hvac_mode is heat,
3 are in heat and one is off. And because 06:33:35 is > 06:33:17 + 10sec, this return is taken into account. So VTherm takes this change as it have been done directly on underlying, and the loop starts.
The main reason is that your underlying react to the second hvac_off command with 18 sec late which is > of the 10 sec of throttling and are not synchronized.
On the other hand, I've got some users that are annoying by this 10 sec throttling because its preventing from changing the target temperature on a physical thermostat (see #331).
If you look at the code at thermostat_climate.py
line 661, you will see that:
# Issue #120 - Some TRV are changing target temperature a very long time (6 sec) after the change.
# In that case a loop is possible if a user change multiple times during this 6 sec.
This is exactly your case but with 18 sec and not 6 sec. I suppose this is because of the hvac_heat just one second before the hvac_off command from HA. TRV are long to change their internal state.
Maybe I should ignore command from HA if all underlyings are not synchronized (which means a previous command is running).
Just a quick reply from my phone: yes, this is exactly what I was thinking about. hvac_mode should not be derived from the underlyings. Unless maybe configured to do so.
It would be better to have a master hvac_mode and a derived effective_hvac_mode from the underlyings. The latter one can be used for the frontend visualization.
With temperature this is much less of a problem, this can actually be derived from the underlyings.
Waiting until all underlyings are in sync is another important safety layer.
Thanks for your effort, I really appreciate it!
Jean-Marc Collin @.***> schrieb am Sa., 13. Jan. 2024, 10:21:
If you look at the code at thermostat_climate.py line 661, you will see that:
Issue #120 - Some TRV are changing target temperature a very long time (6 sec) after the change.
In that case a loop is possible if a user change multiple times during this 6 sec.
This is exactly your case but with 18 sec and not 6 sec. I suppose this is because of the hvac_heat just one second before the hvac_off command from HA. TRV are long to change their internal state.
Maybe I should ignore command from HA if all underlyings are not synchronized (which means a previous command is running).
— Reply to this email directly, view it on GitHub https://github.com/jmcollin78/versatile_thermostat/issues/334#issuecomment-1890391987, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANIEFT2WNYU5GLID45QTDDYOJGYDAVCNFSM6AAAAABBWXGAOCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJQGM4TCOJYG4 . You are receiving this because you were mentioned.Message ID: @.***>
Hello @gpayer,
I try a fix that should work and robustify once more the loop issue. This is embedded in this release: https://github.com/jmcollin78/versatile_thermostat/releases/tag/5.2.3.beta1
Can you please give it a try because I cannot reproduce this in my environment ?
To be significant, you should:
logger:
default: info
logs:
custom_components.versatile_thermostat: debug
over_climate
with multiple underlying climate entities,If this loop one again (always possible), you can just reload all VTherm entities with the Dev Tools / Reload configuration / Versatile Thermostat. It will stop the loop.
After the test (ok or not), I would you to send me the logs so that I will have the proof that the bug is now fixed.
Thanks a lot for your help !
After the test don't forget to put the log level back to INFO, VTherm is very verbose in debug mode.
Ok, I'll test it, but due to external circumstances, I won't be able to test it until this evening or tomorrow.
And one stupid question, how can I upgrade to a beta version?
Thank you!
Jean-Marc Collin @.***> schrieb am Sa., 13. Jan. 2024, 12:39:
Hello @gpayer https://github.com/gpayer,
I try a fix that should work and robustify once more the loop issue. This is embedded in this release: https://github.com/jmcollin78/versatile_thermostat/releases/tag/5.2.3.beta1
Can you please give it a try because I cannot reproduce this in my environment ?
To be significant, you should:
- put the Versatile Thermostat log level to debug (in your configuration.yaml add something like:
logger: default: info logs: custom_components.versatile_thermostat: debug
- have a VTherm over_climate with multiple underlying climate entities,
- stop the VTherm (with the HA interface),
- immedialty after stop, try to set the Heat mode (with the HA interface)
If this loop one again (always possible), you can just reload all VTherm entities with the Dev Tools / Reload configuration / Versatile Thermostat. It will stop the loop.
After the test (ok or not), I would you to send me the logs so that I will have the proof that the bug is now fixed.
Thanks a lot for your help !
— Reply to this email directly, view it on GitHub https://github.com/jmcollin78/versatile_thermostat/issues/334#issuecomment-1890426528, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANIEFS6ELZDR6M6BHYUTBDYOJW6JAVCNFSM6AAAAABBWXGAOCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJQGQZDMNJSHA . You are receiving this because you were mentioned.Message ID: @.***>
In hacs you have to select the integration click on the menu and select 'download' (retélécharger in French). Then you will be able to check 'Display beta release' and to select the beta release.
Tomorrow this will be perfect.
Thank you for your help.
I was able to trigger your new failsafe by switching the VTherm wildly between heat and off, so everything worked very well! After this the underlying thermostats are not in sync, but this is perfectly fine as it is the expected behaviour. It's very easy to fix this via the app.
Your code change is very nice as well, if we were in the same team and I were to do the code review, I would merge it without further discussion. :smiley:
Thank you for your fine report !
If you want, we could be in the same team and if think so I would be please to propose a review to reviewer. I'm alone to code and review which is certainly not a good practice.
I have a major enhancement running (central boiler control) and then I will release hopefully next week. Keep you beta until the release.
Thank you for your fine report !
If you want, we could be in the same team and if think so I would be please to propose a review to reviewer. I'm alone to code and review which is certainly not a good practice.
Sure, you can add me as reviewer!
Version of the custom_component
5.2.0
Configuration
My VTherm attributes are the following:
Describe the bug
If a VTherm controlling multiple climate devices is turned on (hvac_mode heat) and off (hvac_mode off) again very quickly, it enters an endless loop turning itself on and off all the time and also sending endless on/off commands to the underlying climate devices.
I'm trying to: Turn off the VTherm immediately after I turned it on accidently.
And I expect: The VTherm must stay in in the state I set it to, in this case off.
But I observe this .... And endless loop of the VTherm turning itself on then off again. The only solution is then to deactivate it, wait until the command storm disappears (restarting zigbee2mqtt and mosquito might have helped as well) and then activate it again.
My idea for a solution would be
And finally: never blast out commands without throttling! There really should be a failsafe, so if the VTherm finds itself sending out commands way too quickly, then it should just deactivate itself.
I read the documentation on the README.md file and I don't find any relevant information about this issue.
Debug log