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

Cell Temperatur display in Venus does not reflect cells #1072

Closed
endurance1968 opened this issue Jun 2, 2024 · 13 comments
Closed

Cell Temperatur display in Venus does not reflect cells #1072

endurance1968 opened this issue Jun 2, 2024 · 13 comments
Labels
bug Something isn't working

Comments

@endurance1968
Copy link

Describe the bug

Hello,

image

image

looks like cell temperatures are more the BMS and env temp. not the cells as such?
Can it be configured, that BMS and env temp are maybe ignored here?

How to reproduce

Nothing specific to be done just compare values

Expected behavior

Venus is showing cell temperatures

Driver version

v1.2.20240408

Venus OS device type

Cerbo GX

Venus OS version

3.32

BMS type

Seplos

Cell count

16

Battery count

1

Connection type

Serial USB adapter to RS485

Config file

[DEFAULT]

LOGGING = DEBUG

; If you want to add custom values/settings, then check the values/settings you want to change in "config.default.ini"
; and insert them below to persist future driver updates.

; Example (remove the semicolon ";" to uncomment and activate the value/setting):
MAX_BATTERY_CHARGE_CURRENT = 150.0
MAX_BATTERY_DISCHARGE_CURRENT = 140.0

; --------- Cell Voltages ---------
; Description:
;     Cell min/max voltages which are used to calculate the min/max battery voltage
; Example:
;     16 cells * 3.45V/cell = 55.2V max charge voltage. 16 cells * 2.90V = 46.4V min discharge voltage
MIN_CELL_VOLTAGE   = 2.900
; Max voltage (can seen as absorption voltage)
MAX_CELL_VOLTAGE   = 3.45
; Float voltage (can be seen as resting voltage)
FLOAT_CELL_VOLTAGE = 3.4
BMS_TYPE = Seplos
SOC_LOW_WARNING = 15
SOC_LOW_ALARM = 10

Relevant log output

