Update overscaling for better mixed setup management #1298
Conversation
leoai-81
force-pushed
the
development
branch
2 times, most recently
from
e0242b8
to
8a75484
Compare
leoai-81
changed the title
AndreasBoehm
force-pushed
the
development
branch
from
8a75484
to
66e7af2
Compare
|
Ha prettier, that was it. I was wondering what was the code format for the vue files. Should be fine now, can you run the workflows again ? |
| @@ -225,6 +225,7 @@ void PowerLimiterClass::loop() | |||
|
|
|||
| // Check if NTP time is set and next inverter restart not calculated yet | |||
| if ((config.PowerLimiter.RestartHour >= 0) && (_nextInverterRestart == 0) ) { | |||
| MessageOutput.printf("[DPL::loop] Setting next restart \r\n"); | |||
There was a problem hiding this comment.
Why is there a space before the carriage return and why is this output even included in the changeset for this PR?
| if (config.PowerLimiter.IsInverterSolarPowered) { | ||
| _nextInverterRestart = 1; | ||
| MessageOutput.println("[DPL::calcNextInverterRestart] not restarting solar-powered inverters"); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
wait... what? if this change should make it into the project, we need a clear reason/motivation, a respective change in the documentation, etc... why are we suddenly restarting solar-powered inverters? because you are treating yours as such, even though it is actually connected to a battery? sorry, it does not work this way. if you need overscaling even though your inverter is battery-powered, we need to find a way to set this up properly. this is not okay.
There was a problem hiding this comment.
Well, good point. Right now the restart function allows you not to restart if you want. So blocking this from code for solar powered inverter makes no sense. Coming to this effort, you're looking at a mixed setup (panels / nothing, panels / battery, different orientation of panels per mppt...). Battery management being fully external, you do not need to rely on the work done for battery management, therefore considering it as "solar powered".
There was a problem hiding this comment.
Battery management being fully external, you do not need to rely on the work done for battery management
You can effectively disable "battery management" by setting the start and stop thresholds to very small values. So you can treat your inverter as battery-powered. You then lose overscaling...
So you want to enable overscaling regardless of whether or not the inverter is solar- or battery-powered. This can make sense, as I am planning to implement a linear overscaling for underperforming battery-powered inverters in #1216. However, that is not the kind of overscaling you want when you mark the inverter as battery-powered... Oh boy...
You know what: Maybe this change isn't so bad. I won't have an effect on actual solar-powered inverters, since they are unreachable in the middle of the night. Ah! Let's make this if-statement if (inverter->isReachable()) (or similar) and only return in the if's body? That would solve your problem, it would still make sense, and has a low risk of introducting a regression -- I think.
@AndreasBoehm Does this make sense to you?
Hm, another thing: How did you enable restarting the inverter in the first place? The setting is hidden in the web UI when you enable "solar-powered inverter". So that would need to change. And, as I wrote already, the docs must be updated.
There was a problem hiding this comment.
@schlimmchen , well, on what I can comment... Battery scaling is not immediate, meaning that a -shot- surge (ie, 30sec) in power need will always introduce some loss one way or another... Either by "not patching up quick enough (up or down)" or by patching up "too aggressively"...
There was a problem hiding this comment.
@schlimmchen makes sense to check if the inverter reachable to allow the restart
webapp/src/locales/de.json
Outdated
| @@ -685,6 +685,7 @@ | |||
| "VoltageLoadCorrectionFactor": "Lastkorrekturfaktor", | |||
| "BatterySocInfo": "<b>Hinweis:</b> Die Akku SoC (State of Charge) Werte werden nur benutzt, wenn die Batterie-Kommunikationsschnittstelle innerhalb der letzten Minute gültige Werte geschickt hat. Andernfalls werden als Fallback-Option die Spannungseinstellungen verwendet.", | |||
| "InverterIsBehindPowerMeter": "Stromzählermessung beinhaltet Wechselrichter", | |||
| "ShadedFactor": "Factor to consider if panels are shaded 0-100, in % vs expected channel power", | |||
There was a problem hiding this comment.
@AndreasBoehm Is is okay that I don't understand what this does or must this be explained further, maybe in the docs?
There was a problem hiding this comment.
It's just the percentage for which you consider the channel "shaded". Ie. if a channel provides 90% of the desired output, then you consider it as fully producing. Previously was was a fixed value of 98%. But that's totally arbitrary and that can greatly change depending on installation.
There was a problem hiding this comment.
Could you work on that text a bit @leoai-81 ?
For me its also hard to understand, even though i know what its about.
There was a problem hiding this comment.
@AndreasBoehm Sure, would something like "Percentage of shade for which shading situation is considered, in % vs expected channel power" be fine ?
There was a problem hiding this comment.
What about Set the minimum power output threshold (%). Channels below this percentage are considered shaded and excluded from scaling. ?
There was a problem hiding this comment.
Sure, can settle on that one. Should be aligned with latest branch update.
| :label="$t('powerlimiteradmin.ShadedFactor')" | ||
| v-model="powerLimiterConfigList.shaded_factor" | ||
| :tooltip="$t('powerlimiteradmin.ShadedFactorHint')" | ||
| placeholder="60" |
There was a problem hiding this comment.
placeholder should match the default value of 98%
webapp/src/locales/de.json
Outdated
| @@ -685,6 +685,8 @@ | |||
| "VoltageLoadCorrectionFactor": "Lastkorrekturfaktor", | |||
| "BatterySocInfo": "<b>Hinweis:</b> Die Akku SoC (State of Charge) Werte werden nur benutzt, wenn die Batterie-Kommunikationsschnittstelle innerhalb der letzten Minute gültige Werte geschickt hat. Andernfalls werden als Fallback-Option die Spannungseinstellungen verwendet.", | |||
| "InverterIsBehindPowerMeter": "Stromzählermessung beinhaltet Wechselrichter", | |||
| "ShadedFactor": "Shading factor %", | |||
There was a problem hiding this comment.
You already set Postfix="%", which is correct, so the unit shall be removed from the label. We don't do units in labels anyhwere.
src/PowerLimiter.cpp
Outdated
| // 98% of the expected power is good enough | ||
| auto expectedAcPowerPerChannel = (currentLimitWatts / dcTotalChnls) * 0.98; | ||
| // defined % of the expected power is good enough | ||
| auto factor_shaded = static_cast<float>(config.PowerLimiter.ShadedFactor)/static_cast<float>(100); |
There was a problem hiding this comment.
snake_case? Please change to camelCase.
| } | ||
|
|
||
| // newly calculated limit minus the production of the shaded panels | ||
| auto newLeanLimit = static_cast<float>(newLimit - static_cast<int>(shadedChannelACPowerSum)); |
There was a problem hiding this comment.
Won't it result in a wrongly calculated new limit if you always consider one channel as non-shaded but still have the AC-power of that channel within this calculation?
Thats actually different from what the 'old' calculation did
There was a problem hiding this comment.
Yes, that's indeed different. You're in a scenario where the system outputs way less power than what you'd expect. So one of the channel becomes the only hope of the inverter to provide that power... Should you directly set it to max power ? I did not want it to be so. This way, the power limit will gradually increase so hopefully one of the channel will bring balance to the overall system. The more the actual power diverges from asked, the higher the inverter limit will be. The closer the actual power is from what's asked, the closer the inverter limit will be.
So yes, if you ask 200w, your inverter produces 10... your limit will become the max possible (1600w in my HMS case)... Do you care ? No. As soon as the power produced will get closer to what's asked, the limit will also get closer to that (more or less abruptly depending of shaded channels vs total).
|
@leoai-81 are you able to push the requested changes? If not, are you fine with me taking care of adjusting this PR? |
|
@AndreasBoehm , yes, should be able to push what's been discussed today |
|
Hi @AndreasBoehm , any chance to see that merged ? Requested changes should be processed by the latest update. |
|
Hi @leoai-81, there are open comments from @schlimmchen that need to be addressed before i do another round of review. Thanks |
I've mostly based this work considering HMS1600 and Marstek battery (externally managed). It tries to overscale no matter the number of shaded channels. Shaded factor added as variable as it can depends on how many channels are subject to shade vs others. Will likely continue to work on it in the future, giving very promising results in terms of adaptation to load changes. Future development could be to integrate control of Marstek B2500 loadfirst/passthrough setting for given timeperiods to accommodate different expositions and solar + battery in the same setup.