Refactor Hue: phue -> aiohue #13043

balloob · 2018-03-10T08:40:55Z

Description:

Hue is going async! 🎉

We used to use phue. It was a sync library and it only allowed to store auth on disk. It could not work with auth that was passed in. So for config entries I decided to roll aiohue to manage linking and getting basic info.

Since then aiohue has matured, 1.0.0 has been released and so I decided to migrate our hue component from phue to aiohue.

This is a backwards compatible change for the auth files. The Hue component will consume and generate the silly config files from phue.

Breaking change: If you specify a bridge in your config, the host key is now required and needs to be a valid ip address. Option allow_in_emulated_hue has been removed. Components should not be aware nor care about other components. Exclude the lights via the emulated hue config.

Example entry for `configuration.yaml` (if applicable):

hue:

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass

If the code does not interact with devices:

Tests have been added to verify that the new code works.

balloob · 2018-03-10T08:41:23Z

homeassistant/components/hue.py

-        allow_unreachable = bridge.get(CONF_ALLOW_UNREACHABLE)
-        allow_in_emulated_hue = bridge.get(CONF_ALLOW_IN_EMULATED_HUE)
-        allow_hue_groups = bridge.get(CONF_ALLOW_HUE_GROUPS)
+        allow_unreachable = bridge.get(CONF_ALLOW_UNREACHABLE,


Pass defaults because when discovering bridges via nupnp, we don't go through config schema.

balloob · 2018-03-10T08:46:09Z

homeassistant/components/light/hue.py

 ATTR_IS_HUE_GROUP = 'is_hue_group'

-# Legacy configuration, will be removed in 0.60


All of this wasn't used and could be removed.

houndci-bot · 2018-03-10T08:47:56Z

homeassistant/components/hue.py

+    """Store config."""
+    with open(hass.config.path(filename), 'w') as outp:
+        json.dump({
+            host: {'username': bridge.username}


undefined name 'bridge'

OttoWinter

Awesome 🎉 I don't have any Hue lights so I can't test it, but the code looks good to me 👍

OttoWinter · 2018-03-10T08:59:50Z

homeassistant/components/hue.py

-                            self.host)
-            self.request_configuration()
+        except aiohue.AiohueException:
+            _LOGGER.exception('Uknown Hue linking error occurred')


Typo. _LOGGER.exception("Unknown Hue linking error occurred")

MartinHjelmare · 2018-03-10T09:43:44Z

homeassistant/components/light/hue.py

-    if not isinstance(api_groups, dict):
-        _LOGGER.error('Got unexpected result from Hue API')
-        return []
+    if not allow_groups:


Maybe also check if new_lights is truthy?

MartinHjelmare · 2018-03-10T09:53:21Z

homeassistant/components/hue.py

-                       allow_in_emulated_hue, allow_hue_groups)
-    bridge.setup()
+                       allow_hue_groups)
+    hass.async_add_job(bridge.async_setup())


hass.add_job

point-4ward · 2018-03-12T01:00:16Z

Any chance of an official integration of the Hue sensors? I'm using @robmarkcole 's custom component atm and it's ace, and I use the readings from the motion sensors for automations (and I have a couple that react to long presses on dimmer switches too)

Obviously this PR is going to throw a spanner in the works somewhere along the line, would be good to get an official integration for the other hue devices at the same time, or at least 'around the corner'.

balloob · 2018-03-12T02:49:57Z

@mf-social yes, sensors are on the roadmap after this PR lands.

point-4ward · 2018-03-12T10:24:00Z

@balloob - excellent 🙏

MartinHjelmare · 2018-03-12T12:59:35Z

homeassistant/components/light/hue.py


+class BoolProxy:
+    """Proxy value for a boolean."""


I think this requires a longer explanation. What does this solve?

Good point. I'll remove it and just pass bridge object through like the old implementation.

What it was solving btw was that we're sharing a boolean value with all the light objects and are able to change it without having to update each light object individually.

houndci-bot · 2018-03-13T20:01:25Z

tests/components/light/test_hue.py

        update_lights_cb=None,
-        allow_in_emulated_hue=False,
+        bridge=Mock(allow_unreachable=False),


undefined name 'Mock'

houndci-bot · 2018-03-13T20:01:25Z

tests/components/light/test_hue.py

-
-        light_id=None,
-        bridge=mock.Mock(),
+        light=Mock(state={'reachable': False}),


undefined name 'Mock'

houndci-bot · 2018-03-13T20:06:44Z

tests/components/light/test_hue.py

        update_lights_cb=None,
-        allow_in_emulated_hue=False,
+        bridge=Mock(allow_unreachable=False),


undefined name 'Mock'

houndci-bot · 2018-03-13T20:06:44Z

tests/components/light/test_hue.py

-
-        light_id=None,
-        bridge=mock.Mock(),
+        light=Mock(state={'reachable': False}),


undefined name 'Mock'

balloob · 2018-03-13T20:27:15Z

Alright updated the code, we're no longer updating things twice. It's lean and efficient.

MartinHjelmare · 2018-03-13T22:55:54Z

Logic looks good to me.

houndci-bot · 2018-03-14T04:48:59Z

tests/components/light/test_hue.py

-import unittest
-import unittest.mock as mock
-from unittest.mock import call, MagicMock, patch
+from unittest.mock import patch, Mock


'unittest.mock.patch' imported but unused

houndci-bot · 2018-03-14T04:48:59Z

tests/components/light/test_hue.py

-
+import asyncio
+from collections import deque
+from copy import deepcopy


'copy.deepcopy' imported but unused

tests/components/hue/conftest.py

+    """Mock the HueBridge from initializing."""
+    patch('homeassistant.components.hue._find_username_from_config',
+          return_value=None), \
+            patch('homeassistant.components.hue.HueBridge') as mock_bridge:


houndci-bot · 2018-03-14T07:20:27Z

homeassistant/components/zwave/__init__.py

@@ -203,8 +203,7 @@ def get_config_value(node, value_index, tries=5):
    return None


-@asyncio.coroutine
-def async_setup_platform(hass, config, async_add_devices, discovery_info=None):
+async def async_setup_platform(hass, config, async_add_devices, discovery_info=None):


line too long (85 > 79 characters)

balloob · 2018-03-14T07:21:01Z

PyLint sometimes gets confused when mixing async/await with coroutine, hence I had to include some extra async/await conversions to satisfy pylint: 9e07108

mcfarlde · 2018-04-02T02:14:31Z

This refactor caused the connections to tasmota ESP 8266s using HUe emulation to be lost. They are not discovered and the config that used to work with them no longer works. It would have been good to leave backwards compatibility so that existing devices would not be lost. I think I can either wait until arendst updates the tasmota firmware or move the device to MQTT which I do not currently use. Itchy finger on that update button burned me again!

balloob · 2018-04-02T16:55:45Z

If Tasmota does not work with this integration, it's the problem in their emulation layer. This integration is made for Philips Hue.

mcfarlde · 2018-04-02T17:02:25Z

Ya, figured that. Theo Arends suggests moving to MQTT as there is no room in the firmware for a full HUE emulation. I guess I was lucky to have it working for the year prior to this integration.

balloob added the breaking-change label Mar 10, 2018

balloob requested a review from andrey-git as a code owner March 10, 2018 08:40

homeassistant added cla-signed integration: hue platform: light.hue labels Mar 10, 2018

balloob commented Mar 10, 2018

View reviewed changes

homeassistant added the cla-signed label Mar 10, 2018

houndci-bot reviewed Mar 10, 2018

View reviewed changes

OttoWinter reviewed Mar 10, 2018

View reviewed changes

MartinHjelmare reviewed Mar 10, 2018

View reviewed changes

balloob force-pushed the hue-aiohue branch from a181026 to 48ddd3f Compare March 11, 2018 16:52

homeassistant added the cla-signed label Mar 11, 2018

balloob force-pushed the hue-aiohue branch from 62d46fa to e5f46d9 Compare March 11, 2018 16:53

homeassistant added the cla-signed label Mar 11, 2018

balloob force-pushed the hue-aiohue branch from e5f46d9 to fe9fd1f Compare March 11, 2018 19:31

homeassistant added the cla-signed label Mar 11, 2018