@40000000665c2c6f1bd7c4fc DEBUG:SerialBattery:wrote 20 bytes to serial port /dev/ttyUSB0, command=b'~20004644E00201FD34\r'
@40000000665c2c702163eb94 DEBUG:SerialBattery:returning info data of length 98, info_length is 98 : b'00001000000000000000000000000000000000060000000000000000140000000000000300000200000000000000000002'
@40000000665c2c7021cf4024 DEBUG:SerialBattery:alarm info raw b'00001000000000000000000000000000000000060000000000000000140000000000000300000200000000000000000002'
@40000000665c2c7021dab1d4 DEBUG:SerialBattery:alarm info decoded b'\x00\x00\x10\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x06\x00\x00\x00\x00\x00\x00\x00\x00\x14\x00\x00\x00\x00\x00\x00\x03\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02'
@40000000665c2c70246a6534 DEBUG:SerialBattery:logged to dbus [20.3]
@40000000665c2c7024a08c7c DEBUG:SerialBattery:Cells:[1]3.248V [2]3.249V [3]3.249V [4]3.249V [5]3.248V [6]3.252V [7]3.251V [8]3.251V [9]3.25V [10]3.252V [11]3.25V [12]3.251V [13]3.254V [14]3.252V [15]3.251V [16]3.251V
@40000000665c2c7307f70254 DEBUG:SerialBattery:read status data
@40000000665c2c730807f244 DEBUG:SerialBattery:read serial data seplos
@40000000665c2c730847e174 DEBUG:SerialBattery:wrote 20 bytes to serial port /dev/ttyUSB0, command=b'~20004642E00201FD36\r'
@40000000665c2c7411effcd4 DEBUG:SerialBattery:returning info data of length 150, info_length is 150 : b'0000100CB00CB10CB10CB10CB00CB40CB30CB30CB20CB40CB20CB30CB50CB40CB30CB3060BB00BAE0BB40BAF0BBD0B9C0095145118360A76C000CB76C0000803E8145200110000037D034F'
@40000000665c2c74125d2a0c DEBUG:SerialBattery:Voltage cell[0]=3.248V
@40000000665c2c741269767c DEBUG:SerialBattery:Voltage cell[1]=3.249V
@40000000665c2c7412772e34 DEBUG:SerialBattery:Voltage cell[2]=3.249V
@40000000665c2c741284b70c DEBUG:SerialBattery:Voltage cell[3]=3.249V
@40000000665c2c741290961c DEBUG:SerialBattery:Voltage cell[4]=3.248V
@40000000665c2c74129d05b4 DEBUG:SerialBattery:Voltage cell[5]=3.252V
@40000000665c2c7412a8d90c DEBUG:SerialBattery:Voltage cell[6]=3.251V
@40000000665c2c7412b3c204 DEBUG:SerialBattery:Voltage cell[7]=3.251V
@40000000665c2c7412bf9d2c DEBUG:SerialBattery:Voltage cell[8]=3.25V
@40000000665c2c7412cacc74 DEBUG:SerialBattery:Voltage cell[9]=3.252V
@40000000665c2c7412d5674c DEBUG:SerialBattery:Voltage cell[10]=3.25V
@40000000665c2c7412e11394 DEBUG:SerialBattery:Voltage cell[11]=3.251V
@40000000665c2c7412ebc1f4 DEBUG:SerialBattery:Voltage cell[12]=3.253V
@40000000665c2c7412f64944 DEBUG:SerialBattery:Voltage cell[13]=3.252V
@40000000665c2c741300a984 DEBUG:SerialBattery:Voltage cell[14]=3.251V
@40000000665c2c74130d53b4 DEBUG:SerialBattery:Voltage cell[15]=3.251V
@40000000665c2c74131b3a4c DEBUG:SerialBattery:Temp cell[0]=26.1°C
@40000000665c2c74132976d4 DEBUG:SerialBattery:Temp cell[1]=25.9°C
@40000000665c2c741332d15c DEBUG:SerialBattery:Temp cell[2]=26.5°C
@40000000665c2c74133dfcbc DEBUG:SerialBattery:Temp cell[3]=26.0°C
@40000000665c2c74134bbc44 DEBUG:SerialBattery:Current = 1.49A , Voltage = 52.01V
@40000000665c2c741355ca7c DEBUG:SerialBattery:Capacity = 61.98/304.0Ah , SOC = 20.3%
@40000000665c2c74135e2eec DEBUG:SerialBattery:Cycles = 8
@40000000665c2c7413693724 DEBUG:SerialBattery:Environment temp = 27.4°C ,  Power temp = 24.1°C
@40000000665c2c7413751e04 DEBUG:SerialBattery:HW:Seplos BMS 16 cells

Any other information that may be helpful

NA

@endurance1968 endurance1968 added the bug Something isn't working label Jun 2, 2024
@mr-manuel
Copy link
Collaborator

@wollew can you take a look?

@endurance1968 in the meanwhile did you read the config.default.ini?

; Battery temperature
; Specify how the battery temperature is assembled
; 0 Get mean of temperature sensor 1 to sensor 4
; 1 Get only temperature from temperature sensor 1
; 2 Get only temperature from temperature sensor 2
; 3 Get only temperature from temperature sensor 3
; 4 Get only temperature from temperature sensor 4
TEMP_BATTERY = 0
; Temperature sensor 1 name
TEMP_1_NAME = Temp 1
; Temperature sensor 2 name
TEMP_2_NAME = Temp 2
; Temperature sensor 2 name
TEMP_3_NAME = Temp 3
; Temperature sensor 2 name
TEMP_4_NAME = Temp 4

@endurance1968
Copy link
Author

Hi,

thanks for quick reaction.

I have seen this section but honestly was not sure how to use it / if it is relevant for the above scenario.
Venus reports min and max cell temp - config decsribes either mean or fixed assigned ONE batt. temp.
It total the debug log shows 6 temperatures (cell 0-3 + env + power). For me it looks like min max cell reported to venus are taken from the env+power instead of cell 0-3

@wollew
Copy link
Contributor

wollew commented Jun 2, 2024

@mr-manuel: the Seplos driver fills in Cell[1..4].temp for the 4 known cell temperatures and Battery.temp1,2 for the other two measured temperatures. But quickly looking at the rest of the code it seems Cell.temp isn't really used (any more?). Should this be changed in the Seplos driver?

@mr-manuel
Copy link
Collaborator

So the Seplos BMS has a total of 6 temperature sensors?

In this driver we always handled the temperatures like this

def to_temp(self, sensor: int, value: float) -> None:
"""
Keep the temp value between -20 and 100 to handle sensor issues or no data.
The BMS should have already protected before those limits have been reached.
:param sensor: temperature sensor number
:param value: the sensor value
:return:
"""
if sensor == 0:
self.temp_mos = round(min(max(value, -20), 100), 1)
if sensor == 1:
self.temp1 = round(min(max(value, -20), 100), 1)
if sensor == 2:
self.temp2 = round(min(max(value, -20), 100), 1)
if sensor == 3:
self.temp3 = round(min(max(value, -20), 100), 1)
if sensor == 4:
self.temp4 = round(min(max(value, -20), 100), 1)

There where no self.cells[i].temp values that I'm aware of like in the Seplos driver

for i in range(min(4, self.cell_count)):
temp = (
Seplos.int_from_2byte_hex_ascii(data, temps_offset + i * 4) - 2731
) / 10
self.cells[i].temp = temp
logger.debug("Temp cell[{}]={}°C".format(i, temp))

My proposal would be to add the cell temperatures as temp1 to temp4 and maybe one of the remaining sensors as mosfet temperature, if it matches?

If you'd like we can also add temp5 and temp6. But I think that would be a bit too much. Should be enough, if you get the min/max temperatures of the cells and the other two. So you have 4 in total.

@endurance1968
Copy link
Author

endurance1968 commented Jun 2, 2024

From my point of view most important to get the cell temp min/max correct.
Being able to see mosfet (I assume it is equal to power) and environment (ambient) would be great add on.

And yes according to BMS docu:
By detecting the temperature of cells (4 of the 16 cells), ambient temperature, and temperature of PCB board
it should be 6 sensors for temp.
Screenshot from BMS Software
image

@wollew
Copy link
Contributor

wollew commented Jun 2, 2024

Yes, I also assume that what is described as "Power" in the Seplos protocol is the mosfet temperature. So this should go into temp_mos. We can use temp1..4 for the cell temperatures. Adding temp5 for the ambient temperature would break the "calculation" for cell min/max temperature, so if we want to provide that sensor value to the driver, we'd need something else, temp_ambient or something along these lines.

@mr-manuel: do you want me to prepare a PR? This sh/would include removing "temp" from the Cell class, this where I got the initial idea from:

class Cell:
"""
This class holds information about a single Cell
"""
voltage = None
balance = None
temp = None

@mr-manuel
Copy link
Collaborator

Ok, so in this case I would propose:

  • Use power temperature as mosfet temperature (best would be to test if this temperature gets higher than the ambient temperature under high load and long duration)
  • Use cell temperatures for temp1-4
  • Remote the temp from the cell class, as you said

Yes, please make a PR for this in my repository.

@endurance1968
Copy link
Author

endurance1968 commented Jun 2, 2024

2024-06-02_21h28_16
2024-06-02_21h44_21
image

power gets higher than ambient

@mr-manuel
Copy link
Collaborator

Perfect, than this should be the mosfet temperature sensor.

mr-manuel added a commit to mr-manuel/venus-os_dbus-serialbattery that referenced this issue Jun 3, 2024
@mr-manuel
Copy link
Collaborator

Can you install v1.3.20240603dev and see if all is working as expected?

@wollew
Copy link
Contributor

wollew commented Jun 3, 2024

Sorry, I hadn't seen that you wanted the PR against a different repo.

@mr-manuel
Copy link
Collaborator

No problem :-)

@endurance1968
Copy link
Author

2024-06-03_18h24_28
2024-06-03_18h41_05
looks good.
2024-06-03_18h41_44
2024-06-03_18h41_52

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

Successfully merging a pull request may close this issue.

3 participants