Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ homeassistant/components/discogs/* @thibmaek
homeassistant/components/doorbird/* @oblogic7
homeassistant/components/dsmr_reader/* @depl0y
homeassistant/components/dweet/* @fabaff
homeassistant/components/dynalite/* @ziv1234
homeassistant/components/dyson/* @etheralm
homeassistant/components/ecobee/* @marthoc
homeassistant/components/ecovacs/* @OverloadUT
Expand Down
93 changes: 93 additions & 0 deletions homeassistant/components/dynalite/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
"""Support for the Dynalite networks."""
from dynalite_devices_lib import BRIDGE_CONFIG_SCHEMA
import voluptuous as vol

from homeassistant import config_entries
from homeassistant.const import CONF_HOST
from homeassistant.helpers import config_validation as cv

# Loading the config flow file will register the flow
from .bridge import DynaliteBridge
from .config_flow import configured_hosts
from .const import CONF_BRIDGES, DATA_CONFIGS, DOMAIN, LOGGER

CONFIG_SCHEMA = vol.Schema(
{
DOMAIN: vol.Schema(
{
vol.Optional(CONF_BRIDGES): vol.All(
cv.ensure_list, [BRIDGE_CONFIG_SCHEMA]
)
}
)
},
extra=vol.ALLOW_EXTRA,
)


async def async_setup(hass, config):
"""Set up the Dynalite platform."""

conf = config.get(DOMAIN)
LOGGER.debug("Setting up dynalite component config = %s", conf)

if conf is None:
conf = {}

hass.data[DOMAIN] = {}
hass.data[DOMAIN][DATA_CONFIGS] = {}

configured = configured_hosts(hass)

# User has configured bridges
if CONF_BRIDGES not in conf:
return True

bridges = conf[CONF_BRIDGES]

for bridge_conf in bridges:
host = bridge_conf[CONF_HOST]
LOGGER.debug("async_setup host=%s conf=%s", host, bridge_conf)

# Store config in hass.data so the config entry can find it
hass.data[DOMAIN][DATA_CONFIGS][host] = bridge_conf

if host in configured:
LOGGER.debug("async_setup host=%s already configured", host)
continue

hass.async_create_task(
hass.config_entries.flow.async_init(
DOMAIN,
context={"source": config_entries.SOURCE_IMPORT},
data={CONF_HOST: bridge_conf[CONF_HOST]},
)
)

return True


async def async_setup_entry(hass, entry):
"""Set up a bridge from a config entry."""
LOGGER.debug("__init async_setup_entry %s", entry.data)
host = entry.data[CONF_HOST]
config = hass.data[DOMAIN][DATA_CONFIGS].get(host)

if config is None:
LOGGER.error("__init async_setup_entry empty config for host %s", host)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should not allow this to happen. This should be caught earlier in the config flow.

return False

bridge = DynaliteBridge(hass, entry)

if not await bridge.async_setup():
LOGGER.error("bridge.async_setup failed")
return False
hass.data[DOMAIN][entry.entry_id] = bridge
return True


async def async_unload_entry(hass, entry):
"""Unload a config entry."""
LOGGER.error("async_unload_entry %s", entry.data)
bridge = hass.data[DOMAIN].pop(entry.entry_id)
return await bridge.async_reset()
118 changes: 118 additions & 0 deletions homeassistant/components/dynalite/bridge.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
"""Code to handle a Dynalite bridge."""

from dynalite_devices_lib import DynaliteDevices
from dynalite_lib import CONF_ALL

from homeassistant.const import CONF_HOST
from homeassistant.core import callback

from .const import DATA_CONFIGS, DOMAIN, LOGGER
from .light import DynaliteLight
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We're leaking entity implementation details from the light platform to the bridge module. That's not a good pattern. The platform details should stay within the platform.

It's ok to extract a common function that's used by multiple platforms but the platform should still be responsible for creating and adding entities etc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed. will be pushed shortly



class BridgeError(Exception):
"""Class to throw exceptions from DynaliteBridge."""

def __init__(self, message):
"""Initialize the exception."""
super().__init__()
self.message = message


class DynaliteBridge:
"""Manages a single Dynalite bridge."""

def __init__(self, hass, config_entry):
"""Initialize the system based on host parameter."""
self.config_entry = config_entry
self.hass = hass
self.area = {}
self.async_add_entities = None
self.waiting_entities = []
self.all_entities = {}
self.config = None
self.host = config_entry.data[CONF_HOST]
if self.host not in hass.data[DOMAIN][DATA_CONFIGS]:
LOGGER.info("invalid host - %s", self.host)
raise BridgeError(f"invalid host - {self.host}")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We want to avoid side effects like logging or raising an error in init method. Please move that outside.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

self.config = hass.data[DOMAIN][DATA_CONFIGS][self.host]
# Configure the dynalite devices
self.dynalite_devices = DynaliteDevices(
config=self.config,
newDeviceFunc=self.add_devices,
updateDeviceFunc=self.update_device,
)

async def async_setup(self, tries=0):
"""Set up a Dynalite bridge."""
# Configure the dynalite devices
await self.dynalite_devices.async_setup()

self.hass.async_create_task(
self.hass.config_entries.async_forward_entry_setup(
self.config_entry, "light"
)
)

return True

@callback
def add_devices(self, devices):
"""Call when devices should be added to home assistant."""
added_entities = []

for device in devices:
if device.category == "light":
entity = DynaliteLight(device, self)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The entity should be created and added within its platform.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed. will be pushed shortly

else:
LOGGER.debug("Illegal device category %s", device.category)
continue
added_entities.append(entity)
self.all_entities[entity.unique_id] = entity
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should not store the entity instances outside their platforms in a shared container. It's ok to store a reference to the entity, eg the entity_id, to keep track of added entities.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed. will be pushed shortly


if added_entities:
self.add_entities_when_registered(added_entities)

@callback
def update_device(self, device):
"""Call when a device or all devices should be updated."""
if device == CONF_ALL:
# This is used to signal connection or disconnection, so all devices may become available or not.
if self.dynalite_devices.available:
LOGGER.info("Connected to dynalite host")
else:
LOGGER.info("Disconnected from dynalite host")
for uid in self.all_entities:
self.all_entities[uid].try_schedule_ha()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed. will be pushed shortly

else:
uid = device.unique_id
if uid in self.all_entities:
self.all_entities[uid].try_schedule_ha()

@callback
def register_add_entities(self, async_add_entities):
"""Add an async_add_entities for a category."""
self.async_add_entities = async_add_entities
if self.waiting_entities:
self.async_add_entities(self.waiting_entities)

def add_entities_when_registered(self, entities):
"""Add the entities to HA if async_add_entities was registered, otherwise queue until it is."""
if not entities:
return
if self.async_add_entities:
self.async_add_entities(entities)
else: # handle it later when it is registered
self.waiting_entities.extend(entities)

async def async_reset(self):
"""Reset this bridge to default state.

Will cancel any scheduled setup retry and will unload
the config entry.
"""
result = await self.hass.config_entries.async_forward_entry_unload(
self.config_entry, "light"
)
# None and True are OK
return result
58 changes: 58 additions & 0 deletions homeassistant/components/dynalite/config_flow.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
"""Config flow to configure Dynalite hub."""
import asyncio

from homeassistant import config_entries
from homeassistant.const import CONF_HOST
from homeassistant.core import callback

from .const import DOMAIN, LOGGER


@callback
def configured_hosts(hass):
"""Return a set of the configured hosts."""
return set(
entry.data[CONF_HOST] for entry in hass.config_entries.async_entries(DOMAIN)
)


class DynaliteFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
"""Handle a Dynalite config flow."""

VERSION = 1
CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_POLL

# pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167

def __init__(self):
"""Initialize the Dynalite flow."""
self.host = None

async def async_step_import(self, import_info):
"""Import a new bridge as a config entry."""
LOGGER.debug("async_step_import - %s", import_info)
host = self.context[CONF_HOST] = import_info[CONF_HOST]
return await self._entry_from_bridge(host)

async def _entry_from_bridge(self, host):
"""Return a config entry from an initialized bridge."""
LOGGER.debug("entry_from_bridge - %s", host)
# Remove all other entries of hubs with same ID or host

same_hub_entries = [
entry.entry_id
for entry in self.hass.config_entries.async_entries(DOMAIN)
if entry.data[CONF_HOST] == host
]

LOGGER.debug("entry_from_bridge same_hub - %s", same_hub_entries)

if same_hub_entries:
await asyncio.wait(
[
self.hass.config_entries.async_remove(entry_id)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a legacy pattern. We now have the unique_id api for config flow and config entries.

https://developers.home-assistant.io/docs/en/config_entries_config_flow_handler.html#unique-ids

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is quite difficult to understand the config_flow. Do you know if there is any example component that has the most simple config flow (only initiated from configuration.yaml)?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think there's an integration with config flow and only import step that uses the unique_id api. That api is still quite new.

Often the import step just forwards the config to the user step, since the user step often can accept a subset of the items in the config import.

The rainmachine config flow is quite simple and has just import + user step. It doesn't use the unique_id api yet though.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I found a simple flow with just import + user step and unique_id api: gdacs.

for entry_id in same_hub_entries
]
)

return self.async_create_entry(title=host, data={CONF_HOST: host})
11 changes: 11 additions & 0 deletions homeassistant/components/dynalite/const.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
"""Constants for the Dynalite component."""
import logging

LOGGER = logging.getLogger(__package__)
DOMAIN = "dynalite"
DATA_CONFIGS = "dynalite_configs"

CONF_BRIDGES = "bridges"

DEFAULT_NAME = "dynalite"
DEFAULT_PORT = 12345
84 changes: 84 additions & 0 deletions homeassistant/components/dynalite/light.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
"""Support for Dynalite channels as lights."""
from homeassistant.components.light import SUPPORT_BRIGHTNESS, Light
from homeassistant.core import callback

from .const import DOMAIN, LOGGER


async def async_setup_entry(hass, config_entry, async_add_entities):
"""Record the async_add_entities function to add them later when received from Dynalite."""
LOGGER.debug("async_setup_entry light entry = %s", config_entry.data)
bridge = hass.data[DOMAIN][config_entry.entry_id]
bridge.register_add_entities(async_add_entities)


class DynaliteLight(Light):
"""Representation of a Dynalite Channel as a Home Assistant Light."""

def __init__(self, device, bridge):
"""Initialize the base class."""
self._device = device
self._bridge = bridge

@property
def device(self):
"""Return the underlying device - mostly for testing."""
return self._device

@property
def name(self):
"""Return the name of the entity."""
return self._device.name

@property
def unique_id(self):
"""Return the unique ID of the entity."""
return self._device.unique_id

@property
def available(self):
"""Return if entity is available."""
return self._device.available

@property
def hidden(self):
"""Return true if this entity should be hidden from UI."""
return self._device.hidden

async def async_update(self):
"""Update the entity."""
return

@property
def device_info(self):
"""Device info for this entity."""
return self._device.device_info

@property
def brightness(self):
"""Return the brightness of this light between 0..255."""
return self._device.brightness

@property
def is_on(self):
"""Return true if device is on."""
return self._device.is_on

async def async_turn_on(self, **kwargs):
"""Turn the light on."""
await self._device.async_turn_on(**kwargs)

async def async_turn_off(self, **kwargs):
"""Turn the light off."""
await self._device.async_turn_off(**kwargs)

@property
def supported_features(self):
"""Flag supported features."""
return SUPPORT_BRIGHTNESS

@callback
def try_schedule_ha(self):
"""Schedule update HA state if configured."""
if self.hass:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's a bad sign that we have to have this check. Entity state update methods should not be called from outside the entity. That's error prone, and that's why this check is needed.

Instead, if we need to tell an entity to update from outside its platform, we can use our dispatch helper to signal the entity to update.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed. will be pushed shortly

self.schedule_update_ha_state()
9 changes: 9 additions & 0 deletions homeassistant/components/dynalite/manifest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"domain": "dynalite",
"name": "Philips Dynalite",
"config_flow": true,
"documentation": "https://www.home-assistant.io/integrations/dynalite",
"dependencies": [],
"codeowners": ["@ziv1234"],
"requirements": ["dynalite_devices==0.1.17"]
}
1 change: 1 addition & 0 deletions homeassistant/generated/config_flows.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"daikin",
"deconz",
"dialogflow",
"dynalite",
"ecobee",
"elgato",
"emulated_roku",
Expand Down
Loading