Skip to content
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

Remove round function for soc_calc_capacity_remain #100

Closed
jojo18222 opened this issue Oct 30, 2024 · 9 comments
Closed

Remove round function for soc_calc_capacity_remain #100

jojo18222 opened this issue Oct 30, 2024 · 9 comments
Labels
bug Something isn't working

Comments

@jojo18222
Copy link

Describe the bug

In File battery.py Line 470 the calculatated soc_calc_capacity_remain is rounded to 3 decimal places. This should removed because small current values have no affect and so the soc_calc_capacity_remain and Soc value doesn't change.

How to reproduce

SOC_CALCULATION = True
SOC_CALC_CURRENT_REPORTED_BY_BMS = -300, 0, 300
SOC_CALC_CURRENT_MEASURED_BY_USER = -300, -0.1, 300
Low Soc is reached, BMS current is 0A, calculatated Soc stays forever at the low Soc value.

Expected behavior

Soc should slowly reduced.

Driver version of the currently installed driver

1.4.20240902dev

Driver version of the last known working driver

No response

Venus OS device type

Raspberry Pi 4

Venus OS version

3.50

BMS type

Daly BMS

Cell count

16

Battery count

1

Connection type

Serial USB adapter to RS485

Config file

SOC_CALCULATION   = True
SOC_CALC_CURRENT_REPORTED_BY_BMS  = -300, 0, 300
SOC_CALC_CURRENT_MEASURED_BY_USER = -300, -0.1, 300

Relevant log output

-

Any other information that may be helpful

No response

@jojo18222 jojo18222 added the bug Something isn't working label Oct 30, 2024
@mr-manuel
Copy link
Owner

Currents smaller than three decimals are so low that your measurement reading is probably not accurate enough to measure it. Even a SmartShunt has a maximum accuracy of 0.1 A. I don't think that a Daly BMS is that accurate. Probably your correction table should be updated.

@jojo18222
Copy link
Author

Correct, the Daly BMS current starts at 0.4A I think.
I have connected Venus os raspi with a DC/DC converter directly to the battery. The clamp meter shows round about 0.1A.
So in the correction table I set if BMS shows 0A, the real value is -0.1A.
With an older version of serial driver, the Soc was reduced slowly if the multiplus was in idle, for example at low Soc situation.
Now, with the rounded value, Soc stays forever at a Soc value and I think this is not how it should work.

@mr-manuel
Copy link
Owner

Which older version? I don‘t think that this is the issue. You should calibrate the current measurement on your Daly BMS, if this is possible with Daly. I tested with a simulated current of 0.01 A and it worked correctly. The current measurement for that current is simply too small.

@jojo18222
Copy link
Author

Here is the round function not implemented:
https://github.com/Louisvdw/dbus-serialbattery/blob/fd0fc1a193427df6ca0fadd141aa57826f761819/etc/dbus-serialbattery/battery.py#L280
Ok, I don't have further arguments. I can't calibrate the daly bms, because the current of the raspi is to low (5 Watts).
And if soc_calc_capacity_remain is rounded and the Multiplus 2 is in idle or off, the soc never get lower although the raspi consumes 5 Watts. After some days or longer, the real soc is way lower then the calculated soc.
I don't understand the advantages of the soc_calc_capacity_remain is rounded, because a line before, the soc_calc_capacity_remain is calculated without rounding and later, the soc value is rounded with 2 digits anyway.

@mr-manuel mr-manuel reopened this Nov 4, 2024
@mr-manuel
Copy link
Owner

Could you try to change the rounding to 6 decimals? Some values are rounded, to filter out stupid values like 45.0000000000001 or 29.99999999999998 that happen sometimes.

@jojo18222
Copy link
Author

Sure, I can test 6 decimal places. But as I can see, this value (soc_calc_capacity_remain) is never published to dbus, so it is only a internal value for soc calculaction, and the soc value is rounded to 2 decimal places, which makes sense.
https://github.com/mr-manuel/venus-os_dbus-serialbattery/blob/7113c1fe0feb436c53d62195cae8fa5bf3e16879/etc/dbus-serialbattery/battery.py#L527
So there is no advantages to round soc_calc_capacity_remain, only that the soc value is more inexact in my opinion.

@mr-manuel
Copy link
Owner

Now I had more time to check the code. Indeed it does not make any sense. It was rounded since the first PR. Currently I have a bigger change in the pipeline, therefore it will take some time, but the rounding will be removed in the next version. Until then I leave the issue open.

mr-manuel added a commit that referenced this issue Nov 6, 2024
@jojo18222
Copy link
Author

Hey, thanks for the change. Sorry to bother you again. You also changed the digital places for soc_calc from 2 to 3.
But this has no effect because in dbushelper.py soc_calc is rounded back to 2 decimal places.

self._dbusservice["/Soc"] = round(self.battery.soc_calc, 2) if self.battery.soc_calc is not None else None

@mr-manuel
Copy link
Owner

Yes I changed that also. This is only relevant for a more pricise saving of the calculated SOC to the dbus. Useful for batteries with >1.000 Ah.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants