Move RainMachine to component/hub model #14085

bachya · 2018-04-26T02:57:03Z

Description:

This is the first of several PRs designed to add more functionality to the RainMachine integration. The first is this, which moves the integration from existing fully in a switch platform to being a combo component + switch platform (a la Arlo, et. al.). No functionality changes, per se, in this PR.

This will be the master PR; fine to merge once it passes tests and review, but others will be held back until this is merged.

BREAKING CHANGE 1: with the migration to the component/hub model, existing configuration files will break. The new format (below) should be used.

BREAKING CHANGE 2: after intense amounts of fiddling, it's clear to me that the RainMachine remote API is broken, clunky, etc. After some investigation, I haven't found a single user who's using it, so using this PR as a time to remove it and references to it.

Related issue (if applicable): N/A

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

Example entry for `configuration.yaml` (if applicable):

rainmachine:
  ip_address: 192.168.1.100
  password: abc123

switch:
  - platform: rainmachine
    zone_run_time: 300

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass

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

Documentation added/updated in home-assistant.github.io

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

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.

houndci-bot · 2018-04-26T02:57:10Z

homeassistant/components/rainmachine.py

+from datetime import timedelta
+
+import voluptuous as vol
+from requests.exceptions import HTTPError, ConnectTimeout


'requests.exceptions.HTTPError' imported but unused
'requests.exceptions.ConnectTimeout' imported but unused

MartinHjelmare · 2018-04-26T04:30:53Z

homeassistant/components/switch/rainmachine.py

