From 32d4f92f6b6de39ef40f5c035e480125a66d2963 Mon Sep 17 00:00:00 2001 From: mletenay Date: Mon, 30 Aug 2021 10:17:27 +0200 Subject: [PATCH] Implement proper modbus response checks --- custom_components/goodwe/goodwe/dt.py | 4 ++-- custom_components/goodwe/goodwe/eh.py | 14 +++++++------- custom_components/goodwe/goodwe/et.py | 14 +++++++------- custom_components/goodwe/goodwe/modbus.py | 17 +++++++++++++---- custom_components/goodwe/goodwe/protocol.py | 19 ++++++++++++------- custom_components/goodwe/goodwe/sensor.py | 4 ---- 6 files changed, 41 insertions(+), 31 deletions(-) diff --git a/custom_components/goodwe/goodwe/dt.py b/custom_components/goodwe/goodwe/dt.py index 7cec3a4..d272e59 100644 --- a/custom_components/goodwe/goodwe/dt.py +++ b/custom_components/goodwe/goodwe/dt.py @@ -9,8 +9,8 @@ class DT(Inverter): """Class representing inverter of DT, D-NS and XS families""" - _READ_DEVICE_VERSION_INFO: ProtocolCommand = ModbusReadCommand(0x7531, 0x0028, 87) - _READ_DEVICE_RUNNING_DATA: ProtocolCommand = ModbusReadCommand(0x7594, 0x0049, 153) + _READ_DEVICE_VERSION_INFO: ProtocolCommand = ModbusReadCommand(0x7531, 0x0028) + _READ_DEVICE_RUNNING_DATA: ProtocolCommand = ModbusReadCommand(0x7594, 0x0049) __sensors: Tuple[Sensor, ...] = ( Timestamp("timestamp", 0, "Timestamp"), diff --git a/custom_components/goodwe/goodwe/eh.py b/custom_components/goodwe/goodwe/eh.py index 8e2d796..5945bc4 100644 --- a/custom_components/goodwe/goodwe/eh.py +++ b/custom_components/goodwe/goodwe/eh.py @@ -9,11 +9,11 @@ class EH(Inverter): """Class representing inverter of EH family""" - _READ_DEVICE_VERSION_INFO: ProtocolCommand = ModbusReadCommand(0x88b8, 0x0021, 146) - _READ_DEVICE_RUNNING_DATA1: ProtocolCommand = ModbusReadCommand(0x891c, 0x007d, 257) - _READ_DEVICE_RUNNING_DATA2: ProtocolCommand = ModbusReadCommand(0x8ca0, 0x001b, 61) - _READ_BATTERY_INFO: ProtocolCommand = ModbusReadCommand(0x9088, 0x000b, 29) - _GET_WORK_MODE: ProtocolCommand = ModbusReadCommand(0xb798, 0x0001, 9) + _READ_DEVICE_VERSION_INFO: ProtocolCommand = ModbusReadCommand(0x88b8, 0x0021) + _READ_DEVICE_RUNNING_DATA1: ProtocolCommand = ModbusReadCommand(0x891c, 0x007d) + _READ_DEVICE_RUNNING_DATA2: ProtocolCommand = ModbusReadCommand(0x8ca0, 0x001b) + _READ_BATTERY_INFO: ProtocolCommand = ModbusReadCommand(0x9088, 0x000b) + _GET_WORK_MODE: ProtocolCommand = ModbusReadCommand(0xb798, 0x0001) __sensors: Tuple[Sensor, ...] = ( Voltage("vpv1", 6, "PV1 Voltage", Kind.PV), @@ -123,7 +123,7 @@ class EH(Inverter): async def read_device_info(self): response = await self._read_from_socket(self._READ_DEVICE_VERSION_INFO) - response = response[12:22] + response = response[5:-2] self.modbus_version = read_unsigned_int(response, 0) self.rated_power = read_unsigned_int(response, 2) self.ac_output_type = read_unsigned_int(response, 4) @@ -150,7 +150,7 @@ async def set_work_mode(self, work_mode: int): async def set_ongrid_battery_dod(self, dod: int): if 0 <= dod <= 89: - await self._read_from_socket(ModbusWriteCommand(0xb12c, 100 - dod, 10)) + await self._read_from_socket(ModbusWriteCommand(0xb12c, 100 - dod)) @classmethod def sensors(cls) -> Tuple[Sensor, ...]: diff --git a/custom_components/goodwe/goodwe/et.py b/custom_components/goodwe/goodwe/et.py index a942103..e84d6cd 100644 --- a/custom_components/goodwe/goodwe/et.py +++ b/custom_components/goodwe/goodwe/et.py @@ -9,11 +9,11 @@ class ET(Inverter): """Class representing inverter of ET family""" - _READ_DEVICE_VERSION_INFO: ProtocolCommand = ModbusReadCommand(0x88b8, 0x0021, 73) - _READ_RUNNING_DATA: ProtocolCommand = ModbusReadCommand(0x891c, 0x007d, 257) - _READ_METER_DATA: ProtocolCommand = ModbusReadCommand(0x8ca0, 0x0011, 41) - _READ_BATTERY_INFO: ProtocolCommand = ModbusReadCommand(0x9088, 0x000b, 29) - _GET_WORK_MODE: ProtocolCommand = ModbusReadCommand(0xb798, 0x0001, 9) + _READ_DEVICE_VERSION_INFO: ProtocolCommand = ModbusReadCommand(0x88b8, 0x0021) + _READ_RUNNING_DATA: ProtocolCommand = ModbusReadCommand(0x891c, 0x007d) + _READ_METER_DATA: ProtocolCommand = ModbusReadCommand(0x8ca0, 0x0011) + _READ_BATTERY_INFO: ProtocolCommand = ModbusReadCommand(0x9088, 0x000b) + _GET_WORK_MODE: ProtocolCommand = ModbusReadCommand(0xb798, 0x0001) # Modbus registers from offset 0x891c (35100), count 0x7d (125) __sensors: Tuple[Sensor, ...] = ( @@ -208,7 +208,7 @@ async def read_runtime_data(self, include_unknown_sensors: bool = False) -> Dict async def read_settings(self, setting_id: str) -> Any: setting = {s.id_: s for s in self.settings()}.get(setting_id) - raw_data = await self._read_from_socket(ModbusReadCommand(setting.offset, 1, 9)) + raw_data = await self._read_from_socket(ModbusReadCommand(setting.offset, 1)) with io.BytesIO(raw_data[5:-2]) as buffer: return setting.read_value(buffer) @@ -225,7 +225,7 @@ async def set_work_mode(self, work_mode: int): async def set_ongrid_battery_dod(self, dod: int): if 0 <= dod <= 89: - await self._read_from_socket(ModbusWriteCommand(0xb12c, 100 - dod, 10)) + await self._read_from_socket(ModbusWriteCommand(0xb12c, 100 - dod)) @classmethod def sensors(cls) -> Tuple[Sensor, ...]: diff --git a/custom_components/goodwe/goodwe/modbus.py b/custom_components/goodwe/goodwe/modbus.py index 9e36785..9232afc 100644 --- a/custom_components/goodwe/goodwe/modbus.py +++ b/custom_components/goodwe/goodwe/modbus.py @@ -1,5 +1,9 @@ +import logging + from typing import Union +logger = logging.getLogger(__name__) + # default inverter modbus address _INVERTER_ADDRESS = 0xf7 @@ -69,14 +73,19 @@ def append_modbus_checksum(payload: str) -> bytes: return bytes(data) -def validate_modbus_response(data: bytes, response_len: int) -> bool: +def validate_modbus_response(data: bytes) -> bool: """ Validate the modbus response. data[0:1] is header data[2:3] is response type - data[4] is response payload length ?? + data[4] is response payload length data[-2:] is crc-16 checksum """ - if len(data) <= 4 or (response_len != 0 and response_len != len(data)): + if len(data) <= 4 or (data[4] > len(data) - 7): + logger.debug(f'Response has unexpected length: {len(data)}, expected {data[4] + 7}.') + return False + checksum_offset = data[4] + 5 + if _modbus_checksum(data[2:checksum_offset]) != ((data[checksum_offset + 1] << 8) + data[checksum_offset]): + logger.debug(f'Response CRC-16 checksum does not match.') return False - return _modbus_checksum(data[2:-2]) == ((data[-1] << 8) + data[-2]) + return True diff --git a/custom_components/goodwe/goodwe/protocol.py b/custom_components/goodwe/goodwe/protocol.py index 41dcecf..cbd3719 100644 --- a/custom_components/goodwe/goodwe/protocol.py +++ b/custom_components/goodwe/goodwe/protocol.py @@ -163,12 +163,17 @@ def _validate_response(data: bytes, response_type: str) -> bool: or len(data) != data[6] + 9 or (response_type and int(response_type, 16) != int.from_bytes(data[4:6], byteorder="big", signed=True)) ): + logger.debug(f'Response has unexpected length: {len(data)}, expected {data[6] + 9}.') return False else: checksum = 0 for each in data[:-2]: checksum += each - return checksum == int.from_bytes(data[-2:], byteorder="big", signed=True) + if checksum != int.from_bytes(data[-2:], byteorder="big", signed=True): + logger.debug(f'Response checksum does not match.') + return False + return True + class ModbusProtocolCommand(ProtocolCommand): @@ -194,10 +199,10 @@ class ModbusProtocolCommand(ProtocolCommand): Last 2 bytes of response is again the CRC-16 of the response. """ - def __init__(self, payload: str, response_len: int = 0): + def __init__(self, payload: str): super().__init__( append_modbus_checksum(payload), - lambda x: validate_modbus_response(x, response_len), + lambda x: validate_modbus_response(x), ) @@ -206,10 +211,10 @@ class ModbusReadCommand(ProtocolCommand): Inverter modbus READ command for retrieving modbus registers starting at register # """ - def __init__(self, offset: int, count: int, response_len: int = 0): + def __init__(self, offset: int, count: int): super().__init__( create_modbus_request(MODBUS_READ_CMD, offset, count), - lambda x: validate_modbus_response(x, response_len), + lambda x: validate_modbus_response(x), ) @@ -218,8 +223,8 @@ class ModbusWriteCommand(ProtocolCommand): Inverter modbus WRITE command setting to modbus register # value """ - def __init__(self, register: int, value: int, response_len: int = 0): + def __init__(self, register: int, value: int): super().__init__( create_modbus_request(MODBUS_WRITE_CMD, register, value), - lambda x: validate_modbus_response(x, response_len), + lambda x: validate_modbus_response(x), ) diff --git a/custom_components/goodwe/goodwe/sensor.py b/custom_components/goodwe/goodwe/sensor.py index 32a63f6..1f81fcb 100644 --- a/custom_components/goodwe/goodwe/sensor.py +++ b/custom_components/goodwe/goodwe/sensor.py @@ -220,8 +220,6 @@ def read_current(buffer: io.BytesIO, offset: int = None) -> float: if offset: buffer.seek(offset) value = int.from_bytes(buffer.read(2), byteorder="big", signed=True) - if value > 32768: - value = value - 65535 return float(value) / 10 @@ -240,8 +238,6 @@ def read_power2(buffer: io.BytesIO, offset: int = None) -> int: if offset: buffer.seek(offset) value = int.from_bytes(buffer.read(2), byteorder="big", signed=True) - if value > 32768: - value = value - 65535 return value