zha: additional sensors by dmulcahey · Pull Request #12515 · home-assistant/core

dmulcahey · 2018-02-19T14:17:55Z

Description:

Updates ZHA with the following changes:

Adds the groundwork for several new sensor types to be supported by ZHA:

Metering (energy consumption for connected devices)
PowerConfiguration (battery sensors - currently limited to Centralite devices)
IlluminanceMeasurement
ElectricalMeasurement

houndci-bot · 2018-02-19T14:18:06Z

blank line at end of file
blank line contains whitespace

houndci-bot · 2018-02-19T14:18:06Z

trailing whitespace

houndci-bot · 2018-02-19T14:18:07Z

blank line at end of file
blank line contains whitespace

houndci-bot · 2018-02-19T14:18:07Z

continuation line under-indented for visual indent

houndci-bot · 2018-02-19T14:18:07Z

line too long (80 > 79 characters)

houndci-bot · 2018-02-19T14:18:08Z

missing whitespace after ':'

houndci-bot · 2018-02-19T14:18:08Z

missing whitespace after ':'

houndci-bot · 2018-02-19T14:18:08Z

missing whitespace after ':'

houndci-bot · 2018-02-19T14:18:08Z

missing whitespace after ':'

houndci-bot · 2018-02-19T14:18:08Z

missing whitespace after ':'

houndci-bot · 2018-02-19T14:23:53Z

no newline at end of file

houndci-bot · 2018-02-19T14:23:54Z

no newline at end of file

houndci-bot · 2018-02-19T14:23:54Z

block comment should start with '# '

houndci-bot · 2018-02-19T14:23:54Z

block comment should start with '# '

houndci-bot · 2018-02-19T14:23:54Z

continuation line under-indented for visual indent

houndci-bot · 2018-02-19T14:23:54Z

blank line contains whitespace

rcloran

This mostly looks good to me.

I'd prefer to handle the device quirks (Centralite-specific battery sensor) in zigpy, where we can expose a consistent interface to hass. I'd be OKish with this going in and changing to use the normal BatterySensor when zigpy quirks handling is ready, though.

Please send in PR to zigpy for the battery size enum, happy to add that in and bump the release for you.

rcloran · 2018-02-20T04:36:04Z

I assume this kind of device is normally wired, and so polling it is OK? Have you tried configuring reporting on your device? That'd be a better option.

Yep, they're wired devices and the electrical measurement cluster attributes don't support reporting according to the ZCL docs.

rcloran · 2018-02-20T04:49:52Z

This should go into zigpy as an enum, as it is part of the standard.

I'll submit a PR to zigpy for this

rcloran · 2018-02-20T04:54:09Z

I don't think this should live in Home Assistant.

"I'd be OKish with this going in and changing to use the normal BatterySensor when zigpy quirks handling is ready, though."

Will leave here until quirks mode is ready.

ryanwinter · 2018-04-03T04:24:33Z

Is there still plans to push this out?

dshokouhi · 2018-04-03T14:18:36Z

I agree this PR has some very critical functionality. Battery reporting for a contact sensor is really important. Not knowing when your device battery is going to run out poses a security risk for any alarm.

rcloran · 2018-04-04T02:52:41Z

I've just force pushed to https://github.com/dmulcahey/home-assistant/tree/dfm/zha-sensors to get travis to re-run.

Adminiuga · 2018-04-04T13:05:51Z

should @asyncio.coroutine/yeiled from" be updated to use "async def/await" syntax?

dshokouhi · 2018-04-23T15:25:52Z

Any chance we can at least split out the Battery aspect of this PR and merge it in? Many users are waiting for this functionality so we can know when a zha battery device is going to die. I would love to see all of it get merged if possible.

dshokouhi · 2018-05-01T15:04:53Z

@dmulcahey and @rcloran I wanted to provide some feedback on this PR as yesterday I had time to pull in this PR into my own environment to test things out. I can confirm the energy reporting is working as expected. I am not so certain about the battery reporting. As of this morning some of my Iris Contact Sensors had checked in but the battery level reported is 100% which can't be right. On top of that it seems I got duplicate battery entity ID's as well. Looking at the code in sensor/zha.py I can see the values being interpreted to 100% so it seems like we are getting the values but maybe we need to update the percentage reading (line 187-200)? When I switched from SmartThings most my devices were around 60-70% battery life so I don't know how accurate the 100% reading really is.

ryanwinter · 2018-05-01T16:42:51Z

I might give this a whirl if I have time this week. I only have smart thing devices though and @dshokouhi looks like he already confirmed this.

balloob · 2018-05-01T20:23:21Z

This PR is a great example why one should limit PRs to a single feature. We could have merged the working parts already.

balloob · 2018-05-01T20:23:56Z

What needs to be done to get this finished and who is going to do this? Would someone want to open smaller PRs containing a single feature from this PR ?

dmulcahey · 2018-05-01T21:09:42Z

@balloob from my perspective it all works. It is my understanding that others are expecting the battery reporting to work for the rebranded smart things centralite sensors. That's where the concerns seem to be on battery reporting not working. I am assuming that they modified the manufacturer check to reference smartthings instead of centralite. I could be completely wrong but that is my guess. This is tested and it is currently working for stock centralite sensors and I have been running it locally since I opened the PR. I will gladly break this up into multiple PR's if it makes things easier. Sorry for causing a stir with my first PR!

dshokouhi · 2018-05-01T21:19:17Z

@dmulcahey the device I used this PR on is a Iris Contact Sensor second generation. I believe that is the same one you used correct? For Energy Reporting I am using a Iris Smart Plug second generation. I do agree that I have seen others mention the SmartThings rebranded ones but personally I did not modify your PR at all in that regard so to me it is working.

