ksheumaker / homeassistant-apsystems_ecur

Home Assistant custom component for local querying of APSystems ECU-R Solar System
Apache License 2.0
166 stars 42 forks source link

Removed `last_reset` property completely #35

Closed renini closed 1 year ago

renini commented 2 years ago

sensor.ecu_current_power has a last_reset property set

This is deprecated and resulting in the following warning:

Entity sensor.ecu_current_power (<class 'custom_components.apsystems_ecur.sensor.APSystemsECUSensor'>) with state_class measurement has set last_reset. Setting last_reset for entities with state_class other than 'total' is deprecated and will be removed from Home Assistant Core 2021.11. Please update your configuration if state_class is manually configured, otherwise report it to the custom component author.

tv3 commented 2 years ago

Shouldn't whole property be removed?

Read a lot of comments about the last_reset property causing lots of issues in integrations.
This morning had a reading of lifetime energy reset to zero causing massive solar production (which would be great, be it that it was fake). And all other data was dwarfed. Had to hack the SQLite database to fix it.

renini commented 2 years ago

With this it's not set anymore. But probably the whole def can be removed. Thought maybe it will be needed some day again for daily total... thats why i figured to change it this way

ksheumaker commented 2 years ago

Yea I'm going to remove it when I get a chance, there doesn't seem to be a good reason to include it anymore.

@tv3 did you have the latest version? I'm now attempting to catch a 0 result from the ECU and thrown an exception and using the cached data from the last successful read instead.

tv3 commented 2 years ago

Yeah. Running latest versions.

renini commented 2 years ago

I removed the whole last_reset property since it's not used at all. And for the ecu_today_energy entity HA will correctly notice the last reset by itself anyways i believe.

For what it's worth: In my HA energy dashboard i'm using the following entities, and for now this seems to correctly work for me:

note, these dsmr stuff are for dutch p1 metered electricity readings. But for me the daily consumption and solar production seems to work better, then using overal totals.

In my setup i'm currently running with the last_reset property removed like in this PR, and so far it seems oke. Yesterday when i was fiddling some more, i had correct the energy dashboard solar production entity, since the duplicate ecu sensors with the "2" suffix appeared again. Not sure where they came from, but for now it seem's oke.

ksheumaker commented 2 years ago

@renini I think I have figured out the source of the duplicate entries like sensor.ecu_lifetime_energy_2 entries. I outlined my testing and what I found in #27. Once I've had a chance to test those changes i'll merge this, and that and release a new version.

@tv3 On the time that you got a 0 result you should have seen something like this in your log: ECU returned 0 for lifetime energy, raw data={self.ecu_raw_data} - see line 118 of APSystemsECUR.py. Can you provide me what the raw data was?

I can't really perform that check on ecu_today_energy since it being 0 is a valid condition every morning.

tv3 commented 2 years ago

@ksheumaker haven't seen that entry in the log. So I really don't know how it got in there, as the other entries (total/current power) seem ok at that timeframe. But I am looking at hourly stats from the statistics table. Is HA calculating those values from multiple raw inputs?

HAEdwin commented 1 year ago

last_rest property was removed in the 1.1.2. release