External Sensor updates are not processed in a timely manner

[RubenKelevra]

Description

I've noticed that the sensor updates by my external sensor are not processed in a timely manner, but instead very delayed in some instances, some updates are also skipped completely. This leads to changes in temperature not being correctly processed and thus the TRV not correctly adjusted for long durations.

The external sensor sees:

18.6 °C before
18.7 °C at 19:23:03
19.4 °C at 19:30:58
20.0 °C at 19:41:24

• Better Thermostat is processing the 18.7 °C at 19:41:24 - so with 18 minutes and 21 seconds delay.
• Better Thermostat has never processed the 19.4 °C
• Better Thermostat is processing the 20.0 °C at 19:41:25 - so just one second after the 18.7 °C is processed.

Screenshots

And here is what Better Thermostat does as state changes:

Steps to Reproduce

unknown

Expected behavior:

Better Thermostat should process the temperature as soon as there are updates and should process all incoming values.

Actual behavior:

Better Thermostat delays processing for 18 minutes and 21 seconds delay and skips values completely.

Versions

HA: 2024.11.1
BT: 1.6.1 with PR #1472 and #1473 applied - otherwise unusable.

Additional Information

[folfy]

Yeah something is rly off, will try to look into this, like this can and sure does cause a lot of trouble for some 😐

[folfy]

BT ignores and discards any sensor update within 60s after the last change:

better_thermostat/custom_components/better_thermostat/events/temperature.py

Line 40 in 6385a79

_time_diff = 60

better_thermostat/custom_components/better_thermostat/events/temperature.py

Lines 67 to 78 in 6385a79

	if (
	_incoming_temperature != self.cur_temp
	and (datetime.now() - self.last_external_sensor_change).total_seconds()
	> _time_diff
	):
	_LOGGER.debug(
	"better_thermostat %s: external_temperature changed from %s to %s",
	self.device_name,
	self.cur_temp,
	_incoming_temperature,
	)
	self.cur_temp = _incoming_temperature

Actually this used to be 5s before v1.7.0:
afe3110#diff-a4d9a716d67945623fe5d45acc98ff88affec88d27db0e44bc0b940d0bc52229L39-R39
@KartoffelToby Do you have any rational for why you increased this to 60s?

Same for updates from the TRV, although it's just 5s of discarding updates:

better_thermostat/custom_components/better_thermostat/events/trv.py

Lines 75 to 93 in 6385a79

	_time_diff = 5
	try:
	for trv in self.all_trvs:
	if trv["advanced"][CONF_HOMEMATICIP]:
	_time_diff = 600
	except KeyError:
	pass
	if (
	_new_current_temp is not None
	and self.real_trvs[entity_id]["current_temperature"] != _new_current_temp
	and (
	(datetime.now() - self.last_internal_sensor_change).total_seconds()
	> _time_diff
	or (
	self.real_trvs[entity_id]["calibration_received"] is False
	and self.real_trvs[entity_id]["calibration"] != 1
	)
	)
	):

Also here we discard TRV updates, if the controller is processing TRV commands:

better_thermostat/custom_components/better_thermostat/events/trv.py

Line 114 in 6385a79

if self.ignore_states:

Overall we would wanna delay triggering re-iteration of the control loop and commanding a new target to the TRV too frequently, if triggered by sensors, but not totally discard them. Especially we don't wanna discard a sensor update.

Proposal

Therefore the existing limits probably should be removed, and reimplemented on control loop side for example, limiting the number of updates / re-iterations. For the control queue, it would probably even be enough to have a simple, binary "control update pending" flag.

Special care must be taken, to avoid an infinite loop of sensor update -> calculating now command from control loop -> triggering new sensor update again (like mainly a risk in "local calibration" mode).
The current implementation for this, might also be the cause of #1513 - We're maybe in such control loop, just a rather slow one.

HomematicIP

Way lower rate-limits need to be ensured ofc, where currently one update per sensor source is processed every 10min at max. When properly limiting on the control queue:

• User inputs ofc still should be processed "almost" immediately (I think HA anyway delays the update, till the user stopped touching the TRV for a second or so)
• We might want to also process big changes more urgently, or for example when changing between heat<>idle states
  Something like processing updates every 15min by default, but if "significant change", process up to 4 more events per hour.

[RubenKelevra]

Yeah I remember @KartoffelToby and I were talking bout this issue in the first place. Some TRVs will "flicker" between two temperatures. Mine does this sometimes for up to a minute.

I think the better logic would be to process the first update always instantly, and if another one comes in within say 30 seconds, we set a callback for the end of 30 seconds being completed before processing the latest update at that time. This will effectively pace updates to 30 seconds, while always processing with the latest values, while processing single updates without delay, as they should.

And if the last values happen to have the same values as the last calculation input, we just skip it. This way we ignore if the flickering flipped back to the last value we used before.

@folfy does this approach sound reasonable?

[folfy]

Yeah, essentially how I'd do it for the basic rate-limiting.

Just we also have to consider an issue in the calculation or TRV behaviour (for example I'm still not 100% sure all TRVs are actually reflecting the calibration value in the reported temperature, as assumed in the control loop atm), which can still lead to constantly changing sensor values and calculated setpoint values -> In the given example we could detect this after lets say 8 TRV induced updates/changes within 5min, and simply start discarding self-induced TRV updates for a bit, to at least get out of the loop, while also throwing a repair indication for the user (prompting to create an issue with proper logs here).

[RubenKelevra]

Just we also have to consider an issue in the calculation or TRV behaviour (for example I'm still not 100% sure all TRVs are actually reflecting the calibration value in the reported temperature, as assumed in the control loop atm), which can still lead to constantly changing sensor values and calculated setpoint values -> In the given example we could detect this after lets say 8 TRV induced updates/changes within 5min, and simply start discarding self-induced TRV updates for a bit, to at least get out of the loop, while also throwing a repair indication for the user (prompting to create an issue with proper logs here).

That's a seperate issue which needs investigation. As I don't use calibration this is no factor here.

Please open a dedicated issue for this and let's keep on topic here :)

[folfy]

Well, it will be part of this, because if we rework the existing function as described without considering such side-effect, we would throw this up as yet another major regression, like happening waaaay more often 😅

Also can happen with target temp calibration in certain scenarios just as well, it was just an example. The "bad" code that is causing this bug here, is at the same time the existing safeguard against such problems happening more often than in a few edge-cases.