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

Update pyhomematic to 0.1.53 #19056

Merged
merged 1 commit into from
Dec 6, 2018
Merged

Update pyhomematic to 0.1.53 #19056

merged 1 commit into from
Dec 6, 2018

Conversation

danielperna84
Copy link
Contributor

@danielperna84 danielperna84 commented Dec 5, 2018

Description:

  • Fixes unknown-parameter-errors
  • Adds support for SSL connections with CCU3
  • Adds support for authentication with CCU3
  • Adds support for hostnames as XML-RPC endpoints
  • Adds support for devices: HmIP-eTRV-B, HmIP-FDT, HB-UW-Sen-THPL-O, HB-UW-Sen-THPL-I, HmIP-B1

I have set DEFAULT_VERIFY_SSL to False because I am pretty sure, that at most an insignificant minority of users will have valid certificates configured on their CCU3.

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

Example entry for configuration.yaml (if applicable):

# Example for connecting to a SSL enabled CCU3 with active authentication
homematic:
  interfaces:
    wireless:
      host: 192.168.1.23
      port: 42001
      ssl: true
      verify_ssl: false # Added for completeness since false is the default
      username: Admin
      password: secret

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 or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.

@@ -173,14 +175,17 @@
DEFAULT_PATH = ''
DEFAULT_USERNAME = 'Admin'
DEFAULT_PASSWORD = ''
DEFAULT_SSL = False
DEFAULT_VERIFY_SSL = False
Copy link
Member

Choose a reason for hiding this comment

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

We should default verify to True.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although I generally agree, for this case (and comparable ones) it wouldn't make sense to verify by default. The reason is, that the CCU-device never is a cloud device, it's always on premise. It can be exposed to the web, but that's a really bad idea. So typically nobody does that. And it's unrealistic, that people will use trusted certificates for a machine that's only local. They might have their custom CA. But last time I checked I couldn't make Home Assistant use a custom CA (virtualenv).
So essentially: having verify_ssl with True as default will be a broken-by-default configuration.

@balloob balloob merged commit 47320ad into home-assistant:dev Dec 6, 2018
@ghost ghost removed the in progress label Dec 6, 2018
@balloob balloob mentioned this pull request Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants