frenck / spook

A scary 👻 powerful toolbox 🧰 for Home Assistant 🏡
https://spook.boo
MIT License
381 stars 36 forks source link

Spook sensors should have state_class: measurement instead of total #582

Closed rrozema closed 3 months ago

rrozema commented 3 months ago

What version of Spook are you using?

v2.1.1

What version of Home Assistant are you using?

2024.1.6

The problem

The sensors created by Spook all seem to have a state_class: total attribute. This means the recorder creates statistics for them that have has_sum = 1 and has_mean = 0. has_sum = 1 however is to be used for sensors that have an increasing value, like f.e. energy usage, gas usage, costs, etc. The proper state_class on Spook's sensors would be measurement, so that more meaningful statistic min, max and mean values will be recorded for them.

Anything in the logs? Paste it here!

No response

frenck commented 3 months ago

You are now creating a definition based from some values you want.

Looking at the definition, these are totals.

rrozema commented 3 months ago

edit: shortened my response.

Giving the entities a state_class: total results in statistics_meta rows being created for those entities that have has_sum = 1. This is incorrect, as Spook's entities are not meter values like f.e. an energy meter. They are momentary measurements for which a cumulative sum of the difference over the interval period (= the contents of the sum column) is meaningless. I see why you want Spook's entities to go into the state column, but giving the entities the state_class: total has the side-effect of also setting the sum, which is incorrect and makes it hard to add generic code that handles the statistics and statistics_short_term tables correctly.

I suggest making a change to recorder in that whenever an attribute state_class: exists on an entity it always stores this entity's state value at the start of the interval period into the state column. Next to that state column it should only when state_class: equals measurement set the three columns min, max, mean. And likewise, only when state_class: equals total or total_increasing recorder should set the [sum] column next to that state column. Spook's entities can then either get the state_class: measurement and the measured number of components at the start of the interval will go into the state column as it does now. Next to that it would also set the min, max and mean values, which is legit in itself. Although I see no reason for it, you could choose to introduce a new state_class, for example state_class: state_only that would only set the state column and neither of sum, mean, min or max, resulting in statistics_meta rows with both has_sum=0 and has_mean=0.

rrozema commented 3 months ago

I am not creating this definition: The blog post New sensor state class: total dated September 20, 2012 by Erik Montnémery describes the intended use for the 3 available state_class: values:

frenck commented 3 months ago

Sorry you are wrong.

The current value represents a total and not a measurement.

The currently set state class is correct.

../Frenck

rrozema commented 3 months ago

So it is a total because you've counted the number of objects? Counting a number of items is a measurement. For example: counting the number of lines the fluid inside a thermometer rose does not result in a total for degrees celcius. No, that is called a measurement. How is counting those lines different than counting f.e. the number of sensors in my installation?

frenck commented 3 months ago

Sorry, thanks for the suggestion. This change won't be happening.

Thanks 👍

../Frenck