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

Clean up Light Groups #12962

Merged
merged 4 commits into from
Mar 9, 2018
Merged

Conversation

OttoWinter
Copy link
Member

@OttoWinter OttoWinter commented Mar 7, 2018

Description:

  • In the docs and some of the code, we refer to "Light Groups"; the "Group Light" name is from the old GroupedLight implementation and makes less sense.
  • Move getting the initial state from async_add_devices to async_added_to_hass (Fix flaky light group test #12843 (comment)); The former does the async_update before we register our callback. Theoretically it could happen that a state is published between when add_deviceswhen our subscription is established. Then we would miss a state.
  • Remove light groups from .coveragerc. I didn't know what that file did when doing the initial PR.

Related PR (if applicable): #12229

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4739

Example entry for configuration.yaml (if applicable):

light:
  - platform: group
    name: Kitchen Lights
    entities:
      - light.kitchen_ceiling_lights
      - light.kitchen_under_cabinet_lights
      - light.kitchen_spot_lights
      - light.pendant_lights

Checklist:

  • The code change is tested and works locally.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@@ -79,10 +79,11 @@ def async_state_changed_listener(entity_id: str, old_state: State,

self._async_unsub_state_changed = async_track_state_change(
self.hass, self._entity_ids, async_state_changed_listener)
self.async_schedule_update_ha_state(True)
Copy link
Member

Choose a reason for hiding this comment

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

You should just call await self.async_update() here, updating state is done right after the call to async_added_to_hass: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/helpers/entity_platform.py#L257-L260

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, thanks.

c727 pushed a commit to home-assistant/home-assistant.io that referenced this pull request Mar 9, 2018
* Add Light Group platform docs

* Group Light →Light Group

* A picture tells a thousand words

* Update for Light Group default name change.

See home-assistant/core#12962
@OttoWinter OttoWinter added this to the 0.65 milestone Mar 9, 2018
@OttoWinter
Copy link
Member Author

Putting this in the 0.65 milestone because the merged docs have the updated default name "Light Group".

corneyl pushed a commit to corneyl/home-assistant.github.io that referenced this pull request Mar 9, 2018
* Add Light Group platform docs

* Group Light →Light Group

* A picture tells a thousand words

* Update for Light Group default name change.

See home-assistant/core#12962
@balloob balloob merged commit ca5f470 into home-assistant:dev Mar 9, 2018
balloob pushed a commit that referenced this pull request Mar 9, 2018
* Clean up Light Groups

* Fix tests

* Remove light group from .coveragerc

* async_schedule_update_ha_state called anyway
@balloob balloob mentioned this pull request Mar 9, 2018
@OttoWinter OttoWinter deleted the light-group-name branch March 13, 2018 19:16
@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.

None yet

3 participants