Add Water device class to Overkiz #87139

tetienne · 2023-02-02T08:16:41Z

Breaking change

Proposed change

Since the introduction of the device class water, we didn’t update the Overkiz integration to support it. It’s now the case, and found two sensors eligible.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes Water Consumption Energy Settings - Unable to select any entity #83736
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant · 2023-02-02T08:16:49Z

Hey there @iMicknl, @vlebourl, @nyroDev, mind taking a look at this pull request as it has been labeled with an integration (overkiz) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of overkiz 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 overkiz Removes the current integration label and assignees on the issue, add the integration domain after the command.

iMicknl

Good find!

epenet · 2023-02-02T08:21:23Z

homeassistant/components/overkiz/sensor.py

@@ -92,7 +92,7 @@ class OverkizSensorDescription(SensorEntityDescription):
        name="Water volume estimation at 40 °C",
        icon="mdi:water",
        native_unit_of_measurement=UnitOfVolume.LITERS,
-        device_class=SensorDeviceClass.VOLUME,
+        device_class=SensorDeviceClass.WATER,


I am not sure about this one.
WATER device class relates to water meters - not to a volume in a tank.

Edit: if it really is a consumption/water meter then the state class also needs to be adjusted.

You are correct, this actually is a volume, not the consumption.

According to the end user, it’s look like we can read it as a comsumption device: #83736

@MassimoL06 Can you confirm?

I think he has the other sensor device? Or am I missing something.

epenet · 2023-02-02T08:22:09Z

homeassistant/components/overkiz/sensor.py

@@ -101,7 +101,7 @@ class OverkizSensorDescription(SensorEntityDescription):
        name="Water consumption",
        icon="mdi:water",
        native_unit_of_measurement=UnitOfVolume.LITERS,
-        device_class=SensorDeviceClass.VOLUME,
+        device_class=SensorDeviceClass.WATER,


If this one is a water meter, then the state_class should be TOTAL or TOTAL_INCREASING.

@tetienne this would be a good one to check for other meters (like electricity) as well, not sure if everything is mapped correctly.

@epenet Since it was introduced, I’m confused with TOTAL and TOTAL_INCREASING. I read many time the definition, but still don’t get it.

TOTAL goes up AND down
TOTAL_INCREASING only goes up (automatically detects reset)

Meters should probably be always TOTAL_INCREASING.

Note: for me the confusion is between TOTAL and MEASUREMENT...

The name TOTAL_INCREASING is very confusing, we should have named it TOTAL_AUTO_RESET or something like that.
@tetienne did you read this part too: https://developers.home-assistant.io/docs/core/entity/sensor/#how-to-choose-state_class-and-last_reset ?

Thx, this is indeed more clear with these examples. I asked to @gbuico to explain me what’s this device. But it looks like that’s not something really clear...

Can it be that the integration is just swapping the name of the entities??
If I check the entity sensor.dhwp_actuator_water_volume_estimation_at_40_degc , this is really an "always increasing value (i.e. I have at the moment 42428 liters)
At the same time, looking at the history of sensor.dhwp_actuator_water_consumption I see values going up and done as from the chart I sent you @tetienne and this is more likely to be the estimation of remaining water at 40 degrees.

I also had a further thought on the readings of that variable and I may have found an explanation.

Problem: in my graph I see the water consumpion value float between 60 and 140 L while me heater is 65 L

My new explanation : Since my middel water temperature is around 65° it is reasonable that a 65 L heater can provide 140 L of water at 40°

Bottom line, I think the measured values are correct... there is just a swap of names

According to my own DHWP, water consumption seem to be a volume that goes down as it's "consumed" then up as it gets replenished.

In the snapshot, the steep slopes are actual hot water consumption (shower, bath, ...), the slow ones are just the water cooling down on its own inside the tank! To me this means the name of the entity is actually missleading, and should be something like available hot water rather than water consumption.

What about the new device class VOLUME_STORAGE? It seems more accurate in this case.

@tetienne VOLUME_STORAGE sounds fine by me! Shall we make this change and see if we can merge this one in?

MassimoL06 · 2023-02-04T18:53:33Z

Hello,
when I force "device_class: water" and "state_class: total_increasing" to my water sensor entity from the Developer Tools TAB like this, I am able to add this entity to the energy dashboard.

Hope it helps.

vlebourl · 2023-02-28T08:16:20Z

I had a closer look at both dhwp_actuator_water_consumption and dhwp_actuator_water_volume_estimation_at_40_degc. Here you have both on the same timeline

From this, I would argue that

Water volume estimation at 40 °C is indeed a water consumption, altghough it's an estimate rather than a direct measurement, and its state_class should be TOTAL_INCREASING
Water consumption is not a water consumption and should stay a VOLUME device class (the slow decreases correspond to cooling inside the tank, not consumption).

epenet · 2023-03-03T07:53:40Z

Please also take a look at #88312 which adds a new device class

MassimoL06 · 2023-03-03T16:07:08Z

Hello @vlebourl
I agree with your comment regarding dhwp_actuator_water_consumption and dhwp_actuator_water_volume_estimation_at_40_degc.
Another concern regarding dhwp_actuator_water_volume_estimation_at_40_degc is that it has a maximum value of 65535, then retarts to 0. Is it possible to extend the range value by using a 32 bits variable ?

How do you create your ApexChart-Card ?
Regards

epenet

From what I gather from the discussion, I still don't think they should be set to WATER.
Maybe VOLUME is good, or maybe VOLUME_STORAGE would be better.
Either way, there still seems to be some confusion.
Is it even consistent across devices?

home-assistant · 2023-03-14T08:53:21Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

frenck · 2023-03-20T11:01:25Z

homeassistant/components/overkiz/sensor.py

@@ -92,7 +92,7 @@ class OverkizSensorDescription(SensorEntityDescription):
        name="Water volume estimation at 40 °C",
        icon="mdi:water",
        native_unit_of_measurement=UnitOfVolume.LITERS,
-        device_class=SensorDeviceClass.VOLUME,
+        device_class=SensorDeviceClass.WATER,


This is the device class available for this right now:

Suggested change

device_class=SensorDeviceClass.WATER,

device_class=SensorDeviceClass.VOLUME_STORAGE,

How that's a nice addition. Thx!

Co-authored-by: Franck Nijhof <[email protected]>

github-actions · 2023-08-02T13:06:53Z

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

Migrate from volume device class to water

8d9fa16

home-assistant bot added bugfix cla-signed integration: overkiz small-pr PRs with less than 30 lines. by-code-owner Quality Scale: No score labels Feb 2, 2023

iMicknl approved these changes Feb 2, 2023

View reviewed changes

tetienne mentioned this pull request Feb 2, 2023

Water Consumption Energy Settings - Unable to select any entity #83736

Closed

epenet requested changes Feb 2, 2023

View reviewed changes

Quentame assigned epenet Mar 13, 2023

epenet requested changes Mar 14, 2023

View reviewed changes

home-assistant bot marked this pull request as draft March 14, 2023 08:53

epenet mentioned this pull request Mar 20, 2023

Fix Overkiz wrong measurement warning #89948

Closed

20 tasks

frenck requested changes Mar 20, 2023

View reviewed changes

Update homeassistant/components/overkiz/sensor.py

8e0f4f3

Co-authored-by: Franck Nijhof <[email protected]>

github-actions bot added the stale label Aug 2, 2023

github-actions bot closed this Aug 9, 2023

github-actions bot locked and limited conversation to collaborators Aug 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Water device class to Overkiz #87139

Add Water device class to Overkiz #87139

tetienne commented Feb 2, 2023

home-assistant bot commented Feb 2, 2023

iMicknl left a comment

epenet Feb 2, 2023 •

edited

Loading

iMicknl Feb 2, 2023

tetienne Feb 2, 2023

iMicknl Feb 2, 2023

epenet Feb 2, 2023

iMicknl Feb 2, 2023

tetienne Feb 2, 2023

epenet Feb 2, 2023

emontnemery Feb 2, 2023

tetienne Feb 7, 2023

gbuico Feb 7, 2023

vlebourl Feb 27, 2023

tetienne Mar 20, 2023

iMicknl May 4, 2023

MassimoL06 commented Feb 4, 2023

vlebourl commented Feb 28, 2023

epenet commented Mar 3, 2023

MassimoL06 commented Mar 3, 2023

epenet left a comment

home-assistant bot commented Mar 14, 2023

frenck Mar 20, 2023

tetienne Mar 20, 2023

github-actions bot commented Aug 2, 2023

	device_class=SensorDeviceClass.WATER,
	device_class=SensorDeviceClass.VOLUME_STORAGE,

Add Water device class to Overkiz #87139

Add Water device class to Overkiz #87139

Conversation

tetienne commented Feb 2, 2023

Breaking change

Proposed change

Type of change

Additional information

Checklist

home-assistant bot commented Feb 2, 2023

iMicknl left a comment

Choose a reason for hiding this comment

epenet Feb 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MassimoL06 commented Feb 4, 2023

vlebourl commented Feb 28, 2023

epenet commented Mar 3, 2023

MassimoL06 commented Mar 3, 2023

epenet left a comment

Choose a reason for hiding this comment

home-assistant bot commented Mar 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 2, 2023

epenet Feb 2, 2023 •

edited

Loading