Merge pull request #749 from canton7/feature/vendor-pymodbus

Vendor pymodbus, rather than relying on the version which HA installs

.devcontainer/devcontainer.json

@@ -11,7 +11,6 @@
       "extensions": [
         "ms-python.python",
         "charliermarsh.ruff",
-        "ms-python.black-formatter",
         "ms-python.vscode-pylance",
         "github.vscode-pull-request-github",
         "ryanluker.vscode-coverage-gutters",

.pre-commit-config.yaml

@@ -5,22 +5,15 @@ repos:
       - id: check-added-large-files
       - id: check-yaml
       - id: end-of-file-fixer
+        exclude: ^custom_components\/foxess_modbus\/vendor
       - id: trailing-whitespace
-  - repo: local
-    hooks:
-      - id: black
-        name: black
-        entry: black
-        language: system
-        types: [python]
-        require_serial: true
   - repo: https://github.com/pre-commit/mirrors-prettier
     rev: v2.2.1
     hooks:
       - id: prettier
   - repo: https://github.com/astral-sh/ruff-pre-commit
     # Ruff version.
-    rev: v0.0.275
+    rev: v0.9.4
     hooks:
       - id: ruff
 # Temporarily disabled due to crash, probably caused by us having to use HA stubs which are out of date?

.prettierignore

@@ -1 +1,2 @@
 tests/__snapshots__
+custom_components/foxess_modbus/vendor

.vscode/settings.json

@@ -26,6 +26,9 @@
     "editor.codeActionsOnSave": {
       "source.fixAll": "explicit"
     },
-    "editor.defaultFormatter": "ms-python.black-formatter"
-  }
+    "editor.defaultFormatter": "charliermarsh.ruff"
+  },
+  "python.analysis.extraPaths": [
+    "./custom_components/foxess_modbus/vendor/pymodbus/pymodbus-3.6.9"
+  ]
 }

CONTRIBUTING.md

@@ -15,9 +15,8 @@ Pull requests are the best way to propose changes to the codebase.
 
 1. Fork the repo and create your branch from `master`.
 2. If you've changed something, update the documentation.
-3. Make sure your code lints (using black).
-4. Test you contribution.
-5. Issue that pull request!
+3. Test you contribution.
+4. Issue that pull request!
 
 ## Any contributions you make will be under the MIT Software License
 
@@ -44,11 +43,7 @@ People _love_ thorough bug reports. I'm not even kidding.
 
 ## Use a Consistent Coding Style
 
-Use [black](https://github.com/ambv/black) and [prettier](https://prettier.io/)
-to make sure the code follows the style.
-
-Or use the `pre-commit` settings implemented in this repository
-(see deicated section below).
+Use the `pre-commit` settings implemented in this repository (see dedicated section below).
 
 ## Test your code modification
 

custom_components/foxess_modbus/client/custom_modbus_tcp_client.py

@@ -5,23 +5,23 @@
 from typing import Any
 from typing import cast
 
-from pymodbus.client import ModbusTcpClient
-from pymodbus.exceptions import ConnectionException
+from ..vendor.pymodbus import ConnectionException
+from ..vendor.pymodbus import ModbusTcpClient
 
 _LOGGER = logging.getLogger(__name__)
 
 
 class CustomModbusTcpClient(ModbusTcpClient):
     """Custom ModbusTcpClient subclass with some hacks"""
 
-    def __init__(self, delay_on_connect: int | None = None, **kwargs: Any) -> None:
+    def __init__(self, delay_on_connect: int | None, **kwargs: Any) -> None:
         super().__init__(**kwargs)
         self._delay_on_connect = delay_on_connect
 
     def connect(self) -> bool:
         was_connected = self.socket is not None
         if not was_connected:
-            _LOGGER.debug("Connecting to %s", self.comm_params)
+            _LOGGER.debug("Connecting to %s", self.params)
         is_connected = cast(bool, super().connect())
         # pymodbus doesn't disable Nagle's algorithm. This slows down reads quite substantially as the
         # TCP stack waits to see if we're going to send anything else. Disable it ourselves.
@@ -34,7 +34,7 @@ def connect(self) -> bool:
 
     # Replacement of ModbusTcpClient to use poll rather than select, see
     # https://github.com/nathanmarlor/foxess_modbus/issues/275
-    def recv(self, size: int | None) -> bytes:
+    def recv(self, size: int) -> bytes:
         """Read data from the underlying descriptor."""
         super(ModbusTcpClient, self).recv(size)
         if not self.socket:
@@ -48,9 +48,9 @@ def recv(self, size: int | None) -> bytes:
         # is received or timeout is expired.
         # If timeout expires returns the read data, also if its length is
         # less than the expected size.
-        self.socket.setblocking(False)
+        self.socket.setblocking(0)
 
-        timeout = self.comm_params.timeout_connect or 0
+        timeout = self.comm_params.timeout_connect
 
         # If size isn't specified read up to 4096 bytes at a time.
         if size is None:
@@ -90,5 +90,20 @@ def recv(self, size: int | None) -> bytes:
             if time_ > end:
                 break
 
-        self.last_frame_end = round(time.time(), 6)
         return b"".join(data)
+
+    # Replacement of ModbusTcpClient to use poll rather than select, see
+    # https://github.com/nathanmarlor/foxess_modbus/issues/275
+    def _check_read_buffer(self) -> bytes | None:
+        """Check read buffer."""
+        time_ = time.time()
+        end = time_ + self.params.timeout
+        data = None
+
+        assert self.socket is not None
+        poll = select.poll()
+        poll.register(self.socket, select.POLLIN)
+        poll_res = poll.poll(end - time_)
+        if len(poll_res) > 0:
+            data = self.socket.recv(1024)
+        return data

custom_components/foxess_modbus/client/modbus_client.py

@@ -11,14 +11,6 @@
 
 import serial
 from homeassistant.core import HomeAssistant
-from pymodbus.client import ModbusSerialClient
-from pymodbus.client import ModbusUdpClient
-from pymodbus.framer import FramerType
-from pymodbus.pdu import ModbusPDU
-from pymodbus.pdu.register_read_message import ReadHoldingRegistersResponse
-from pymodbus.pdu.register_read_message import ReadInputRegistersResponse
-from pymodbus.pdu.register_write_message import WriteMultipleRegistersResponse
-from pymodbus.pdu.register_write_message import WriteSingleRegisterResponse
 
 from .. import client
 from ..common.types import ConnectionType
@@ -28,6 +20,15 @@
 from ..const import TCP
 from ..const import UDP
 from ..inverter_adapters import InverterAdapter
+from ..vendor.pymodbus import ModbusResponse
+from ..vendor.pymodbus import ModbusRtuFramer
+from ..vendor.pymodbus import ModbusSerialClient
+from ..vendor.pymodbus import ModbusSocketFramer
+from ..vendor.pymodbus import ModbusUdpClient
+from ..vendor.pymodbus import ReadHoldingRegistersResponse
+from ..vendor.pymodbus import ReadInputRegistersResponse
+from ..vendor.pymodbus import WriteMultipleRegistersResponse
+from ..vendor.pymodbus import WriteSingleRegisterResponse
 from .custom_modbus_tcp_client import CustomModbusTcpClient
 
 _LOGGER = logging.getLogger(__name__)
@@ -38,19 +39,19 @@
 _CLIENTS: dict[str, dict[str, Any]] = {
     SERIAL: {
         "client": ModbusSerialClient,
-        "framer": FramerType.RTU,
+        "framer": ModbusRtuFramer,
     },
     TCP: {
         "client": CustomModbusTcpClient,
-        "framer": FramerType.SOCKET,
+        "framer": ModbusSocketFramer,
     },
     UDP: {
         "client": ModbusUdpClient,
-        "framer": FramerType.SOCKET,
+        "framer": ModbusSocketFramer,
     },
     RTU_OVER_TCP: {
         "client": CustomModbusTcpClient,
-        "framer": FramerType.RTU,
+        "framer": ModbusRtuFramer,
     },
 }
 
@@ -69,16 +70,14 @@ def __init__(self, hass: HomeAssistant, protocol: str, adapter: InverterAdapter,
 
         client = _CLIENTS[protocol]
 
+        # Delaying for a second after establishing a connection seems to help the inverter stability,
+        # see https://github.com/nathanmarlor/foxess_modbus/discussions/132
         config = {
             **config,
             "framer": client["framer"],
+            "delay_on_connect": 1 if adapter.connection_type == ConnectionType.LAN else None,
         }
 
-        # Delaying for a second after establishing a connection seems to help the inverter stability,
-        # see https://github.com/nathanmarlor/foxess_modbus/discussions/132
-        if adapter.connection_type == ConnectionType.LAN:
-            config["delay_on_connect"] = 1
-
         # If our custom PosixPollSerial hack is supported, use that. This uses poll rather than select, which means we
         # don't break when there are more than 1024 fds. See #457.
         # Only supported on posix, see https://github.com/pyserial/pyserial/blob/7aeea35429d15f3eefed10bbb659674638903e3a/serial/__init__.py#L31
@@ -225,7 +224,7 @@ def __str__(self) -> str:
 class ModbusClientFailedError(Exception):
     """Raised when the ModbusClient fails to read/write"""
 
-    def __init__(self, message: str, client: ModbusClient, response: ModbusPDU | Exception) -> None:
+    def __init__(self, message: str, client: ModbusClient, response: ModbusResponse | Exception) -> None:
         super().__init__(f"{message} from {client}: {response}")
         self.message = message
         self.client = client

custom_components/foxess_modbus/client/protocol_pollserial.py

@@ -68,8 +68,8 @@ def read(self, size: int = 1) -> bytes:
                     result == _PollResult.TIMEOUT
                     or result == _PollResult.ABORT
                     or timeout.expired()
-                    or (self._inter_byte_timeout is not None and self._inter_byte_timeout > 0)
-                    and not buf
+                    or ((self._inter_byte_timeout is not None and self._inter_byte_timeout > 0)
+                    and not buf)
                 ):
                     break  # early abort on timeout
         return bytes(read)

custom_components/foxess_modbus/common/types.py

@@ -1,4 +1,4 @@
-"""Defines RegisterType"""
+"""Defines RegisterType"""  # noqa: A005
 
 from enum import Enum
 from enum import Flag

custom_components/foxess_modbus/entities/base_validator.py

@@ -7,5 +7,5 @@ class BaseValidator(ABC):
     """Base validator"""
 
     @abstractmethod
-    def validate(self, data: int | float) -> bool:
+    def validate(self, data: float) -> bool:
         """Validate a value against a set of rules"""

custom_components/foxess_modbus/entities/modbus_entity_mixin.py

@@ -153,8 +153,8 @@ def _get_entity_id(self, platform: Platform) -> str:
     def _validate(
         self,
         rules: list[BaseValidator],
-        processed: float | int,
-        original: float | int | None = None,
+        processed: float,
+        original: float | None = None,
         address_override: int | None = None,
     ) -> bool:
         """Validate against a set of rules"""

custom_components/foxess_modbus/entities/validation.py

@@ -11,7 +11,7 @@ def __init__(self, min_value: float, max_value: float) -> None:
         self._min = min_value
         self._max = max_value
 
-    def validate(self, data: int | float) -> bool:
+    def validate(self, data: float) -> bool:
         """Validate a value against a set of rules"""
 
         return self._min <= data <= self._max
@@ -24,7 +24,7 @@ def __init__(self, min_value: float) -> None:
         """Init"""
         self._min = min_value
 
-    def validate(self, data: int | float) -> bool:
+    def validate(self, data: float) -> bool:
         """Validate a value against a set of rules"""
 
         return data >= self._min
@@ -37,7 +37,7 @@ def __init__(self, max_value: float) -> None:
         """Init"""
         self._max = max_value
 
-    def validate(self, data: int | float) -> bool:
+    def validate(self, data: float) -> bool:
         """Validate a value against a set of rules"""
 
         return data <= self._max
@@ -46,6 +46,6 @@ def validate(self, data: int | float) -> bool:
 class Time(BaseValidator):
     """Time validator"""
 
-    def validate(self, data: int | float) -> bool:
+    def validate(self, data: float) -> bool:
         """Validate a value against a set of rules"""
         return isinstance(data, int) and is_time_value_valid(data)

custom_components/foxess_modbus/flow/adapter_flow_segment.py

@@ -7,8 +7,6 @@
 from homeassistant.data_entry_flow import FlowResult
 from homeassistant.helpers import config_validation as cv
 from homeassistant.helpers.selector import selector
-from pymodbus.exceptions import ConnectionException
-from pymodbus.exceptions import ModbusIOException
 
 from ..client.modbus_client import ModbusClient
 from ..client.modbus_client import ModbusClientFailedError
@@ -23,6 +21,8 @@
 from ..inverter_adapters import InverterAdapter
 from ..inverter_adapters import InverterAdapterType
 from ..modbus_controller import ModbusController
+from ..vendor.pymodbus import ConnectionException
+from ..vendor.pymodbus import ModbusIOException
 from .flow_handler_mixin import FlowHandlerMixin
 from .flow_handler_mixin import ValidationFailedError
 from .inverter_data import InverterData

custom_components/foxess_modbus/manifest.json

@@ -8,6 +8,5 @@
   "integration_type": "service",
   "iot_class": "local_push",
   "issue_tracker": "https://github.com/nathanmarlor/foxess_modbus/issues",
-  "requirements": ["pymodbus>=3.7.4"],
   "version": "1.0.0"
 }