homeassistant added the cla-signed label Mar 12, 2018

MartinHjelmare reviewed Mar 12, 2018

View reviewed changes

balloob added 8 commits March 12, 2018 20:24

phue -> aiohue

19c8d32

Clean up

86274e4

Fix config

51b58de

Address comments

0e2a9de

Typo

dd977c0

Fix rebase error

5ccc678

Mark light as unavailable when bridge is disconnected

da7a503

Tests

1f3f0c3

houndci-bot reviewed Mar 13, 2018

View reviewed changes

Another comment update

99bd4a2

houndci-bot reviewed Mar 13, 2018

View reviewed changes

Wrap up tests

2d4454f

houndci-bot reviewed Mar 14, 2018

View reviewed changes

balloob added 3 commits March 13, 2018 21:49

Lint

a1a2d5f

Fix tests

b3e81dc

PyLint does not like mix 'n match async and coroutine

9e07108

balloob requested a review from a team as a code owner March 14, 2018 07:20

homeassistant added the cla-signed label Mar 14, 2018

houndci-bot reviewed Mar 14, 2018

View reviewed changes

balloob added 4 commits March 14, 2018 00:21

Lint

43cbcf9

Update aiohue to 1.2

bbd5782

Lint

0a971a1

Fix await MagicMock

230a8f6

balloob mentioned this pull request Mar 14, 2018

Hue Groups being imported when configured not to #13213

Closed

balloob merged commit 5a9013c into dev Mar 17, 2018

balloob deleted the hue-aiohue branch March 17, 2018 03:27

MartinHjelmare mentioned this pull request Mar 17, 2018

Philips Hue Component does not support scan-interval. #13144

Closed

balloob mentioned this pull request Mar 30, 2018

0.66.0 #13554

Merged

mcfarlde mentioned this pull request Apr 2, 2018

Hue Emulation arendst/Tasmota#2322

Closed

bart-roos mentioned this pull request Apr 6, 2018

"hue.hue_activate_scene" not working consistently since 0.66 #13685

Closed

home-assistant locked and limited conversation to collaborators Jul 26, 2018

ghost removed the platform: light.hue label Mar 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Hue: phue -> aiohue #13043

Refactor Hue: phue -> aiohue #13043

balloob commented Mar 10, 2018 •

edited

balloob Mar 10, 2018

balloob Mar 10, 2018

houndci-bot Mar 10, 2018

OttoWinter left a comment

OttoWinter Mar 10, 2018

MartinHjelmare Mar 10, 2018

MartinHjelmare Mar 10, 2018

point-4ward commented Mar 12, 2018

balloob commented Mar 12, 2018

point-4ward commented Mar 12, 2018

MartinHjelmare Mar 12, 2018

balloob Mar 12, 2018

balloob Mar 13, 2018

MartinHjelmare Mar 13, 2018

houndci-bot Mar 13, 2018

houndci-bot Mar 13, 2018

houndci-bot Mar 13, 2018

houndci-bot Mar 13, 2018

balloob commented Mar 13, 2018

MartinHjelmare commented Mar 13, 2018

houndci-bot Mar 14, 2018

houndci-bot Mar 14, 2018

This comment was marked as resolved.

houndci-bot Mar 14, 2018

balloob commented Mar 14, 2018

mcfarlde commented Apr 2, 2018

balloob commented Apr 2, 2018

mcfarlde commented Apr 2, 2018

		ATTR_IS_HUE_GROUP = 'is_hue_group'

		# Legacy configuration, will be removed in 0.60

Refactor Hue: phue -> aiohue #13043

Refactor Hue: phue -> aiohue #13043

Conversation

balloob commented Mar 10, 2018 • edited

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OttoWinter left a 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

point-4ward commented Mar 12, 2018

balloob commented Mar 12, 2018

point-4ward commented Mar 12, 2018

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

balloob commented Mar 13, 2018

MartinHjelmare commented Mar 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

balloob commented Mar 14, 2018

mcfarlde commented Apr 2, 2018

balloob commented Apr 2, 2018

mcfarlde commented Apr 2, 2018

balloob commented Mar 10, 2018 •

edited

Example entry for `configuration.yaml` (if applicable):