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.54k stars 30.72k forks source link

HomeWizard meter statistics are wrong format #104072

Closed DavidNordin closed 7 months ago

DavidNordin commented 11 months ago

The problem

The entities for statistics are wrong/undefined for use with home assistant energy.

The entities need to be defined correctly

What version of Home Assistant Core has the issue?

core-2023.11.2

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

HomeWizard integration

Link to integration documentation on our website

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

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 11 months ago

Hey there @dcsbl, mind taking a look at this issue as it has been labeled with an integration (homewizard) you are listed as a code owner for? Thanks!

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

(message by CodeOwnersMention)


homewizard documentation homewizard source (message by IssueLinks)

frenck commented 11 months ago

The entities for statistics are wrong/undefined for use with home assistant energy.

I'm actually actively working on HomeWizard myself, but I don't see that issue?

Could you clarify exactly what is incorrect?

Thanks! 👍

../Frenck

DCSBL commented 11 months ago

Can you tell us what is "wrong/undefined"? What do you expect and what is currently used?

DCSBL commented 11 months ago

@DavidNordin, is this the same issue you have? → https://github.com/home-assistant/core/issues/104082

DavidNordin commented 11 months ago

I'll get back to you on this more detailed but I short the statistics is undefined and doesn't work properly together with the energy dashboard for example.

Den tors 16 nov. 2023 17:06Duco Sebel @.***> skrev:

@DavidNordin https://github.com/DavidNordin, is this the same issue you have? → #104082 https://github.com/home-assistant/core/issues/104082

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/issues/104072#issuecomment-1814760271, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMLMIEMRRW6QY3DZ4ATOW5TYEY2Z5AVCNFSM6AAAAAA7OBEDROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJUG43DAMRXGE . You are receiving this because you were mentioned.Message ID: @.***>

frenck commented 11 months ago

I'll get back to you on this more detailed but I short the statistics is undefined and doesn't work properly together with the energy dashboard for example.

They are defined in source, hence us asking for more details :)

DavidNordin commented 11 months ago

Right. I'm sorry for being so vague initially.

I only got errors adding the metrics in the energy-setup.

"Statistics not defined" and something about state class.

I'll try to replicate when I get homw

Den tors 16 nov. 2023 17:27Franck Nijhof @.***> skrev:

I'll get back to you on this more detailed but I short the statistics is undefined and doesn't work properly together with the energy dashboard for example.

They are defined in source, hence us asking for more details :)

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/issues/104072#issuecomment-1814796202, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMLMIEONQXDSV7LH65USLILYEY5HHAVCNFSM6AAAAAA7OBEDROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJUG44TMMRQGI . You are receiving this because you were mentioned.Message ID: @.***>

davidsmoker commented 10 months ago

hello,

normal we don't see the export from the wifi kwh meter (sdm230 wifi)

Total energy export : This entity is unavailable.

thanks

DCSBL commented 10 months ago

@DavidNordin Have you been able to reproduce your issue?

@davidsmoker This is maybe because the export sensor is disabled when the value is '0'. We assume when the meter has not seen any export value the sensor itself is not useful. You can enable the sensor in in its settings.

HansMerken commented 9 months ago

I have the same issue. I want to see the average monthly peak in my Statistics graph Card. The sensor is not listed in the dropdown - and I get a link to: https://www.home-assistant.io/more-info/statistics/ The peak_demand_current_month entity is enabled, and in another card I can get the value, and I can see the history. I tried adding the sensor via YAML, but it does not appear on the card.

DCSBL commented 9 months ago

Are you the first to mention we are talking about the "Average monthly peak" sensor? If that is the case I finally understand what you guys actually mean.

It seems this sensor does not have state_class set. I am not sure if you can set that via customisation or something. But I will pick this up ASAP.

https://github.com/home-assistant/core/blob/6bc36666b1551c9fdc567b02d122398fd687e787/homeassistant/components/homewizard/sensor.py#L379-L394

HansMerken commented 9 months ago

I confirm that, for me, it is about the average monthly peak. I cannot find any way to add state_class: measurement.

Thank you for the super fast reply!

frenck commented 9 months ago

average monthly peak.

Those are calculated averages unsuitable for assigned state classes or long-term statistics.

Them not having a state class is thus correct.

measurement would be incorrect, as documented in our developer documentation as well:

The state represents a measurement in present time, not a historical aggregation such as statistics or a prediction of the future.

These are statistical averages and not measurements in the current present time.

../Frenck

DCSBL commented 9 months ago

Just to confirm, because it was quite some time ago this sensor added; @HansMerken, can you send a screenshot of some historical data from "Active average demand" and "Peak demand current month"?

Active average demand is a value directly extracted from the smart meter which very maybe can be seen as a statistical value. I can see some value to see the lowest, average and highest peaks from this sensor.

HansMerken commented 9 months ago

Wow, you guys are really on top of things. @frenck I am so sorry, but I am very new to all this, and I cannot understand anything of what you have written. Your reply raises more questions than it solves. Could you please answer the following questions:

thank you for your valuable replies.

@DCSBL image History as requested.

image The location where I would like to use the value.

image If you combine the peak value with the active power, you can see when the value increases, and we would be able to raise an alarm.

DCSBL commented 9 months ago

Where are the values calcu...

All of these questions belong on Discord and/or the forum. Feel free to ask them there. GitHub issues is used track issues, in this case for Home Assistant Core.

https://www.home-assistant.io/help/


Thanks for the screenshots, @frenck What do you think?

frenck commented 9 months ago

I'm not sure why it matter what I think. These are averages 🤷‍♂️

They are, documented, not allowed to have a state class.

HansMerken commented 9 months ago

Could you please point me to this documentation, because I am having trouble locating it?

The documentation from the integration, which you can find if you click on the following Uniform Resource Locator (URL) )(https://www.home-assistant.io/integrations/homewizard) says the following: "the peak measured this month". I would like to see the peak measured this month in a graph. Not the average. It is not an average.

frenck commented 9 months ago

Could you please point me to this documentation, because I am having trouble locating it?

Please note, this is a discussion about the implementation.

Developer documentation about the sensor entities can be found here: https://developers.home-assistant.io/docs/core/entity/sensor

From the non-development perspective/user perspective the answer would be: Home Assistant doesn't support that. If you want to do such things, you could maybe build your own template sensor or use the statistical helper.

"the peak measured this month". I would like to see the peak measured this month in a graph. Not the average. It is not an average.

A peak is a statistical value, not a current value. Thus not eligible for a state class.

HansMerken commented 9 months ago

Thank you for the link. It appears you are right! It says right here in the definition of state_class: Type of state. If not None, the sensor is assumed to be numerical and will be displayed as a line-chart in the frontend instead of as discrete values. Therefore, the state_class should not be none, because it is numerical.

Looking further below, I found Home Assistant has support for storing sensors as long-term statistics if the entity has the right properties. So it is a matter of setting the right properties. In this case: Entities representing a total amount Examples ...

_> The sensor's value may reset to 0, and its value can only increase: state class totalincreasing. Examples: energy consumption aligned with a billing cycle, e.g. monthly, an energy meter resetting to 0

Could you please point me to the part where it says that it is forbidden, because I cannot find that part.

frenck commented 9 months ago

It is quoted above and also on that page.

CleanShot 2024-01-16 at 20 55 57@2x

Meaning neither of the available state classes is a match.

HansMerken commented 9 months ago

Thank you for pointing this out to me. The conclusion is that you think that the value is non-numerical. In the meantime I found a workaround.

Kind regards.

frenck commented 9 months ago

The conclusion is that you think that the value is non-numerical.

No, that is not the conclusion or what I think. Please read my previous posts again, as it ain't a current value.

HansMerken commented 9 months ago

I have read it again.

Meaning neither of the available state classes is a match.

This means that according to @frenck the value for state_class should be "None". image

If the value is "None", it is considered to be non-numerical.

frenck commented 9 months ago

From the state class perspective, yes; however, this is not the only variable used to determine if a sensor is numeric or not. You are mixing up two different things right now; state class provides a hint for the long-term storage + is a possible factor in determining whether a sensor is numeric. Other factors in are, for example, a device class.

That said, a numeric sensor doesn't automatically support long-term statistic (state_class), a sensor with a state class set, however, is always considered numeric.

The discussion was about the long-term statistics, which this sensor isn't viable for, as it ain't a current value.

DCSBL commented 7 months ago

Closing this issue as wont-fix

DCSBL commented 7 months ago

@home-assistant close