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

Add Notification model and handle notification event #85

Merged
merged 6 commits into from
Feb 1, 2021

Conversation

raman325
Copy link
Contributor

@raman325 raman325 commented Feb 1, 2021

I plan to submit a PR similar to home-assistant/core#45729 where we fire an event when a notification is received for a node.

@MartinHjelmare
Copy link
Contributor

Please rebase to solve the merge conflict.

@marcelveldt
Copy link
Contributor

@raman325 maybe use the same logic to fire the event ? I can adjust my PR so it will also forward these events ?

@raman325
Copy link
Contributor Author

raman325 commented Feb 1, 2021

The event data will be different but otherwise yeah that makes sense. I was thinking it would be nice eventually to tie these events to the primary "entity" but that might be easier said than done.

@MartinHjelmare
Copy link
Contributor

Entities don't fire events, besides the built in state changed events, in Home Assistant.

Copy link
Contributor

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Are we happy to merge?

@raman325
Copy link
Contributor Author

raman325 commented Feb 1, 2021

sure I meant add entity_id as part of the event data to use as criteria for automations

@MartinHjelmare
Copy link
Contributor

We can discuss that.

@MartinHjelmare MartinHjelmare merged commit 8a4ba91 into home-assistant-libs:master Feb 1, 2021
@raman325 raman325 deleted the notifications branch February 1, 2021 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants