Skip to content

Add basic support for native Hue sensors#22598

Merged
balloob merged 28 commits into
home-assistant:devfrom
mitchellrj:add-basic-hue-sensor-support
Apr 18, 2019
Merged

Add basic support for native Hue sensors#22598
balloob merged 28 commits into
home-assistant:devfrom
mitchellrj:add-basic-hue-sensor-support

Conversation

@mitchellrj
Copy link
Copy Markdown
Contributor

@mitchellrj mitchellrj commented Mar 31, 2019

Description:

This PR adds support for Hue motion sensors and their integrated light-level & temperature sensors.

In future, this could be expanded to include support for Hue switches and artificial sensors.

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#9084

Example entry for configuration.yaml (if applicable):

unchanged

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

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

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

  • New files were added to .coveragerc.

Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/bridge.py Outdated
Comment thread homeassistant/components/hue/__init__.py Outdated
@ghost ghost added the in progress label Mar 31, 2019
@mitchellrj mitchellrj force-pushed the add-basic-hue-sensor-support branch 3 times, most recently from 43346a1 to e1cab48 Compare March 31, 2019 19:49
Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will require tests to be written.

Comment thread homeassistant/components/hue/__init__.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py
Comment thread homeassistant/components/hue/sensor.py
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
Comment thread homeassistant/components/hue/sensor.py Outdated
@mitchellrj
Copy link
Copy Markdown
Contributor Author

I'll address these shortly, thanks for the feedback. Some of these corrections are an upshot of me copying code from the light module and not understanding broader codebase guidelines.

Comment thread homeassistant/components/hue/__init__.py Outdated
Comment thread homeassistant/components/hue/__init__.py Outdated
@mitchellrj mitchellrj force-pushed the add-basic-hue-sensor-support branch from fc4f92b to 74fb6f1 Compare April 17, 2019 16:36
@mitchellrj
Copy link
Copy Markdown
Contributor Author

Pushed a rebase to trigger tests again - last run failed due to an unstable test in another component.

@balloob, how do you feel about this now? Personally, I think it's in pretty good shape and much better than the original request thanks to people's feedback. @MartinHjelmare makes a good point about handling updates for newly added sensors, but I hope my response makes sense and that others agree with that.

Comment thread homeassistant/components/hue/sensor_base.py Outdated
Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 🎉 🎉 🎉

@balloob balloob added this to the 0.92.0 milestone Apr 17, 2019
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2019

Codecov Report

Merging #22598 into dev will increase coverage by <.01%.
The diff coverage is 96.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev   #22598      +/-   ##
==========================================
+ Coverage   94.19%    94.2%   +<.01%     
==========================================
  Files         453      456       +3     
  Lines       36917    37084     +167     
==========================================
+ Hits        34775    34936     +161     
- Misses       2142     2148       +6
Impacted Files Coverage Δ
homeassistant/components/hue/binary_sensor.py 100% <100%> (ø)
homeassistant/components/hue/bridge.py 74.07% <100%> (+0.99%) ⬆️
homeassistant/components/hue/sensor.py 100% <100%> (ø)
homeassistant/components/hue/sensor_base.py 95.34% <95.34%> (ø)
homeassistant/components/uk_transport/sensor.py 93.43% <0%> (-0.73%) ⬇️
...ssistant/components/islamic_prayer_times/sensor.py 95.74% <0%> (+1.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b33d95...9958a6f. Read the comment docs.

@balloob balloob merged commit 474ac8b into home-assistant:dev Apr 18, 2019
balloob pushed a commit that referenced this pull request Apr 18, 2019
* Add basic support for native Hue sensors

* Update coveragerc

* Simplify attributes

* Remove config option

* Refactor and document device-ness and update mechanism

* Entity docstrings

* Remove lingering config for sensors

* Whitespace

* Remove redundant entity ID generation and hass assignment.

* More meaningful variable name.

* Add new 'not-darkness' pseudo-sensor.

* Refactor sensors into separate binary, non-binary, and shared modules.

* formatting

* make linter happy.

* Refactor again, fix update mechanism, and address comments.

* Remove unnecessary assignment

* Small fixes.

* docstring

* Another refactor: only call API once and make testing easier

* Tests & test fixes

* Flake & lint

* Use gather and dispatcher

* Remove unnecessary whitespace change.

* Move component related stuff out of the shared module

* Remove unused remnant of failed approach.

* Increase test coverage

* Don't get too upset if we're already trying to update an entity before it has finished adding

* relative imports
@jimbob1001
Copy link
Copy Markdown

Hi, Is there a particular reason the motion sensor binary_sensors have device_class "presence" rather than "motion"?
Cheers,
James

@mitchellrj
Copy link
Copy Markdown
Contributor Author

mitchellrj commented Apr 18, 2019 via email

@mitchellrj
Copy link
Copy Markdown
Contributor Author

#23208 for actual naming

@mvdwetering
Copy link
Copy Markdown
Contributor

mvdwetering commented Apr 22, 2019

I was looking through the code to see how it compared to my personal hacks to get the Hue sensors work with hass and noticed 2 small issues.

According to https://www.home-assistant.io/components/sensor/ the light unit should be lx (for lux) or lm (for lumen), however current implementation seems to use 'Lux'

Light level in the Hue API is not in lux, it is in another unit. So in my custom code I had the following to convert to lux. It seemed right, but I had some issues getting it working so it may have issues return round(10 ** ((int(self.sensor.state['lightlevel'])-1)/10000.0))
This is the info from the Hue developer documentation:

Light level in 10000 log10 (lux) +1 measured by sensor. Logarithm scale used because the human eye adjusts to light levels and small changes at low lux levels are more noticeable than at high lux levels.

@mitchellrj
Copy link
Copy Markdown
Contributor Author

Excellent info, thanks @mvdwetering! I'll get this updated right away.

@mitchellrj
Copy link
Copy Markdown
Contributor Author

@mvdwetering #23309

@Mariusthvdb
Copy link
Copy Markdown
Contributor

Mariusthvdb commented Apr 22, 2019

not really being sure, so please forgive me if this is an unnecessary question, but it would really be very nice if we got these values with the native sensors, since they are all part of templates being used throughtout the setup:

Schermafbeelding 2019-04-22 om 22 15 30

about the lux setting: in the current custom component, lx is calculated from light level as follows:

            lx = round(float(10 ** ((lightlevel - 1) / 10000)), 2)

if you need more info, please let me help if, be glad to.

@mitchellrj
Copy link
Copy Markdown
Contributor Author

mitchellrj commented Apr 22, 2019 via email

@Mariusthvdb
Copy link
Copy Markdown
Contributor

Mariusthvdb commented Apr 22, 2019

ok thanks, please do. Some of them are simply there for the taking. adding them is a matter of adding a line (at least it was like that in the CC...)

Hope you can add them like that too?

def parse_sml(response):
    """Parse the json for a SML Hue motion sensor and return the data."""
    if response["type"] == "ZLLLightLevel":
        lightlevel = response["state"].get("lightlevel")
        tholddark = response["config"].get("tholddark")
        if lightlevel is not None:
            lx = round(float(10 ** ((lightlevel - 1) / 10000)), 2)
            dark = response["state"]["dark"]
            daylight = response["state"]["daylight"]
            data = {
                "light_level": lightlevel,
                "lx": lx,
                "dark": dark,
                "daylight": daylight,
                "threshold": tholddark,

'reachable' and 'on' are truly important, hope these are already included right now?

@MartinHjelmare
Copy link
Copy Markdown
Member

Please open an issue if you suspect a bug.

Merged PRs should not be used for enhancement discussion or bug reports. If you've found a bug it's ok to make a review with inline comments and link to an issue that reports the bug.

Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Apr 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants