datatype value is actually not used

[kbabioch]

The base class for all datatypes is Base, which looks as follows:

python-luxtronik/luxtronik/datatypes.py

Lines 6 to 28 in 34f4e66

	class Base:
	"""Base datatype, no conversions."""
	measurement_type = None
	def __init__(self, name, writeable=False):
	self.value = None
	self.name = name
	self.writeable = writeable
	def to_heatpump(self, value):
	"""Convert a value into heatpump units."""
	return value
	def from_heatpump(self, value):
	"""Convert a value from heatpump units."""
	return value
	def __repr__(self):
	return str(self.value)
	def __str__(self):
	return str(self.value)

All other datatypes are derived from that. I'm currently wondering what the attribute value is actually used for in this context. The __repr__ and __str__ methods are referring to it, but actually the value is never set (other than during initialization). Any invocation of from_heatpump and to_heatpump will just calculate and return the value, but never set it.

Is this attribute actually needed and if so, what for and how is it used? Am I missing something here?

[Bouni]

I just had to look what I've been doing back then 😅

When all the data is read from the heatpump, we pass the raw value to from_heatpump and store the result in .value, here for example:

https://github.com/Bouni/python-luxtronik/blob/main/luxtronik/parameters.py#L1171

When you then get a parameter (tvl = l.calculations.get("ID_WEB_Temperatur_TVL")) you use it either like print(tvl) which uses __str__ or like print(tvl.value)

As i read the code and think about it, not very nice what I did there 🥴

Maybe we should read all data, store it in .raw_data, and convert it on the fly:

class Base:
    """Base datatype, no conversions."""

    measurement_type = None

    def __init__(self, name, writeable=False):
        self.raw_value = None
        self.name = name
        self.writeable = writeable

    def _to_heatpump(self):
        """Converts value into heatpump units."""
        return self.raw_value

    def _from_heatpump(self):
        """Converts value from heatpump units."""
        return raw_value

    @raw_value.setter
    def set_raw_value(self, value):
        self.raw_value = self._to_heatpump(value)
        return self.raw_value

    @property
    def value(self):
        return self._from_heatpump(self.raw_value)

    def __repr__(self):
        return str(self.value)

    def __str__(self):
        return str(self.value)

That way we could have print(tvl) as well as print(tvl.value) and print(tvl.raw_value), and for writing self.queue[index] = parameter.value = 12345 (https://github.com/Bouni/python-luxtronik/blob/main/luxtronik/parameters.py#L1210)

These are just thoughts, nothing tested ...

[kbabioch]

In general I like your approach, that might turn out useful.

After re-reading the code that the value attribute of datatype is actually used, although only indirectly via the code you've referenced:

python-luxtronik/luxtronik/parameters.py

Lines 1162 to 1172 in fec29e9

	def parse(self, raw_data):
	"""Parse raw parameter data."""
	for index, data in enumerate(raw_data):
	parameter = self.parameters.get(index, False)
	if parameter is not False:
	parameter.value = parameter.from_heatpump(data)
	else:
	#LOGGER.warning("Parameter '%d' not in list of parameters", index)
	parameter = Unknown(f"Unknown_Parameter_{index}")
	parameter.value = parameter.from_heatpump(data)
	self.parameters[index] = parameter

As you've pointed our yourself, this is not be the most elegant solution.

I like you're proposal to manage the raw_value along with the value attribute. It seems to be more flexible. This will essentially help us to address #46 eventually.

However I'm missing something with your proposal: It seems to be not possible to set the value, only the raw_value. To have more flexibility I propose to change it in the following way:

• Manage / store only the raw data as attribute (just like you're proposing)
• Allow for raw_data to be accessed regularly, i.e. you can read and write raw data if you happen to know how data should be represented
• Have a "getter" to retrieve / translate the raw data into a readable value, i.e. the value property that you've outlined above
• Have a "setter" function to translate readable values into raw data that will eventually be sent to the heatpump.

Putting this into the code, it might look something like this:

class Base:
    """Base datatype, no conversions."""

    measurement_type = None

    def __init__(self, name, writeable=False):
        self.name = name
        self.writeable = writeable
        self.raw_value = None

    def _to_heatpump(self, value):
        """Converts value into heatpump units."""
        return value

    def _from_heatpump(self, value):
        """Converts value from heatpump units."""
        return value

    @property
    def value(self):
        return self._from_heatpump(self.raw_value)

    @value.setter
    def value(self, value):
        self.raw_value = self._to_heatpump(value)

    def __repr__(self):
        return f'{self.__class__.__name__}(name: {self.name}, writeable: {self.writeable}, value: {self.value})'

    def __str__(self):
        return str(self.value)

What do you think about this? Anything I've missed? Potentially _to_heatpump and _from_heatpump could also become abstract and/or static methods, but that's not too important from my point of view.

Data can be read and written in the following way:

>>> b = base.Base("b")
>>> b.value = 2
>>> b.raw_value
2
>>> b.raw_value = 3
>>> b.value
3

This way it is possible to read and write data raw (raw_value) as well as interpreted (value).

[kbabioch]

Taking #57 into account, this is my current proposal:

class Base:
    """Base datatype, no conversions."""

    type = None
    unit = None

    def __init__(self, name, writeable=False):
        self.name = name
        self.writeable = writeable
        self.raw_value = None

    def _to_heatpump(self, value):
        """Converts value into heatpump units."""
        return value

    def _from_heatpump(self, value):
        """Converts value from heatpump units."""
        return value

    @property
    def value(self):
        return self._from_heatpump(self.raw_value)

    @value.setter
    def value(self, value):
        self.raw_value = self._to_heatpump(value)

    def __repr__(self):
        return f'{self.__class__.__name__} (name: {self.name}, writeable: {self.writeable}, raw_value: {self.raw_value}, value: {self.value}, unit: {self.unit})'

    def __str__(self):
        return str(self.value)

class Celsius(Base):

    type = "temperature"
    unit = "°C"

    def _to_heatpump(self, value):
        """Converts value into heatpump units."""
        return int(float(value) * 10) 
 
    def _from_heatpump(self, value):
        """Converts value from heatpump units."""
        return value / 10

class Kelvin(Base):

    type = "temperature"
    unit = "K"

    def _to_heatpump(self, value):
        """Converts value into heatpump units."""
        return int(float(value) * 10) 
 
    def _from_heatpump(self, value):
        """Converts value from heatpump units."""
        return value / 10

This way we can deal with different kind of data (e.g. temperature vs. boolean via the type attribute, as well as with different kind of units, e.g. Kelvin vs. Celsius.

In all cases we can read and write data directly (raw_value) or in a nice format (value).

Anything still missing? Did I overlook something?

[Guzz-T]

Do the changes in #92 fix this issue too?

[kbabioch]

value is no longer available and has been removed in favor of _raw along with some setters and properties (see #96). Therefore this issue has been addressed and can be closed.

#57 still remains open, but can be addressed independent of this issue here.