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.

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

Add Lutron Homeworks component #18311

Merged
merged 10 commits into from Dec 21, 2018

Conversation

@dubnom
Copy link
Contributor

dubnom commented Nov 7, 2018

Description:

Lutron made the Homeworks series 4 & 8 systems about 15 years ago. These components interface with Homeworks.

Related issue (if applicable): fixes #

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

Example entry for configuration.yaml (if applicable):

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 dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
  • New dependencies are only imported inside functions that use them ([example][ex-import]).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.
@pvizeli
Copy link
Member

pvizeli left a comment

Use helper/dispatcher to send a signal from the main callback handler to platform. That help also to hold the worker time small for your library. Make sure that the platform first listens on dispatcher on async_added_to_hass. There exists some example of how that will work.

@dubnom

This comment has been minimized.

Copy link
Contributor

dubnom commented Nov 9, 2018

I'm used to Python 2.7, not all of the cool async features for 3.5+. I'm getting there. I also come from CVS source code control world, so moving to Git has been fun. I had already switched my code to async_added_to_hass when I tested with a much larger set of devices. Do you have a good example component that uses help/dispatcher? Also, if I make these mods (or other mods), what is the correct way of getting these changes into the dev build?
Thanks - Mike

@pvizeli

This comment has been minimized.

Copy link
Member

pvizeli commented Nov 9, 2018

Example:
https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/android_ip_webcam.py

So we support only python > 3.5.2 . You should use a python3 developer environment for Home Assistant.

@dubnom

This comment has been minimized.

Copy link
Contributor

dubnom commented Nov 9, 2018

Thanks for the examples. I'm fusing 3.6.5 in a virtual environment.

Show resolved Hide resolved homeassistant/components/binary_sensor/homeworks.py Outdated
Show resolved Hide resolved homeassistant/components/binary_sensor/homeworks.py Outdated
Show resolved Hide resolved homeassistant/components/binary_sensor/homeworks.py Outdated
vol.Required(CONF_NAME): cv.string,
vol.Required(CONF_BUTTONS): vol.All(cv.ensure_list, [BUTTON_SCHEMA])
})
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Nov 13, 2018

Member

It's better to keep all the config under the component and load platforms via discovery. See the rainmachine component for an example.

Show resolved Hide resolved homeassistant/components/binary_sensor/homeworks.py Outdated
Show resolved Hide resolved homeassistant/components/light/homeworks.py Outdated
Show resolved Hide resolved homeassistant/components/light/homeworks.py Outdated
Show resolved Hide resolved homeassistant/components/light/homeworks.py Outdated
Show resolved Hide resolved homeassistant/components/light/homeworks.py Outdated
Show resolved Hide resolved homeassistant/components/light/homeworks.py Outdated

@MartinHjelmare MartinHjelmare changed the title Added Lutron Homeworks components. Add Lutron Homeworks component Nov 13, 2018

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Nov 13, 2018

Make sure we explain in the docs how this integration is different than the existing lutron integrations:
https://www.home-assistant.io/components/lutron/
https://www.home-assistant.io/components/lutron_caseta/

@dubnom

This comment has been minimized.

Copy link
Contributor

dubnom commented Dec 14, 2018

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Dec 15, 2018

  1. Devices can still be configured individually from the component. Look at the octoprint component, or eg IHC component.
  2. Look at the deconz component for how to fire events for eg buttons.
  3. The Entity base class doesn't have an init method, so there's no confusion about multiple inheritance.
  4. Use of dispatcher looks good after a quick glance. I'll have another look asap.
from pyhomeworks.pyhomeworks import (
HW_BUTTON_PRESSED, HW_BUTTON_RELEASED)

msg_type, values = data

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Dec 15, 2018

Member

We already have accesst to msg_type as an argument. The dispatch helper can send any number of arguments to the target _update_callback. So we can remove this line.

_LOGGER.debug('callback: %s, %s', msg_type, values)
addr = values[0]
signal = ENTITY_SIGNAL.format(addr)
dispatcher_send(hass, signal, (msg_type, values))

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Dec 15, 2018

Member

Send the arguments separately.

Suggested change Beta
dispatcher_send(hass, signal, (msg_type, values))
dispatcher_send(hass, signal, msg_type, values)
return self._level != 0

@callback
def _update_callback(self, data):

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Dec 15, 2018

Member
Suggested change Beta
def _update_callback(self, data):
def _update_callback(self, msg_type, values):
from pyhomeworks.pyhomeworks import HW_LIGHT_CHANGED

_LOGGER.debug('_update_callback %s', data)
msg_type, values = data

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Dec 15, 2018

Member

Remove this.

"ButtonNumber": self._num}

@callback
def _update_callback(self, msg_type, data):

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Dec 15, 2018

Member
Suggested change Beta
def _update_callback(self, msg_type, data):
def _update_callback(self, msg_type, values):
@property
def device_state_attributes(self):
"""Return state attributes."""
return {"HomeworksAddress": self._addr,

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Dec 15, 2018

Member
Suggested change Beta
return {"HomeworksAddress": self._addr,
return {'homeworks_address': self._addr,
@property
def device_state_attributes(self):
"""Supported attributes."""
return {'HomeworksAddress': self._addr}

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Dec 15, 2018

Member
Suggested change Beta
return {'HomeworksAddress': self._addr}
return {'homeworks_address': self._addr}
controller.register(self)

@property
def addr(self):

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Dec 15, 2018

Member

Just remove this method. We don't seem to need it.

Removed binary_sensor.
Implemented new signal/events for button presses.
Cleaned up some data passing.
else:
return
self._hass.bus.async_fire(event,
{CONF_ID: self._id, CONF_NAME: self._name, 'button': values[1]})

This comment has been minimized.

@houndci-bot

houndci-bot Dec 15, 2018

continuation line under-indented for visual indent

addr = key_config[CONF_ADDR]
name = key_config[CONF_NAME]
whatever = HomeworksKeypadEvent(hass, addr, name)

This comment has been minimized.

@houndci-bot

houndci-bot Dec 15, 2018

blank line contains whitespace

for key_config in config.get(CONF_KEYPADS,[]):
addr = key_config[CONF_ADDR]
name = key_config[CONF_NAME]
whatever = HomeworksKeypadEvent(hass, addr, name)

This comment has been minimized.

@houndci-bot

houndci-bot Dec 15, 2018

local variable 'whatever' is assigned to but never used

dimmers = config.get(CONF_DIMMERS,[])
load_platform(hass, 'light', DOMAIN, {CONF_DIMMERS: dimmers}, base_config)

for key_config in config.get(CONF_KEYPADS,[]):

This comment has been minimized.

@houndci-bot

houndci-bot Dec 15, 2018

missing whitespace after ','


hass.bus.listen_once(EVENT_HOMEASSISTANT_STOP, cleanup)

dimmers = config.get(CONF_DIMMERS,[])

This comment has been minimized.

@houndci-bot

houndci-bot Dec 15, 2018

missing whitespace after ','

import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.discovery import load_platform
from homeassistant.helpers.dispatcher import (dispatcher_send,
async_dispatcher_connect)

This comment has been minimized.

@houndci-bot

houndci-bot Dec 15, 2018

continuation line under-indented for visual indent

@dubnom

This comment has been minimized.

Copy link
Contributor

dubnom commented Dec 15, 2018

@dubnom

This comment has been minimized.

Copy link
Contributor

dubnom commented Dec 15, 2018

Got rid of unused config stuff.
Reordered imports.

_LOGGER = logging.getLogger(__name__)

def setup_platform(hass, config, add_entities, discover_info=None):

This comment has been minimized.

@houndci-bot

houndci-bot Dec 16, 2018

expected 2 blank lines, found 1

"""
import logging

import voluptuous as vol

This comment has been minimized.

@houndci-bot

houndci-bot Dec 16, 2018

'voluptuous as vol' imported but unused

Show resolved Hide resolved homeassistant/components/homeworks.py Outdated
DOMAIN: vol.Schema({
vol.Required(CONF_HOST): cv.string,
vol.Required(CONF_PORT): cv.port,
vol.Optional(CONF_DIMMERS): vol.All(cv.ensure_list, [DIMMER_SCHEMA]),

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Dec 16, 2018

Member

We can set the default empty list in the config schema here.

Should we require at least either dimmers or keypads in the config? Without any of those, the component will be useless.

We can use cv.has_at_least_one_key for this.

Show resolved Hide resolved homeassistant/components/homeworks.py Outdated
Show resolved Hide resolved homeassistant/components/light/homeworks.py
Show resolved Hide resolved homeassistant/components/light/homeworks.py Outdated
Show resolved Hide resolved requirements_all.txt Outdated
More requested changes
Removed HomeworksController, it wasn't needed.
@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Almost done.

dispatcher_send(hass, signal, msg_type, values)

config = base_config.get(DOMAIN)
# host = config[CONF_HOST]

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Dec 16, 2018

Member

Stale code.

@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Great!

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Dec 16, 2018

Can be merged when build passes.

@dubnom

This comment has been minimized.

Copy link
Contributor

dubnom commented Dec 16, 2018

@cgarwood

This comment has been minimized.

Copy link
Member

cgarwood commented Dec 21, 2018

Travis doesn't like one of the docstrings:
./homeassistant/components/light/homeworks.py:48:1: D401 First line should be in imperative mood

@cgarwood cgarwood merged commit 30841ef into home-assistant:dev Dec 21, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.008%) to 93.03%
Details

@wafflebot wafflebot bot removed the in progress label Dec 21, 2018

mxworm added a commit to mxworm/home-assistant that referenced this pull request Dec 22, 2018

Merge branch 'dev' into current
* dev: (22 commits)
  Update yale smart alarm client to v0.1.6 (home-assistant#19495)
  Add deprecation warning (home-assistant#19515)
  Restore state for zha binary_sensors on restart. (home-assistant#19314)
  Add ZHA battery sensor (home-assistant#19363)
  Clean up RFLink tests and add two tests (home-assistant#19511)
  Allow scrape sensor to retry setting up platform if initial setup fails (home-assistant#19498)
  Add Lutron Homeworks component (home-assistant#18311)
  Disable creating port mappings from UI, add discovery from component (home-assistant#18565)
  Bumped version to 0.84.6
  Remove check if base url is local (home-assistant#19494)
  Remove check if base url is local (home-assistant#19494)
  Bumped version to 0.84.5
  Bump pyharmony (home-assistant#19460)
  Bumped version to 0.84.4
  Use web sockets for Harmony HUB (home-assistant#19440)
  Fix IHC config schema (home-assistant#19415)
  Updated frontend to 20181211.2
  Bumped version to 0.84.3
  Fix not being able to update entities (home-assistant#19344)
  Fix restore state for manual alarm control panel (home-assistant#19284)
  ...

dshokouhi added a commit to dshokouhi/home-assistant that referenced this pull request Dec 25, 2018

Add Lutron Homeworks component (home-assistant#18311)
* Added Lutron Homeworks components.

* Made all requested changes other than config.

* Removed commented out code.

* Removed binary_sensor.
Implemented new signal/events for button presses.
Cleaned up some data passing.

* Fixed minor formatting.

* Got rid of unused config stuff.
Reordered imports.

* Missed removing vol, and forgot an extra line.

* More requested changes
Removed HomeworksController, it wasn't needed.

* Removed stale code.

* Imperative doc change.

@dubnom dubnom referenced this pull request Dec 30, 2018

Merged

Added Lutron Homeworks components. #7422

2 of 2 tasks complete

@balloob balloob referenced this pull request Jan 10, 2019

Merged

0.85.0 #19897

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment