Skip to content
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

deCONZ allow unloading of config entry #14115

Merged
merged 7 commits into from Apr 29, 2018

Conversation

Kane610
Copy link
Member

@Kane610 Kane610 commented Apr 27, 2018

Description:

Allow unloading deCONZ config entries and all its' entities

  • Unload works for binary sensor, light, scene and sensor components
  • Make removing DeconzEvents possible
  • Add new tests

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.

@Kane610 Kane610 requested a review from a team as a code owner April 27, 2018 11:16
@Kane610 Kane610 changed the title deCONZ allow unloading of config entry WIP: deCONZ allow unloading of config entry Apr 27, 2018
hass.services.async_remove(DOMAIN, SERVICE_DECONZ)
deconz.close()
for component in ['binary_sensor', 'light', 'scene', 'sensor']:
hass.async_add_job(hass.config_entries.async_forward_entry_unload(
Copy link
Member

Choose a reason for hiding this comment

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

No reason to async_add_job, you can just await them.

@@ -35,7 +35,7 @@
for sensor in sensors.values():
if sensor and sensor.type in DECONZ_SENSOR:
if sensor.type in DECONZ_REMOTE:
DeconzEvent(hass, sensor)
hass.data[DATA_DECONZ_EVENT].append(DeconzEvent(hass, sensor))
Copy link
Member Author

Choose a reason for hiding this comment

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

@balloob do you want me to move the creation of the events and the class definition to be created by the setup entry method instead?

Copy link
Member

Choose a reason for hiding this comment

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

Since it's just firing events, it should indeed be handled at the component level. It should not be done in the sensor platform.

@Kane610 Kane610 changed the title WIP: deCONZ allow unloading of config entry deCONZ allow unloading of config entry Apr 28, 2018
@@ -82,6 +87,10 @@
for component in ['binary_sensor', 'light', 'scene', 'sensor']:
hass.async_add_job(hass.config_entries.async_forward_entry_setup(
config_entry, component))

hass.data[DATA_DECONZ_EVENT] = [DeconzEvent(hass, sensor)
for sensor in deconz.sensors.values() if sensor.type in DECONZ_REMOTE]

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

CONF_API_KEY, CONF_HOST, CONF_PORT, EVENT_HOMEASSISTANT_STOP)
from homeassistant.core import callback
from homeassistant.const import (CONF_API_KEY, CONF_EVENT,
CONF_HOST, CONF_ID, CONF_PORT, EVENT_HOMEASSISTANT_STOP)

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

@balloob
Copy link
Member

balloob commented Apr 29, 2018

Looks good! Ok to merge when tests pass.

Btw I've added some config entry documentation to https://developers.home-assistant.io and would love some feedback on it since you're the biggest user now 🙂

@Kane610
Copy link
Member Author

Kane610 commented Apr 29, 2018

I'll have a look at it during the day 👍

@balloob balloob merged commit 3fd4987 into home-assistant:dev Apr 29, 2018
@balloob balloob mentioned this pull request May 11, 2018
@Kane610 Kane610 deleted the deconz-allow-unload-config-entry branch May 18, 2018 21:18
@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants