Add config flow for Rain Bird by allenporter · Pull Request #85271 · home-assistant/core

allenporter · 2023-01-06T00:55:12Z

Breaking change

Rainbird is now configured via the UI and configuration from YAML has been imported. However, there are a few notable changes:

Custom per zone overrides for irrigation time are no longer supported.
The Rain delay binary_sensor that duplicates the Rain delay time sensor has been removed. You should move all uses to the sensor value and check for a non-zero value to get equivalent behavior.
The Rain sensor that duplicates the Rain binary_sensor has been removed. You may check the binary_sensor state to get equivalent behavior.
The set_rain_delay now requires a configuration entry parameter. Previously the service had undefined behavior when multiple devices were configured.

Note for release notes maintainer: The last bullet can be superseded by the breaking change note in #86208

Proposed change

Add a config flow for Rain Bird. Existing YAML configuration is imported, with the exception of per-zone overrides for irrigation time which are not supported. Irrigation times can be specified with a service call.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request: Update Rain Bird documentation for config flow and yaml deprecation home-assistant.io#25614

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

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

Documentation added/updated for www.home-assistant.io

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

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

Convert rainbird to a config flow. Still need to handle irrigation numbers.

home-assistant · 2023-01-06T00:55:17Z

Hey there @konikvranik, mind taking a look at this pull request as it has been labeled with an integration (rainbird) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of rainbird can trigger bot actions by commenting:

@home-assistant close Closes the issue.
@home-assistant rename Awesome new title Change the title of the issue.
@home-assistant reopen Reopen the issue.
@home-assistant unassign rainbird Removes the current integration label and assignees on the issue, add the integration domain after the command.

MartinHjelmare · 2023-01-06T02:55:38Z

+        hass, "Rain delay", controller.get_rain_delay
+    )
+
+    hass.data[DOMAIN][entry.entry_id] = {


I recommend using a dataclass to store multiple items in hass.data.

Based on this comment and the comment about initializing the update coordinators in the platforms, I decided to just simplify the overall interactions between the device and the integration, moving all the rpcs into a single data update coordinator. This required changing the tests, so i added additional test fixtures to help manage the request setup and ordering expected by the integration.

The consequence of this is now a single update coordinator handles rpcs for all platforms so there isn't independence for each sensor, but in hindsight that is probably simpler overall (e.g. if the device is offline, there is just one coordinator trying to hit it rather than 3).

While I was here, I also removed some unnecessary duplicate sensors. Today there is a rain sensor and a rain delay sensor both exported as a sensor and binary sensor and it doesn't really make a lot of sense to me. I've just left the sensors that provide unique value and have updated the breaking changes in the description.

If this is too much for a single PR, I can break into smaller chunks, or move into another branch to not have to worry about leaving dev in a bad state.

It's not too much.

What did the four binary sensors and sensors represent? How would a user replace the removed sensors?

There were four total sensors representing two values that were redundant. I updated the PR to describe but can reword if it is not clear.

The "rain" sensor was a binary sensor and a sensor with a string boolean value. This represents whether the device knows it is raining (I'm not sure if it's from a separate sensors only or also from a cloud weather report)

The "rain delay" represents the number of days that the sprinkler is paused due to rain. This is a sensor and also had a binary sensor when it was non-zero. The rain delay service can update this value so maybe this should just be a Number?

Ok.

Yeah, if the rain delay service and sensor are in sync it should be a number entity.

We can deprecate the service and raise a repair issue telling users to move to the number entity and service when that's in place.

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

…ssistant-core into rainbird-config-entry

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

…e config entry

MartinHjelmare

Great!

Further work is good to do in follow ups. 👍

allenporter added 4 commits January 4, 2023 07:56

Rainbird config flow

48bbb21

Convert rainbird to a config flow. Still need to handle irrigation numbers.

Add options for irrigation time and deprecate yaml

ffd2b5f

Combine exception handling paths to get 100% test coverage

e16b0c6

Bump the rainird config deprecation release

49dcb7c

home-assistant Bot added cla-signed config-flow This integration migrates to the UI by adding a config flow has-tests integration: rainbird new-feature by-code-owner Quality Scale: No score labels Jan 6, 2023

MartinHjelmare requested changes Jan 6, 2023

View reviewed changes

allenporter and others added 8 commits January 5, 2023 20:10

Apply suggestions from code review

797cf9e

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

Merge branch 'rainbird-config-entry' of github.com:allenporter/home-a…

fb8ce86

…ssistant-core into rainbird-config-entry

Remove unnecessary sensor/binary sensor and address some PR feedback

cf019cc

Simplify configuration flow and options based on PR feedback

868a2de

Consolidate data update coordinators to simplify overall integration

8c1aa07

Fix type error on python3.9

1e019a6

Handle yaml name import

f82a533

Fix naming import post serialization

9b12dee

allenporter requested a review from MartinHjelmare January 6, 2023 19:47

Parallelize requests to the device

0cc4ec9

allenporter mentioned this pull request Jan 6, 2023

Update Rain Bird documentation for config flow and yaml deprecation home-assistant/home-assistant.io#25614

Merged

8 tasks

Complete conversion to entity service

9f8e2d7

MartinHjelmare reviewed Jan 6, 2023

View reviewed changes

Comment thread homeassistant/components/rainbird/switch.py Outdated

Comment thread homeassistant/components/rainbird/config_flow.py Outdated

Comment thread homeassistant/components/rainbird/switch.py Outdated

allenporter and others added 2 commits January 6, 2023 14:43

Update homeassistant/components/rainbird/switch.py

d8eb12e

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

Update homeassistant/components/rainbird/config_flow.py

63f43fb

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

MartinHjelmare reviewed Jan 6, 2023

View reviewed changes

Comment thread homeassistant/components/rainbird/__init__.py Outdated

MartinHjelmare added the breaking-change label Jan 6, 2023

allenporter added 3 commits January 6, 2023 17:34

Remove unused import

009c08d

Set default duration in options used in tests

b91275a

Add separate devices for each sprinkler zone and update service to us…

15edee1

…e config entry

MartinHjelmare approved these changes Jan 7, 2023

View reviewed changes

allenporter merged commit 5000c42 into home-assistant:dev Jan 7, 2023

github-actions Bot locked and limited conversation to collaborators Jan 8, 2023

Uh oh!

Conversation

allenporter commented Jan 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Breaking change

Proposed change

Type of change

Additional information

Checklist

Uh oh!

home-assistant Bot commented Jan 6, 2023

Uh oh!

Uh oh!

MartinHjelmare Jan 6, 2023

Choose a reason for hiding this comment

Uh oh!

allenporter Jan 6, 2023

Choose a reason for hiding this comment

Uh oh!

allenporter Jan 6, 2023

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare Jan 6, 2023

Choose a reason for hiding this comment

Uh oh!

allenporter Jan 6, 2023

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare Jan 7, 2023

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare Jan 7, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

allenporter commented Jan 6, 2023 •

edited

Loading