Skip to content

Prefix all xiaomi_aqara events#17354

Merged
syssi merged 1 commit into
home-assistant:devfrom
syssi:feature/prefix-xiaomi-aqara-events
Nov 19, 2018
Merged

Prefix all xiaomi_aqara events#17354
syssi merged 1 commit into
home-assistant:devfrom
syssi:feature/prefix-xiaomi-aqara-events

Conversation

@syssi
Copy link
Copy Markdown
Member

@syssi 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 October 12, 2018 07:03
@ghost ghost assigned syssi Oct 12, 2018
@ghost ghost added the in progress label Oct 12, 2018
@syssi syssi changed the title Prefix all xiaomi_aqara events RFC: Prefix all xiaomi_aqara events Oct 12, 2018
@Kane610
Copy link
Copy Markdown
Member

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
Copy link
Copy Markdown
Member

balloob commented Oct 12, 2018

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

@rytilahti
Copy link
Copy Markdown
Member

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
Copy link
Copy Markdown
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
Copy link
Copy Markdown
Member Author

syssi commented Nov 17, 2018

We should standardize the events first. Closing this.

@syssi syssi closed this Nov 17, 2018
@ghost ghost removed the in progress label Nov 17, 2018
@balloob
Copy link
Copy Markdown
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
Copy link
Copy Markdown
Member Author

syssi commented Nov 18, 2018

I fear the breaking change.

@balloob
Copy link
Copy Markdown
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
@ghost ghost added the in progress label Nov 18, 2018
@balloob
Copy link
Copy Markdown
Member

balloob commented Nov 19, 2018

so let's merge? 👍

@syssi syssi merged commit 14ad742 into home-assistant:dev Nov 19, 2018
@ghost ghost removed the in progress label Nov 19, 2018
@syssi syssi changed the title RFC: Prefix all xiaomi_aqara events Prefix all xiaomi_aqara events Nov 19, 2018
@frenck
Copy link
Copy Markdown
Member

frenck commented Nov 19, 2018

@syssi Docs?!

@syssi
Copy link
Copy Markdown
Member Author

syssi commented Nov 19, 2018

@frenck
Copy link
Copy Markdown
Member

frenck commented Nov 19, 2018

Thanks, @syssi! 👍

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants