From cab3c0ad2e9a8d4fe8e432263b6afadfad693902 Mon Sep 17 00:00:00 2001 From: David Rapan Date: Sat, 5 Oct 2024 22:59:31 +0200 Subject: [PATCH] refactor: Code cleanup --- custom_components/solarman/__init__.py | 36 ++++++------- custom_components/solarman/api.py | 62 ++++++++--------------- custom_components/solarman/config_flow.py | 18 +++---- custom_components/solarman/const.py | 12 ++--- custom_components/solarman/coordinator.py | 13 ++--- 5 files changed, 58 insertions(+), 83 deletions(-) diff --git a/custom_components/solarman/__init__.py b/custom_components/solarman/__init__.py index b922f53..20cc5f1 100644 --- a/custom_components/solarman/__init__.py +++ b/custom_components/solarman/__init__.py @@ -24,42 +24,38 @@ async def async_setup_entry(hass: HomeAssistant, config: ConfigEntry) -> bool: data = config.data name = data.get(CONF_NAME) - inverter_serial = data.get(CONF_INVERTER_SERIAL) + serial = data.get(CONF_SERIAL) options = config.options inverter_host = options.get(CONF_INVERTER_HOST) inverter_port = options.get(CONF_INVERTER_PORT) - inverter_mb_slave_id = options.get(CONF_INVERTER_MB_SLAVE_ID) + mb_slave_id = options.get(CONF_MB_SLAVE_ID) inverter_mac = None lookup_file = options.get(CONF_LOOKUP_FILE) lookup_path = hass.config.path(LOOKUP_DIRECTORY_PATH) - try: - ipaddr = IPv4Address(inverter_host) - except AddressValueError: - ipaddr = IPv4Address(socket.gethostbyname(inverter_host)) - if ipaddr.is_private and (discover := await InverterDiscovery(hass, inverter_host, inverter_serial).discover()): - if device := get_or_default(discover, inverter_serial): - inverter_host = device["ip"] - inverter_mac = device["mac"] - elif device := get_or_default(discover, (serial := next(iter([k for k, v in discover.items() if v["ip"] == inverter_host]), None))): - _LOGGER.warning(f"Host {inverter_host} has '{serial}' serial number but is configured with '{inverter_serial}'.") - inverter_serial = serial - inverter_mac = device["mac"] - + if serial is None: + raise vol.Invalid("Configuration parameter [inverter_serial] does not have a value") if inverter_host is None: raise vol.Invalid("Configuration parameter [inverter_host] does not have a value") - if inverter_serial is None: - raise vol.Invalid("Configuration parameter [inverter_serial] does not have a value") if inverter_port is None: raise vol.Invalid("Configuration parameter [inverter_port] does not have a value") - if inverter_mb_slave_id is None: - inverter_mb_slave_id = DEFAULT_INVERTER_MB_SLAVE_ID if lookup_file is None: raise vol.Invalid("Configuration parameter [lookup_file] does not have a value") - inverter = Inverter(inverter_host, inverter_serial, inverter_port, inverter_mb_slave_id) + try: + ipaddr = IPv4Address(inverter_host) + except AddressValueError: + ipaddr = IPv4Address(socket.gethostbyname(inverter_host)) + if ipaddr.is_private and (discover := await InverterDiscovery(hass, inverter_host, serial).discover()): + if device := get_or_default(discover, serial): + inverter_host = device["ip"] + inverter_mac = device["mac"] + elif device := get_or_default(discover, (s := next(iter([k for k, v in discover.items() if v["ip"] == inverter_host]), None))): + raise vol.Invalid(f"Host {inverter_host} has serial number {s} but is configured with {serial}.") + + inverter = Inverter(inverter_host, serial, inverter_port, mb_slave_id if mb_slave_id else DEFAULT_MB_SLAVE_ID) await inverter.load(name, inverter_mac, lookup_path, lookup_file) diff --git a/custom_components/solarman/api.py b/custom_components/solarman/api.py index 335de12..b97aa5a 100644 --- a/custom_components/solarman/api.py +++ b/custom_components/solarman/api.py @@ -22,8 +22,7 @@ _LOGGER = logging.getLogger(__name__) class PySolarmanV5AsyncWrapper(PySolarmanV5Async): - sm_start = bytes.fromhex("AA") - sm_passthrough = False + _passthrough = False def __init__(self, address, serial, port, mb_slave_id): super().__init__(address, serial, port = port, mb_slave_id = mb_slave_id, logger = _LOGGER, auto_reconnect = AUTO_RECONNECT, socket_timeout = TIMINGS_SOCKET_TIMEOUT) @@ -32,14 +31,14 @@ async def _tcp_parse_response_adu(self, mb_request_frame): return parse_response_adu(await self._send_receive_v5_frame(mb_request_frame), mb_request_frame) def _received_frame_is_valid(self, frame): - if self.sm_passthrough: + if self._passthrough: return True if not frame.startswith(self.v5_start): self.log.debug("[%s] V5_MISMATCH: %s", self.serial, frame.hex(" ")) return False if frame[5] != self.sequence_number and is_ethernet_frame(frame): self.log.debug("[%s] V5_ETHERNET_DETECTED: %s", self.serial, frame.hex(" ")) - self.sm_passthrough = True + self._passthrough = True return False if frame[5] != self.sequence_number: self.log.debug("[%s] V5_SEQ_NO_MISMATCH: %s", self.serial, frame.hex(" ")) @@ -68,42 +67,42 @@ async def disconnect(self) -> None: self.writer = None async def read_coils(self, register_addr, quantity): - if not self.sm_passthrough: + if not self._passthrough: return await super().read_coils(register_addr, quantity) return await self._tcp_parse_response_adu(read_coils(self.mb_slave_id, register_addr, quantity)) async def read_discrete_inputs(self, register_addr, quantity): - if not self.sm_passthrough: + if not self._passthrough: return await super().read_discrete_inputs(register_addr, quantity) return await self._tcp_parse_response_adu(read_discrete_inputs(self.mb_slave_id, register_addr, quantity)) async def read_input_registers(self, register_addr, quantity): - if not self.sm_passthrough: + if not self._passthrough: return await super().read_input_registers(register_addr, quantity) return await self._tcp_parse_response_adu(read_input_registers(self.mb_slave_id, register_addr, quantity)) async def read_holding_registers(self, register_addr, quantity): - if not self.sm_passthrough: + if not self._passthrough: return await super().read_holding_registers(register_addr, quantity) return await self._tcp_parse_response_adu(read_holding_registers(self.mb_slave_id, register_addr, quantity)) async def write_single_coil(self, register_addr, value): - if not self.sm_passthrough: + if not self._passthrough: return await super().write_single_coil(register_addr, value) return await self._tcp_parse_response_adu(write_single_coil(self.mb_slave_id, register_addr, value)) async def write_multiple_coils(self, register_addr, values): - if not self.sm_passthrough: + if not self._passthrough: return await super().write_multiple_coils(register_addr, values) return await self._tcp_parse_response_adu(write_multiple_coils(self.mb_slave_id, register_addr, values)) async def write_holding_register(self, register_addr, value): - if not self.sm_passthrough: + if not self._passthrough: return await super().write_holding_register(register_addr, value) return await self._tcp_parse_response_adu(write_single_register(self.mb_slave_id, register_addr, value)) async def write_multiple_holding_registers(self, register_addr, values): - if not self.sm_passthrough: + if not self._passthrough: return await super().write_multiple_holding_registers(register_addr, values) return await self._tcp_parse_response_adu(write_multiple_registers(self.mb_slave_id, register_addr, values)) @@ -210,14 +209,14 @@ def get_result(self, middleware = None): return result - async def get_failed(self, message = None): + async def get_failed(self): _LOGGER.debug(f"[{self.serial}] Request failed. [Previous State: {self.get_connection_state()} ({self.state})]") + self.state = 0 if self.state == 1 else -1 await self.disconnect() - if message and self.state == -1: - raise UpdateFailed(message) + return self.state == -1 async def get(self, runtime = 0): requests = self.profile.get_requests(runtime) @@ -250,12 +249,9 @@ async def get(self, runtime = 0): try: self.profile.parse(await self.read_write(code, start, quantity), start, quantity) results[i] = 1 + _LOGGER.debug(f"[{self.serial}] Querying {start_end} succeeded.") except (V5FrameError, TimeoutError, Exception) as e: - results[i] = 0 - - #if ((not isinstance(e, TimeoutError) or not attempts_left >= 1) and not (not isinstance(e, TimeoutError) or (e.__cause__ and isinstance(e.__cause__, OSError) and e.__cause__.errno == errno.EHOSTUNREACH))) or _LOGGER.isEnabledFor(logging.DEBUG): - # _LOGGER.warning(f"[{self.serial}] Querying {start_end} failed. #{runtime} [{format_exception(e)}]") - _LOGGER.debug(f"[{self.serial}] Querying {start_end} failed. [{format_exception(e)}]") + _LOGGER.debug(f"[{self.serial}] Querying {start_end} failed, attempts left: {attempts_left}{'' if attempts_left > 0 else ', aborting.'} [{format_exception(e)}]") if not self.auto_reconnect: await self.disconnect() @@ -265,32 +261,18 @@ async def get(self, runtime = 0): await asyncio.sleep((ACTION_ATTEMPTS - attempts_left) * TIMINGS_WAIT_SLEEP) - _LOGGER.debug(f"[{self.serial}] Querying {start_end} {'succeeded.' if results[i] == 1 else f'attempts left: {attempts_left}{'' if attempts_left > 0 else ', aborting.'}'}") - - if results[i] == 0: - break - - if not 0 in results: - return self.get_result(self.profile) - else: - await self.get_failed(f"[{self.serial}] Querying {self.address}:{self.port} failed: {results}") - except TimeoutError: - last_state = self.state - await self.get_failed() - if last_state < 1: + if await self.get_failed(): raise - else: - _LOGGER.debug(f"[{self.serial}] Timeout fetching {self.name} data") - except UpdateFailed: - raise + _LOGGER.debug(f"[{self.serial}] Timeout fetching {self.name} data") except Exception as e: - #await self.get_failed(f"[{self.serial}] Querying {self.address}:{self.port} failed: {results} with exception: {format_exception(e)}.") - await self.get_failed(f"[{self.serial}] {format_exception(e)}: {results}") + if await self.get_failed(): + raise UpdateFailed(f"[{self.serial}] {format_exception(e)} {results}") from e + _LOGGER.debug(f"[{self.serial}] Error fetching {self.name} data: {e} {results}") finally: self._is_busy = 0 - return self.get_result() + return self.get_result(self.profile) async def call(self, code, start, arg, wait_for_attempts = ACTION_ATTEMPTS) -> bool: _LOGGER.debug(f"[{self.serial}] call code {code}: {start} | 0x{start:04X}, arg: {arg}, wait_for_attempts: {wait_for_attempts}") diff --git a/custom_components/solarman/config_flow.py b/custom_components/solarman/config_flow.py index a8fc2c2..3a984b8 100644 --- a/custom_components/solarman/config_flow.py +++ b/custom_components/solarman/config_flow.py @@ -29,9 +29,9 @@ async def async_update_listener(hass: HomeAssistant, entry: ConfigEntry) -> None def step_user_data_prefill(ip, serial): _LOGGER.debug(f"step_user_data_prefill: IP: {ip}, serial: {serial}") - return { CONF_NAME: DEFAULT_NAME, CONF_DISCOVERY: DEFAULT_DISCOVERY, CONF_INVERTER_HOST: ip, CONF_INVERTER_SERIAL: serial, CONF_INVERTER_PORT: DEFAULT_INVERTER_PORT, CONF_INVERTER_MB_SLAVE_ID: DEFAULT_INVERTER_MB_SLAVE_ID, CONF_PASSTHROUGH: DEFAULT_PASSTHROUGH, CONF_LOOKUP_FILE: DEFAULT_LOOKUP_FILE, CONF_BATTERY_NOMINAL_VOLTAGE: DEFAULT_BATTERY_NOMINAL_VOLTAGE, CONF_BATTERY_LIFE_CYCLE_RATING: DEFAULT_BATTERY_LIFE_CYCLE_RATING } + return { CONF_NAME: DEFAULT_NAME, CONF_INVERTER_HOST: ip, CONF_SERIAL: serial, CONF_INVERTER_PORT: DEFAULT_INVERTER_PORT, CONF_MB_SLAVE_ID: DEFAULT_MB_SLAVE_ID, CONF_LOOKUP_FILE: DEFAULT_LOOKUP_FILE, CONF_BATTERY_NOMINAL_VOLTAGE: DEFAULT_BATTERY_NOMINAL_VOLTAGE, CONF_BATTERY_LIFE_CYCLE_RATING: DEFAULT_BATTERY_LIFE_CYCLE_RATING } -async def step_user_data_schema(hass: HomeAssistant, data: dict[str, Any] = { CONF_NAME: DEFAULT_NAME, CONF_DISCOVERY: DEFAULT_DISCOVERY, CONF_INVERTER_PORT: DEFAULT_INVERTER_PORT, CONF_INVERTER_MB_SLAVE_ID: DEFAULT_INVERTER_MB_SLAVE_ID, CONF_PASSTHROUGH: DEFAULT_PASSTHROUGH, CONF_LOOKUP_FILE: DEFAULT_LOOKUP_FILE, CONF_BATTERY_NOMINAL_VOLTAGE: DEFAULT_BATTERY_NOMINAL_VOLTAGE, CONF_BATTERY_LIFE_CYCLE_RATING: DEFAULT_BATTERY_LIFE_CYCLE_RATING }, wname: bool = True) -> vol.Schema: +async def step_user_data_schema(hass: HomeAssistant, data: dict[str, Any] = { CONF_NAME: DEFAULT_NAME, CONF_INVERTER_PORT: DEFAULT_INVERTER_PORT, CONF_MB_SLAVE_ID: DEFAULT_MB_SLAVE_ID, CONF_LOOKUP_FILE: DEFAULT_LOOKUP_FILE, CONF_BATTERY_NOMINAL_VOLTAGE: DEFAULT_BATTERY_NOMINAL_VOLTAGE, CONF_BATTERY_LIFE_CYCLE_RATING: DEFAULT_BATTERY_LIFE_CYCLE_RATING }) -> vol.Schema: lookup_files = await async_listdir(hass.config.path(LOOKUP_DIRECTORY_PATH)) + await async_listdir(hass.config.path(LOOKUP_CUSTOM_DIRECTORY_PATH), "custom/") _LOGGER.debug(f"step_user_data_schema: data: {data}, {LOOKUP_DIRECTORY_PATH}: {lookup_files}") #data_schema = vol.Schema({ vol.Required(CONF_NAME, default = data.get(CONF_NAME)): str }, extra = vol.PREVENT_EXTRA) if wname else vol.Schema({}, extra = vol.PREVENT_EXTRA) @@ -40,10 +40,10 @@ async def step_user_data_schema(hass: HomeAssistant, data: dict[str, Any] = { CO { #vol.Optional("Example of"): "some text to display in the config flow..." vol.Required(CONF_NAME, default = data.get(CONF_NAME)): str, + vol.Required(CONF_SERIAL, default = data.get(CONF_SERIAL)): cv.positive_int, vol.Required(CONF_INVERTER_HOST, default = data.get(CONF_INVERTER_HOST)): str, - vol.Required(CONF_INVERTER_SERIAL, default = data.get(CONF_INVERTER_SERIAL)): cv.positive_int, vol.Optional(CONF_INVERTER_PORT, default = data.get(CONF_INVERTER_PORT)): cv.port, - vol.Optional(CONF_INVERTER_MB_SLAVE_ID, default = data.get(CONF_INVERTER_MB_SLAVE_ID)): cv.positive_int, + vol.Optional(CONF_MB_SLAVE_ID, default = data.get(CONF_MB_SLAVE_ID)): cv.positive_int, vol.Optional(CONF_LOOKUP_FILE, default = data.get(CONF_LOOKUP_FILE)): vol.In(lookup_files), vol.Optional(CONF_BATTERY_NOMINAL_VOLTAGE, default = data.get(CONF_BATTERY_NOMINAL_VOLTAGE)): cv.positive_int, vol.Optional(CONF_BATTERY_LIFE_CYCLE_RATING, default = data.get(CONF_BATTERY_LIFE_CYCLE_RATING)): cv.positive_int, @@ -53,14 +53,14 @@ async def step_user_data_schema(hass: HomeAssistant, data: dict[str, Any] = { CO _LOGGER.debug(f"step_user_data_schema: data_schema: {data_schema}") return data_schema -async def step_init_data_schema(hass: HomeAssistant, data: dict[str, Any] = { CONF_NAME: DEFAULT_NAME, CONF_DISCOVERY: DEFAULT_DISCOVERY, CONF_INVERTER_PORT: DEFAULT_INVERTER_PORT, CONF_INVERTER_MB_SLAVE_ID: DEFAULT_INVERTER_MB_SLAVE_ID, CONF_PASSTHROUGH: DEFAULT_PASSTHROUGH, CONF_LOOKUP_FILE: DEFAULT_LOOKUP_FILE, CONF_BATTERY_NOMINAL_VOLTAGE: DEFAULT_BATTERY_NOMINAL_VOLTAGE, CONF_BATTERY_LIFE_CYCLE_RATING: DEFAULT_BATTERY_LIFE_CYCLE_RATING }, wname: bool = True) -> vol.Schema: +async def step_init_data_schema(hass: HomeAssistant, data: dict[str, Any] = { CONF_NAME: DEFAULT_NAME, CONF_INVERTER_PORT: DEFAULT_INVERTER_PORT, CONF_MB_SLAVE_ID: DEFAULT_MB_SLAVE_ID, CONF_LOOKUP_FILE: DEFAULT_LOOKUP_FILE, CONF_BATTERY_NOMINAL_VOLTAGE: DEFAULT_BATTERY_NOMINAL_VOLTAGE, CONF_BATTERY_LIFE_CYCLE_RATING: DEFAULT_BATTERY_LIFE_CYCLE_RATING }) -> vol.Schema: lookup_files = await async_listdir(hass.config.path(LOOKUP_DIRECTORY_PATH)) + await async_listdir(hass.config.path(LOOKUP_CUSTOM_DIRECTORY_PATH), "custom/") _LOGGER.debug(f"step_init_data_schema: data: {data}, {LOOKUP_DIRECTORY_PATH}: {lookup_files}") data_schema = vol.Schema( { vol.Required(CONF_INVERTER_HOST, default = data.get(CONF_INVERTER_HOST)): str, vol.Optional(CONF_INVERTER_PORT, default = data.get(CONF_INVERTER_PORT)): cv.port, - vol.Optional(CONF_INVERTER_MB_SLAVE_ID, default = data.get(CONF_INVERTER_MB_SLAVE_ID)): cv.positive_int, + vol.Optional(CONF_MB_SLAVE_ID, default = data.get(CONF_MB_SLAVE_ID)): cv.positive_int, vol.Optional(CONF_LOOKUP_FILE, default = data.get(CONF_LOOKUP_FILE)): vol.In(lookup_files), vol.Optional(CONF_BATTERY_NOMINAL_VOLTAGE, default = data.get(CONF_BATTERY_NOMINAL_VOLTAGE)): cv.positive_int, vol.Optional(CONF_BATTERY_LIFE_CYCLE_RATING, default = data.get(CONF_BATTERY_LIFE_CYCLE_RATING)): cv.positive_int, @@ -137,7 +137,7 @@ async def async_step_user(self, user_input: dict[str, Any] | None = None) -> Con errors["base"] = "unknown" else: _LOGGER.debug(f"ConfigFlowHandler.async_step_user: validation passed: {user_input}") - await self.async_set_unique_id(f"solarman_{user_input[CONF_INVERTER_SERIAL]}") #self._abort_if_unique_id_configured(updates={CONF_HOST: url.host}) + await self.async_set_unique_id(f"solarman_{user_input[CONF_SERIAL]}") #self._abort_if_unique_id_configured(updates={CONF_HOST: url.host}) self._abort_if_unique_id_configured() return self.async_create_entry(title = user_input[CONF_NAME], data = user_input, options = user_input) @@ -164,7 +164,7 @@ async def async_step_init(self, user_input: dict[str, Any] | None = None) -> Con """Handle options flow.""" _LOGGER.debug(f"OptionsFlowHandler.async_step_init: user_input: {user_input}, current: {self.entry.options}") if user_input is None: - return self.async_show_form(step_id = "init", data_schema = await step_init_data_schema(self.hass, self.entry.options, False)) + return self.async_show_form(step_id = "init", data_schema = await step_init_data_schema(self.hass, self.entry.options)) errors = {} @@ -181,7 +181,7 @@ async def async_step_init(self, user_input: dict[str, Any] | None = None) -> Con _LOGGER.debug(f"OptionsFlowHandler.async_step_init: validation passed: {user_input}") return self.async_create_entry(title = self.entry.data[CONF_NAME], data = user_input) - return self.async_show_form(step_id = "init", data_schema = await step_init_data_schema(self.hass, user_input, False), errors = errors) + return self.async_show_form(step_id = "init", data_schema = await step_init_data_schema(self.hass, user_input), errors = errors) class InvalidHost(HomeAssistantError): """Error to indicate there is invalid hostname or IP address.""" diff --git a/custom_components/solarman/const.py b/custom_components/solarman/const.py index 57d8ba2..4187e17 100644 --- a/custom_components/solarman/const.py +++ b/custom_components/solarman/const.py @@ -1,5 +1,6 @@ import types import struct + from datetime import timedelta as td DOMAIN = "solarman" @@ -21,22 +22,17 @@ LOOKUP_DIRECTORY_PATH = f"{COMPONENTS_DIRECTORY}/{DOMAIN}/{LOOKUP_DIRECTORY}/" LOOKUP_CUSTOM_DIRECTORY_PATH = f"{COMPONENTS_DIRECTORY}/{DOMAIN}/{LOOKUP_DIRECTORY}/custom/" -CONF_DISCOVERY = "inverter_discovery" -CONF_DISCOVERED = "discovered" +CONF_SERIAL = "inverter_serial" CONF_INVERTER_HOST = "inverter_host" -CONF_INVERTER_SERIAL = "inverter_serial" CONF_INVERTER_PORT = "inverter_port" -CONF_INVERTER_MB_SLAVE_ID = "inverter_mb_slave_id" -CONF_PASSTHROUGH = "inverter_passthrough" +CONF_MB_SLAVE_ID = "inverter_mb_slave_id" CONF_LOOKUP_FILE = "lookup_file" CONF_BATTERY_NOMINAL_VOLTAGE = "battery_nominal_voltage" CONF_BATTERY_LIFE_CYCLE_RATING = "battery_life_cycle_rating" DEFAULT_NAME = "Inverter" -DEFAULT_DISCOVERY = True DEFAULT_INVERTER_PORT = 8899 -DEFAULT_INVERTER_MB_SLAVE_ID = 1 -DEFAULT_PASSTHROUGH = False +DEFAULT_MB_SLAVE_ID = 1 DEFAULT_LOOKUP_FILE = "deye_hybrid.yaml" DEFAULT_BATTERY_NOMINAL_VOLTAGE = 48 DEFAULT_BATTERY_LIFE_CYCLE_RATING = 6000 diff --git a/custom_components/solarman/coordinator.py b/custom_components/solarman/coordinator.py index cb29d57..21b3cee 100644 --- a/custom_components/solarman/coordinator.py +++ b/custom_components/solarman/coordinator.py @@ -15,18 +15,19 @@ class InverterCoordinator(DataUpdateCoordinator[dict[str, Any]]): def __init__(self, hass: HomeAssistant, inverter): super().__init__(hass, _LOGGER, name = inverter.name, update_interval = TIMINGS_UPDATE_INTERVAL, always_update = False) self.inverter = inverter - self._counter = -1 + self._counter = 0 def _accounting(self) -> int: - if self.last_update_success: - self._counter += 1 - - return int(self._counter * self._update_interval_seconds) + try: + return int(self._counter * self._update_interval_seconds) + finally: + if self.last_update_success: + self._counter += 1 async def _async_update_data(self) -> dict[str, Any]: try: return await self.inverter.get(self._accounting()) - except Exception: # Temporary fix to retrieve all data after reconnect + except: self._counter = 0 raise