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

Prefix all xiaomi_aqara events #17354

Merged
merged 1 commit into from Nov 19, 2018

Conversation

Projects
None yet
7 participants
@syssi
Member

syssi commented Oct 12, 2018

Description:

It was wrong from the beginning. This integration now behaves badly by firing events not specifically identifiable.

All events are prefixed now by "xiaomi_aqara". Please update your automations:

motion -> xiaomi_aqara.motion
click -> xiaomi_aqara.click
cube_action -> xiaomi_aqara.cube_action

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

@syssi syssi requested a review from Danielhiversen as a code owner Oct 12, 2018

@wafflebot wafflebot bot added the in progress label Oct 12, 2018

@syssi syssi changed the title from Prefix all xiaomi_aqara events to RFC: Prefix all xiaomi_aqara events Oct 12, 2018

@Kane610

This comment has been minimized.

Contributor

Kane610 commented Oct 12, 2018

Since you're doing breaking changes, you should fix the device types being entities but firing custom events instead of keeping states.

Entities only being exposed using custom events should be handled in the hub and not in a platform.

You can see deCONZ as one example of this.

@balloob

This comment has been minimized.

Member

balloob commented Oct 12, 2018

@Kane610 is right, move these events to the component.

@rytilahti

This comment has been minimized.

Contributor

rytilahti commented Oct 12, 2018

I thought I'll write also my thoughts here, too. I'm wondering what are the pros of prefixing the events like this? The events being fired will deliver the entity id and are thus identifiable already, and I'm afraid that having a variety of naming schemes for similar events leads to more confusion among users. On top of that it makes it harder to share automations and upgrade/swap hardware (e.g. instead of simply updating the entity-id, you have to change also the events). Do the pros really outweigh those cons?

@balloob

This comment has been minimized.

Member

balloob commented Oct 16, 2018

The reason we want unique events because we haven't standardized events yet. If we want to do that, we can open an architecture issue. This hasn't been done and so platform prefixed it is.

@syssi

This comment has been minimized.

Member

syssi commented Nov 17, 2018

We should standardize the events first. Closing this.

@syssi syssi closed this Nov 17, 2018

@wafflebot wafflebot bot removed the in progress label Nov 17, 2018

@balloob

This comment has been minimized.

Member

balloob commented Nov 18, 2018

I disagree. We should rename these events first to make sure they follow our existing policy of event naming. Then we can try to standardize things.

@syssi

This comment has been minimized.

Member

syssi commented Nov 18, 2018

I fear the breaking change.

@balloob

This comment has been minimized.

Member

balloob commented Nov 18, 2018

Imagine that we standardize it, that the format or maybe even the intention of the events change, and we end up triggering a lot more false positive events?

@syssi syssi reopened this Nov 18, 2018

@wafflebot wafflebot bot added the in progress label Nov 18, 2018

@balloob

This comment has been minimized.

Member

balloob commented Nov 19, 2018

so let's merge? 👍

@syssi syssi merged commit 14ad742 into home-assistant:dev Nov 19, 2018

4 of 5 checks passed

coverage/coveralls Coverage decreased (-0.5%) to 93.047%
Details
Hound No violations found. Woof!
WIP Legacy commit status override — see details
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wafflebot wafflebot bot removed the in progress label Nov 19, 2018

@syssi syssi changed the title from RFC: Prefix all xiaomi_aqara events to Prefix all xiaomi_aqara events Nov 19, 2018

mxworm added a commit to mxworm/home-assistant that referenced this pull request Nov 19, 2018

Merge branch 'dev' of https://github.com/home-assistant/home-assistant
…into dev

* 'dev' of https://github.com/home-assistant/home-assistant:
  Bumped ghlocalapi to 0.1.0 (home-assistant#18584)
  Prefix all xiaomi_aqara events (home-assistant#17354)
  Fix MQTT async_added_to_hass (home-assistant#18575)
  Add mikrotik SSL support (home-assistant#17898)
  Reconfigure MQTT binary_sensor component if discovery info is changed (home-assistant#18169)
  Darksky: Expose missing conditions for day 0 forecast (home-assistant#18312)
  Update pyhomematic to 0.1.52 and add features for lights (home-assistant#18499)
  Fix for epson state not updating (home-assistant#18357)
  Support for Point component (home-assistant#17466)
  Template binary sensor to not track all state changes (home-assistant#18573)
  Correct cached stale device tracker handling (home-assistant#18572)
  Add support for sessions (home-assistant#18518)
  Remove turn_on and turn_off feature for clients (home-assistant#18234)
  Log delay and wait_template steps in scripts (home-assistant#18448)
  Logbook speedup (home-assistant#18376)
  Avoid race in entity_platform.async_add_entities() (home-assistant#18445)
  Fix small issue related to topic prefix (home-assistant#18512)
  Enable native support + ADB authentication for Fire TV (home-assistant#17767)
  Re-adding the season attribute (home-assistant#18523)
@frenck

This comment has been minimized.

Member

frenck commented Nov 19, 2018

@syssi Docs?!

@syssi

This comment has been minimized.

Member

syssi commented Nov 19, 2018

@frenck

This comment has been minimized.

Member

frenck commented Nov 19, 2018

Thanks, @syssi! 👍

@frenck frenck removed the docs-missing label Nov 19, 2018

@balloob balloob referenced this pull request Nov 29, 2018

Merged

0.83 #18776

@cnrd cnrd referenced this pull request Nov 29, 2018

Open

Standardizing events #115

@blakadder blakadder referenced this pull request Nov 30, 2018

Merged

get the docs in line with PR #7689

0 of 2 tasks complete

thomaswr added a commit to thomaswr/homeassistant-config that referenced this pull request Dec 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment