Skip to content
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

potential fix for duplicated instances on update #1231

Merged

Conversation

schummar
Copy link
Contributor

@schummar schummar commented Jan 5, 2024

Motivation:

After updating a Better Thermostat instance, two instances of the control logic seem to be running. That creates problems like target temperature updates on the original TRV being recognized as external input and therefore used as new target temperature for BT. Hard to say if this is the only cause for all those "Target temperature changes randomly" issues, but at least that's what's causing it for me.

After adding more debug logs, I found out that the config_entry_update_listener callback is actually called twice in quick succession. No idea why this is, some Home Assistant logic triggers this I guess. I could reliably observe this chain of events:

  1. Update config in the UI
  2. config_entry_update_listener is called
  3. async_unload_entry is called
  4. config_entry_update_listener is called
  5. async_setup_entry is called
  6. async_unload_entry is called
  7. async_unload_entry is called
  8. "ValueError: Config entry was never loaded!"
  9. async_setup_entry is called
  10. async_setup_entry is called
  11. "ValueError: Config entry has already been setup!"
  12. "startup completed"
  13. "startup completed"

As you can see, some beautiful race conditions right there.

Changes:

  1. Use a lock in config_entry_update_listener to prevent to updates overlapping. That was why unload and setup were wildly mixed, causing exceptions.
  2. Removed the async_unload_entry and async_setup_entry calls from config_entry_update_listener. They are already called during hass.config_entries.async_reload. That gets rid of some duplicate unloads/setups.
  3. The startup function is running asynchronously. That means, if the entify is removed during its execution, a bunch of listeners could be attached after removal. The listeners are then not removed anymore, because async_on_remove has no effect, because the entity is already removed. To solve this, I added a check for removal before attaching the listeners.

I tested the changes and they do solve the problem. I am a noob with Python and HA however, so my testing was a bit hacky with me changing files directly in my running HA instance. I have extracted the changes into this PR, but did/could not test them in context of the newest version of the code.
There might be better or cleaner ways to achieve what I did, so feel free to use my changes as inspiration and implement them properly. 😉

Related issue (check one):

Checklist (check one):

  • I did not change any code (e.g. documentation changes)
  • The code change is tested and works locally.

Test-Hardware list (for code changes)

HA Version: 2023.12.1
Zigbee2MQTT Version: None, I use ZHA
TRV Hardware: FRITZ!DECT 301

New device mappings

  • I avoided any changes to other device mappings
  • There are no changes in climate.py

@KartoffelToby
Copy link
Owner

@schummar Thanks for your work!, looks good to me, will be in the next beta 1.5.0 iteration of Beta 4 for testing

@KartoffelToby KartoffelToby merged commit c1ec7f8 into KartoffelToby:master Jan 7, 2024
2 of 3 checks passed
@schummar schummar deleted the fix/duplicateInstanceOnUpdate branch January 7, 2024 20:32
@pascaltsc
Copy link

Hello,

Thank you for your work.
Unfortunately the fix doesn't work for me.
If you change it manually, the set point jumps back to 30°C

Attached is the log file

2024-01-08 06:29:51.144 DEBUG (MainThread) [custom_components.better_thermostat.climate] better_thermostat myBetterTest.WZ: HA set target temperature to 23.0 & None
2024-01-08 06:29:51.146 DEBUG (MainThread) [custom_components.better_thermostat.utils.helpers] better_thermostat myBetterTest.WZ: climate.wohnzimmer / heating_power_valve_position - temp diff: 2.7 - heating power: 0.01 - expected valve position: 100%
2024-01-08 06:29:51.146 DEBUG (MainThread) [custom_components.better_thermostat.calibration] better_thermostat myBetterTest.WZ: climate.wohnzimmer - new setpoint calibration: 30.0 | external_temp: 20.3, target_temp: 23.0, trv_temp: 20.3
2024-01-08 06:29:51.147 DEBUG (MainThread) [custom_components.better_thermostat.utils.controlling] better_thermostat myBetterTest.WZ: TO TRV set_temperature: climate.wohnzimmer from: 20.0 to: 30.0
2024-01-08 06:29:51.251 DEBUG (MainThread) [custom_components.better_thermostat.utils.controlling] better_thermostat myBetterTest.WZ: climate.wohnzimmer / check_target_temp / _last: 30.0 - _current: 20.0
2024-01-08 06:30:53.127 DEBUG (MainThread) [custom_components.better_thermostat.climate] better_thermostat myBetterTest.WZ: HA set target temperature to 19.0 & None
2024-01-08 06:30:53.128 DEBUG (MainThread) [custom_components.better_thermostat.calibration] better_thermostat myBetterTest.WZ: climate.wohnzimmer - new setpoint calibration: 19.0 | external_temp: 20.3, target_temp: 19.0, trv_temp: 20.3
2024-01-08 06:30:53.128 DEBUG (MainThread) [custom_components.better_thermostat.utils.controlling] better_thermostat myBetterTest.WZ: TO TRV set_temperature: climate.wohnzimmer from: 30.0 to: 19.0
2024-01-08 06:30:53.231 DEBUG (MainThread) [custom_components.better_thermostat.utils.controlling] better_thermostat myBetterTest.WZ: climate.wohnzimmer / check_target_temp / _last: 19.0 - _current: 30.0
2024-01-08 06:30:57.496 DEBUG (MainThread) [custom_components.better_thermostat.climate] better_thermostat myBetterTest.WZ: HA set target temperature to 20.0 & None
2024-01-08 06:30:57.497 DEBUG (MainThread) [custom_components.better_thermostat.calibration] better_thermostat myBetterTest.WZ: climate.wohnzimmer - new setpoint calibration: 20.0 | external_temp: 20.3, target_temp: 20.0, trv_temp: 20.3
2024-01-08 06:30:57.497 DEBUG (MainThread) [custom_components.better_thermostat.utils.controlling] better_thermostat myBetterTest.WZ: TO TRV set_temperature: climate.wohnzimmer from: 19.0 to: 20.0
2024-01-08 06:30:58.619 DEBUG (MainThread) [custom_components.better_thermostat.utils.controlling] better_thermostat myBetterTest.WZ: climate.wohnzimmer / check_target_temp / _last: 20.0 - _current: 20.0
2024-01-08 06:31:23.980 DEBUG (MainThread) [custom_components.better_thermostat.climate] better_thermostat myBetterTest.WZ: HA set target temperature to 23.0 & None
2024-01-08 06:31:23.981 DEBUG (MainThread) [custom_components.better_thermostat.utils.helpers] better_thermostat myBetterTest.WZ: climate.wohnzimmer / heating_power_valve_position - temp diff: 2.7 - heating power: 0.01 - expected valve position: 100%
2024-01-08 06:31:23.981 DEBUG (MainThread) [custom_components.better_thermostat.calibration] better_thermostat myBetterTest.WZ: climate.wohnzimmer - new setpoint calibration: 30.0 | external_temp: 20.3, target_temp: 23.0, trv_temp: 20.3
2024-01-08 06:31:23.981 DEBUG (MainThread) [custom_components.better_thermostat.utils.controlling] better_thermostat myBetterTest.WZ: TO TRV set_temperature: climate.wohnzimmer from: 20.0 to: 30.0
2024-01-08 06:31:24.113 DEBUG (MainThread) [custom_components.better_thermostat.utils.controlling] better_thermostat myBetterTest.WZ: climate.wohnzimmer / check_target_temp / _last: 30.0 - _current: 20.0

@schummar
Copy link
Contributor Author

schummar commented Jan 8, 2024

It's entirely possible that there are more issues that cause related problems. Judging from the logs however my impression is that you are observing the "temperature based calibration". E.g. the BT is set to 23°. The real TRV might be set to a completely different target temperature to quickly reach the intended temperature. It depends on you calibration type and mode. See the docs page:
image

More here: https://better-thermostat.org/configuration and https://better-thermostat.org/qanda/modes

@pascaltsc
Copy link

Thanks for checking the logs.
So far I have used the AI-Based option.
Only changing from AI to normal does not cause the TRV to jump when changed manually.

Is the behavior normal for AI-Based?

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.

Target temperature in GUI changes by itself randomly
3 participants