Skip to content

Add config entry for Flu Near You#32858

Merged
balloob merged 12 commits intohome-assistant:devfrom
bachya:fny-config-entry
Apr 2, 2020
Merged

Add config entry for Flu Near You#32858
balloob merged 12 commits intohome-assistant:devfrom
bachya:fny-config-entry

Conversation

@bachya
Copy link
Copy Markdown
Contributor

@bachya bachya commented Mar 16, 2020

Breaking change

The configuration.yaml schema for this integration has changed; users will need to reconfigure the integration.

Proposed change

This PR adds a config flow for Flu Near You. It also removes the ability to set the monitored_conditions configuration key (per ADR #0003).

Now that monitored_conditions cannot be configured, I should manage API usage (since disabling certain entities disables the need for those calls). I'm waiting on an answer #32291 (comment) – if it gets here in time, I'll include it here; otherwise, I'll include it in a future PR.

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)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml
flunearyou:

Additional information

  • This PR fixes or closes issue: N/A
  • This PR is related to issue: N/A
  • Link to documentation pull request: N/A

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.
    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.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

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

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 16, 2020

Monitored conditions should be removed if all the data is fetched from the same API call. If it requires more calls, it is ok to allow enabling/disabling that.

@bachya
Copy link
Copy Markdown
Contributor Author

bachya commented Mar 16, 2020

@balloob Two distinct API calls in this one.

Copy link
Copy Markdown
Member

@Kane610 Kane610 left a comment

Choose a reason for hiding this comment

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

Remember this is a major breaking change in regards to moving away from platform configuration, which isn't really reflected in the breaking change text.

Comment thread homeassistant/components/flunearyou/__init__.py Outdated
@bachya bachya requested a review from Kane610 March 21, 2020 03:42
Comment thread homeassistant/components/flunearyou/.translations/en.json Outdated
Comment thread homeassistant/components/flunearyou/__init__.py Outdated
Comment thread homeassistant/components/flunearyou/__init__.py Outdated
Comment on lines +104 to +111
async def refresh(event_time):
"""Refresh data from Flu Near You."""
await fny.async_update()

hass.data[DOMAIN][DATA_LISTENER][config_entry.entry_id] = async_track_time_interval(
hass, refresh, DEFAULT_SCAN_INTERVAL
)
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

Choose a reason for hiding this comment

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

Because I had no idea this was a thing? 😆

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.

A few things:

  1. FNY requires more than one API call. I could wrap them both in a single callback that gets passed to this, but that doesn't feel great.
    2 The Data Update Coordinator doesn't appear to handle disabling/enabling of entities and the appropriate removal/addition of API calls.

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.

Data Update Coordinators will only refresh if they have at least 1 listener. So you can create two coordinators and let entities subscribe.

@bachya bachya force-pushed the fny-config-entry branch from 8e07f1c to 799ad18 Compare March 31, 2020 19:54
@bachya bachya force-pushed the fny-config-entry branch from 799ad18 to 60f6f50 Compare April 2, 2020 18:21
@balloob balloob merged commit cb058ff into home-assistant:dev Apr 2, 2020
@bachya bachya deleted the fny-config-entry branch April 3, 2020 02:18
Comment thread homeassistant/components/flunearyou/__init__.py
Comment thread homeassistant/components/flunearyou/config_flow.py
Comment thread homeassistant/components/flunearyou/const.py
@bachya
Copy link
Copy Markdown
Contributor Author

bachya commented Apr 3, 2020

@MartinHjelmare Thank you! I’ll address these in a future PR.

@lock lock Bot locked and limited conversation to collaborators Apr 4, 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.

5 participants