DPL: Honor battery-provided discharge power limit

[ranma]

When the BMC provides a discharge current limit, apply the this limit in the DPL to the inverter power target when running from battery.

In my test this worked well, when the limit dropped from 0.5C (25A) to 0.25C (12.5A) at 20% SoC, the actual measured current draw dropped to 12.3A.

See #657

[spcqike]

I like and support this idea.

is there a reason why getBatteryDischargeLimit() is part of powerlimiter and not part of the battery itself?

Do you think it’s a good idea to move this function to the battery? And also if there is an enable/disable switch in the battery menu to let the user choose whether or not to use this option?

Further thinking it would be possible to have this switch as overall battery setting, independent of the chosen interface. For „plain / stupid“ batteries one could than add an input field to let the user define a fixed max current.

As I said somewhere else, on thinking run I think it would be good to move all battery stats to the battery menu (also desired voltages, SoCs and things) and let the DPL just query the battery class and check, if the battery is available and how much power it allows to be drawn.

[ranma]

is there a reason why getBatteryDischargeLimit() is part of powerlimiter and not part of the battery itself?

On the Battery side the limit is normally expressed as a current limit, so currently only the power limiter code needs to convert it to a power limit in watts. This also matches the existing getChargeCurrentLimitation() in the battery interface.

Of course it could also be moved to the BatteryStats base class.

And also if there is an enable/disable switch in the battery menu to let the user choose whether or not to use this option?

Yeah, it would make sense to have the enable/disable switch in the battery menu.

For „plain / stupid“ batteries one could than add an input field to let the user define a fixed max current.

It probably should be voltage threshold based for those. For the user it would be most convenient to select the battery voltage / cell count and capacity and then have the actual threshold voltages / current limits be generated automatically.

[spcqike]

For the user it would be most convenient to select the battery voltage / cell count and capacity and then have the actual threshold voltages / current limits be generated automatically.

well, there might be several reasons why a user wants to set a specific, lower rating. That could be fuse and wire sizing, battery age / degradation and stuff. I’d say, one can’t simply automatically calculate the max ampere rating.

[ranma]

For the user it would be most convenient to select the battery voltage / cell count and capacity and then have the actual threshold voltages / current limits be generated automatically.

well, there might be several reasons why a user wants to set a specific, lower rating. That could be fuse and wire sizing, battery age / degradation and stuff. I’d say, one can’t simply automatically calculate the max ampere rating.

Sure, but automatically calculated would be a good default to provide, which could still be overridden.

is there a reason why getBatteryDischargeLimit() is part of powerlimiter and not part of the battery itself?

Actually, there is one more reason that just came to mind. The power should be "inverterVoltage * batteryDischargeCurrentLimit", to account for possible cable losses.

[AndreasBoehm]

I will try to take a look at this in the upcoming days as i would love to see that feature.

Even though the Pytes battery reported 12,5A discharge current limit (because it was at 11% SoC) it allowed the inverter to pull 45A and i think we should avoid that.
Not only because the BMS somehow didn't do anything to prevent that but also because we should try to not make the BMS stop us from doing something wrong. We should make sure that we stay within the given limits.

[AndreasBoehm]

... if there is an enable/disable switch in the battery menu to let the user choose whether or not to use this option?

Further thinking it would be possible to have this switch as overall battery setting, independent of the chosen interface. For „plain / stupid“ batteries one could than add an input field to let the user define a fixed max current.
...

I like the idea of having a toggle for this feature and also the input field in case the BMS does not provide this information.
IMHO the input field could be a separate PR, feel free to create an issue for that.

[schlimmchen]

we should try to not make the BMS stop us from doing something wrong

Absolutely.

Do you think it’s a good idea to move this function to the battery?

Please keep the changes to the DPL at a minimum, because I have this 🙊

 src/PowerLimiter.cpp | 964
 1 file changed, 509 insertions(+), 455 deletions(-)

On the other hand, the "only" important new thing is to limit the power the inverter(s) can draw from the battery to the new getBatteryDischargeLimit(). If you keep it at that, I'll be fine.

As I said somewhere else, on thinking run I think it would be good to move all battery stats to the battery menu (also desired voltages, SoCs and things) and let the DPL just query the battery class and check, if the battery is available and how much power it allows to be drawn.

Yes, that sounds nice. It's kind of the "battery watchdog" idea but integrated into the battery.

one could than add an input field to let the user define a fixed max current.

I disagree. We have the upper power limit for that. Why shouldn't the user use that to make sure only a sane amount of power is drawn from the battery? If you want to limit the current instead of the power, do the calculation assuming the stop threshold and there you have it. Is this a convenience thing?

And also if there is an enable/disable switch in the battery menu to let the user choose whether or not to use this option?

What should be switchable here? Whether or not the DPL shall honor the battery discharge limit? That's strange, why would one want to disable that? If the discharge current limit is known, use it. Tell it in the (verbose) logs. Add it to the DPL live view card eventually.

[AndreasBoehm]

one could than add an input field to let the user define a fixed max current.

I disagree. We have the upper power limit for that. Why shouldn't the user use that to make sure only a sane amount of power is drawn from the battery? If you want to limit the current instead of the power, do the calculation assuming the stop threshold and there you have it. Is this a convenience thing?

If you need to use the upper power limit for that we will limit the whole system by the battery to e.g. 10A even though we could pull e.g. 25 A from the solar charger and an additional 10 A from the battery when needed.

[spcqike]

I disagree. [...] Is this a convenience thing?

IMO no. its not just for convenience.

you are right, one could use the amps that can be drawn at lower SoC/voltage, but why should we? as we know, the voltage changes with SoC and therefore the current changes, for a fixed power.

if we have like a small 24V battery (or thin cables, small fuse, safty concerns, ... you name it), and we want to draw 25A at max, why should we use the DPL to limit to 500W (which is 20V very lowest voltage * 25A), when we could use up to 730W with a fully charged battery (29.2V * 25A). that ~ 31% less than we could use at max voltage and current. (or we could get ~46% more power with the same current, if we calculate the power based on a fixed current and the real voltage)

on the other hand, it is more intuitive to use a fixed amp rating, as everything is based on that. BMS / battery (dis-)charge currents, fuses, wire sizes, connectors, .....

next, as @ranma pointed out, we should use the voltage, that the inverter measures, to calculate the limit.
right now, the DPLs upper limit is the inverter output limit. it doesn't care about currents or even voltage drops across the wires. on the contrary, we calculate the ouput power based on different voltage sources (BMS, Shunt, MPPT, inverter, which ever happens to be the better / more accurate one and is reachable. which is fine, as we want to know the real battery voltage to protect the battery from deep discharge), but to protect the battery and wires from to high currents, we must use the real currents. if the inverter tries to deliver 800W, and we loose 0.5V from battery to inverter input, the inverter just increases the current to reach its desired output. that's a point we simply can't calculate with the lower SoC / voltage threshold, can we?

another thing:
solar passthrough.
if we limit the DPL to like 500W (25A 24V system), why shoudn't we use surplus solar power from the MPPT charger?

we want to limit the current, that is drawn from the battery. we dont want to limit the DPL to a fixed value. do we?

What should be switchable here?

well, maybe someone still want's to push his battery to the limits and further. shoudn't that be se choice of the user? if the BMS says not to use more than xA now, if the user urges for more, he could do it, like now. but at his own risk.

[schlimmchen]

Good points. So the (DPL or inverter) upper limit is not suitable to implement a maximum battery discharge current.

So there shall be a new config value "maximum battery discharge current" in the battery settings. @spcqike argues that this should be available in any case (all battery providers), as one might want to limit the current base on cabling. Additionally, if the battey provider supports it, the discharge current is further reduced to the BMS-reported discharge current (if that is even lower).

Are we on the same page now? Are there more new config switches/values that I missed?

Note, however, that I find that this gives a false sense of safety. Depending on the wiring, i.e., if the solar charger is connected to the battery poles and the battery poles are connected to the inverter, all current from the solar charger goes through the cables between the inverter(s) and the battery poles. If the user sets a battery discharge current such that these wires are not overloaded, but the DPL sets a higher limit that draws more current than that because the charge controller also produces power and the DPL wants to use that, we still risk overloading the wires. So I think this should strictly be advertised as a battery protection mechanism, not a wiring protection.

[AndreasBoehm]

I also see this as a battery protection and not for wires. I can understand why people use small batteries and limit their solar chargers because of that, also that they want to set a limit of the discharge current for the same reason (small battery without proper BMS)

But protecting wrong fuses or cabling, this should not be something that we should aim for.

People need to understand which wires and fuses to use, if they don't, its safer for everyone if they don't run a 'custom' battery setup at all. And we should not support them running setups like that.

I would not make the input field for maximum discharge current available for all battery providers, will increase the code needed to handle which value overrides which and we should trust the BMS imho.

[spcqike]

Are we on the same page now?

👍

So I think this should strictly be advertised as a battery protection mechanism, not a wiring protection.

I also see this as a battery protection and not for wires.

i guess, you are right.

I would not make the input field for maximum discharge current available for all battery providers, will increase the code needed to handle which value overrides which and we should trust the BMS imho.

what about if someone wants to further protect his battery? even so a US5000 can handle 50A, if one wants to limit it to like 40A? (yes, this probably is a luxury problem as you need a HMT-2250 or bigger (upcoming MIT-4000+ ?) or multiple inverters and a DPL that manages both) but IMO its a legit feature.

