-
-
Notifications
You must be signed in to change notification settings - Fork 742
Add support for UnifiedPush notifications #5261
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lone-faerie
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
app/src/main/kotlin/io/homeassistant/companion/android/util/UnifiedPushExtensions.kt
Fixed
Show fixed
Hide fixed
app/src/main/kotlin/io/homeassistant/companion/android/util/UnifiedPushExtensions.kt
Fixed
Show fixed
Hide fixed
While this works to get started (and is quite similar in some ways to my proof of concept from last year), I feel like there should be a generic interface here. This approach doesn't scale and the push provider can change without the user noticing. |
The push provider should only change during onboarding or if the user uninstalls the distributor app. A generic interface does feel like the more proper approach though, perhaps something like the SensorManager. UnifiedPush also has the ability to handle FCM on it's own, though that would require a bit of backend work since it needs a gateway similar to the current cloud functions. |
Since you are changing the notification, you will also need two others PR in the developer documentation. |
Summary
Adds support for using UnifiedPush as the source for push notifications. This allows both the full and minimal flavors to receive push notifications without a persistent websocket connection and without reliance on Google's Firebase Cloud Messaging. On launch, UnifiedPush is preferred if a distributor is available, otherwise the full flavor falls back to FCM. A setting has also been added to allow the user to manually select the UnifiedPush distributor to use.
Checklist
Screenshots
Link to pull request in documentation repositories
User Documentation: home-assistant/companion.home-assistant#1202
Any other notes
#3174 mentions Home Assistant Core needing to add support before this would work. I'm not sure if it's changed since then, but in my tests UnifiedPush works with Core as-is. A small change to Core would be needed to support encrypting the notifications, but that isn't necessary for UnifiedPush to work.