ScratMan / HASmartThermostat

Smart Thermostat with PID controller for HomeAssistant
336 stars 48 forks source link

Duty cycle messed up - False reset of time_changed variable - Fix #210

Closed DaniM54 closed 4 months ago

DaniM54 commented 4 months ago

I ran into a problem where my actuator was switching on/off more often than my PWM interval was long. This can be reproduced if, for example, you set the pwm to 1800 seconds and the min_cycle_duration to 150 seconds. Since the time_changed variable is incorrectly set to time.time() before the actual switching, the activation duration is extended beyond the calculated on duration, and the duty cycle gets mixed up a lot. The solution to the problem is to remove the variable time_changed completely, which is reset before the service is called, and adapt the check to the variable last_heat_cycle_time set in _async_heater_turn_off/on.

Additionally, I don't understand why last_heat_cycle_time is initialized with the current time. This means that the first control always fails after a reload. Therefore I initialized it with 0. Of course, in practice this will result in the min_cycle_duration being undershot during a reload/restart, but it makes more sense to me in practice. Of course, it would be perfect to maintain the value throughout a reload, but I personally don't see it as necessary.

Cheers and thank you for this project. This is my first time participating in a github project, so let me know if I do anything wrong, thanks.

DaniM54 commented 4 months ago

I closed the Pull Request because I found another related problem that I will solve.