The only real issue I saw was that duplicate battery sensors got added as you can see in my screenshot above.

ryanwinter · 2018-05-01T23:22:37Z

Whats the path for supported other sensor (such as Smartthings) once this is merged.

Is it just keep extending the zha.py and add additional derived GenericBatterySensor classes?

Or do we want to wait to add this functionality into zigpy once the quirks handling is complete?

dmulcahey · 2018-05-01T23:30:26Z

@ryanwinter Quirks handling should help this... but I think we’ll still have a bunch of custom sensor classes just in a different format. @rcloran, any thoughts on this?

MartinHjelmare · 2018-05-06T16:10:14Z

+    def state(self):
+        """Return the state of the entity."""
+        if self._state == 'unknown':
+            return 'unknown'


Return None to represent unknown state.

Why are multiple input values mapped to the same output percentage?
28, 27 and 26 are 100%
Why not 28=100, 27=97, 26=93? (or something like that)

@ryanwinter please move your comment to a general PR comment. I don't see that it's related to my comment or this code line.

MartinHjelmare · 2018-05-06T16:10:46Z

+        elif self._state > self.maxVolts:
+            self._state = self.maxVolts
+
+        return self.values.get(self._state, 'unknown')


MartinHjelmare · 2018-05-06T16:11:07Z

+    def state(self):
+        """Return the state of the entity."""
+        if self._state == 'unknown':
+            return 'unknown'


MartinHjelmare · 2018-05-06T16:11:22Z

+    def state(self):
+        """Return the state of the entity."""
+        if self._state == 'unknown':
+            return 'unknown'


MartinHjelmare · 2018-05-06T16:12:26Z

+    def state(self):
+        """Return the state of the entity."""
+        if self._state == 'unknown':
+            return 'unknown'


Return None.

dmulcahey · 2018-05-12T12:13:51Z

@ryanwinter I pulled that map directly from a smartthings device handler. The code can be found here: https://github.com/varzac/SmartThingsPublic/blob/97bfe61baa464cdac8894a566f19bcd6cc3c68c7/devicetypes/smartthings/smartsense-motion-sensor.src/smartsense-motion-sensor.groovy

dshokouhi · 2018-05-13T20:08:36Z

Hey all I think this PR needs to be rebased or updated now as it has conflicts and parts of it is no longer working for me. After updating to 0.69 energy reporting is no longer working. I made sure to place back the files that got overwritten. Battery reporting is still working so parts of it seems to be ok.

dmulcahey · 2018-05-13T20:45:33Z

@dshokouhi I’m going to close this. I’ll open smaller independent ones.

dshokouhi · 2018-05-13T20:58:27Z

Happy to help test anything if you need it!

dmulcahey · 2018-05-20T14:37:23Z

@dshokouhi #14563 #14562 #14561 have been opened. @MartinHjelmare I addressed the relevant comments in those PR's. @balloob sorry again for all of the churn that this caused!

dshokouhi · 2018-05-20T17:24:47Z

thanks @dmulcahey !! Will check out the PR's soon :) The battery reporting has actually been working pretty well so far too, my centralite devices are definitely reporting. Thanks for much for submitting the PR's :)

homeassistant added cla-signed integration: zha platform: sensor.zha labels Feb 19, 2018

houndci-bot reviewed Feb 19, 2018

View reviewed changes

homeassistant added the cla-signed label Feb 19, 2018

houndci-bot reviewed Feb 19, 2018

View reviewed changes

rcloran reviewed Feb 20, 2018

View reviewed changes

dmulcahey and others added 6 commits April 3, 2018 19:51

zha: additional sensors

c8a6d38

lint

ff5d39c

lint

4205be9

remove logging

5b52890

add sensors to SINGLE_CLUSTER_DEVICE_CLASS mapping

b7bb9d1

removed the skipping of manufacturer specific profiles

3b1d26f

rcloran force-pushed the dfm/zha-sensors branch from 26e8df9 to 3b1d26f Compare April 4, 2018 02:52

homeassistant added the cla-signed label Apr 4, 2018

Lint: docstring formatting

34d267e

MartinHjelmare reviewed May 6, 2018

View reviewed changes

dmulcahey closed this May 13, 2018

Adminiuga mentioned this pull request Sep 3, 2018

Zha battery #16384

Closed

8 tasks

home-assistant locked and limited conversation to collaborators Sep 5, 2018

dmulcahey deleted the dfm/zha-sensors branch December 26, 2018 12:12

ghost removed the platform: sensor.zha label Mar 21, 2019

Uh oh!

Conversation

dmulcahey commented Feb 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rcloran left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanwinter commented Apr 3, 2018

Uh oh!

dshokouhi commented Apr 3, 2018

Uh oh!

rcloran commented Apr 4, 2018

Uh oh!

Adminiuga commented Apr 4, 2018

Uh oh!

dshokouhi commented Apr 23, 2018

Uh oh!

dshokouhi commented May 1, 2018

Uh oh!

ryanwinter commented May 1, 2018

Uh oh!

balloob commented May 1, 2018

Uh oh!

balloob commented May 1, 2018

Uh oh!

dmulcahey commented May 1, 2018

Uh oh!

dshokouhi commented May 1, 2018

Uh oh!

ryanwinter commented May 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmulcahey commented May 1, 2018

Uh oh!

dmulcahey commented Feb 19, 2018 •

edited

Loading

ryanwinter commented May 1, 2018 •

edited

Loading