Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect sensor readout by home assistant #52

Open
tetsuo55 opened this issue Dec 8, 2022 · 13 comments
Open

Incorrect sensor readout by home assistant #52

tetsuo55 opened this issue Dec 8, 2022 · 13 comments

Comments

@tetsuo55
Copy link

tetsuo55 commented Dec 8, 2022

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
Copy link
Owner

Bouni commented Dec 8, 2022

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
Copy link
Author

tetsuo55 commented Dec 8, 2022

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
Copy link
Owner

Bouni commented Dec 8, 2022

What moel of heatpump do you use?

@tetsuo55
Copy link
Author

tetsuo55 commented Dec 8, 2022

alpha innotec Wzs 6hk

@bheindl
Copy link

bheindl commented Dec 11, 2022

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
Copy link
Owner

Bouni commented Dec 14, 2022

Maybe Home Assistant changed the way they interpret timestamps 🤔

@Bouni
Copy link
Owner

Bouni commented Dec 14, 2022

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
Copy link
Owner

Bouni commented Dec 14, 2022

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
Copy link

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
Copy link

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
Copy link

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
Copy link
Owner

Bouni commented Dec 14, 2022

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
Copy link
Owner

Bouni commented Dec 15, 2022

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants