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 sensors and services to RainMachine #14326

Merged
merged 35 commits into from May 29, 2018

Conversation

@bachya
Contributor

bachya commented May 7, 2018

Description:

Adds several (binary) sensors to the RainMachine component:

  • Binary Sensor: Extra Water on Hot Days
  • Binary Sensor: Freeze Protection
  • Binary Sensor: Freeze Restrictions
  • Binary Sensor: Hourly Restrictions
  • Binary Sensor: Month Restrictions
  • Binary Sensor: Rain Delay Restrictions
  • Binary Sensor: Rain Sensor Restrictions
  • Binary Sensor: Weekday Restrictions
  • Sensor: Freeze Protect Temperature

Additionally, this PR adds several services:

  • start_program
  • start_zone
  • stop_all
  • stop_program
  • stop_zone

Related issue (if applicable): N/A

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

Example entry for configuration.yaml (if applicable):

rainmachine:
  ip_address: 192.168.1.100
  password: abc123

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:

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.

@bachya bachya changed the title from Adds sensors and services to RainMachine to WIP: Adds sensors and services to RainMachine May 7, 2018

@bachya bachya referenced this pull request May 7, 2018

Merged

Adds documentation for latest RainMachine PR #5322

2 of 2 tasks complete

@bachya bachya changed the title from WIP: Adds sensors and services to RainMachine to Adds sensors and services to RainMachine May 9, 2018

@bachya

This comment has been minimized.

Contributor

bachya commented May 16, 2018

FYI: been running this in my local HASS for a week or so and everything seems to work fine.

homeassistant/components/binary_sensor/rainmachine.py Outdated
binary_sensors = []
for sensor_type in discovery_info.get(
CONF_MONITORED_CONDITIONS, SENSORS.keys()):

This comment has been minimized.

@balloob

balloob May 18, 2018

Member

No need for .keys()

@@ -1,132 +0,0 @@
"""

This comment has been minimized.

@balloob

balloob May 18, 2018

Member

Please use git mv <X> <Y> to keep history attached to a file

This comment has been minimized.

@bachya

bachya May 18, 2018

Contributor

Not sure what you mean here – I performed git mv homeassistant/components/rainmachine.py homeassistant/components/rainmachine/__init__.py, but the result appears to be the same as before.

homeassistant/components/binary_sensor/rainmachine.py Outdated
ATTR_RAINSENSOR = 'rainsensor'
ATTR_WEEKDAY = 'weekday'
SENSORS = {

This comment has been minimized.

@balloob

balloob May 18, 2018

Member

No device class ?

This comment has been minimized.

@bachya

bachya May 18, 2018

Contributor

These are all device configurations, not representations of conditions. Example: TYPE_FREEZE means "Is the device currently restricted from watering because it's too cold outside?" That doesn't seem to fit the spirit of the cold device class. The same is true for the other sensors.

That said, let me know if you'd rather they be coerced into that format and I'll get it done.

homeassistant/components/binary_sensor/rainmachine.py Outdated
name, icon = SENSORS[sensor_type]
binary_sensors.append(
RainMachineBinarySensor(rainmachine, sensor_type, name, icon))
except KeyError:

This comment has been minimized.

@balloob

balloob May 18, 2018

Member

Don't catch this, instead a voluptuous schema should make sure we never get bad keys.

homeassistant/components/binary_sensor/rainmachine.py Outdated
def __init__(self, rainmachine, sensor_type, name, icon):
"""Initialize."""
super().__init__(
rainmachine, 'binary_sensor', sensor_type, name, icon=icon)

This comment has been minimized.

@balloob

balloob May 18, 2018

Member

This seems like the RainMachineEntity is overcomplicated. The logic is hard to follow.

homeassistant/components/binary_sensor/rainmachine.py Outdated
async def async_added_to_hass(self):
"""Register callbacks."""
def update_data():

This comment has been minimized.

@balloob

balloob May 18, 2018

Member

You need to annotate this as callback or else it will run synchronous

homeassistant/components/binary_sensor/rainmachine.py Outdated
def update(self):
"""Update the state."""
if self._sensor_type == ATTR_FREEZE:

This comment has been minimized.

@balloob

balloob May 18, 2018

Member

Why are the keys called ATTR? Shouldn't it be TYPE or something ?

homeassistant/components/rainmachine/__init__.py Outdated
rainmachine = RainMachine(hass, Client(auth))
rainmachine.update()
hass.data[DATA_RAINMACHINE] = rainmachine
except (HTTPError, ConnectTimeout, UnboundLocalError) as exc_info:

This comment has been minimized.

@balloob

balloob May 18, 2018

Member

What are these errors? Why isn't Rainmachine raising their own error type?

homeassistant/components/rainmachine/__init__.py Outdated
discovery.load_platform(
hass, 'binary_sensor', DOMAIN, binary_sensor_config, config)
_LOGGER.debug('Setting up sensor platform')

This comment has been minimized.

@balloob

balloob May 18, 2018

Member

Please remove some of the debugs.

homeassistant/components/sensor/rainmachine.py Outdated
if discovery_info is None:
return
_LOGGER.debug('Config received: %s', discovery_info)

This comment has been minimized.

@balloob

balloob May 18, 2018

Member

Do not log config

homeassistant/components/sensor/rainmachine.py Outdated
async def async_added_to_hass(self):
"""Register callbacks."""
def update_data():

This comment has been minimized.

@balloob

balloob May 18, 2018

Member

make a callback.

homeassistant/components/binary_sensor/rainmachine.py Outdated
for sensor_type in discovery_info.get(CONF_MONITORED_CONDITIONS,
BINARY_SENSORS):
binary_sensors.append(
RainMachineBinarySensor(rainmachine, sensor_type, name))

This comment has been minimized.

@houndci-bot

houndci-bot May 18, 2018

undefined name 'name'

@bachya

This comment has been minimized.

Contributor

bachya commented May 28, 2018

@balloob I’m sure you’re slammed, but now that 0.70 is out, can we get this stitched up and out the door? Let me know what else you’d like done on this one.

bachya added some commits May 7, 2018

@MartinHjelmare

Ok, merging. 👍

@MartinHjelmare MartinHjelmare changed the title from Adds sensors and services to RainMachine to Add sensors and services to RainMachine May 29, 2018

@MartinHjelmare MartinHjelmare merged commit 084b328 into home-assistant:dev May 29, 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 remained the same at 93.961%
Details

@bachya bachya deleted the bachya:rainmachine-3 branch May 29, 2018

@balloob balloob referenced this pull request Jun 8, 2018

Merged

0.71.0 #14876

cyberjacob pushed a commit to cyberjacob/home-assistant that referenced this pull request Sep 4, 2018

Add sensors and services to RainMachine (home-assistant#14326)
* Starting to add attributes

* All attributes added to programs

* Basic zone attributes in place

* Added advanced properties for zones

* We shouldn't calculate the MAC with every entity

* Small fixes

* Basic framework for push in play

* I THINK IT'S WORKING

* Some state cleanup

* Restart

* Restart part 2

* Added stub for service schema

* Update

* Added services

* Small service description update

* Lint

* Updated CODEOWNERS

* Moving to async methods

* Fixed coverage test

* Lint

* Removed unnecessary hass reference

* Lint

* Lint

* Round 1 of Owner-requested changes

* Round 2 of Owner-requested changes

* Round 3 of Owner-requested changes

* Round 4 (final for now) of Owner-requested changes

* Hound

* Updated package requirements

* Lint

* Collaborator-requested changes

* Collaborator-requested changes

* More small tweaks

* One more small tweak

* Bumping Travis and Coveralls

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

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