+        _LOGGER.debug('Adding program: %s', program)
+        entities.append(
+            RainMachineProgram(
+                client,


Please keep the existing formatting, by having as many parameters on one line as possible.

MartinHjelmare · 2018-04-26T04:35:23Z

homeassistant/components/rainmachine.py

+    from regenmaschine import Authenticator, Client
+    from regenmaschine.exceptions import HTTPError
+
+    _LOGGER.debug('Config data: %s', config)


This will log the whole config. Please remove.

MartinHjelmare · 2018-04-26T04:36:49Z

homeassistant/components/rainmachine.py

+    _LOGGER.debug('Config data: %s', config)
+
+    conf = config[DOMAIN]
+    ip_address = conf.get(CONF_IP_ADDRESS, None)


None is the default value returned.

Not sure what you mean by this; all other components I've looked at return True or False. Are you saying that I should be returning None, or that I shouldn't and I somehow am?

dict.get(key) by default returns None if the key is missing in the dict. So please just write conf.get(CONF_IP_ADDRESS).

Sorry about not being clear.

Got it! Will update.

MartinHjelmare · 2018-04-26T04:37:55Z

homeassistant/components/rainmachine.py

+            port = conf[CONF_PORT]
+            ssl = conf[CONF_SSL]
+            auth = Authenticator.create_local(
+                ip_address,


Please keep as many arguments as possible on one line.

MartinHjelmare · 2018-04-26T04:39:08Z

homeassistant/components/rainmachine.py

+                password,
+                port=port,
+                https=ssl)
+            _LOGGER.debug('Configuring local API: %s', auth)


Will this log credentials? If so remove.

MartinHjelmare · 2018-04-26T04:39:24Z

homeassistant/components/rainmachine.py

+            _LOGGER.debug('Configuring local API: %s', auth)
+        elif email_address:
+            auth = Authenticator.create_remote(email_address, password)
+            _LOGGER.debug('Configuring remote API: %s', auth)


Same as above.

MartinHjelmare · 2018-04-27T04:50:26Z

homeassistant/components/rainmachine.py

+
+    conf = config[DOMAIN]
+    ip_address = conf.get(CONF_IP_ADDRESS)
+    email_address = conf.get(CONF_EMAIL)


This isn't needed anymore with the new config schema.

MartinHjelmare · 2018-04-27T04:51:43Z

homeassistant/components/rainmachine.py

+        _decorator = decorator
+    else:
+
+        @Throttle(MIN_SCAN_TIME_REMOTE, MIN_SCAN_TIME_FORCED)


This can also be removed.

MartinHjelmare · 2018-04-27T04:52:01Z

homeassistant/components/rainmachine.py

+            auth = Authenticator.create_local(
+                ip_address, password, port=port, https=ssl)
+            _LOGGER.debug('Configuring local API: %s', ip_address)
+        elif email_address:


Same as above.

MartinHjelmare · 2018-04-27T04:55:01Z

homeassistant/components/rainmachine.py

+    from regenmaschine.exceptions import HTTPError
+
+    conf = config[DOMAIN]
+    ip_address = conf.get(CONF_IP_ADDRESS)


With the new schema this should be:

ip_address = conf[CONF_IP_ADDRESS]

MartinHjelmare

Looks good! Can be merged when build passes.

bachya added 3 commits April 25, 2018 20:47

Moves RainMachine to component/hub model

0971d71

Updated requirements

8867744

Updated coverage

a2276d2

homeassistant added new-platform platform: switch.rainmachine integration: rainmachine cla-signed labels Apr 26, 2018

houndci-bot reviewed Apr 26, 2018

View reviewed changes

Hound violations

f7db849

This was referenced Apr 26, 2018

Adds docs for RainMachine component/hub home-assistant/home-assistant.io#5251

Merged

WIP: Adds useful attributes to RainMachine programs and zones #14086

Closed

Adds useful attributes to RainMachine programs and zones #14087

Merged

MartinHjelmare reviewed Apr 26, 2018

View reviewed changes

bachya added 4 commits April 26, 2018 10:07

Collaborator-requested changes

bebb949

Small formatting updates

7682b2e

Removed references to remote API

4c19f2a

Collaborator-requested changes

7ef79c1

MartinHjelmare reviewed Apr 27, 2018

View reviewed changes

Collaborator-requested changes

0eab1dd

MartinHjelmare approved these changes Apr 27, 2018

View reviewed changes

Fixed attribution

dd6b241

homeassistant added the cla-signed label Apr 27, 2018

MartinHjelmare added the breaking-change label Apr 28, 2018

MartinHjelmare changed the title ~~Moves RainMachine to component/hub model~~ Move RainMachine to component/hub model Apr 28, 2018

MartinHjelmare merged commit 8bc497b into home-assistant:dev Apr 28, 2018

bachya deleted the rainmachine-1 branch April 30, 2018 21:02

bachya mentioned this pull request May 1, 2018

Converts RainMachine to hub model (part 2) #14225

Merged

3 tasks

balloob mentioned this pull request May 11, 2018

0.69 #14392

Merged

home-assistant locked and limited conversation to collaborators Sep 5, 2018

ghost removed the platform: switch.rainmachine label Mar 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move RainMachine to component/hub model #14085

Move RainMachine to component/hub model #14085

bachya commented Apr 26, 2018 •

edited

Loading

houndci-bot Apr 26, 2018

MartinHjelmare Apr 26, 2018

MartinHjelmare Apr 26, 2018

MartinHjelmare Apr 26, 2018

bachya Apr 26, 2018 •

edited

Loading

MartinHjelmare Apr 26, 2018

bachya Apr 26, 2018

MartinHjelmare Apr 26, 2018

MartinHjelmare Apr 26, 2018

MartinHjelmare Apr 26, 2018

MartinHjelmare Apr 27, 2018

MartinHjelmare Apr 27, 2018

MartinHjelmare Apr 27, 2018

MartinHjelmare Apr 27, 2018

MartinHjelmare left a comment

Move RainMachine to component/hub model #14085

Move RainMachine to component/hub model #14085

Conversation

bachya commented Apr 26, 2018 • edited Loading

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bachya Apr 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare left a comment

Choose a reason for hiding this comment

bachya commented Apr 26, 2018 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):

bachya Apr 26, 2018 •

edited

Loading