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

JBD BMS: Balancing indicator not showing on correct cells #359

Closed
iLeeeZi opened this issue Dec 16, 2022 · 10 comments
Closed

JBD BMS: Balancing indicator not showing on correct cells #359

iLeeeZi opened this issue Dec 16, 2022 · 10 comments
Labels
bug Something isn't working

Comments

@iLeeeZi
Copy link

iLeeeZi commented Dec 16, 2022

Balancing indicators on Cell Voltages page are not showing on correct cells. The red indicators jump between cells showing 1-3 cells balancing and after a while it shows all cells balancing:
Screenshot_2022-12-16-22-20-26-57_bc2aea2f1887607afb8a748e894fe9f8

Checked with dbus-spy and it shows the same:
Screenshot_2022-12-16-22-21-32-49_3d419158bad5872c40592a6c9956e692

Switched to bluetooth adapter and xiaoxiang app is showing only two cells balancing as it should:
Screenshot_2022-12-16-22-25-53-65_3df8a9d4984557553da8791feb0ab07e

I have a JBD-SP04S034 4S 200A BMS with three temperature sensors, so there might be an issue with unusual message length. I need to check if disabling one temperature sensor in xiaoxiang app makes any difference.

Edit: Disabled one temp sensor and still the same issue.

@Louisvdw Louisvdw added the bug Something isn't working label Dec 29, 2022
@mr-manuel
Copy link
Collaborator

Have others with a JBD BMS the same issue?

@mr-manuel
Copy link
Collaborator

@iLeeeZi can you organize the protocol documentation so we can fix this? Please ask your seller or the manufacturer.

@iLeeeZi
Copy link
Author

iLeeeZi commented May 4, 2023

Did some testing and looks like that it is not a protocol issue.

I added logging after L123 in lltjbd.py
https://github.com/Louisvdw/dbus-serialbattery/blob/master/etc/dbus-serialbattery/bms/lltjbd.py#L123

logger.info(str(tmp))

And the data seems correct, but with data showing only two of the cells balancing, the UI indicated after a few seconds that three of the cells are constantly balancing

2023-05-04 15:55:04.967329500 INFO:SerialBattery:0001
2023-05-04 15:55:05.974664500 INFO:SerialBattery:0001
2023-05-04 15:55:06.965833500 INFO:SerialBattery:0001
2023-05-04 15:55:07.972370500 INFO:SerialBattery:0101
2023-05-04 15:55:08.978049500 INFO:SerialBattery:0001
2023-05-04 15:55:09.969268500 INFO:SerialBattery:0001
2023-05-04 15:55:10.976891500 INFO:SerialBattery:0101
2023-05-04 15:55:11.982067500 INFO:SerialBattery:0000
2023-05-04 15:55:12.973059500 INFO:SerialBattery:0001
2023-05-04 15:55:13.979577500 INFO:SerialBattery:0101
2023-05-04 15:55:14.986797500 INFO:SerialBattery:0001
2023-05-04 15:55:16.028883500 INFO:SerialBattery:0001
2023-05-04 15:55:17.018241500 INFO:SerialBattery:0101
2023-05-04 15:55:18.024716500 INFO:SerialBattery:0001
2023-05-04 15:55:19.015430500 INFO:SerialBattery:0001
2023-05-04 15:55:20.021196500 INFO:SerialBattery:0001
2023-05-04 15:55:21.012090500 INFO:SerialBattery:0001
2023-05-04 15:55:22.017786500 INFO:SerialBattery:0001
2023-05-04 15:55:23.025228500 INFO:SerialBattery:0101

@mr-manuel
Copy link
Collaborator

So the log data is correct, but the displayed data not?

@iLeeeZi
Copy link
Author

iLeeeZi commented May 9, 2023

Yes. The bms seems to send correct data, and the driver reads it correctly but dbus and gui is incorrect.

@mr-manuel
Copy link
Collaborator

Could you write me on Discord?

@mr-manuel
Copy link
Collaborator

Could you try to replace