edit:
another thing to think about or discuss:
what current do we use? the BMS and a Smartshunt give us a current. we can also calculate the current based on inverter input voltage and desired output power. which one is better or more accurate? does it matter at all? if we have a BMS/Shunt, do we need to read the current and calculate, how much current we can draw additionaly? (like if we have a second inverter, hoymiles or offgrid, or even another DC load connected to it?)

[schlimmchen]

IMO its a legit feature.

I agree. The implementation effort should be small.

what current do we use?

We calculate a DC power allowance for battery-powered inverter(s) based on the current limit(s) and battery voltage.

currentLimit = min(config.Battery.MaxCurrent, Battery.getDischargeCurrentLimit()) // the config value should be handled in getDischargeCurrentLimit(), I suppose
voltage = DPL.getBatteryVoltage() // this exists, don't know the name by heart
allowance = voltage * currentLimit * currentLimit

[spcqike]

We calculate a DC power allowance for battery-powered inverter(s) based on the current limit(s) and battery voltage.

So how do we deal with voltage drops from battery to inverter? DPL.getbstteryvoltage() gives either the BMS, shunt, MPPT or inverter input voltsge, doesn’t it?

If the BMS reports like 51V, and we want to draw 15A, we can draw 765W. But if we apply 765W there will be a voltage drop, so the inverter may only get like 50.5V. So it will draw about 15.15A. It’s a small error, but it’s there and it will be higher at higher loads and for smaller batteries.

Next the inverter limit is AC, the current limit is DC. So the inverter limit must be reduced by efficiency.

and how to we handle charging while discharging?

Wouldn’t it be easier to use

currentLimit = config.Battery.MaxCurrent-(Battery.isCurrent-inverter.isCurrent)

at least for BMS and shunt based batteries?

If we want a max discharge current of like 15A and the battery gets charged with 5A (which is reported as -5 isn’t it?) and we already use 5A (~250W) we could „draw“ up to 25A at this moment, couldn’t we?

[AndreasBoehm]

I put together a settings UI in the battery configuration, if we can agree that this can work that way i would created an MR based on the current one.

MQTT Battery Provider	Any other provider	Custom limit	Hint

[spcqike]

looks great.

does the custom limit exists for mqtt based batteries, too?

[AndreasBoehm]

Or we go for a different approach and use the provided value as a fallback? Similar to what is done when you use the DPL based on SoC instead of voltage.

@spcqike do you think that we should allow mqtt batteries to provide a default/fallback value as well?

[AndreasBoehm]

This would be a version for the mqtt battery

[spcqike]

Or we go for a different approach and use the provided value as a fallback?

i would use it as overwrite. but only as long as its lower than the value that a BMS gives. so we can set a default value of e.g. 40A, and not use more, even if the battery would allow 50A. but if the SoC is low and the battery reports only 12.5A, this should be the used value. in case of a communication error, 40A could be considered as fallback, as we wouldn't have anything else in this situation.
but, i wouldn't use it as fallback allone, as we could use it as lower default, to reduce the stress of the battery.

do you think that we should allow mqtt batteries to provide a default/fallback value as well?

certainly. as mqtt is just another way to obtain the BMS data. why shouldn't we also define our own max values (and fallback) there? idk but the one or the other user may have a simple esp8266 which reads a BMS and reports the BMS data plain to MQTT, which is than read and used by opendtu-oB. (yes, we could manipulate said BMS data with another external service and create an overwrite there, but it would be easier and straight forward to handle all battery providers the same, i guess)

This would be a version for the mqtt battery

i like it.
just one question, which may be offtopic in this PR: do we need unit selectors here, like we have in the powermeter? do all "MQTT BMS bridges" always report A and V? may they also use mA and mV?

[AndreasBoehm]

i would use it as overwrite. but only as long as its lower than the value that a BMS gives. so we can set a default value of e.g. 40A, and not use more, even if the battery would allow 50A. but if the SoC is low and the battery reports only 12.5A, this should be the used value. in case of a communication error, 40A could be considered as fallback, as we wouldn't have anything else in this situation. but, i wouldn't use it as fallback allone, as we could use it as lower default, to reduce the stress of the battery.

We can do it like this:

certainly. as mqtt is just another way to obtain the BMS data. why shouldn't we also define our own max values (and fallback) there? idk but the one or the other user may have a simple esp8266 which reads a BMS and reports the BMS data plain to MQTT, which is than read and used by opendtu-oB. (yes, we could manipulate said BMS data with another external service and create an overwrite there, but it would be easier and straight forward to handle all battery providers the same, i guess)

Makes sense.

