Skip to content

Remove monitored conditions from RainMachine#31066

Merged
balloob merged 5 commits into
home-assistant:devfrom
bachya:rainmachine-monitored-conditions
Jan 23, 2020
Merged

Remove monitored conditions from RainMachine#31066
balloob merged 5 commits into
home-assistant:devfrom
bachya:rainmachine-monitored-conditions

Conversation

@bachya
Copy link
Copy Markdown
Contributor

@bachya bachya commented Jan 21, 2020

Breaking change

It is no longer possible to specify monitored conditions within the RainMachine integration; all entities are added by default.

Additionally, the zone_run_time parameter is now configured directly within the controller within configuration.yaml.

Proposed change

Per ADR 0003, this PR removes the concept of monitored conditions from RainMachine. All entity types are added by default. The integration uses more than one local API call, but since the created entities all connect together, I didn't see a problem automatically including everything.

This PR also includes a config entry migration to clean up monitored conditions data as necessary.

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix or feature that would cause existing functionality not to work as expected)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

rainmachine:
  controllers:
    - ip_address: !secret rainmachine_ip_address
      password: !secret rainmachine_password

Additional information

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:

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

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

Comment thread homeassistant/components/rainmachine/__init__.py Outdated
Comment thread homeassistant/components/rainmachine/binary_sensor.py Outdated
Comment thread homeassistant/components/rainmachine/binary_sensor.py Outdated
@bachya bachya force-pushed the rainmachine-monitored-conditions branch from b791f59 to fe127f4 Compare January 23, 2020 02:32

tasks = {}

if TYPE_FLOW_SENSOR in self.binary_sensor_conditions or any(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So now you're fetching all data. One more improvement that you can do in a future PR is that each entity will let the data class know what data it is interested in. That way if all entities of a certain type are disabled, you will no longer fetch the data.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

@bachya bachya Jan 23, 2020

Choose a reason for hiding this comment

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

How do you listen for the entity being enabled or disabled?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

async_added_to_hass and async_will_remove_from_hass?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, use the lifecycle methods to opt-in for data. If data is not being fetched yet, that's when you start fetching that data. In your case you could just track sensor types and count how many need them. Ring has a variant on that too: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/ring/__init__.py#L238

@balloob balloob merged commit 73a5582 into home-assistant:dev Jan 23, 2020
@bachya bachya deleted the rainmachine-monitored-conditions branch January 23, 2020 15:34
@bachya bachya mentioned this pull request Jan 23, 2020
20 tasks
@lock lock Bot locked and limited conversation to collaborators Jan 24, 2020
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.

4 participants