Skip to content

Style change follow up #13707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kellerza merged 12 commits into from
Apr 11, 2018
Merged

Style change follow up #13707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
03eb379
Style improvements
cdce8p Apr 5, 2018
5243038
Rewritten get_accessory
cdce8p Apr 9, 2018
8703cb2
Added log statement
cdce8p Apr 9, 2018
2b05f1e
Return None
cdce8p Apr 10, 2018
cb0aad4
Pass config instead of **a_kwargs
cdce8p Apr 10, 2018
dfa7191
Thermostat: Moved feature check to type file
cdce8p Apr 10, 2018
99b67df
Init_setup
cdce8p Apr 10, 2018
2f7ce59
Small changes
cdce8p Apr 10, 2018
2793753
Use super().__init__(*args, category=CATEGORY)
cdce8p Apr 11, 2018
d99f9df
Fix light set_color_temperature call
cdce8p Apr 11, 2018
0af42de
Added setup_char helper function
cdce8p Apr 11, 2018
e0eabb1
Improved type_thermostat optional chars
cdce8p Apr 11, 2018
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
85 changes: 33 additions & 52 deletions homeassistant/components/homekit/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@

import voluptuous as vol

from homeassistant.components.climate import (
SUPPORT_TARGET_TEMPERATURE_HIGH, SUPPORT_TARGET_TEMPERATURE_LOW)
from homeassistant.components.cover import SUPPORT_SET_POSITION
from homeassistant.const import (
ATTR_CODE, ATTR_SUPPORTED_FEATURES, ATTR_UNIT_OF_MEASUREMENT,
ATTR_SUPPORTED_FEATURES, ATTR_UNIT_OF_MEASUREMENT,
CONF_PORT, TEMP_CELSIUS, TEMP_FAHRENHEIT,
EVENT_HOMEASSISTANT_START, EVENT_HOMEASSISTANT_STOP)
import homeassistant.helpers.config_validation as cv
Expand Down Expand Up @@ -79,63 +77,46 @@ def get_accessory(hass, state, aid, config):
state.entity_id)
return None

if state.domain == 'sensor':
unit = state.attributes.get(ATTR_UNIT_OF_MEASUREMENT)
if unit == TEMP_CELSIUS or unit == TEMP_FAHRENHEIT:
_LOGGER.debug('Add "%s" as "%s"',
state.entity_id, 'TemperatureSensor')
return TYPES['TemperatureSensor'](hass, state.entity_id,
state.name, aid=aid)
elif unit == '%':
_LOGGER.debug('Add "%s" as %s"',
state.entity_id, 'HumiditySensor')
return TYPES['HumiditySensor'](hass, state.entity_id, state.name,
aid=aid)
a_type = None
config = config or {}

if state.domain == 'alarm_control_panel':
a_type = 'SecuritySystem'

elif state.domain == 'binary_sensor' or state.domain == 'device_tracker':
_LOGGER.debug('Add "%s" as "%s"', state.entity_id, 'BinarySensor')
return TYPES['BinarySensor'](hass, state.entity_id,
state.name, aid=aid)
a_type = 'BinarySensor'

elif state.domain == 'climate':
a_type = 'Thermostat'

elif state.domain == 'cover':
# Only add covers that support set_cover_position
features = state.attributes.get(ATTR_SUPPORTED_FEATURES, 0)
if features & SUPPORT_SET_POSITION:
_LOGGER.debug('Add "%s" as "%s"',
state.entity_id, 'WindowCovering')
return TYPES['WindowCovering'](hass, state.entity_id, state.name,
aid=aid)

elif state.domain == 'alarm_control_panel':
_LOGGER.debug('Add "%s" as "%s"', state.entity_id, 'SecuritySystem')
return TYPES['SecuritySystem'](hass, state.entity_id, state.name,
alarm_code=config.get(ATTR_CODE),
aid=aid)

elif state.domain == 'climate':
features = state.attributes.get(ATTR_SUPPORTED_FEATURES, 0)
support_temp_range = SUPPORT_TARGET_TEMPERATURE_LOW | \
SUPPORT_TARGET_TEMPERATURE_HIGH
# Check if climate device supports auto mode
support_auto = bool(features & support_temp_range)

_LOGGER.debug('Add "%s" as "%s"', state.entity_id, 'Thermostat')
return TYPES['Thermostat'](hass, state.entity_id,
state.name, support_auto, aid=aid)
a_type = 'WindowCovering'

elif state.domain == 'light':
_LOGGER.debug('Add "%s" as "%s"', state.entity_id, 'Light')
return TYPES['Light'](hass, state.entity_id, state.name, aid=aid)
a_type = 'Light'

elif state.domain == 'lock':
return TYPES['Lock'](hass, state.entity_id, state.name, aid=aid)
a_type = 'Lock'

elif state.domain == 'sensor':
unit = state.attributes.get(ATTR_UNIT_OF_MEASUREMENT)
if unit == TEMP_CELSIUS or unit == TEMP_FAHRENHEIT:
a_type = 'TemperatureSensor'
elif unit == '%':
a_type = 'HumiditySensor'

elif state.domain == 'switch' or state.domain == 'remote' \
or state.domain == 'input_boolean' or state.domain == 'script':
_LOGGER.debug('Add "%s" as "%s"', state.entity_id, 'Switch')
return TYPES['Switch'](hass, state.entity_id, state.name, aid=aid)
a_type = 'Switch'

if a_type is None:
return None

return None
_LOGGER.debug('Add "%s" as "%s"', state.entity_id, a_type)
return TYPES[a_type](hass, state.name, state.entity_id, aid, config=config)


def generate_aid(entity_id):
Expand All @@ -151,7 +132,7 @@ class HomeKit():

def __init__(self, hass, port, entity_filter, entity_config):
"""Initialize a HomeKit object."""
self._hass = hass
self.hass = hass
self._port = port
self._filter = entity_filter
self._config = entity_config
Expand All @@ -164,11 +145,11 @@ def setup(self):
"""Setup bridge and accessory driver."""
from .accessories import HomeBridge, HomeDriver

self._hass.bus.async_listen_once(
self.hass.bus.async_listen_once(
EVENT_HOMEASSISTANT_STOP, self.stop)

path = self._hass.config.path(HOMEKIT_FILE)
self.bridge = HomeBridge(self._hass)
path = self.hass.config.path(HOMEKIT_FILE)
self.bridge = HomeBridge(self.hass)
self.driver = HomeDriver(self.bridge, self._port, get_local_ip(), path)

def add_bridge_accessory(self, state):
Expand All @@ -177,7 +158,7 @@ def add_bridge_accessory(self, state):
return
aid = generate_aid(state.entity_id)
conf = self._config.pop(state.entity_id, {})
acc = get_accessory(self._hass, state, aid, conf)
acc = get_accessory(self.hass, state, aid, conf)
if acc is not None:
self.bridge.add_accessory(acc)

Expand All @@ -192,12 +173,12 @@ def start(self, *args):
type_covers, type_lights, type_locks, type_security_systems,
type_sensors, type_switches, type_thermostats)

for state in self._hass.states.all():
for state in self.hass.states.all():
self.add_bridge_accessory(state)
self.bridge.set_broker(self.driver)

if not self.bridge.paired:
show_setup_message(self.bridge, self._hass)
show_setup_message(self.hass, self.bridge)

_LOGGER.debug('Driver start')
self.driver.start()
Expand Down
59 changes: 42 additions & 17 deletions homeassistant/components/homekit/accessories.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
from pyhap.accessory import Accessory, Bridge, Category
from pyhap.accessory_driver import AccessoryDriver

from homeassistant.core import callback
from homeassistant.core import callback as ha_callback
from homeassistant.helpers.event import (
async_track_state_change, track_point_in_utc_time)
from homeassistant.util import dt as dt_util

from .const import (
DEBOUNCE_TIMEOUT, ACCESSORY_MODEL, ACCESSORY_NAME, BRIDGE_MODEL,
BRIDGE_NAME, MANUFACTURER, SERV_ACCESSORY_INFO, CHAR_MANUFACTURER,
DEBOUNCE_TIMEOUT, BRIDGE_MODEL, BRIDGE_NAME, MANUFACTURER,
SERV_ACCESSORY_INFO, CHAR_MANUFACTURER,
CHAR_MODEL, CHAR_NAME, CHAR_SERIAL_NUMBER)
from .util import (
show_setup_message, dismiss_setup_message)
Expand All @@ -24,7 +24,7 @@

def debounce(func):
"""Decorator function. Debounce callbacks form HomeKit."""
@callback
@ha_callback
def call_later_listener(*args):
"""Callback listener called from call_later."""
# pylint: disable=unsubscriptable-object
Expand Down Expand Up @@ -72,6 +72,18 @@ def add_preload_service(acc, service, chars=None):
return service


def setup_char(char_name, service, value=None, properties=None, callback=None):
"""Helper function to return fully configured characteristic."""
char = service.get_characteristic(char_name)
if value:
char.value = value
if properties:
char.override_properties(properties)
if callback:
char.setter_callback = callback
return char


def set_accessory_info(acc, name, model, manufacturer=MANUFACTURER,
serial_number='0000'):
"""Set the default accessory information."""
Expand All @@ -85,34 +97,47 @@ def set_accessory_info(acc, name, model, manufacturer=MANUFACTURER,
class HomeAccessory(Accessory):
"""Adapter class for Accessory."""

# pylint: disable=no-member

def __init__(self, name=ACCESSORY_NAME, model=ACCESSORY_MODEL,
category='OTHER', **kwargs):
def __init__(self, hass, name, entity_id, aid, category):
"""Initialize a Accessory object."""
super().__init__(name, **kwargs)
set_accessory_info(self, name, model)
super().__init__(name, aid=aid)
Copy link
Copy Markdown
Member

@kellerza kellerza Apr 11, 2018

Choose a reason for hiding this comment

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

Are you sure you never need config to be passed to the Accessory?

Copy link
Copy Markdown
Member Author

@cdce8p cdce8p Apr 11, 2018

Choose a reason for hiding this comment

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

About 90% sure that it isn't needed. The intention behind config was/is to use it for corner cases (e.g. the alarm_code) or to be able to define which type should be used there so it won't have to be in customize.

Copy link
Copy Markdown
Member

@kellerza kellerza Apr 11, 2018

Choose a reason for hiding this comment

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

Ok, because you used to pass it and stopped with the new code... otherwise all seems similar (hate to ask the obvious, but you did test it? 😄)

Copy link
Copy Markdown
Member Author

@cdce8p cdce8p Apr 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Test what?
The config was never passed to super().__init__(), since they haven't been keyword arguments before, only normal ones. **kwargs was there to cover any case that I need to pass more arguments from get_accessory to the HAP-python accessory. However only used for aid.

Edit:
I did test all changes for lights at least. That works. The changes in the other types are mostly identical. I also change the same line in the tests. As you know with PR's that change the base structure it's almost impossible to test everything, but I'm quite confident that it won't introduce new errors.

set_accessory_info(self, name, model=entity_id)
self.category = getattr(Category, category, Category.OTHER)
self.entity_id = entity_id
self.hass = hass

def _set_services(self):
add_preload_service(self, SERV_ACCESSORY_INFO)

def run(self):
"""Method called by accessory after driver is started."""
state = self.hass.states.get(self.entity_id)
self.update_state(new_state=state)
self.update_state_callback(new_state=state)
async_track_state_change(
self.hass, self.entity_id, self.update_state)
self.hass, self.entity_id, self.update_state_callback)

def update_state_callback(self, entity_id=None, old_state=None,
new_state=None):
"""Callback from state change listener."""
_LOGGER.debug('New_state: %s', new_state)
if new_state is None:
return
self.update_state(new_state)

def update_state(self, new_state):
Copy link
Copy Markdown
Member

@kellerza kellerza Apr 7, 2018

Choose a reason for hiding this comment

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

👍 This is a good idea and seem to simplify things as well

"""Method called on state change to update HomeKit value.

Overridden by accessory types.
"""
pass


class HomeBridge(Bridge):
"""Adapter class for Bridge."""

def __init__(self, hass, name=BRIDGE_NAME,
model=BRIDGE_MODEL, **kwargs):
def __init__(self, hass, name=BRIDGE_NAME):
"""Initialize a Bridge object."""
super().__init__(name, **kwargs)
set_accessory_info(self, name, model)
super().__init__(name)
set_accessory_info(self, name, model=BRIDGE_MODEL)
self.hass = hass

def _set_services(self):
Expand All @@ -130,7 +155,7 @@ def add_paired_client(self, client_uuid, client_public):
def remove_paired_client(self, client_uuid):
"""Override super function to show setup message if unpaired."""
super().remove_paired_client(client_uuid)
show_setup_message(self, self.hass)
show_setup_message(self.hass, self)


class HomeDriver(AccessoryDriver):
Expand Down
2 changes: 0 additions & 2 deletions homeassistant/components/homekit/const.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
SERVICE_HOMEKIT_START = 'start'

# #### STRING CONSTANTS ####
ACCESSORY_MODEL = 'homekit.accessory'
ACCESSORY_NAME = 'Home Accessory'
BRIDGE_MODEL = 'homekit.bridge'
BRIDGE_NAME = 'Home Assistant'
MANUFACTURER = 'HomeAssistant'
Expand Down
35 changes: 11 additions & 24 deletions homeassistant/components/homekit/type_covers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@
from homeassistant.components.cover import ATTR_CURRENT_POSITION

from . import TYPES
from .accessories import HomeAccessory, add_preload_service
from .accessories import HomeAccessory, add_preload_service, setup_char
from .const import (
CATEGORY_WINDOW_COVERING, SERV_WINDOW_COVERING,
CHAR_CURRENT_POSITION, CHAR_TARGET_POSITION, CHAR_POSITION_STATE)


_LOGGER = logging.getLogger(__name__)


Expand All @@ -20,29 +19,20 @@ class WindowCovering(HomeAccessory):
The cover entity must support: set_cover_position.
"""

def __init__(self, hass, entity_id, display_name, **kwargs):
def __init__(self, *args, config):
"""Initialize a WindowCovering accessory object."""
super().__init__(display_name, entity_id,
CATEGORY_WINDOW_COVERING, **kwargs)

self.hass = hass
self.entity_id = entity_id

super().__init__(*args, category=CATEGORY_WINDOW_COVERING)
self.current_position = None
self.homekit_target = None

serv_cover = add_preload_service(self, SERV_WINDOW_COVERING)
self.char_current_position = serv_cover. \
get_characteristic(CHAR_CURRENT_POSITION)
self.char_target_position = serv_cover. \
get_characteristic(CHAR_TARGET_POSITION)
self.char_position_state = serv_cover. \
get_characteristic(CHAR_POSITION_STATE)
self.char_current_position.value = 0
self.char_target_position.value = 0
self.char_position_state.value = 0

self.char_target_position.setter_callback = self.move_cover
self.char_current_position = setup_char(
CHAR_CURRENT_POSITION, serv_cover, value=0)
self.char_target_position = setup_char(
CHAR_TARGET_POSITION, serv_cover, value=0,
callback=self.move_cover)
self.char_position_state = setup_char(
CHAR_POSITION_STATE, serv_cover, value=0)

def move_cover(self, value):
"""Move cover to value if call came from HomeKit."""
Expand All @@ -56,11 +46,8 @@ def move_cover(self, value):
self.hass.components.cover.set_cover_position(
value, self.entity_id)

def update_state(self, entity_id=None, old_state=None, new_state=None):
def update_state(self, new_state):
"""Update cover position after state changed."""
if new_state is None:
return

current_position = new_state.attributes.get(ATTR_CURRENT_POSITION)
if isinstance(current_position, int):
self.current_position = current_position
Expand Down
Loading