Bouni / luxtronik

Luxtronik integration for Home Assistant
MIT License
82 stars 13 forks source link

Incorrect sensor readout by home assistant #52

Open tetsuo55 opened 1 year ago

tetsuo55 commented 1 year ago

After updating to the latest version some sensors are broken

These 2 report invalid timestamp, the value should be number of hours

group: parameters id: ID_Einst_Kuhl_Zeit_Ein_akt friendly_name: Koeling aan na BT overschrijding

- group: parameters
  id: ID_Einst_Kuhl_Zeit_Aus_akt
  friendly_name: Koeling uit na BT onderschrijding

These ones are being read as days into the past/future but should be seconds

group: calculations id: ID_WEB_Time_WPein_akt friendly_name: Warmtepomp loopt

group: calculations id: ID_WEB_Time_LGS_akt friendly_name: Thermische desinfectie since

Bouni commented 1 year ago

What do you mean by latest verson? Latest version of this integration?

ID_Einst_Kuhl_Zeit_Aus_akt was not touched in the last 3 years and is of type Hours: https://github.com/Bouni/python-luxtronik/blame/main/luxtronik/parameters.py#L876

ID_WEB_Time_WPein_akt was not touched either and is of type seconds: https://github.com/Bouni/python-luxtronik/blame/main/luxtronik/calculations.py#L110

The same is true for ID_WEB_Time_LGS_akt

Did you upgrade your Heatpump recently?

Are you aware that all sensors / binary_sensors are no longer luxtronik.XXX but sensor.XXX / binary_sensor.XXX since the december releases?

tetsuo55 commented 1 year ago

I was using 1.3.0 and today updated to 2022.12.02. I also updated home assistant to 2022.12.0

I updated all the sensors from luxtronic to (binary_) sensor and now the ones i mentioned are incorrect

Maybe the home assistant update broke it

Bouni commented 1 year ago

What moel of heatpump do you use?

tetsuo55 commented 1 year ago

alpha innotec Wzs 6hk

bheindl commented 1 year ago

I'm having a similar issue with some ID_WEB_Zaehler_BetrZeit* sensors. Either since the HA 2022.12.0 or since the luxtronik integration update. The graphs in the history looks fine, but the entity cards just displaying "invalid timestamp". Seems like the sensors report an invalid format. See attached a screenshot:

Bildschirmfoto von 2022-12-11 14-21-26

Bouni commented 1 year ago

Maybe Home Assistant changed the way they interpret timestamps 🤔

Bouni commented 1 year ago

I think I've figured it out. This is (and was never) a timestamp so device_class: timestamp is wrong and it should have been device_class: duration

This is the ID_WEB_Time_WPein_akt as it is:

grafik

And this is with device_class set to duration using a template sensor (just for the test):

grafik

Bouni commented 1 year ago

I think we need to introduce a new datatype Duration here: https://github.com/Bouni/python-luxtronik/blob/main/luxtronik/datatypes.py

And change some of the Timestamp to Duration in here: https://github.com/Bouni/python-luxtronik/blob/main/luxtronik/calculations.py

Anyone want to send a PR?

kbabioch commented 1 year ago

I can give it a try. The change itself doesn't look to hard, just need to figure out how to best test it.

kbabioch commented 1 year ago

Looked into this in some more details, still have to get familiar with the Home Assistant device classes.

Overall I tried two things:

class Duration(Base):

    measurement_type = "duration"

    def from_heatpump(self, value):
        if value > 0:
            return datetime.timedelta(seconds=value)
        return datetime.timedelta(seconds=0)

    def to_heatpump(self, value):
        return int(value.total_seconds())

As well as a more straight-forward approach:

class Duration(Base):

    measurement_type = "duration"

    def from_heatpump(self, value):
        if value > 0:
            return value
        return 0

    def to_heatpump(self, value):
        return value

However I'm not quite sure what Home Assistant expects. The documentation for duration (https://developers.home-assistant.io/docs/core/entity/sensor/) lists multiple units, so I need to figure out which unit is expected, etc.

Additionally it seems like some of the parameters are reported as seconds (ID_WEB_Time_WPein_akt) while others might be reported as hours:

850: Hours("ID_Einst_Kuhl_Zeit_Ein_akt", True),                 
851: Hours("ID_Einst_Kuhl_Zeit_Aus_akt", True),

Should this be represented by one datatype, or are multiple ones needed here?

kbabioch commented 1 year ago

Also it might be useful to set the state class for those entities to total_increasing as described here: https://developers.home-assistant.io/docs/core/entity/sensor/#available-state-classes

What do you think?

Bouni commented 1 year ago

I just realized that there is already a Seconds data type:

https://github.com/Bouni/python-luxtronik/blob/main/luxtronik/datatypes.py#L83-L87

So we just need a Hours data type.

Maybe some adjustments are necessary as well

Bouni commented 1 year ago

@kbabioch

I was wrong, we already have Seconds and Hours

So I think we only have to add measurement_type = "duration" to both of them.

As you already mentioned, we might add a state_class to all of the available datatypes as well. What needs to be checked first though is if it's a problem when parameters have a state_class because they are static or if we only should add them to calculations. If the later is true, we could dynamically add the state_class in the HA integration based on configuration group = calculations