martinarva / dynamic_energy_cost

35 stars 9 forks source link

Daily/monthly sensors are not reset at midnight/end of the month #30

Open DutchKillerbee opened 1 month ago

DutchKillerbee commented 1 month ago

Describe the bug The daily sensor is not (always) reset a midnight to 0 and the monthly sensor is not rest at the end of the month

Expected behavior I expect that at 0:00 every night the Daily sensor will be set to 0.00 and at the end of the month the monthly sensor will be set to 0

Screenshots Example daily image Example montly image

DutchKillerbee commented 1 month ago

FYI this night it has been reset. So it seems to reset it randomly..... image

gregbarreiro commented 1 month ago

I'm also experiencing the same issue. It only happens to the Sensors based on Power Usage. The sensors based on Energy Usage, resets fine.

DutchKillerbee commented 4 weeks ago

@gregbarreiro Does it work always for the energy based sensors? Daily/Montly/Yearly? (I understand that yearly is difficult to check)

@martinarva I checked the code difference energy_based_sensors.py and power_based_sensors.py code and could see some differences in the calculate_next_reset_time(self) function

if self._interval == "daily":
      next_reset = current_time.replace(hour=0, minute=0, second=0, microsecond=0) + timedelta(days=1)
if self._interval == "daily":
      next_reset = (current_time + timedelta(days=1)).replace(hour=0, minute=0, second=0, microsecond=0)

Could this be the reason why its working for the energy sensors and not for the power sensors?

gregbarreiro commented 4 weeks ago

@gregbarreiro Does it work always for the energy based sensors? Daily/Montly/Yearly? (I understand that yearly is difficult to check)

@martinarva I checked the code difference energy_based_sensors.py and power_based_sensors.py code and could see some differences in the calculate_next_reset_time(self) function

if self._interval == "daily":
      next_reset = current_time.replace(hour=0, minute=0, second=0, microsecond=0) + timedelta(days=1)
if self._interval == "daily":
      next_reset = (current_time + timedelta(days=1)).replace(hour=0, minute=0, second=0, microsecond=0)

Could this be the reason why its working for the energy sensors and not for the power sensors?

@DutchKillerbee Yes, just checked! The Energy Based sensors resets are OK on both month and day sensors. The power based sensors resets varies, some days it resets and some day it doesn't. I've noticed that the day after a HA-reboot it works, the other days it varies. I have only one of these sensors. The monthly reset worked properly. For now I've created an automation that resets the sensor at 12:05. Groet. Greg

Pluimvee commented 4 weeks ago

@DutchKillerbee I'm responding to your question posted in https://github.com/martinarva/dynamic_energy_cost/issues/26#issuecomment-2158029154 over here to keep the comments and issues together.

@Pluimvee I noticed you changed this part of the code only in the energy sensor code image what was the reason for this? for me resetting daily/monthly power sensor doesn't always work. Could your fix also work for the power sensor? issue: #30

I changed the reset schedule as I think the reset issue is caused by a race condition. Exactly at 0:00 there are two events fired: a price change event and a timer event. Due to the async definitions of the methods being invoked, the code is executed concurrently and may cause issues. A proper way to resolve this is by either define the methods synchronous, but I'm sure this is not according HA specs, or by using a mutex. As I'm not a python developer I need to investigate and test. For now I simply changed the reset timer to one second after 0:00 to prevent being fired at the same time as the market price changes. Ps.: as I'm currently using my suggested logic to determine export/import pricing (see https://github.com/martinarva/dynamic_energy_cost/pull/31#issuecomment-2147092179) a fluctuation in net power usage may also trigger a change in my price sensor and could, potentially, also trigger a price event at 1 second passed 0:00. The chances on this are very slim as I do not switch from production to consumption, or vice versa, during midnight.

Why I didn't change this in the power based code is for three reasons:

  1. The above is a temporarily fix

  2. I don't like the power based solution as this code is based on changes in power usage and assumes the power usage is constant between two sensor updates. As such the accuracy of the cost calculation is dependent on the update frequency of the used power sensor. If, for example, the power sensor updates each 15 second the code 'assumes' the power consumption to be constant for 15 seconds. If this less accuracy is acceptable, or the power sensor gives more frequent updates, then I suggest to use the out-of-the-box integral helper to convert power (W) into energy (kWh) and use the latter as input for the energy based sensor of this component. Removing the power based sensor also simplifies this component and reuses the utility sensors in the energy based solution.

  3. Because the power changes more frequent, also during midnight, the risk of having a race condition is higher, and therefore a better solution (mutex) is needed.

DutchKillerbee commented 3 weeks ago

I used the quick fix of @Pluimvee on the power daily/monthly sensors and it seems to work for me.

if self._interval == "daily":
      next_reset = current_time.replace(hour=0, minute=0, second=2, microsecond=0) + timedelta(days=1)