Skip to content
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

upgrade to new pylutron_caseta with TLS #10286

Merged
merged 4 commits into from
Nov 10, 2017

Conversation

mdonoughe
Copy link
Contributor

@mdonoughe mdonoughe commented Nov 2, 2017

Description:

Lutron released a firmware updated for the Caseta system which removed our ability to connect to and control the bridge device over SSH, breaking compatibility with pylutron_caseta and Home Assistant. This is an update to pull in the new version of pylutron_caseta with support for connecting over TLS. See gurumitts/pylutron-caseta#15 and gurumitts/pylutron-caseta#16.

This should all be non-blocking now. I'm not sure if I updated all the appropriate lutron_caseta.py files in Home Assistant to take advantage of that but it seems to work.

The documentation does need to be updated but I'm not sure how best to do it. The setup process is a bit rough because it involves doing an OAuth login, but your Home Assistant instance is not a valid callback URI so the user isn't going to be able to click a link in Home Assistant and sign in to generate the key. What I did to get my instance running was to run the script referenced from the top of gurumitts/pylutron-caseta#15, separately use openssl to retrieve the CA certificate from the bridge (the parent certificate returned from the script is for your certificate chain and is not the certificate of the bridge itself), and collect the parts into the appropriate files. I don't know if there is some precedent for handling this sort of thing. Should Home Assistant have a UI that opens the OAuth login in a new tab and tells the user to copy the URL of the error page at the end? I don't see a way individual Home Assistant users could easily sign up with Lutron as developers and get their own proper OAuth clients registered and I don't know of any easier way of getting client keys.

Related issue (if applicable):
fixes #9730, fixes #9374

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

Example entry for configuration.yaml (if applicable):

lutron_caseta:
  host: 192.168.86.49
  keyfile: caseta.key
  certfile: caseta.crt
  ca_certs: caseta-bridge.crt

Checklist:

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

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

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New 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:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@mdonoughe mdonoughe requested a review from andrey-git as a code owner November 2, 2017 02:48
@homeassistant
Copy link
Contributor

Hi @mdonoughe,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!


_LOGGER = logging.getLogger(__name__)

LUTRON_CASETA_SMARTBRIDGE = 'lutron_smartbridge'

DOMAIN = 'lutron_caseta'

CONF_KEYFILE = 'key'
CONF_CERTFILE = 'cert'
CONF_CA_CERTS = 'ca'
Copy link
Member

Choose a reason for hiding this comment

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

I think the name are a bit wrong. make certfile, keyfile, ca_certs

Copy link
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

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

If you library is now really async, you need convert everything to async. Any platform and any call need run inside loop.

@@ -13,8 +14,8 @@

DEPENDENCIES = ['lutron_caseta']


def setup_platform(hass, config, add_devices, discovery_info=None):
@asyncio.coroutine

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

@mdonoughe
Copy link
Contributor Author

I think I made everything properly async now. The documentation (https://home-assistant.io/developers/asyncio_categorizing_functions/) makes it look like you can mark things as @callback instead of @asyncio.coroutine if you aren't actually writing a coroutine, but that only seems to be true in certain cases which don't apply here.

Copy link
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

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

Looks good

@@ -18,7 +19,8 @@


# pylint: disable=unused-argument
def setup_platform(hass, config, add_devices, discovery_info=None):
@asyncio.coroutine
def async_setup_platform(hass, config, add_devices, discovery_info=None):
Copy link
Member

Choose a reason for hiding this comment

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

rename add_devices to async_add_devices

@pvizeli pvizeli added this to the 0.57 milestone Nov 3, 2017
@pvizeli
Copy link
Member

pvizeli commented Nov 3, 2017

If you fast enough, it can go into 0.57

@mdonoughe
Copy link
Contributor Author

@pvizeli what should I do about the setup process? Should I just document the complicated process for now?

@balloob
Copy link
Member

balloob commented Nov 4, 2017

This is not going to make 0.57 since it's missing a paragraph to add to the release notes about the breaking change and there is no documentation PR ready.

@mdonoughe
Copy link
Contributor Author

I went ahead created a Python fork of the setup Gist and opened a pull request on the documentation project.

@pvizeli pvizeli merged commit e7dc963 into home-assistant:dev Nov 10, 2017
@balloob balloob mentioned this pull request Nov 17, 2017
@Coolie1101
Copy link

I tried following your the instructions given, which was going great, until I got to installing cryptography.

I’ts showing the following error, any ideas?

"Command python setup.py egg_info failed with error code 1 in /tmp/pip-build-j7vd07be/cryptography
Storing debug log for failure in /home/pi/.pip/pip.log"

When I run “python get_lutron_cert.py”

I get the following error:

Traceback (most recent call last):
File "get_lutron_cert.py", line 25, in
import requests
ImportError: No module named 'requests'

@MartinHjelmare
Copy link
Member

Please open an issue if you suspect a bug. If you need help please use our help channels:
https://home-assistant.io/help/#communication-channels

Merged PRs should not be used for support or bug reports.

@home-assistant home-assistant locked and limited conversation to collaborators Nov 19, 2017
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.

Lutron Caseta component is failing to make SSH connection to hub Lutron Caséta Lights missing
8 participants