jmcollin78 / versatile_thermostat

A full featured Thermostat for Home Assistant: presets, window, motion, presence and overpowering management
MIT License
257 stars 25 forks source link

Enhance temperature regulation when working with internal device temperature #453

Closed jdeksup closed 1 month ago

jdeksup commented 2 months ago

Hi, I use Versatile thermostat from several weeks and really appreciate it to regulate my climate devices that have a bad thermal regulation because of the position of the sensor. Thank you for your big work on this. I suggest a small adjustment : a behavior that prevents the system from forgetting the regulation when using the device's internal temperature as an offset.

Although the regulation itself remains unchanged, variations in the device's internal temperature necessitate adjustments to the target temperature to maintain effective regulation. This code modification specifically bypasses the forget mechanism for minor adjustments, while the forget mechanism based on duration is still maintained.

Here is an history of the underlying climate before : image and after (with 2 minutes of interval which is fast) : image

jmcollin78 commented 2 months ago

Hello @jdeksup ,

I understand the idea and it could be interesting. It has one important drawback: we should avoid to send to much regulation command (setpoint change) on some equipment like TRV. Else we will drawn the battery very fast. (see some issue relative to this particular point).

This threshold have been done to avoid this battery drawn and I fear that your improvement will rollback this feature.

Without touching the code, you can do what you want by setting the dtemp threshold to 0, I guess. So it is configurable Vtherm by Vtherm and not forced for every one.

jdeksup commented 2 months ago

I understand your point. I already tried to configure dtemp threshold to 0 and got an error "assert x > 0" on round_to_nearest method, So I just update my pull request to fix this and it works.

jmcollin78 commented 2 months ago

I understand your point. I already tried to configure dtemp threshold to 0 and got an error "assert x > 0" on round_to_nearest method, So I just update my pull request to fix this and it works.

This seems good. Can you please add a unit test with dtemp equal to 0 to prove the correct behavior ? You have a good example in the test method:

async def test_over_climate_regulation(
    hass: HomeAssistant, skip_hass_states_is_state, skip_send_event
):

in the tests/test_auto_regulation.py. Of course I can help if needed.

jdeksup commented 2 months ago

Hi, I added a unit test based on your suggestions. It tests standard regulation with dtemp = 0, and tests if a small temperature change is taken into account.