InventoCasa / ha-advanced-blueprints

Advanced Blueprints combined with pyscript for extra useful automations
139 stars 33 forks source link

Update pv_excess_control.py to improve time trigger management #51

Closed stefan73 closed 4 months ago

stefan73 commented 4 months ago

Old code has problems to roubstly have a time_triggered instance of on_time() running. Often there was no instance. Especially when automation got restarted. This had to be done twice and the user was also not able to understand subsequent issues. To fix this the multiplicty of instance copies in a global array, a local trigger variable was removed. The logics of trying to have just one on_time() method running was changed. The control code now gets only executed for the first instance in the dictionary, but all automations have a on_time() instance. This seems robust.

InventoCasa commented 4 months ago

@stefan73 Thanks for the PR. I will review it hopefully on the upcoming weekend. However I've already seen that you made some changes to the files which I cannot accept; I commented these changes directly at the specific files / code lines. I think those issues arised because you changed a few things regarding doc/readme etc. on your fork and now try to merge all of the changes on the fork back into this project. Most likely this was not intended?

stefan73 commented 4 months ago

I guess something did not work out with this PR. My intention was only to propose the first change for pull at this moment ( Update pv_excess_control.py to improve time trigger management). This fixes - at least for me - the unreliability of having a robustly running on:time() instance. This occurs especially if you are in the beginning starting to set up automations (and also remove them) due to the global which gets never reset and the multiplicity of references kept in the two arrays.

For the other changes I assume to be on my fork for now as a kind of alpha devel. This is e.g. why I deleted the readme because it is incomplete / inconsistent to the changes done on the fork. Or I changes the headline of the automation so that users understand that they are using the forked version. What gets merged back from there needs to be seen. Let us do that in other PRs.

InventoCasa commented 4 months ago

Alright, in that case you may just update this PR (or create a new one) so that it only contains the fix for the Python script. Then I can review / merge it.

The other commits got added because unmerged branch PRs automatically get updated If you commit more changes to the branch.

stefan73 commented 4 months ago

Closing this - resubmitting a branch so that only the intended change can be pulled at any time.