i like it. just one question, which may be offtopic in this PR: do we need unit selectors here, like we have in the powermeter? do all "MQTT BMS bridges" always report A and V? may they also use mA and mV?

I was thinking about this as well, should be straight forward to add this.

[AndreasBoehm]

i like it. just one question, which may be offtopic in this PR: do we need unit selectors here, like we have in the powermeter? do all "MQTT BMS bridges" always report A and V? may they also use mA and mV?

[AndreasBoehm]

To sum it up, this is how it would look.

MQTT	Everything else	Hint

[AndreasBoehm]

Should we get rid of the Override Limit toggle and update the hint to include (to be rephrased) The lowest limit will be used in any case ?

[schlimmchen]

Nice progress!

I had the chance to look at the final result without too many thoughts about the intermediate results or the discussion and I have to say that it took me a while longer than I would have liked to understand the switches and settings

I would go the other way around:

• Nothing is shown except for "Limit Discharge Current" until "Limit Discharge Current" is enabled.
• Enabling "Limit Discharge Current" shows "max. Discharge Current" and "Use Battery's Limit", nothing more.
• Force a value in "max. Discharge Current", but user may enter 0 which effectively disables the fallback.
• Enabling "Use Battery's Limit" shows only Hint-Text for non-MQTT batteries, otherwise also shows Topic, JSON Path, and value setting.

Maybe rename "Use Battery's Limit" to "Further Reduce to Battery's Limit", which potentially makes the tooltip obsolete? (I am not happy with "Battery's Limit"... Maybe "Battery-Reported Limit"?)

Is this easier to understand oder just a different style? I think it is more intuitve.

[ranma]

I would not make the input field for maximum discharge current available for all battery providers, will increase the code needed to handle which value overrides which and we should trust the BMS imho.

There is actually additional reasons why having a battery-side limit that is separate from the DPL limit can make sense:

• AFAIU it is best for the battery life and to minimize aging to have an average SoC that is close to 50%
• Resistive loss is (I^2)R, so by halving the current you quarter the loss from cable and cell resistances
• At the ends of the battery SoC curve the current has to go down to allow for accurate SoC estimation / calibration (AFIU this is the main reason why the battery reduces the allowed charge/discharge currents towards the SoC ends)

e.g. in my case, the Battery lasts only until ~23:00, so spends a fair amount of the night at very low SoC (the selected DPL discharge cutoff threshold).

My ideal feature would actually be to have a discharge target end time for this so I could say "regulate the discharge current so that we finish discharging the battery 04:00 in the morning", or even better would be to keep track of average SoC and manage the night-time discharge start/end time automatically based on that.
The simpler version of this would be to set a battery-set artificial discharge power limit (possible only applying to night-time discharge, or being based on SoC threshold).

e.g. https://www.sciencedirect.com/science/article/pii/S2352152X2201622X:

"the lowest cyclic aging was found in the range of 45 to 55% SOC" "the highest in the range of 90 to 100% SOC" (though this was for NMC instead of LPF, it also says "the primary aging mechanisms and their stress factors remain the same across LFP, NMC and NCA").

[ranma]

Or we go for a different approach and use the provided value as a fallback? Similar to what is done when you use the DPL based on SoC instead of voltage.

Or use min(batteryProvidedValue, manuallyProvidedValue)?

[spcqike]

the lowest cyclic aging was found in the range of 45 to 55% SOC" "the highest in the range of 90 to 100% SOC

thats why most people use their battery within a range from 20-80%. chemical aging happens at the very low (0-20) and very high (80-100) ends.

i guess it wouldnt be smart to only use 10% of your battery (SoC range from 45-55%), as a 1.000€ 5kWh battery would only provide 0.5kWh "usable" capacity...

Or use min(batteryProvidedValue, manuallyProvidedValue)?

so, as @schlimmchen suggestet 4 days ago?

[AndreasBoehm]

@ranma are you fine with me taking over this MR/topic? Or do you want to take care of adding the settings in the UI and in the code?

Just let me know what you prefer, i can provide you a patch for this MR or i create a new MR on top of your commits if you don't want to handle the UI and settings part.

[ranma]

@ranma are you fine with me taking over this MR/topic? Or do you want to take care of adding the settings in the UI and in the code?

Just let me know what you prefer, i can provide you a patch for this MR or i create a new MR on top of your commits if you don't want to handle the UI and settings part.

I haven't looked much into updating the UI (in particular if you also want to move settings from DPL to Battery), so feel free to take over this MR/topic :)

[AndreasBoehm]

I created a new MR based on this one that also includes the settings UI for it: #1245

[schlimmchen]

I checked that this changeset is still the valid base for #1245 and will merge this first, then rebase and squash #1245 on top of it. That should work and give the cleanest kind of attribution to @ranma for his part of this feature.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns.