8000 Cleanup Xiaomi Aqara by balloob · Pull Request #10302 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Cleanup Xiaomi Aqara #10302

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

Merged
merged 1 commit into from
Nov 3, 2017
Merged

Cleanup Xiaomi Aqara #10302

merged 1 commit into from
Nov 3, 2017

Conversation

balloob
Copy link
Member
@balloob balloob commented Nov 3, 2017

Description:

The Xiaomi Aqara component was broken by #10150. This fixes the services and cleans it up.

When opening a PR, please make sure that you run the code that you are submitting! If this would have gone out before 0.57, it would have broken for tons of users, impact our reputation and force us to spend time on doing a hot fix. That's not cool.

Bonus in this PR: gateway is optional in service schemas when the user has only 1 gateway.

Example entry for configuration.yaml (if applicable):

xiaomi_aqara:
  gateways:
   - mac: "00:00:00:00:00:00"
     key: "aaaaaaaaaaaaaaaa"

Checklist:

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 config.get(CONF_KEY) is None:
_LOGGER.warning(
'Key is not provided for gateway %s. Controlling the gateway '
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this a warning or should we mark this config as invalid?

Copy link
Member

Choose a reason for hiding this comment

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

It's a warning! We want to be able to use the gateway without a proper key.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we connect if we don't have a key?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. We are able the receive measurements. We are unable to control an actor without a key.

Copy link
Member Author

Choose a reason for hiding this comment

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

ooh nice, I didn't realise that. Very cool.

Copy link
Member
@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Code looks good. I don't know the xiaomi aqara system though.

@syssi
Copy link
Member
syssi commented Nov 3, 2017

I'm sorry for the mess. It was my fault!

@balloob
Copy link
Member Author
balloob commented Nov 3, 2017

It's okay @syssi, managed to catch it in time. Please run your code next time locally 👍

@balloob balloob merged commit 8f774e9 into dev Nov 3, 2017
@balloob balloob deleted the fix-xiaomi-aqara branch November 3, 2017 05:18
@syssi
Copy link
Member
syssi commented Nov 3, 2017

Promised!

balloob added a commit that referenced this pull request Nov 4, 2017
@balloob balloob mentioned this pull request Nov 4, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Mar 2, 2018
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.

5 participants
0