-
Notifications
You must be signed in to change notification settings - Fork 115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve Solar divert logic #679
base: master
Are you sure you want to change the base?
Conversation
What is the reasoning for removing the PV divert ratio? I think it still provides valuable functionality in some use cases. |
Also thank you for the contribution. |
Hey Jeremy. I don't think is needed with the divert_reserve parameter that I introduced but I might be wrong. For me it was also hard to understand the value of this ratio, which is applied to a constant of 1000Watts instead to the excess current. Maybe you can elaborate in which case is useful. Could you describe what scenario would need it that isn't covered with the new divert code? If we would want to thread fine, we could add a PID to track available current but I think is threading too fine. For me the changes that I did work very well. If anything I would add an hysteresis threshold between current steps to reduce changes in charge rate. |
Any will to get this merged? Or any improvement I can do in that direction? @jeremypoulter |
Sorry I haven't had a chance to test. However we moved from a fixed reserve because some users wanted to make use of more of the solar power, even at the expense of importing power from the grid. So if the value is greater than 1 it acts as a reserve like you have but if less than 1 it acts as a percentage of how much solar/grid energy to use. I will have to look to see why we combined the option. Probably because the options are mutually exclusive. |
@jeremypoulter what are next steps to get this merged? I think the changes are pretty good. Have tested them for weeks, added a watchdog safety mechanism as well. |
The main problem is the removal of the PV ratio, this is still needed for some use cases. Some users prefer to import a small amount of grid energy to make more use of the PV. |
Instead of this ratio we set it now in watts. Please explain why this is a problem at all? |
This option has a number of mutually exclusive operations, one is the reserver you are proposing, when set to a value greater than 1, but when less than 1 it defines how much to import to make more use of the solar energy. The other related thing this does (when <1) is change the current the charging on PV will start/stop again to make more use of the solar. I think it is best to just change the UI to better communicate these options. The other option is to add multiple options, but would have to do something both together. |
I think the best way forward is to update the UI to better explain these options, eg a select box for the mode, an input for setting the watt of in 'reserve' mode or an input for setting the percentage if in the import mode |
My 2 cents: this change makes sense to me. The current implementation and the fact that
Personally this PR's hysteresis / reserve power concepts would have been clearer for me to understand 👍 Not entirely convinced about the smoothed set point, though. I can't see why this is useful as non-smoothed variations of the set-point can't cause any mechanical wear I'm aware of. |
Improved ECO divert logic.