Skip to content

Show a notification when a config entry is discovered#14022

Merged
pvizeli merged 4 commits intodevfrom
config-entry-discovery-notification
Apr 22, 2018
Merged

Show a notification when a config entry is discovered#14022
pvizeli merged 4 commits intodevfrom
config-entry-discovery-notification

Conversation

@balloob
Copy link
Copy Markdown
Member

@balloob balloob commented Apr 20, 2018

Description:

When we migrated Hue to use config entries for discovery, the user was no longer notified when we discovered it. This will reinstate that behavior with a generic notification for any config flow that is initiated by discovery. We will also automatically dismiss the notification when all discovered flows are finished.

Example entry for configuration.yaml (if applicable):

discovery:

Checklist:

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

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@balloob balloob force-pushed the config-entry-discovery-notification branch from 027ff34 to 119baeb Compare April 20, 2018 20:42
Comment thread homeassistant/config_entries.py Outdated
await async_setup_component(
self.hass, entry.domain, self._hass_config)

# remove notification if 0 flows with discovery source in progress
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.

This comment sounds weird looking at the code below it. I don't get it.

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.

I like the idea behind this code and make it faster but the comment was wrong

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

When we finish the last config flow that has source=discovery, we should remove the notification.

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.

Yes, but that's not what the code is doing. This comment was a misplaced duplicate from the one below.

Comment thread homeassistant/config_entries.py Outdated
hass.components.persistent_notification.async_create(
title='New devices discovered',
message=("We have discovered new devices on your network. "
"[Check it out](/config/integrations)"),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

Comment thread homeassistant/config_entries.py Outdated

# Create notification.
if source in DISCOVERY_SOURCES:
hass.components.persistent_notification.async_create(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

undefined name 'hass'

Comment thread homeassistant/config_entries.py Outdated
# If no discovery config entries in progress, remove notification.
if not any(ent['source'] in DISCOVERY_SOURCES for ent
in self.hass.config_entries.flow.async_progress()):
hass.components.persistent_notification.async_dismiss(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

undefined name 'hass'

Comment thread homeassistant/config_entries.py Outdated
hass.components.persistent_notification.async_create(
title='New devices discovered',
message=("We have discovered new devices on your network. "
"[Check it out](/config/integrations)"),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

Comment thread homeassistant/config_entries.py Outdated

# Create notification.
if source in DISCOVERY_SOURCES:
hass.components.persistent_notification.async_create(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

undefined name 'hass'

Comment thread homeassistant/config_entries.py Outdated
# If no discovery config entries in progress, remove notification.
if not any(ent['source'] in DISCOVERY_SOURCES for ent
in self.hass.config_entries.flow.async_progress()):
hass.components.persistent_notification.async_dismiss(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

undefined name 'hass'

@pvizeli pvizeli merged commit 5d34712 into dev Apr 22, 2018
@pvizeli pvizeli deleted the config-entry-discovery-notification branch April 22, 2018 19:00
@balloob balloob mentioned this pull request May 11, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Jul 26, 2018
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