Remove the parameter queue and use the write_pending flag instead

README.md

@@ -238,16 +238,17 @@ from luxtronik import Luxtronik, Parameters
 
 l = Luxtronik('192.168.1.23', 8889)
 
-# Queue a parameter change
-# In this example, the domestic hot water temperature is set to 45 degrees.
+# Set the value of a field
+# In this example, the domestic hot water temperature
+# is set (for the time being only in this field) to 45 degrees
 l.parameters.set("ID_Soll_BWS_akt", 45.0)
 
-# Write all queued changes to the heat pump
+# Then write the data of all changed fields to the to the heat pump
 l.write()
 
 # Another possibility to write parameters
 parameters = Parameters()
-parameters.set("ID_Ba_Hz_akt", "Party")
+parameters["ID_Ba_Hz_akt"] = "Party"
 l.write(parameters)
 
 # If you're not sure what values to write, you can get all available options:
@@ -261,7 +262,7 @@ l.holdings["heating_mode"] = "Offset" # Set the value to activate the offset mod
 l.write()                             # Write down the values to the heatpump
 ```
 
-**NOTE:** Writing values to the heat pump is particulary dangerous as this is
+**NOTE:** Writing values to the heat pump is particularly dangerous as this is
 an undocumented API. By default a safe guard is in place, which will prevent
 writing parameters that are not (yet) understood.
 

luxtronik/cfi/interface.py

@@ -162,23 +162,24 @@ def _write_and_read(self, parameters, data):
         return self._read(data)
 
     def _write(self, parameters):
-        for index, value in parameters.queue.items():
-            if not isinstance(index, int) or not isinstance(value, int):
-                LOGGER.warning(
-                    "%s: Parameter id '%s' or value '%s' invalid!",
-                    self._host,
-                    index,
-                    value,
-                )
-                continue
-            LOGGER.info("%s: Parameter '%d' set to '%s'", self._host, index, value)
-            self._send_ints(LUXTRONIK_PARAMETERS_WRITE, index, value)
-            cmd = self._read_int()
-            LOGGER.debug("%s: Command %s", self._host, cmd)
-            val = self._read_int()
-            LOGGER.debug("%s: Value %s", self._host, val)
-        # Flush queue after writing all values
-        parameters.queue = {}
+        for index, field in parameters.data.items():
+            if field.write_pending:
+                field.write_pending = False
+                value = field.raw
+                if not isinstance(index, int) or not field.check_for_write(parameters.safe):
+                    LOGGER.warning(
+                        "%s: Parameter id '%s' or value '%s' invalid!",
+                        self._host,
+                        index,
+                        value,
+                    )
+                    continue
+                LOGGER.info("%s: Parameter '%d' set to '%s'", self._host, index, value)
+                self._send_ints(LUXTRONIK_PARAMETERS_WRITE, index, value)
+                cmd = self._read_int()
+                LOGGER.debug("%s: Command %s", self._host, cmd)
+                val = self._read_int()
+                LOGGER.debug("%s: Value %s", self._host, val)
         # Give the heatpump a short time to handle the value changes/calculations:
         time.sleep(WAIT_TIME_AFTER_PARAMETER_WRITE)
 
@@ -285,6 +286,8 @@ def _parse(self, data_vector, raw_data):
             # remove all used indices from the list of undefined indices
             for index in range(definition.index, next_idx):
                 undefined.discard(index)
+            # integrate_data() also resets the write_pending flag,
+            # intentionally only for read fields
             pair.integrate_data(raw_data, LUXTRONIK_CFI_REGISTER_BIT_SIZE)
 
         # create an unknown field for additional data

luxtronik/cfi/parameters.py

@@ -33,25 +33,9 @@ def __init__(self, safe=True):
         """Initialize parameters class."""
         super().__init__()
         self.safe = safe
-        self.queue = {}
         for d in PARAMETERS_DEFINITIONS:
             self._data.add(d, d.create_field())
 
     @property
     def parameters(self):
         return self._data
-
-    def set(self, target, value):
-        """Set parameter to new value."""
-        index, parameter = self._lookup(target, with_index=True)
-        if index is not None:
-            if parameter.writeable or not self.safe:
-                raw = parameter.to_heatpump(value)
-                if isinstance(raw, int):
-                    self.queue[index] = raw
-                else:
-                    LOGGER.error("Value '%s' for Parameter '%s' not valid!", value, parameter.name)
-            else:
-                LOGGER.warning("Parameter '%s' not safe for writing!", parameter.name)
-        else:
-            LOGGER.warning("Parameter '%s' not found", target)

luxtronik/data_vector.py

@@ -8,6 +8,7 @@
 )
 
 from luxtronik.collections import LuxtronikFieldsDictionary
+from luxtronik.datatypes import Base
 
 
 LOGGER = logging.getLogger(__name__)
@@ -121,5 +122,14 @@ def get(self, target):
         return entry
 
     def set(self, target, value):
-        "TODO: Placeholder for future changes"
-        pass
+        """
+        Set the value of a field to the given value.
+
+        The value is set, even if the field marked as non-writeable.
+        No data validation is performed either.
+        """
+        field = target
+        if not isinstance(field, Base):
+            field = self.get(target)
+        if field is not None:
+            field.value = value

luxtronik/datatypes.py

@@ -1,6 +1,7 @@
 """datatype conversions."""
 
 import datetime
+import logging
 import socket
 import struct
 
@@ -14,6 +15,9 @@
 from functools import total_ordering
 
 
+LOGGER = logging.getLogger(__name__)
+
+
 @total_ordering
 class Base:
     """Base datatype, no conversions."""
@@ -83,6 +87,8 @@ def value(self):
     def value(self, value):
         """Converts the value into heatpump units and store it."""
         self._raw = self.to_heatpump(value)
+        if self._raw is None:
+            LOGGER.warning(f"Value '{value}' not valid for field '{self.name}'")
         self.write_pending = True
 
     @property
@@ -92,7 +98,7 @@ def raw(self):
 
     @raw.setter
     def raw(self, raw):
-        """Store the raw data."""
+        """Store the raw data. For internal use only"""
         self._raw = raw
         self.write_pending = False
 
@@ -141,6 +147,30 @@ def __lt__(self, other):
             and self.datatype_unit == other.datatype_unit
         )
 
+    def check_for_write(self, safe=True):
+        """
+        Returns true if the field is writable and the field data is valid.
+
+        Args:
+            safe (bool, Default: True): Flag for blocking write operations
+                if the field is not marked as writable
+
+        Returns:
+            bool: True if the data is writable, otherwise False.
+        """
+        if self.writeable or not safe:
+            # We support integers
+            if isinstance(self._raw, int):
+                return True
+            # and list of integers
+            elif isinstance(self._raw, list) and all(isinstance(value, int) for value in self._raw):
+                return True
+            else:
+                LOGGER.error(f"Value of '{self.name}' invalid!")
+        else:
+            LOGGER.warning(f"'{self.name}' not safe for writing!")
+        return False
+
 
 class SelectionBase(Base):
     """Selection base datatype, converts from and to list of codes."""

luxtronik/shi/interface.py

@@ -321,6 +321,8 @@ def _integrate_data(self, telegrams_data):
         success = True
         for block, telegram, read_not_write in telegrams_data:
             if (read_not_write == READ):
+                # integrate_data() also resets the write_pending flag,
+                # intentionally only for read fields
                 valid = block.integrate_data(telegram.data)
                 if not valid:
                     LOGGER.debug(f"Failed to integrate read data into {block}")
@@ -379,16 +381,14 @@ def _prepare_write_field(self, definition, field, safe, data):
         if not field.write_pending and data is None:
             return False
 
-        # Abort if field is not writeable
-        if safe and not (definition.writeable and field.writeable):
-            LOGGER.warning("Field marked as non-writeable: " \
-                + f"name={definition.name}, data={field.raw}")
-            return False
-
         # Override the field's data with the provided data
         if data is not None:
             field.value = data
 
+        # Abort if field is not writeable or the value is invalid
+        if not field.check_for_write(safe):
+            return False
+
         # Abort if insufficient data is provided
         if not get_data_arr(definition, field, LUXTRONIK_SHI_REGISTER_BIT_SIZE):
             LOGGER.warning("Data error / insufficient data provided: " \

luxtronik/shi/vector.py

@@ -287,23 +287,4 @@ def get(self, def_name_or_idx, default=None):
             If multiple fields added for the same index/name,
             the last added takes precedence.
         """
-        return self._data.get(def_name_or_idx, default)
-
-    def set(self, def_field_name_or_idx, value):
-        """
-        Set field to new value.
-
-        The value is set, even if the field marked as non-writeable.
-        No data validation is performed either.
-
-        Args:
-            def_field_name_or_idx (LuxtronikDefinition | Base | int | str):
-                Definition, name, or register index to be used to search for the field.
-                It is also possible to pass the field itself.
-            value (int | List[int]): Value to set
-        """
-        field = def_field_name_or_idx
-        if not isinstance(field, Base):
-            field = self.get(def_field_name_or_idx)
-        if field is not None:
-            field.value = value
+        return self._data.get(def_name_or_idx, default)

tests/cfi/test_cfi_interface.py

@@ -18,35 +18,53 @@ def test_parse(self):
         n = 2000
         t = list(range(0, n + 1))
 
+        parameters[0].write_pending = True
         lux._parse(parameters, t)
         p = parameters.get(n)
         assert p.name == f"unknown_parameter_{n}"
         assert p.raw == n
+        assert not p.write_pending
 
+        calculations[0].write_pending = True
         lux._parse(calculations, t)
         c = calculations.get(n)
         assert c.name == f"unknown_calculation_{n}"
         assert c.raw == n
+        assert not c.write_pending
 
+        visibilities[0].write_pending = True
         lux._parse(visibilities, t)
         v = visibilities.get(n)
         assert v.name == f"unknown_visibility_{n}"
         assert v.raw == n
+        assert not v.write_pending
 
         n = 10
         t = list(range(0, n + 1))
 
+        parameters[0].write_pending = True
+        parameters[20].write_pending = True
+        parameters[40].write_pending = True
         lux._parse(parameters, t)
         for definition, field in parameters.data.pairs():
             if definition.index > n:
                 assert field.raw is None
+            assert not field.write_pending
 
+        calculations[0].write_pending = True
+        calculations[20].write_pending = True
+        calculations[40].write_pending = True
         lux._parse(calculations, t)
         for definition, field in calculations.data.pairs():
             if definition.index > n:
                 assert field.raw is None
+            assert not field.write_pending
 
+        visibilities[0].write_pending = True
+        visibilities[20].write_pending = True
+        visibilities[40].write_pending = True
         lux._parse(visibilities, t)
         for definition, field in visibilities.data.pairs():
             if definition.index > n:
-                assert field.raw is None
+                assert field.raw is None
+            assert not field.write_pending

tests/cfi/test_cfi_parameters.py

@@ -15,11 +15,9 @@ def test_init(self):
         assert parameters.name == "parameter"
         assert parameters.parameters == parameters._data
         assert parameters.safe
-        assert len(parameters.queue) == 0
 
         parameters = Parameters(False)
         assert not parameters.safe
-        assert len(parameters.queue) == 0
 
     def test_data(self):
         """Test cases for the data dictionary"""
@@ -84,18 +82,21 @@ def test_set(self):
 
         # Set something which does not exist
         parameters.set("BarFoo", 0)
-        assert len(parameters.queue) == 0
+        assert parameters["BarFoo"] is None
 
-        # Set something which is not allowed to be set
-        parameters.set("ID_Transfert_LuxNet", 0)
-        assert len(parameters.queue) == 0
+        # Set something which was previously (v0.3.14) not allowed to be set
+        parameters.set("ID_Transfert_LuxNet", 1)
+        assert parameters["ID_Transfert_LuxNet"].raw == 1
+        assert parameters["ID_Transfert_LuxNet"].write_pending
 
         # Set something which is allowed to be set
-        parameters.set("ID_Einst_WK_akt", 0)
-        assert len(parameters.queue) == 1
+        parameters.set("ID_Einst_WK_akt", 2)
+        assert parameters["ID_Einst_WK_akt"].raw == 20
+        assert parameters["ID_Einst_WK_akt"].write_pending
 
         parameters = Parameters(safe=False)
 
-        # Set something which is not allowed to be set, but we are brave.
-        parameters.set("ID_Transfert_LuxNet", 0)
-        assert len(parameters.queue) == 1
+        # Set something which was previously (v0.3.14) not allowed to be set, but we are brave.
+        parameters.set("ID_Transfert_LuxNet", 4)
+        assert parameters["ID_Transfert_LuxNet"].raw == 4
+        assert parameters["ID_Transfert_LuxNet"].write_pending