diff --git a/custom_components/bms_ble/plugins/jikong_bms.py b/custom_components/bms_ble/plugins/jikong_bms.py index 064cd99..98c408b 100644 --- a/custom_components/bms_ble/plugins/jikong_bms.py +++ b/custom_components/bms_ble/plugins/jikong_bms.py @@ -37,6 +37,7 @@ class BMS(BaseBMS): HEAD_RSP: Final = bytes([0x55, 0xAA, 0xEB, 0x90]) # header for responses HEAD_CMD: Final = bytes([0xAA, 0x55, 0x90, 0xEB]) # header for commands (endiness!) BT_MODULE_MSG: Final = bytes([0x41, 0x54, 0x0D, 0x0A]) # AT\r\n from BLE module + TYPE_POS: Final[int] = 4 # frame type is right after the header INFO_LEN: Final[int] = 300 def __init__(self, ble_device: BLEDevice, reconnect: bool = False) -> None: @@ -45,7 +46,7 @@ def __init__(self, ble_device: BLEDevice, reconnect: bool = False) -> None: self._data: bytearray = bytearray() self._data_final: bytearray | None = None self._char_write_handle: int | None = None - self._valid_replies: list[int] = [0x2] + self._valid_replies: list[int] = [0x2] # BMS ready confirmation self._FIELDS: Final[ list[tuple[str, int, int, bool, Callable[[int], int | float]]] ] = [ # Protocol: JK02_32S; JK02_24S has offset -32 @@ -105,14 +106,15 @@ def _calc_values() -> set[str]: def _notification_handler(self, _sender, data: bytearray) -> None: """Retrieve BMS data update.""" - if data[0 : len(self.BT_MODULE_MSG)] == self.BT_MODULE_MSG: - LOGGER.debug("(%s) filtering AT cmd", self._ble_device.name) + if data.startswith(self.BT_MODULE_MSG): + LOGGER.debug("(%s) filtering AT cmd", self.name) if len(data) == len(self.BT_MODULE_MSG): return data = data[len(self.BT_MODULE_MSG) :] if ( - len(self._data) >= self.INFO_LEN and data.startswith(self.HEAD_RSP) + len(self._data) >= self.INFO_LEN + and (data.startswith(self.HEAD_RSP) or data.startswith(self.HEAD_CMD)) ) or not self._data.startswith(self.HEAD_RSP): self._data = bytearray() @@ -120,32 +122,45 @@ def _notification_handler(self, _sender, data: bytearray) -> None: LOGGER.debug( "(%s) Rx BLE data (%s): %s", - self._ble_device.name, + self.name, "start" if data == self._data else "cnt.", data, ) - # verify that data long enough and if answer is cell info (0x2) - if len(self._data) < self.INFO_LEN: + # verify that data long enough + if ( + len(self._data) < self.INFO_LEN and self._data.startswith(self.HEAD_RSP) + ) or len(self._data) < self.TYPE_POS: return - if self._data[4] not in self._valid_replies: + # check that message type is expected + if self._data[self.TYPE_POS] not in self._valid_replies: LOGGER.debug( - "(%s) wrong message type %i (length %i): %s", + "(%s) unexpected message type 0x%x (length %i): %s", self.name, - self._data[4], + self._data[self.TYPE_POS], len(self._data), self._data, ) return - crc = self._crc(self._data[0 : self.INFO_LEN - 1]) - if self._data[self.INFO_LEN - 1] != crc: + # trim message in case oversized + if len(self._data) > self.INFO_LEN: + LOGGER.debug( + "(%s) Wrong data length (%i): %s", + self.name, + len(self._data), + self._data, + ) + self._data = self._data[: self.INFO_LEN] + + crc = self._crc(self._data[:-1]) + if self._data[-1] != crc: LOGGER.debug( "(%s) Rx data CRC is invalid: %i != %i", - self._ble_device.name, - self._data[self.INFO_LEN - 1], - self._crc(self._data[0 : self.INFO_LEN - 1]), + self.name, + self._data[-1], + self._crc(self._data[:-1]), ) self._data_final = None # reset invalid data else: @@ -161,7 +176,7 @@ async def _init_characteristics(self) -> None: for char in service.characteristics: LOGGER.debug( "(%s) Discovered %s (#%i): %s", - self._ble_device.name, + self.name, char.uuid, char.handle, char.properties, @@ -177,14 +192,14 @@ async def _init_characteristics(self) -> None: ): self._char_write_handle = char.handle if char_notify_handle is None or self._char_write_handle is None: - LOGGER.debug("(%s) Failed to detect characteristics", self._ble_device.name) + LOGGER.debug("(%s) Failed to detect characteristics", self.name) await self._client.disconnect() raise ConnectionError( - f"Failed to detect characteristics from {self._ble_device.name}." + f"Failed to detect characteristics from {self.name}." ) LOGGER.debug( "(%s) Using characteristics handle #%i (notify), #%i (write)", - self._ble_device.name, + self.name, char_notify_handle, self._char_write_handle, ) @@ -192,13 +207,13 @@ async def _init_characteristics(self) -> None: char_notify_handle or 0, self._notification_handler ) - # query device info frame (0x3) - self._valid_replies.append(0x3) + # query device info frame and wait for BMS ready (0xC8) + self._valid_replies.append(0xC8) await self._client.write_gatt_char( self._char_write_handle or 0, data=self._cmd(b"\x97") ) await asyncio.wait_for(self._wait_event(), timeout=BAT_TIMEOUT) - self._valid_replies.remove(0x3) + self._valid_replies.remove(0xC8) def _crc(self, frame: bytes) -> int: """Calculate Jikong frame CRC.""" @@ -239,7 +254,7 @@ async def _async_update(self) -> BMSsample: """Update battery status information.""" if not self._data_event.is_set(): # request cell info (only if data is not constantly published) - LOGGER.debug("(%s) request cell info", self._ble_device.name) + LOGGER.debug("(%s) request cell info", self.name) await self._client.write_gatt_char( self._char_write_handle or 0, data=self._cmd(b"\x96") ) @@ -248,14 +263,6 @@ async def _async_update(self) -> BMSsample: if self._data_final is None: return {} - if len(self._data_final) != self.INFO_LEN: - LOGGER.debug( - "(%s) Wrong data length (%i): %s", - self._ble_device.name, - len(self._data_final), - self._data_final, - ) - data = self._decode_data(self._data_final) data.update(self._cell_voltages(self._data_final, int(data[KEY_CELL_COUNT]))) diff --git a/tests/test_jikong_bms.py b/tests/test_jikong_bms.py index 44e190a..ab38d15 100644 --- a/tests/test_jikong_bms.py +++ b/tests/test_jikong_bms.py @@ -22,14 +22,14 @@ class MockJikongBleakClient(MockBleakClient): HEAD_CMD = bytearray(b"\xAA\x55\x90\xEB") CMD_INFO = bytearray(b"\x96") + DEV_INFO = bytearray(b"\x97") def _response( self, char_specifier: BleakGATTCharacteristic | int | str | UUID, data: Buffer ) -> bytearray: - if ( - char_specifier == 3 - and bytearray(data)[0:5] == self.HEAD_CMD + self.CMD_INFO - ): + if char_specifier != 3: + return bytearray() + if bytearray(data)[0:5] == self.HEAD_CMD + self.CMD_INFO: return bytearray( b"\x41\x54\x0d\x0a" # added AT\r\n command b"\x55\xaa\xeb\x90\x02\xc6\xc1\x0c\xc1\x0c\xc1\x0c\xc1\x0c\xc1\x0c\xc1\x0c" @@ -50,6 +50,25 @@ def _response( b"\x80\x51\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xfe" b"\xff\x7f\xdc\x2f\x01\x01\xb0\x07\x00\x00\x00\xd0" ) # {"temperature": 18.4, "voltage": 52.234, "current": -10.595, "battery_level": 42, "cycle_charge": 117.575, "cycles": 2} + if bytearray(data)[0:5] == self.HEAD_CMD + self.DEV_INFO: + return bytearray( + b"\x55\xaa\xeb\x90\x03\xa3\x4a\x4b\x5f\x42\x32\x41\x38\x53\x32\x30\x50\x00\x00\x00" + b"\x00\x00\x31\x31\x2e\x58\x41\x00\x00\x00\x31\x31\x2e\x34\x38\x00\x00\x00\xe4\xa7" + b"\x46\x00\x07\x00\x00\x00\x31\x32\x76\x34\x32\x30\x61\x00\x00\x00\x00\x00\x00\x00" + b"\x00\x00\x31\x32\x33\x34\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x32\x34" + b"\x30\x37\x30\x34\x00\x00\x34\x30\x34\x30\x39\x32\x43\x32\x32\x36\x32\x00\x30\x30" + b"\x30\x00\x49\x6e\x70\x75\x74\x20\x55\x73\x65\x72\x64\x61\x74\x61\x00\x00\x31\x34" + b"\x30\x37\x30\x33\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" + b"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xfe\xf9\xff\xff\x1f\x2d\x00\x02\x00\x00" + b"\x00\x00\x90\x1f\x00\x00\x00\x00\xc0\xd8\xe7\x32\x00\x00\x00\x01\x00\x00\x00\x00" + b"\x00\x00\x00\x00\x00\x00\x07\x04\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" + b"\x00\x00\x41\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" + b"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x09\x00\x00\x00\x64\x00" + b"\x00\x00\x5f\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" + b"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" + b"\x00\x00\x00\x00\x00\x00\x00\xfe\xbf\x21\x06\x00\x00\x00\x00\x00\x00\x00\x00\xd8" +# b"\xaa\x55\x90\xeb\xc8\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x44" + ) # Vendor_ID: JK_B2A8S20P, SN: 404092C2262, HW: V11.XA, SW: V11.48, power-on: 7, Version: 4.28.0 return bytearray() @@ -72,6 +91,8 @@ async def write_gatt_char( resp[i : i + BT_FRAME_SIZE] for i in range(0, len(resp), BT_FRAME_SIZE) ]: self._notify_callback("MockJikongBleakClient", notify_data) + if bytearray(data)[0:5] == self.HEAD_CMD + self.DEV_INFO: + self._notify_callback("MockJikongBleakClient", b"\xaa\x55\x90\xeb\xc8\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x44") class JKservice(BleakGATTService): """Mock the main battery info service from JiKong BMS.""" @@ -174,7 +195,7 @@ def characteristics(self) -> list[BleakGATTCharacteristic]: return [ self.CharNotify(None, lambda: 350), self.CharWrite(None, lambda: 350), - self.CharFaulty(None, lambda: 350), # leave last! + self.CharFaulty(None, lambda: 350), # leave last! ] def add_characteristic(self, characteristic: BleakGATTCharacteristic) -> None: @@ -207,13 +228,13 @@ def services(self) -> BleakGATTServiceCollection: class MockInvalidBleakClient(MockJikongBleakClient): """Emulate a Jikong BMS BleakClient returning wrong data.""" - def _response( - self, char_specifier: BleakGATTCharacteristic | int | str | UUID, data: Buffer - ) -> bytearray: - if char_specifier == 3: - return bytearray(b"\x55\xaa\xeb\x90\x02") + bytearray(295) + # def _response( + # self, char_specifier: BleakGATTCharacteristic | int | str | UUID, data: Buffer + # ) -> bytearray: + # if char_specifier == 3: + # return bytearray(b"\x55\xaa\xeb\x90\x02") + bytearray(295) - return bytearray() + # return bytearray() async def disconnect(self) -> bool: """Mock disconnect to raise BleakError.""" @@ -303,7 +324,7 @@ async def test_update(monkeypatch, reconnect_fixture) -> None: "temp#1": 18.1, "temp#2": 18.2, "temp#3": 18.0, - "temp#4": 18.3, + "temp#4": 18.3, } # query again to check already connected state @@ -315,9 +336,27 @@ async def test_update(monkeypatch, reconnect_fixture) -> None: await bms.disconnect() +# @pytest.fixture( +# name="wrong_response", +# params=[ +# bytearray(b"\x55\xaa\xeb\x90\x02") + bytearray(295), # incorrect CRC +# bytearray(b"\x55\xaa\xeb\x90\x05") + bytearray(295), # invalid frame type (0x5) +# ], +# ) +# def response(request): +# """Return all possible BMS variants.""" +# return request.param + + async def test_invalid_response(monkeypatch) -> None: """Test data update with BMS returning invalid data.""" + monkeypatch.setattr( + "tests.test_jikong_bms.MockInvalidBleakClient._response", + lambda _s, _c_, d: bytearray(b"\x55\xaa\xeb\x90\x02") + + bytearray(295), # incorrect CRC, + ) + monkeypatch.setattr( "custom_components.bms_ble.plugins.basebms.BleakClient", MockInvalidBleakClient, @@ -332,6 +371,35 @@ async def test_invalid_response(monkeypatch) -> None: await bms.disconnect() +async def test_invalid_frame_type(monkeypatch) -> None: + """Test data update with BMS returning invalid data.""" + + monkeypatch.setattr( + "custom_components.bms_ble.plugins.jikong_bms.BAT_TIMEOUT", + 0.1, + ) + + monkeypatch.setattr( + "tests.test_jikong_bms.MockInvalidBleakClient._response", + lambda _s, _c_, d: bytearray(b"\x55\xaa\xeb\x90\x05") + + bytearray(295), # invalid frame type (0x5) + ) + + monkeypatch.setattr( + "custom_components.bms_ble.plugins.basebms.BleakClient", + MockInvalidBleakClient, + ) + + bms = BMS(generate_ble_device("cc:cc:cc:cc:cc:cc", "MockBLEdevice", None, -73)) + + result = {} + with pytest.raises(TimeoutError): + result = await bms.async_update() + assert result == {} + + await bms.disconnect() + + async def test_oversized_response(monkeypatch) -> None: """Test data update with BMS returning oversized data, result shall still be ok.""" @@ -377,7 +445,7 @@ async def test_oversized_response(monkeypatch) -> None: "temp#1": 18.1, "temp#2": 18.2, "temp#3": 18.0, - "temp#4": 18.3, + "temp#4": 18.3, } await bms.disconnect()