Merge pull request #142 from kbialek/split-long-reg-ranges

Split register ranges longer than configured limit

README.md

@@ -200,6 +200,11 @@ In order to do this run the following commands:
    1. `Connected to <ip>` - connection works fine. The next step is to enable DEBUG logs in `config.env` and open a github issue
    2. `telnet: can't connect to remote host (<ip>): Connection refused` - The next step is: fix your network configuration
 
+#### Random read timeouts 
+
+For best performance, multiple Modbus registers are read at once, in so called register ranges. It's been reported [here](https://github.com/kbialek/deye-inverter-mqtt/issues/141) that Deye-SUN-5K-SG03LP1 reading times out when more than 16 registers is requested at once. To mitigate this problem you may try to set `DEYE_LOGGER_MAX_REG_RANGE_LENGTH` to lower number.
+
+
 ## Configuration
 All configuration options are controlled through environment variables.
 
@@ -228,6 +233,7 @@ All configuration options are controlled through environment variables.
 * `DEYE_LOGGER_IP_ADDRESS` - inverter data logger IP address
 * `DEYE_LOGGER_PORT` - inverter data logger communication port, optional, defaults to 8899 for Modbus/TCP, and 48899 for Modbus/AT
 * `DEYE_LOGGER_PROTOCOL` - inverter communication protocol, optional, either `tcp` for Modbus/TCP, or `at` for Modbus/AT, defaults to `tcp`
+* `DEYE_LOGGER_MAX_REG_RANGE_LENGTH` - controls maximum number of registers to be read in a single Modbus registers read operation, defaults to 256
 * `DEYE_FEATURE_MQTT_PUBLISHER` - controls, if the service will publish metrics over mqtt, defaults to `true`
 * `DEYE_FEATURE_SET_TIME` - when set to `true`, the service will automatically set the inverter/logger time, defaults to `false`
 * `DEYE_FEATURE_ACTIVE_POWER_REGULATION` - enables active power regulation control over MQTT command topic

config.env.example

@@ -2,6 +2,7 @@ DEYE_LOGGER_IP_ADDRESS=192.168.x.x
 # DEYE_LOGGER_PORT=8899
 DEYE_LOGGER_SERIAL_NUMBER=1234567890
 # DEYE_LOGGER_PROTOCOL=tcp
+# DEYE_LOGGER_MAX_REG_RANGE_LENGTH=256
 
 MQTT_HOST=192.168.x.x
 # MQTT_PORT=1883

src/deye_config.py

@@ -155,7 +155,14 @@ class DeyeLoggerConfig:
     with the device.
     """
 
-    def __init__(self, serial_number: int, ip_address: str, port: int, protocol: str = "tcp"):
+    def __init__(
+        self,
+        serial_number: int,
+        ip_address: str,
+        port: int,
+        protocol: str = "tcp",
+        max_register_range_length: int = 256,
+    ):
         self.serial_number = serial_number
         self.ip_address = ip_address
         if protocol not in ["tcp", "at"]:
@@ -167,6 +174,7 @@ def __init__(self, serial_number: int, ip_address: str, port: int, protocol: str
             self.port = 48899
         else:
             self.port = port
+        self.max_register_range_length = max_register_range_length
 
     @staticmethod
     def from_env():
@@ -175,6 +183,7 @@ def from_env():
             ip_address=DeyeEnv.string("DEYE_LOGGER_IP_ADDRESS"),
             port=DeyeEnv.integer("DEYE_LOGGER_PORT", 0),
             protocol=DeyeEnv.string("DEYE_LOGGER_PROTOCOL", "tcp"),
+            max_register_range_length=DeyeEnv.integer("DEYE_LOGGER_MAX_REG_RANGE_LENGTH", 256),
         )
 
 

src/deye_daemon.py

@@ -30,7 +30,7 @@
 from deye_mqtt_subscriber import DeyeMqttSubscriber
 from deye_observation import Observation
 from deye_plugin_loader import DeyePluginContext, DeyePluginLoader
-from deye_sensor import SensorRegisterRange
+from deye_sensor import SensorRegisterRanges
 from deye_sensors import sensor_list, sensor_register_ranges
 from deye_set_time_processor import DeyeSetTimeProcessor
 from deye_command_handlers import DeyeActivePowerRegulationCommandHandler
@@ -49,8 +49,10 @@ def __init__(self, config: DeyeConfig):
         connector = DeyeConnectorFactory(config).create_connector()
         self.modbus = DeyeModbus(connector)
         self.sensors = [s for s in sensor_list if s.in_any_group(self.__config.metric_groups)]
-        self.reg_ranges = [r for r in sensor_register_ranges if r.in_any_group(self.__config.metric_groups)]
-        self.reg_ranges = self.__remove_duplicated_reg_ranges(self.reg_ranges)
+        reg_ranges = [r for r in sensor_register_ranges if r.in_any_group(self.__config.metric_groups)]
+        self.reg_ranges = SensorRegisterRanges(
+            reg_ranges, max_range_length=config.logger.max_register_range_length
+        ).ranges
 
         mqtt_client = DeyeMqttClient(self.__config)
 
@@ -120,13 +122,6 @@ def __get_observations_from_reg_values(self, regs: dict[int, bytearray]) -> list
                 self.__log.debug(f"{observation.sensor.name}: {observation.value_as_str()}")
         return events
 
-    def __remove_duplicated_reg_ranges(self, reg_ranges: list[SensorRegisterRange]) -> list[SensorRegisterRange]:
-        result: list[SensorRegisterRange] = []
-        for reg_range in reg_ranges:
-            if not [r for r in result if r.is_same_range(reg_range)]:
-                result.append(reg_range)
-        return result
-
     def __is_device_observation_changed(self, events: DeyeEventList) -> bool:
         """Check if the received event observations have changed compared to last published
 

src/deye_sensor.py

@@ -15,6 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 
+import math
 from abc import abstractmethod
 
 
