Referenced Current values to MAX_BATTERY_CHARGE_CURRENT & MAX_BATTERY_DISCHARGE_CURRENT

[vincegod]

Hi,

I've changed a lot of the fixed Current values to be referenced MAX_BATTERY_CHARGE_CURRENT & MAX_BATTERY_DISCHARGE_CURRENT by multiplying by 0.1, 0.2, 0.4, 0.5 etc.

I've also tweaked the temperature limit steps a little.

I'm using a JBD and will be adding a second battery late this month.

Thanks for a great driver

Vince

PS. I'm new to Python, GitHub and such so hopefully I've not broke anything :)

[Louisvdw]

Hi @vincegod
The changes to ant.py is a removal of a previous change and it seems like that is not part of your changes. If you can remove the ant bms changes I can approve the rest

[vincegod]

I have removed the ant bms changes now, sorry I never intended to changed this file.

[ppuetsch]

@vincegod @Louisvdw
In https://github.com/ppuetsch/dbus-serialbattery/tree/improvements_with_config I introduced reading config values from an ini file (instead of letting users change utils.py).

However, I now have some difficulties with the configuration of the attributes that were changed in this MR.

I do see the following possibilities to resolve this:
1 Only max current is configured - and the array setpoints as well as their values are derived from that. (e.g.:
CELL_VOLTAGES_WHILE_CHARGINGANDMAX_CHARGE_CURRENT_CV` reside in utils.py and are not configurable
2 The setpoints (so in the example above CELL_VOLTAGES_WHILE_CHARGING) can be configured, but the corresponding limits will be derived based on max current. This approach kind of resembles what is already present - but I don't like it as it makes the configuration strange
3.: Both arrays can be configured. This gives the most flexibility - however it's kind of reverting this PR here.

What do you think?

[mr-manuel]

Would it also be possible to configure CELL_VOLTAGES_WHILE_CHARGING, MAX_CHARGE_CURRENT_CV, CELL_VOLTAGES_WHILE_DISCHARGING, MAX_DISCHARGE_CURRENT_CV, TEMPERATURE_LIMITS_WHILE_CHARGING, MAX_CHARGE_CURRENT_T, TEMPERATURE_LIMITS_WHILE_DISCHARGING, MAX_DISCHARGE_CURRENT_T, CC_CURRENT_LIMIT1, CC_CURRENT_LIMIT2, CC_CURRENT_LIMIT3, DC_CURRENT_LIMIT1, DC_CURRENT_LIMIT2, DC_CURRENT_LIMIT3 as it is now in the INI file with fixed values and if this values are not defined or commented out (maybe as default) the calculations of this PR are used?

[Louisvdw]

The problem with seperate settings for each of the different CCCM types are that it makes it difficult for the average user to change their max current limits. To make it easier should be the goal.

On the other side we want to be flexable so that users can adjust to their setup.

We can do something like @mr-manuel suggest where a default apply with ratios like in this PR, but then you can override that in the settings. However I can already see that people would then not know which values are applied.

For the GUI Settings I planned to have values in the settings, but when you apply a new (dis)charge limit it would adjust all the values according to a ratio like in this PR which the user can then adjust again. So the ratio only comes into play in the GUI when the "master charge limits" are changed. Not sure if we can do something similar here?

[ppuetsch]

I will implement it like follows: The Current limits in the configuration will always be relative to max_(dis)charge_current - e.g.:

CELL_VOLTAGES_WHILE_DISCHARGING = 2.70,2.80,2.90,3.10
MAX_DISCHARGE_CURRENT_CV = 0,0.08,0.5,1

So one can configure everything in a fine grained manner, but for most cases it's sufficient to only set charge/discharge limits