Modbus: verbleibende ungesperrte Delegate-Zugriffe schliessen

packages/modules/chargepoints/openwb_series2_satellit/chargepoint_module.py

@@ -147,16 +147,16 @@ def switch_phases(self, phases_to_use: int) -> None:
                             self._client.evse_client.set_current(0)
                             time.sleep(5)
                             if phases_to_use == 1:
-                                self._client.client.delegate.write_register(
+                                self._client.client.write_single_register_raw(
                                     0x0001, 256, unit=self.ID_PHASE_SWITCH_UNIT)
                                 time.sleep(1)
-                                self._client.client.delegate.write_register(
+                                self._client.client.write_single_register_raw(
                                     0x0001, 512, unit=self.ID_PHASE_SWITCH_UNIT)
                             else:
-                                self._client.client.delegate.write_register(
+                                self._client.client.write_single_register_raw(
                                     0x0002, 512, unit=self.ID_PHASE_SWITCH_UNIT)
                                 time.sleep(1)
-                                self._client.client.delegate.write_register(
+                                self._client.client.write_single_register_raw(
                                     0x0002, 256, unit=self.ID_PHASE_SWITCH_UNIT)
                     except AttributeError:
                         self._create_client()

packages/modules/common/hardware_check.py

@@ -89,8 +89,7 @@ def request_and_check_hardware(self: ClientHandlerProtocol,
                         evse_state = self.evse_client.get_evse_state()
                         evse_check_passed = True
                         break
-                    except (pymodbus.exceptions.ModbusIOException,
-                            pymodbus.exceptions.ConnectionException) as e:
+                    except Exception as e:
                         evse_check_passed = self.handle_exception(e)
                         # nur warten, wenn danach noch ein Versuch folgt
                         if attempt < MAX_ATTEMPTS - 1 and evse_check_passed is False:

packages/modules/common/hardware_check_test.py

@@ -66,6 +66,32 @@ def test_hardware_check_fails(evse_side_effect,
         ClientHandler(0, client, [1], Mock())
 
 
+@pytest.mark.parametrize(
+    "transient_exception",
+    [pytest.param(Exception("Modbus"), id="response.isError() in modbus.py"),
+     pytest.param(ValueError("Unbekannter Zustand der EVSE"), id="EvseStatusCode.FAILURE in evse.py")]
+)
+def test_hardware_check_retries_transient_exception(transient_exception, monkeypatch):
+    # Neither modbus.py's response.isError() case nor evse.py's EvseStatusCode.FAILURE case raise
+    # ModbusIOException/ConnectionException. The retry loop must catch those too, or a single
+    # transient glitch fails the whole readout with no retry.
+    mock_evse_client = Mock(spec=Evse, version=18,
+                            get_evse_state=Mock(side_effect=[transient_exception, Mock(spec=EvseState)]))
+    monkeypatch.setattr(ClientHandler, "_evse_factory", Mock(return_value=mock_evse_client))
+
+    counter_state_mock = Mock(spec=CounterState, voltages=[230]*3, currents=[0, 0, 0], powers=[0, 0, 0],
+                              power=0, serial_number="1234")
+    mock_meter_client = Mock(spec=sdm.Sdm630_72, get_counter_state=Mock(return_value=counter_state_mock))
+    monkeypatch.setattr(ClientHandler, "find_meter_client", Mock(return_value=mock_meter_client))
+
+    client = Mock(spec=ModbusSerialClient_, __enter__=Mock(return_value=None), __exit__=Mock(return_value=None))
+
+    # execution and evaluation
+    # keine Exception: der zweite Versuch muss noch innerhalb von request_and_check_hardware laufen
+    ClientHandler(0, client, [1], Mock())
+    assert mock_evse_client.get_evse_state.call_count == 2
+
+
 def test_hardware_check_succeeds(monkeypatch):
     # setup
     mock_evse_client = Mock(spec=Evse, get_evse_state=Mock(return_value=Mock(spec=EvseState)), version=17)

packages/modules/common/modbus.py

@@ -6,6 +6,7 @@
 """
 import logging
 import struct
+import threading
 from enum import Enum
 import time
 from types import TracebackType
@@ -58,32 +59,39 @@ def __init__(self,
         self.address = address
         self.port = port
         self.sleep_after_connect = sleep_after_connect
+        # Serialisiert alle Zugriffe auf die (ggf. von mehreren Threads geteilte) Verbindung, zB.
+        # Haupt-Poll-Thread und Phasenumschaltungs-/CP-Unterbrechungs-Thread bei internem Ladepunkt.
+        self._lock = threading.RLock()
 
     def __enter__(self):
-        try:
-            self._delegate.__enter__()
-            time.sleep(self.sleep_after_connect)
-        except pymodbus.exceptions.ConnectionException as e:
-            e.args += (NO_CONNECTION.format(self.address, self.port),)
-            raise e
+        with self._lock:
+            try:
+                self._delegate.__enter__()
+                time.sleep(self.sleep_after_connect)
+            except pymodbus.exceptions.ConnectionException as e:
+                e.args += (NO_CONNECTION.format(self.address, self.port),)
+                raise e
         return self
 
     def __exit__(self,
                  exc_type: Optional[Type[BaseException]],
                  exc_value: Optional[BaseException],
                  exc_traceback: Optional[TracebackType]):
-        self._delegate.__exit__(exc_type, exc_value, exc_traceback)
+        with self._lock:
+            self._delegate.__exit__(exc_type, exc_value, exc_traceback)
 
     def connect(self) -> None:
-        self._delegate.connect()
-        time.sleep(self.sleep_after_connect)
+        with self._lock:
+            self._delegate.connect()
+            time.sleep(self.sleep_after_connect)
 
     def close(self) -> None:
-        try:
-            log.debug("Close Modbus TCP connection")
-            self._delegate.close()
-        except Exception as e:
-            raise Exception(__name__+" "+str(type(e))+" " + str(e)) from e
+        with self._lock:
+            try:
+                log.debug("Close Modbus TCP connection")
+                self._delegate.close()
+            except Exception as e:
+                raise Exception(__name__+" "+str(type(e))+" " + str(e)) from e
 
     def is_socket_open(self) -> bool:
         return self._delegate.is_socket_open()
@@ -94,37 +102,38 @@ def __read_registers(self, read_register_method: Callable[[int, int], Any],
                          byteorder: str = Endian.Big,
                          wordorder: str = Endian.Big,
                          **kwargs: Any):
-        if self.is_socket_open() is False:
-            self.connect()
-        try:
-            multi_request = isinstance(types, Iterable)
-            if not multi_request:
-                types = [types]
-
-            def divide_rounding_up(numerator: int, denominator: int):
-                return -(-numerator // denominator)
-
-            number_of_addresses = sum(divide_rounding_up(
-                t.bits, _MODBUS_HOLDING_REGISTER_SIZE) for t in types)
-            response = read_register_method(
-                address, number_of_addresses, **kwargs)
-            if response.isError():
-                raise Exception(__name__+" "+str(response))
-            decoder = BinaryPayloadDecoder.fromRegisters(response.registers, byteorder, wordorder)
-            result = [struct.unpack(">e", struct.pack(">H", decoder.decode_16bit_uint())) if t ==
-                      ModbusDataType.FLOAT_16 else getattr(decoder, t.decoding_method)() for t in types]
-            return result if multi_request else result[0]
-        except pymodbus.exceptions.ConnectionException as e:
-            self.close()
-            e.args += (NO_CONNECTION.format(self.address, self.port),)
-            raise e
-        except pymodbus.exceptions.ModbusIOException as e:
-            self.close()
-            e.args += (NO_VALUES.format(self.address, self.port),)
-            raise e
-        except Exception as e:
-            self.close()
-            raise Exception(__name__+" "+str(type(e))+" " + str(e)) from e
+        with self._lock:
+            if self.is_socket_open() is False:
+                self.connect()
+            try:
+                multi_request = isinstance(types, Iterable)
+                if not multi_request:
+                    types = [types]
+
+                def divide_rounding_up(numerator: int, denominator: int):
+                    return -(-numerator // denominator)
+
+                number_of_addresses = sum(divide_rounding_up(
+                    t.bits, _MODBUS_HOLDING_REGISTER_SIZE) for t in types)
+                response = read_register_method(
+                    address, number_of_addresses, **kwargs)
+                if response.isError():
+                    raise Exception(__name__+" "+str(response))
+                decoder = BinaryPayloadDecoder.fromRegisters(response.registers, byteorder, wordorder)
+                result = [struct.unpack(">e", struct.pack(">H", decoder.decode_16bit_uint())) if t ==
+                          ModbusDataType.FLOAT_16 else getattr(decoder, t.decoding_method)() for t in types]
+                return result if multi_request else result[0]
+            except pymodbus.exceptions.ConnectionException as e:
+                self.close()
+                e.args += (NO_CONNECTION.format(self.address, self.port),)
+                raise e
+            except pymodbus.exceptions.ModbusIOException as e:
+                self.close()
+                e.args += (NO_VALUES.format(self.address, self.port),)
+                raise e
+            except Exception as e:
+                self.close()
+                raise Exception(__name__+" "+str(type(e))+" " + str(e)) from e
 
     @overload
     def read_holding_registers(self, address: int, types: Iterable[ModbusDataType], byteorder: str = Endian.Big,
@@ -177,17 +186,18 @@ def read_coils(self, address: int, count: int, **kwargs: Any) -> Union[bool, Lis
         pass
 
     def read_coils(self, address: int, count: int = 1, **kwargs: Any) -> Union[bool, List[bool]]:
-        try:
-            response = self._delegate.read_coils(address, count, **kwargs)
-            if response.isError():
-                raise Exception(__name__+" "+str(response))
-            return response.bits[0] if count == 1 else response.bits[:count]
-        except pymodbus.exceptions.ConnectionException as e:
-            e.args += (NO_CONNECTION.format(self.address, self.port),)
-            raise e
-        except pymodbus.exceptions.ModbusIOException as e:
-            e.args += (NO_VALUES.format(self.address, self.port),)
-            raise e
+        with self._lock:
+            try:
+                response = self._delegate.read_coils(address, count, **kwargs)
+                if response.isError():
+                    raise Exception(__name__+" "+str(response))
+                return response.bits[0] if count == 1 else response.bits[:count]
+            except pymodbus.exceptions.ConnectionException as e:
+                e.args += (NO_CONNECTION.format(self.address, self.port),)
+                raise e
+            except pymodbus.exceptions.ModbusIOException as e:
+                e.args += (NO_VALUES.format(self.address, self.port),)
+                raise e
 
     def _build_binary_payload(self,
                               value: Union[int, float],
@@ -208,24 +218,33 @@ def _build_binary_payload(self,
 
     def write_register(self, address: int, value: Union[int, float], data_type: Optional[ModbusDataType] = None,
                        byteorder: str = Endian.Big, wordorder: str = Endian.Big, **kwargs: Any):
-        if data_type is not None:
-            if data_type.bits > 16 or data_type in [ModbusDataType.FLOAT_16,
-                                                    ModbusDataType.FLOAT_32,
-                                                    ModbusDataType.FLOAT_64]:
-                registers = self._build_binary_payload(value, data_type, byteorder, wordorder)
-                self._delegate.write_registers(address, registers, **kwargs)
+        with self._lock:
+            if data_type is not None:
+                if data_type.bits > 16 or data_type in [ModbusDataType.FLOAT_16,
+                                                        ModbusDataType.FLOAT_32,
+                                                        ModbusDataType.FLOAT_64]:
+                    registers = self._build_binary_payload(value, data_type, byteorder, wordorder)
+                    self._delegate.write_registers(address, registers, **kwargs)
+                else:
+                    # Einfache 16-bit oder kleinere Werte können direkt geschrieben werden
+                    self._delegate.write_registers(address, [value], **kwargs)
             else:
-                # Einfache 16-bit oder kleinere Werte können direkt geschrieben werden
-                self._delegate.write_registers(address, [value], **kwargs)
-        else:
-            # Fallback für bestehenden Code ohne data_type
-            self._delegate.write_registers(address, value, **kwargs)
+                # Fallback für bestehenden Code ohne data_type
+                self._delegate.write_registers(address, value, **kwargs)
+
+    def write_single_register_raw(self, address: int, value: int, **kwargs: Any):
+        # FC06 (Write Single Register) statt FC16 wie write_register() oben - manche Geräte akzeptieren
+        # FC16 nicht zuverlässig für ein einzelnes Register.
+        with self._lock:
+            self._delegate.write_register(address, value, **kwargs)
 
     def write_single_coil(self, address: int, value: bool, **kwargs: Any):
-        self._delegate.write_coil(address, value, **kwargs)
+        with self._lock:
+            self._delegate.write_coil(address, value, **kwargs)
 
     def write_coils(self, address: int, value: List[bool], **kwargs: Any):
-        self._delegate.write_coils(address, value, **kwargs)
+        with self._lock:
+            self._delegate.write_coils(address, value, **kwargs)
 
     def __read_bulk(self,
                     read_register_method: Callable[[int, int], Any],
@@ -239,36 +258,37 @@ def __read_bulk(self,
         Liest einen Registerbereich und gibt ein dict mit reg als Key und dekodiertem Wert als Value zurück.
         mapping: Liste von Tupeln (reg, ModbusDataType)
         """
-        if self.is_socket_open() is False:
-            self.connect()
-        try:
-            response = read_register_method(start_address, count, **kwargs)
-            if response.isError():
-                raise Exception(__name__+" "+str(response))
-            decoder = BinaryPayloadDecoder.fromRegisters(response.registers, byteorder, wordorder)
-            results = {}
-            for register_address, data_type in mapping:
-                multiple_register_requested = isinstance(data_type, Iterable)
-                if not multiple_register_requested:
-                    data_type = [data_type]
-                offset = register_address - start_address
-                decoder.reset()
-                decoder.skip_bytes(offset * 2)
-                val = [struct.unpack(">e", struct.pack(">H", decoder.decode_16bit_uint())) if t ==
-                       ModbusDataType.FLOAT_16 else getattr(decoder, t.decoding_method)() for t in data_type]
-                results[register_address] = val if multiple_register_requested else val[0]
-            return results
-        except pymodbus.exceptions.ConnectionException as e:
-            self.close()
-            e.args += (NO_CONNECTION.format(self.address, self.port),)
-            raise e
-        except pymodbus.exceptions.ModbusIOException as e:
-            self.close()
-            e.args += (NO_VALUES.format(self.address, self.port),)
-            raise e
-        except Exception as e:
-            self.close()
-            raise Exception(__name__+" "+str(type(e))+" " + str(e)) from e
+        with self._lock:
+            if self.is_socket_open() is False:
+                self.connect()
+            try:
+                response = read_register_method(start_address, count, **kwargs)
+                if response.isError():
+                    raise Exception(__name__+" "+str(response))
+                decoder = BinaryPayloadDecoder.fromRegisters(response.registers, byteorder, wordorder)
+                results = {}
+                for register_address, data_type in mapping:
+                    multiple_register_requested = isinstance(data_type, Iterable)
+                    if not multiple_register_requested:
+                        data_type = [data_type]
+                    offset = register_address - start_address
+                    decoder.reset()
+                    decoder.skip_bytes(offset * 2)
+                    val = [struct.unpack(">e", struct.pack(">H", decoder.decode_16bit_uint())) if t ==
+                           ModbusDataType.FLOAT_16 else getattr(decoder, t.decoding_method)() for t in data_type]
+                    results[register_address] = val if multiple_register_requested else val[0]
+                return results
+            except pymodbus.exceptions.ConnectionException as e:
+                self.close()
+                e.args += (NO_CONNECTION.format(self.address, self.port),)
+                raise e
+            except pymodbus.exceptions.ModbusIOException as e:
+                self.close()
+                e.args += (NO_VALUES.format(self.address, self.port),)
+                raise e
+            except Exception as e:
+                self.close()
+                raise Exception(__name__+" "+str(type(e))+" " + str(e)) from e
 
     def read_input_registers_bulk(self,
                                   start_address: int,