custom_components/foxess_modbus/modbus_controller.py

@@ -18,7 +18,6 @@
 from homeassistant.helpers import issue_registry
 from homeassistant.helpers.event import async_track_time_interval
 from homeassistant.helpers.issue_registry import IssueSeverity
-from pymodbus.exceptions import ConnectionException
 
 from .client.modbus_client import ModbusClient
 from .client.modbus_client import ModbusClientFailedError
@@ -38,6 +37,7 @@
 from .inverter_profiles import INVERTER_PROFILES
 from .inverter_profiles import InverterModelConnectionTypeProfile
 from .remote_control_manager import RemoteControlManager
+from .vendor.pymodbus import ConnectionException
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -234,7 +234,7 @@ async def write_registers(self, start_address: int, values: list[int]) -> None:
                 self._notify_update(changed_addresses)
         except Exception as ex:
             # Failed writes are always bad
-            _LOGGER.error("Failed to write registers", exc_info=True)
+            _LOGGER.exception("Failed to write registers")
             raise ex
 
     async def _refresh(self, _time: datetime) -> None:
@@ -538,7 +538,7 @@ async def autodetect(client: ModbusClient, slave: int, adapter_config: dict[str,
             _LOGGER.error("Did not recognise inverter model '%s' (%s)", full_model, register_values)
             raise UnsupportedInverterError(full_model)
         except Exception as ex:
-            _LOGGER.error("Autodetect: failed to connect to (%s)", client, exc_info=True)
+            _LOGGER.exceptino("Autodetect: failed to connect to (%s)", client)
             raise AutoconnectFailedError(spy_handler.records) from ex
         finally:
             pymodbus_logger.removeHandler(spy_handler)

custom_components/foxess_modbus/select.py

@@ -1,4 +1,4 @@
-"""Sensor platform for foxess_modbus."""
+"""Sensor platform for foxess_modbus."""  # noqa: A005
 
 import logging
 

custom_components/foxess_modbus/services/update_charge_period_service.py

@@ -10,13 +10,13 @@
 from homeassistant.core import ServiceCall
 from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import config_validation as cv
-from pymodbus.exceptions import ModbusIOException
 
 from ..const import DOMAIN
 from ..entities.modbus_charge_period_sensors import is_time_value_valid
 from ..entities.modbus_charge_period_sensors import parse_time_value
 from ..entities.modbus_charge_period_sensors import serialize_time_to_value
 from ..modbus_controller import ModbusController
+from ..vendor.pymodbus import ModbusIOException
 from .utils import get_controller_from_friendly_name_or_device_id
 
 _LOGGER: logging.Logger = logging.getLogger(__package__)

custom_components/foxess_modbus/services/write_registers_service.py

@@ -8,10 +8,10 @@
 from homeassistant.core import ServiceCall
 from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import config_validation as cv
-from pymodbus.exceptions import ModbusIOException
 
 from ..const import DOMAIN
 from ..modbus_controller import ModbusController
+from ..vendor.pymodbus import ModbusIOException
 from .utils import get_controller_from_friendly_name_or_device_id
 
 _LOGGER: logging.Logger = logging.getLogger(__package__)