home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.71k stars 30.84k forks source link

SensorStateClass.TOTAL_INCREASING valid for SensorDeviceClass.MONETARY? #88457

Closed alexhk90 closed 1 year ago

alexhk90 commented 1 year ago

The problem

For cost tracking in the Energy dashboard (i.e. under "Configure grid consumption" - "Use an entity tracking the total costs"), I have an entity configured with device_class: "monetary" and state_class: "total_increasing". This has been working well however I have noticed recently this Warning in the logs:

Logger: homeassistant.components.sensor
Source: components/sensor/__init__.py:503
Integration: Sensor ([documentation](https://www.home-assistant.io/integrations/sensor), [issues](https://github.com/home-assistant/home-assistant/issues?q=is%3Aissue+is%3Aopen+label%3A%22integration%3A+sensor%22))
First occurred: 1:19:00 PM (2 occurrences)
Last logged: 1:19:09 PM

    Entity sensor.smart_meter_electricity_cost_today (<class 'homeassistant.components.template.sensor.SensorTemplate'>) is using state class 'total_increasing' which is impossible considering device class ('monetary') it is using; expected None or one of 'total'; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/home-assistant/core/issues?q=is%3Aopen+is%3Aissue+label%3A%22integration%3A+template%22

Given that the underlying "energy" device class entity for the meter reading has state_class: "total_increasing", does it make sense to also allow "total_increasing" as a valid state class for the corresponding "monetary" device class entity tracking the cost?

What version of Home Assistant Core has the issue?

2023.2.5

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

sensor

Link to integration documentation on our website

https://www.home-assistant.io/integrations/sensor

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

Related change appears to be: https://github.com/home-assistant/core/pull/84402

home-assistant[bot] commented 1 year ago

Hey there @home-assistant/core, mind taking a look at this issue as it has been labeled with an integration (sensor) you are listed as a code owner for? Thanks!

Code owner commands Code owners of `sensor` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Change the title of the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign sensor` Removes the current integration label and assignees on the issue, add the integration domain after the command.

(message by CodeOwnersMention)


sensor documentation sensor source (message by IssueLinks)

oywino commented 1 year ago

Perhaps this is related?

Entity sensor.nordpool (<class 'custom_components.nordpool.sensor.NordpoolSensor'>) is using state class 'measurement' which is impossible considering device class ('monetary') it is using; expected None or one of 'total'; Please update your configuration if your entity is manually configured, otherwise report it to the custom integration author.
Entity sensor.accumulated_cost_home (<class 'homeassistant.components.tibber.sensor.TibberSensorRT'>) is using state class 'measurement' which is impossible considering device class ('monetary') it is using; expected None or one of 'total'; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/home-assistant/core/issues?q=is%3Aopen+is%3Aissue+label%3A%22integration%3A+tibber%22
Entity sensor.monthly_cost_middelthunet (<class 'homeassistant.components.tibber.sensor.TibberDataSensor'>) is using state class 'measurement' which is impossible considering device class ('monetary') it is using; expected None or one of 'total'; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/home-assistant/core/issues?q=is%3Aopen+is%3Aissue+label%3A%22integration%3A+tibber%22
mmillmor commented 1 year ago

+1 to this. I have a sensor which measures the amount of money generated by solar panels each day. The money amount goes up each day and resets to zero at midnight. It is not an amount which can go up and down, so not "total".

Vinblastine commented 1 year ago

+1 to this also. I am using to tract the cost of an item per day using energy tariff * KWh of a device. As above the money amount goes up each day and resets to zero at midnight therefore total_increasing should be a valid device class?

metalblue commented 1 year ago

+1 to this. MEASUREMENT should also be valid as well as TOTAL_INCREASING. I could have a sensor that displays the current cost of running a device for example.

frenck commented 1 year ago

From the OP:

<class 'homeassistant.components.template.sensor.SensorTemplate'>

This is a sensor template, meaning you create it; not Home Assistant. Please remove either the device class or change the state class. The current ones you have configured are not compatible. This is not a bug in Home Assistant, but a configuration error.

Total increasing is not a valid use case for us at this point, as the monetary device class is used to indicate a monetary balance value (which is a netto value going up and down). The total_increasing is mostly designed for use with meter values (like energy meters).

Closing this issue for that reason.

../Frenck

metalblue commented 1 year ago

@frenck which device class would you suggest we use to keep track of the current daily costs of gas and electricity for use with the energy dashboard?

frenck commented 1 year ago

@frenck which device class would you suggest we use to keep track of the current daily costs of gas and electricity for use with the energy dashboard?

None. Please note, device classes are not mandatory.

Edit: added missing not in the above sentence, sorry about that.

metalblue commented 1 year ago

@frenck which device class would you suggest we use to keep track of the current daily costs of gas and electricity for use with the energy dashboard?

None. Please note, device classes are mandatory.

The energy dash board complains and doesn't update the costs if I try and use a monetary sensor value for Total costs (of either Gas or Electricity) with a state class of None.

I get the error:

Unexpected state class
The following entities do not have the expected state class:
sensor.smart_meter_electricity_cost_today
frenck commented 1 year ago

I'm not following the issue here. total_increasing is made for metered things like an energy/electricity meter. Monetary values/aggregations are net balances. If it is a balance, use the total state class.

../Frenck

metalblue commented 1 year ago

I'm not following the issue here. total_increasing is made for metered things like an energy/electricity meter. Monetary values/aggregations are net balances. If it is a balance, use the total state class.

../Frenck

OK, so the current total cost of electricity for the day should be state class total and not total_increasing, even though its only ever going to increase (throughout the day) and is directly related to the meter?

Separately, should measurement not be a valid state class for the monetary sensor because it can be used to store the current cost of something which may vary in time. Examples would be unit cost of energy, or share prices?

frenck commented 1 year ago

OK, so the current total cost of electricity for the day should be state class total and not total_increasing, even though its only ever going to increase (throughout the day) and is directly related to the meter?

No, there are also net meters.

Separately, should measurement not be a valid state class for the monetary sensor because it can be used to store the current cost of something which may vary in time. Examples would be unit cost of energy, or share prices?

No, as it is not an aggregated value, which is what the device class represents.

../Frenck

metalblue commented 1 year ago

OK, so the current total cost of electricity for the day should be state class total and not total_increasing, even though its only ever going to increase (throughout the day) and is directly related to the meter?

No, there are also net meters.

Separately, should measurement not be a valid state class for the monetary sensor because it can be used to store the current cost of something which may vary in time. Examples would be unit cost of energy, or share prices?

No, as it is not an aggregated value, which is what the device class represents.

../Frenck

I still don't understand why measurement isn't a valid state class for a monetary sensor. Even this official blog post suggests it should be https://developers.home-assistant.io/blog/2021/08/16/state_class_total/

frenck commented 1 year ago

I still don't understand why measurement isn't a valid state class for a monetary sensor

Measurement isn't an aggregation.

metalblue commented 1 year ago

I still don't understand why measurement isn't a valid state class for a monetary sensor

Measurement isn't an aggregation.

So what is the monetary device class supposed to be used for?

frenck commented 1 year ago

I don't want to be impolite, but I've provided that answer multiple times.

../Frenck

metalblue commented 1 year ago

Frenck, I'm not trying to be annoying and I do appreciate your responses, I just still genuinely don't understand what the purpose of the device class is and its relationship to the state class is and why measurement isn't permitted for the monetary device class. It may be my understanding of what the monetary device class is intended to cover is incorrect.

In my mind any sensor that stores a monetary value, whether that be current unit cost of electricity/gas or a stock price should have device class monetary. Is this assumption correct?

Secondly, from reading the docs I understand the measurement state class as being used to "represents a measurement in present time". The current unit cost of electricity/gas or a stock price would seem to fit this description.

frenck commented 1 year ago

I just still genuinely don't understand what the purpose of the device class is and its relationship to the state class is and why measurement isn't permitted for the monetary device class

Both have different proposes. A state class merely defined what the values if for a long-term storage/statistics. The device class defined what it is representing. In the case of the monetary, it represents a net monetary value (balance).

The state classes are limited to match the goals of the device class.

The current unit cost of electricity/gas or a stock price would seem to fit this description.

Sure, that fits the state class, but not the device class; hence, the system is not accepting that combination.

metalblue commented 1 year ago

Thanks, that makes sense. Please can I make the following two suggestions, both of which I'm happy to help with if you agree.

  1. can the documentation be updated to better clarify the purpose of the monetary device class. Currently it just says "Monetary value with a currency." which isn't particularly helpful.
  2. can a new device class be added to represent the current cost/price of something?
alexhk90 commented 1 year ago

I'm not following the issue here. total_increasing is made for metered things like an energy/electricity meter. Monetary values/aggregations are net balances. If it is a balance, use the total state class.

../Frenck

Thanks, @frenck, for looking into this. For context, I created this issue because the energy dashboard expects certain state class for the (monetary) entity provided for "User an entity tracking the total costs": https://community.home-assistant.io/t/glow-hildebrand-display-local-mqtt-access-template-help/428638/285?u=alexhk90

As mentioned by @metalblue, perhaps we need different device classes to distinguish between monetary measures that have different purposes/usages?

Or maybe I'm missing the right combination of device class and state class that would satisfy both the energy dashboard and other validations?

metalblue commented 1 year ago

@alexhk90 I'm using that same plugin, so I've been battling the same issues.

I've changed my local version of the plugin to use device class monetary and state class total for the daily totals, which I think is correct based on my discussions with @frenck above. I think this is also working correctly in the energy dashboard, although I'm leaving it for a few days to be sure - I need to see what happens when they reset to 0 at the start of the new day.

I've left the unit costs and standing charge as device class monetary, which I now think is incorrect based upon the above. I think these should either be device class None, or we need a new device class (which I proposed above) to cover these.

mmillmor commented 1 year ago

I think the primary issue here is that there are multiple use cases, generally around energy costs, that seem to exactly meet the documented definitions of state_class and device_class - i.e. they are measuring financial amounts, and they fall into the category of either a spot price that can go up and down (e.g. current price per kWh) or an incrementing and resetting amount of money (e.g. amount of money spent today/this week/this bill). They fit the measurement and total_increasing state class respectively, and seem to be the right thing for people who want to be monitoring the statistics on those.

There is a warning that monetary amounts can't be measurements or total_increasing. It's not clear what harm that warning is trying to protect people from. Multiple users have examples where they believe it is what they need. It may be that there is a very good reason why people should make sure that they don't do that, but the reason why that is a bad idea is not documented, and the best practice of what to do instead is also not documented. I'd be happy to contribute and either update the documentation or change the code myself if I understood the underlying problems or potential future direction or whatever it is that this warning has been added for, but I have to confess that I just can't get my head round why this warning is in place.

frenck commented 1 year ago

can a new device class be added to represent the current cost/price of something?

, generally around energy costs, that seem to exactly meet the documented definitions of state_class and device_class - i.e. they are measuring financial amounts, and they fall into the category of either a spot price that can go up and down (e.g. current price per kWh) or an incrementing and resetting amount of money (e.g. amount of money spent today/this week/this bill).

Those are not monetary values, those are rates. The example of spot price is a good example, it is e.g., EUR/kWh, note how those values are an amount per unit (and not just a monetary value).

We don't have a device class for such things in Home Assistant, as we don't provide purpose for those either at this point (no direct use case). Additionally, unit conversion of rates can be tricky as well; we currently aren't able to support those either.

I do often see sensor set to e.g., the rate of a spot price with a unit of measurement in EUR (or another currency) and trying to at monetary as a device class; however, that is incorrect in multiple levels as per above.

mmillmor commented 1 year ago

Thanks for the explanation. I disagree that those things are rates. A rate represents how fast something is happening at any time, e.g. for energy the rate is W. For rain it is mm/hr. For spending on energy it is EUR/kW or EUR/hr. The accumulated amount spent on energy so far today is not a rate though, it is a total, just like kWh. For energy, the rate of power is W, and the rate of money is EUR/kWh. The total energy is kW, and the total money is EUR. kWh and EUR are different ways of measuring the same thing . So it's inconsistent that we are allowed to have a sensor with a device class of energy and a state class of total_increasing, but are not allowed to have a sensor with a device class representing a financial amount and a state class of total_increasing.

I can see you really don't want to make the change for the device class of monetary though. I don't understand why, but rather than argue about it, what if we introduced some new device classes;

PRICE - represents a point in time financial amount associated with something - i.e. a stock price or an energy price. Allowed state class = measurement MONETARY_AMOUNT - represents an amount of money (e.g. the amount spent on energy so far today). Allowed state class = total, total_increasing

Is that reasonable? If yes, I am happy to make the change and submit a pull request

frenck commented 1 year ago

Is that reasonable?

I don't know, as this is not the place to discuss architectural changes. My personal vote would not be to add such device classes. Mostly because we have no direct use for them, nor are we able to provide an UI advantage for them (in terms of conversions and things like that).

Please note, things do not have to have device classes. Nothing having a device class isn't bad or wrong. As a matter of fact, most entities don't have device classes.

Somehow, I feel like there is an obsession/feeling that things need to be classified in a device class, which is not true at all.

alexhk90 commented 1 year ago

@frenck which device class would you suggest we use to keep track of the current daily costs of gas and electricity for use with the energy dashboard?

None. Please note, device classes are mandatory.

Is there a missing word here? Should this read "device classes are not mandatory"? If so then maybe a simple solution for the energy dashboard "Use an entity tracking the total costs" entity could be to not have a device class at all (instead of "MONETARY")?

killarema commented 1 year ago

No не за деньги

frenck commented 1 year ago

Is there a missing word here? Should this read "device classes are not mandatory"?

Correct, sorry about that, will edit the above message.

If so then maybe a simple solution for the energy dashboard "Use an entity tracking the total costs" entity could be to not have a device class at all (instead of "MONETARY")?

It was a typo, as per above. The entities tracking a net total monetary value should have the monetary device class.

alexhk90 commented 1 year ago

@alexhk90 I'm using that same plugin, so I've been battling the same issues.

I've changed my local version of the plugin to use device class monetary and state class total for the daily totals, which I think is correct based on my discussions with @frenck above. I think this is also working correctly in the energy dashboard, although I'm leaving it for a few days to be sure - I need to see what happens when they reset to 0 at the start of the new day.

I've left the unit costs and standing charge as device class monetary, which I now think is incorrect based upon the above. I think these should either be device class None, or we need a new device class (which I proposed above) to cover these.

Did setting device class monetary and state class total work for you? I tried changing it (the daily cost entity) to this yesterday and the energy dashboard is now showing a negative cost for me.

Is there a missing word here? Should this read "device classes are not mandatory"?

Correct, sorry about that, will edit the above message.

If so then maybe a simple solution for the energy dashboard "Use an entity tracking the total costs" entity could be to not have a device class at all (instead of "MONETARY")?

It was a typo, as per above. The entities tracking a net total monetary value should have the monetary device class.

Thanks. Do you know what the state class should be for these "MONETARY" device class entities for the energy dashboard ("Use an entity tracking the total costs")? It works well with "TOTAL_INCREASING" (but has the warnings in the log, which is why I created this issue) but not with "TOTAL" (shows negative cost figures in the energy dashboard). Or is this a different question on the way the energy dashboard works and I should create a separate issue for this?

frenck commented 1 year ago

(but has the warnings in the log, which is why I created this issue) but not with "TOTAL" (shows negative cost figures in the energy dashboard)

If that happens, it means the sensor source resets it balance, however, it didn't tell HA about it (thus it considers it a drop, as it is a net state class). The sensor is missing the last_reset property.

../Frenck

mmillmor commented 1 year ago

I don't know, as this is not the place to discuss architectural changes. My personal vote would not be to add such device classes. Mostly because we have no direct use for them, nor are we able to provide an UI advantage for them (in terms of conversions and things like that).

There are use cases that require this. I have a dashboard which uses a statistic card to show the amount spent on energy each day;

This is a key thing to track for energy usage when you have a variable energy rate. Simply knowing the total kWh consumed (as the energy dashboard shows) is not particularly informative because it doesn't show how much of your energy consumption you have managed to move to greener times of day. The amount of money spent is a good way of measuring that because it combines energy used with cost per kWh at the time you used it, which is in turn a measure of how green it is to consume energy at that time (lower cost = more green).

I am only able to build that bar chart because I have a sensor of device_class=MONETARY and state_class=TOTAL_INCREASING. That produces the statistics required to be able to display that data on a dashboard. Without the state class, it can't be added like that

frenck commented 1 year ago

I am only able to build that bar chart because I have a sensor of device_class=MONETARY and state_class=TOTAL_INCREASING. That produces the statistics required to be able to display that data on a dashboard.

Seems odd, total_increasing is meant for things like electricity meters. In this case, you're resetting daily, which means it should be using a last_reset to indicate to Home Assistant when it resetted in combination with the total state class.

../Frenck

mmillmor commented 1 year ago

That may be the case, but it doesn't change the underlying use case - there is a need to have TOTAL and/or TOTAL_INCREASING against things that record money. If you can confirm that a device_class of MONETARY will never have that, I'll add a new device class. It'll be great to have your confirmation of that in here so that I can point to a voice of authority when submitting the pull request.

frenck commented 1 year ago

there is a need to have TOTAL and/or TOTAL_INCREASING against things that record money.

I beg the differ, as pointed out above, you need to use total and set the proper reset time, as it is a daily resetting sensor for your case.

I'll add a new device class.

I don't agree to that, which I already pointed out in the conversation above.

mmillmor commented 1 year ago

TOTAL works well for external systems that provide a reset time. But some don't, and the TOTAL_INCREASING device class takes care of noticing that the value has gone back down and assumes it was reset. The documentation explicitly notes this;

"Similar to total, with the restriction that the state represents a monotonically increasing positive total which periodically restarts counting from 0, e.g. a daily amount of consumed gas, weekly water consumption or lifetime energy consumption. Statistics of the accumulated growth of the sensor's value since it was first added is updated every 5 minutes. A decreasing value is interpreted as the start of a new meter cycle or the replacement of the meter."

The only difference between TOTAL and TOTAL_INCREASING is that TOTAL_INCREASING always goes up and takes care of tracking the reset time when the integration is unable to provide it. This fits the use case of tracking the money spent in a time period perfectly, and that is (typically) only increasing. However, if you are saying that monetary and TOTAL_INCREASING cannot be compatible, the only solution is a new device class to track money that is allowed to be TOTAL_INCREASING. Please help us all to do the right thing for the environment by allowing proper tracking of energy use by either removing the warning for monetary, or say you are happy for a new device class.

metalblue commented 1 year ago

Did setting device class monetary and state class total work for you? I tried changing it (the daily cost entity) to this yesterday and the energy dashboard is now showing a negative cost for me.

No. It worked for the first day, but didn't handle the reset to 0 correctly. I too have a negative cost in the energy dash board.

The only difference between TOTAL and TOTAL_INCREASING is that TOTAL_INCREASING always goes up and takes care of tracking the reset time when the integration is unable to provide it. This fits the use case of tracking the money spent in a time period perfectly, and that is (typically) only increasing. However, if you are saying that monetary and TOTAL_INCREASING cannot be compatible, the only solution is a new device class to track money that is allowed to be TOTAL_INCREASING. Please help us all to do the right thing for the environment by allowing proper tracking of energy use by either removing the warning for monetary, or say you are happy for a new device class.

+1

frenck commented 1 year ago

However, if you are saying that monetary and TOTAL_INCREASING cannot be compatible

Correct, as stated many times before now above, that total_increasing is designed for handling meters originally. As monetary handles a net total (a balance), the total is the matching one.

The use of total_increasing has been discussed with Core members when the validation was added (as it was missing) and has been declined for the reasons I've mentioned now a couple of times. It is a net value. If it is resetting at some point, a last_reset can be provided.

the only solution is a new device class to track money that is allowed to be TOTAL_INCREASING

No, it is not. As per all of the above. You are missing setting a last_reset for your use case.

Please help us all to do the right thing for the environment by allowing proper tracking of energy use by either removing the warning for monetary, or say you are happy for a new device class.

Neither of those, as pointed out a couple of times now.

I feel like you make me run in circles, and I'm going to halt that at this point.

../Frenck

alexhk90 commented 1 year ago

Thanks for all the discussion. If I'm understanding correctly, we should use device class "MONETARY" with state class "TOTAL" for the for the energy dashboard cost entity ("Use an entity tracking the total costs"), and ensure this entity provides the "last_reset" property?

Can this be configured with the "template" sensor? I couldn't find any reference to "last_reset" in the documentation: https://www.home-assistant.io/integrations/template

frenck commented 1 year ago

Can this be configured with the "template" sensor?

Sure, add it as an attribute to the sensor.

../Frenck

alexhk90 commented 1 year ago

Can this be configured with the "template" sensor?

Sure, add it as an attribute to the sensor.

../Frenck

Thanks. I've added it like this for now:

attributes:
        last_reset: "{{ today_at('00:00') }}" 

There is probably a better/more dynamic way of doing it (i.e. actually checking when the value went back to zero), but I'm hoping this will cause the required behaviour of the energy dashboard recognising the reset at midnight each day.

It looks correct in that there is now a "last_reset" attribute set to midnight today for the entity under "States" in "Developer Tools":

state_class: total
last_reset: 2023-05-08 00:00:00+01:00
unit_of_measurement: GBP
device_class: monetary
icon: mdi:cash
friendly_name: Smart Meter Electricity: Cost (Today)

I'll monitor over the next couple of days to check if this fixes the negative cost values on the energy dashboard.

mmillmor commented 1 year ago

That certainly helps with some uses cases. Thanks for sharing both of you. I'd be happy to help with updating documentation to make that easier for others to discover if that would be welcome?

alexhk90 commented 1 year ago

That certainly helps with some uses cases. Thanks for sharing both of you. I'd be happy to help with updating documentation to make that easier for others to discover if that would be welcome?

Thanks; agreed that more detailed documentation would be helpful on this, as it wasn't obvious (at least to me) to work out how to resolve this switching from an entity with monetary device class and total_increasing state class.

To confirm, for the energy dashboard "Configure grid consumption" "Use an entity tracking total costs", I have been using the following entity (monetary device class, total state class, with last_reset property/attribute) for a couple of days now and it has been working well (expected - non-negative - cost figures on the energy dashboard, no warnings in the log):

template:
  sensor:
    - name: "Smart Meter Electricity: Cost (Today)"
      unique_id: smart_meter_electricity_cost_today
      device_class: "monetary"
      state_class: "total" # requires "last_reset" for use to track costs in energy dashboard
      unit_of_measurement: "GBP"
      icon: mdi:cash
      state: "{{ (
        states('sensor.smart_meter_electricity_import_today') | float
        * states('sensor.smart_meter_electricity_import_unit_rate') | float
        + states('sensor.smart_meter_electricity_import_standing_charge') | float
        ) | round(2) }}"
      attributes:
        last_reset: "{{ today_at('00:00') }}" # required for use as "total" in energy dashboard