@@ -266,7 +267,54 @@ def is_same_range(self, other: "SensorRegisterRange") -> bool:
         """
         return self.first_reg_address == other.first_reg_address and self.last_reg_address == other.last_reg_address
 
+    @property
+    def length(self) -> int:
+        return self.last_reg_address - self.first_reg_address + 1
+
+    def split(self, sub_range_len: int) -> list["SensorRegisterRange"]:
+        """Splits this register range into sub-ranges
+
+        Args:
+            sub_range_len (int): only ranges longer than this value are splitted
+
+        Returns:
+            list[SensorRegisterRange]: created sub-ranges
+        """
+        sub_ranges: list[SensorRegisterRange] = []
+        sub_ranges_count = math.ceil(self.length / sub_range_len)
+        for i in range(0, sub_ranges_count):
+            sub_range_first_reg = self.first_reg_address + i * sub_range_len
+            sub_range_last_reg = min(sub_range_first_reg + sub_range_len - 1, self.last_reg_address)
+            sub_ranges.append(SensorRegisterRange(self.group, sub_range_first_reg, sub_range_last_reg))
+        return sub_ranges
+
     def __str__(self):
         return "metrics group: {}, range: {:04x}-{:04x}".format(
             self.group, self.first_reg_address, self.last_reg_address
         )
+
+
+class SensorRegisterRanges:
+    def __init__(self, ranges: list[SensorRegisterRange], max_range_length: int):
+        unique_ranges = SensorRegisterRanges.__remove_duplicated_reg_ranges(ranges)
+        self.ranges = SensorRegisterRanges.__split_long_reg_ranges(unique_ranges, max_range_length)
+
+    @staticmethod
+    def __split_long_reg_ranges(
+        reg_ranges: list[SensorRegisterRange], max_range_length: int
+    ) -> list[SensorRegisterRange]:
+        result: list[SensorRegisterRange] = []
+        for reg_range in reg_ranges:
+            if reg_range.length <= max_range_length:
+                result.append(reg_range)
+            else:
+                result += reg_range.split(max_range_length)
+        return result
+
+    @staticmethod
+    def __remove_duplicated_reg_ranges(reg_ranges: list[SensorRegisterRange]) -> list[SensorRegisterRange]:
+        result: list[SensorRegisterRange] = []
+        for reg_range in reg_ranges:
+            if not [r for r in result if r.is_same_range(reg_range)]:
+                result.append(reg_range)
+        return result

tests/deye_sensor_test.py

@@ -24,6 +24,7 @@
     SignedMagnitudeSingleRegisterSensor,
     SignedMagnitudeDoubleRegisterSensor,
     SensorRegisterRange,
+    SensorRegisterRanges,
 )
 
 
@@ -189,22 +190,77 @@ def test_single_reg_sensor_write_signed(self):
         # then
         self.assertEqual(result, {0: bytearray.fromhex("fb2e")})
 
+    def test_split_long_register_range(self):
+        # given
+        sut = SensorRegisterRange("test", 10, 50)
+
+        # when
+        result = sut.split(21)
+
+        # then
+        self.assertEqual(len(result), 2)
+        self.assertEqual(result[0].length, 21)
+        self.assertEqual(result[0].first_reg_address, 10)
+        self.assertEqual(result[0].last_reg_address, 30)
+        self.assertEqual(result[1].length, 20)
+        self.assertEqual(result[1].first_reg_address, 31)
+        self.assertEqual(result[1].last_reg_address, 50)
+
+    def test_split_short_register_range(self):
+        # given
+        sut = SensorRegisterRange("test", 10, 50)
+
+        # when
+        result = sut.split(45)
+
+        # then
+        self.assertEqual(len(result), 1)
+        self.assertEqual(result[0].length, 41)
+        self.assertEqual(result[0].first_reg_address, 10)
+        self.assertEqual(result[0].last_reg_address, 50)
+
+    def test_prep_register_ranges(self):
+        # when
+        sut = SensorRegisterRanges(
+            ranges=[
+                SensorRegisterRange("a", 1, 10),
+                SensorRegisterRange("b", 20, 40),
+                SensorRegisterRange("c", 60, 70),
+            ],
+            max_range_length=15,
+        )
+
+        # then
+        self.assertEqual(len(sut.ranges), 4)
+        self.assertEqual(sut.ranges[0].group, {"a"})
+        self.assertEqual(sut.ranges[0].first_reg_address, 1)
+        self.assertEqual(sut.ranges[0].last_reg_address, 10)
+        self.assertEqual(sut.ranges[1].group, {"b"})
+        self.assertEqual(sut.ranges[1].first_reg_address, 20)
+        self.assertEqual(sut.ranges[1].last_reg_address, 34)
+        self.assertEqual(sut.ranges[2].group, {"b"})
+        self.assertEqual(sut.ranges[2].first_reg_address, 35)
+        self.assertEqual(sut.ranges[2].last_reg_address, 40)
+        self.assertEqual(sut.ranges[3].group, {"c"})
+        self.assertEqual(sut.ranges[3].first_reg_address, 60)
+        self.assertEqual(sut.ranges[3].last_reg_address, 70)
+
     def test_registry_range_single_group_name(self):
         # given
         sut = SensorRegisterRange("a", 1, 2)
 
         # expect
-        self.assertTrue(sut.in_any_group("a"))
-        self.assertFalse(sut.in_any_group("b"))
+        self.assertTrue(sut.in_any_group({"a"}))
+        self.assertFalse(sut.in_any_group({"b"}))
 
     def test_registry_range_multiple_groups_names(self):
         # given
         sut = SensorRegisterRange({"a", "b"}, 1, 2)
 
         # expect
-        self.assertTrue(sut.in_any_group("a"))
-        self.assertTrue(sut.in_any_group("b"))
-        self.assertFalse(sut.in_any_group("c"))
+        self.assertTrue(sut.in_any_group({"a"}))
+        self.assertTrue(sut.in_any_group({"b"}))
+        self.assertFalse(sut.in_any_group({"c"}))
 
 
 if __name__ == "__main__":