def to_cell_bits(self, byte_data, byte_data_high):
# clear the list
for c in self.cells:
self.cells.remove(c)
# get up to the first 16 cells
tmp = bin(byte_data)[2:].rjust(min(self.cell_count, 16), utils.zero_char)
for bit in reversed(tmp):
self.cells.append(Cell(is_bit_set(bit)))
# get any cells above 16
if self.cell_count > 16:
tmp = bin(byte_data_high)[2:].rjust(self.cell_count - 16, utils.zero_char)
for bit in reversed(tmp):
self.cells.append(Cell(is_bit_set(bit)))

with

    def to_cell_bits(self, byte_data, byte_data_high):
        try:
            # get up to the first 16 cells
            tmp = bin(byte_data)[2:].rjust(min(self.cell_count, 16), utils.zero_char)
            logger.info("tmp: type: " + type(tmp) + " - value: " + tmp)
            # tmp = 0101
            tmp_reversed = reversed(tmp)
            # tmp_reversed = 1010
            logger.info("tmp_reversed: type: " + type(tmp_reversed) + " - value: " + tmp_reversed)

            test = tmp_reversed + reversed(tmp)
            logger.info("test: type: " + type(test) + " - value: " + test)

            if self.cell_count > 16:
                tmp2 = bin(byte_data_high)[2:].rjust(self.cell_count - 16, utils.zero_char)
                logger.info("tmp2: type: " + type(tmp2) + " - value: " + tmp2)
                tmp_reversed = tmp_reversed + reversed(tmp2)
                logger.info("tmp_reversed: type: " + type(tmp_reversed) + " - value: " + tmp_reversed)
            # tmp = 1010
            for c in range(self.cell_count):
                if is_bit_set(tmp_reversed[c]):
                    logger.info("balancing cell " + c)
                    self.cells[c].balance = True
                else:
                    self.cells[c].balance = False

        except Exception as err:
            logger.error(f"Unexpected {err=}, {type(err)=}")

?

It could be that nothing works, since I'm not sure about the var types and I have no JBD BMS to test. But the driver shouldn't crash.

@iLeeeZi
Copy link
Author

iLeeeZi commented May 10, 2023

I got errors on reversed() so changed it to [::-1]:

    def to_cell_bits(self, byte_data, byte_data_high):
        try:
            # get up to the first 16 cells
            tmp = bin(byte_data)[2:].rjust(min(self.cell_count, 16), utils.zero_char)
            logger.info("tmp: type: " + str(type(tmp)) + " - value: " + tmp)
            tmp_reversed = tmp[::-1]
            logger.info("tmp_reversed: type: " + str(type(tmp_reversed)) + " - value: " + tmp_reversed)

            test = tmp_reversed + tmp[::-1]
            logger.info("test: type: " + str(type(test)) + " - value: " + str(test))

            if self.cell_count > 16:
                tmp2 = bin(byte_data_high)[2:].rjust(self.cell_count - 16, utils.zero_char)
                logger.info("tmp2: type: " + str(type(tmp2)) + " - value: " + tmp2)
                tmp_reversed = tmp_reversed + tmp2[::-1]
                logger.info("tmp_reversed: type: " + str(type(tmp_reversed)) + " - value: " + tmp_reversed)

            for c in range(self.cell_count):
                if is_bit_set(tmp_reversed[c]):
                    logger.info("balancing cell " + str(c+1))
                    #self.cells.append(Cell(True))
                    self.cells[c].balance = True
                else:
                    #self.cells.append(Cell(False))
                    self.cells[c].balance = False

        except Exception as err:
            logger.error(f"Unexpected {err=}, {type(err)=}")

But without self.cells.append(Cell()) I get

2023-05-10 15:53:14.552053500 ERROR:SerialBattery:Unexpected err=IndexError('list index out of range'), type(err)=<class 'IndexError'>

In some other function

@mr-manuel
Copy link
Collaborator

Will be fixed with the next (pre)release.

@ngr58
Copy link

ngr58 commented Jun 16, 2023

Thank you @mr-manuel for fixing this. I have a JBD-AP20S003S BMS with 18 cells. I suspected that the red balancing indicators in SerialBattery were wrong ever since cell voltages became available but never had a chance to compare them to the xiaoxiang app. Saw this note in the changelog, upgraded to v1.0.20230531 and now they look much more normal! Thank you.

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

4 participants