-
Notifications
You must be signed in to change notification settings - Fork 66
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
Add SoC threshold for battery current limit. #1293
Add SoC threshold for battery current limit. #1293
Conversation
53070c8
to
242613d
Compare
Do you thinking it’s possible to respect the „ignore SoC“ switch? (Does the switch still exist? I thought so but haven’t found it without a battery (: ) |
242613d
to
08dbfea
Compare
Added a check for "ignore SoC" and also a voltage threshold setting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, however, I want #1294 merged first, then I will squash and rebase this onto development. Also, I will then change the web UI (1) to use wide input field labels and (2) to hide elements which do not apply until "enable interface" is checked.
I merged development into this branch so you can go ahead with your adjustments @schlimmchen without worrying about merge conflicts. |
1f06871
to
25f1902
Compare
Is this what you had in mind @schlimmchen ? |
Ah, those are two different screenshots... In that case, yes, I guess, except that the hint is also not applicable if the battery interface is disabled. I'll tell you when you push the respective changes to the webapp 😉 It's probably correct. |
b5875ab
to
8bb5ac5
Compare
Changes pushed :) |
|
Good catch, but the MQTT Battery Provider should be allowed as well to enable the topic input and I adjusted the SBS Battery provider to provide the discharge current limit in the correct way and added it to the list of allowed providers as well. |
What "Ignore Battery SoC" setting -- will a bunch of people ask. This cross-dependency is... unfortunate. Well, we are not going to solve this overnight. Let's clarify in the tooltip that a DPL setting is meant here. In the long run, all battery-related settings (DPL thresholds and the "ignore SoC" setting in particular shall move to the battery. Other people suggested this as well already. I suspect the condition for the hint is not correct. I switched to MQTT as the provider, enabled "use battery limits", then switched to JK BMS. Add a template-tag (or a div) around the "use battery limit" switch and the hint, then make that container dependent on the provider. Ah, wait. The input switch for "use battery limit" should move to the template below it, and that template should have the check for the provider setting. Do you know what I mean? |
I adjusted the texts and fixed displaying the alert for wrong providers. |
SBS CAN receiver implementation was not using the correct way to provide discharge current limit.
This changes the custom current limit so the custom limit is only applied when any of: - SoC is valid and not ignored and SoC < threshold - Voltage is valid and Voltage < threshold - Voltage is invalid Independently, if "Use Battery-Reported limit" is enabled and valid, it is applied (unless a lower custom limit already was applied).
* use wide labels for all battery settings * dynamically show and hide valid battery discharge limit settings
6537d9d
to
77283ca
Compare
I'd like to only limit the discharge current below a configurable state of charge (in my case currently "limit to base consumption below 70% SoC").
In principle the current limit could also be extended to support multiple SoC/voltage thresholds for different current limits, which might be useful for users of "dumb" batteries to approximate the multiple steps of discharge current reduction as the voltage or SoC approaches "empty". However currently this only adds the SoC threshold for the single current limit.