-
-
Notifications
You must be signed in to change notification settings - Fork 677
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
Support multiple subscriptions of same type in websocket and use in template widget #2801
Support multiple subscriptions of same type in websocket and use in template widget #2801
Conversation
- Change the structure of how the websocket repository tracks individual subscriptions to allow subscribing multiple times with the same type, but different data
- Instead of depending on entity state changes and refreshing any time there is a change, use the render_template subscription for the template widget to limit the amount of data and power used. To make this possible without too much abstractions, the TemplateWidget no longer implements BaseWidgetProvider. - Fix hardcoded "Loading" string
- Implements home-assistant#2795 for changes to tracked subscription ID
- When subscribing fails, return null and don't store an active subscription instead of continuing and returning a flow (which would never emit messages)
Does anyone know why there are all these unrelated intents in the widget's intent receiver? android/app/src/main/AndroidManifest.xml Lines 156 to 162 in e7dcdac
|
I think this was when we first added widgets as it was a best effort to keep them as up to date as possible. Then shortly after was when we added the screen on updates and now websocket updates :) |
So far initial testing looks good. This will cut down on a lot of events! Really impressive. This PR made me realize that my current template was inefficient so to anyone not seeing instant updates this means you need to optimize your templates 😄 . To determine if your template is actually inefficient test the output in Developer Tools > Templates and if you don't see instant updates there then that means you need to optimize it. https://www.home-assistant.io/integrations/template/#rate-limiting-updates |
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.
In my testing things look good here, it is nice to see less state updates coming for the template widget. I haven't observed any issues, just less noise when the screen is on 😃
…e-render-template # Conflicts: # common/src/main/java/io/homeassistant/companion/android/common/data/websocket/impl/WebSocketRepositoryImpl.kt
Summary
This PR updates the app's websocket implementation to support multiple subscriptions of the same type, and partially implements #2769.
1. Websocket update
The app will now not only track the
type
of a subscription but also any additional data, in order to support multiple subscriptions where the type is the same but the data is not (like templates, or specific entity updates).The logic for these subscriptions is the same as before: if an identical subscription already exists, it will return the existing Flow instead of starting another subscription.
2. Template widget update
The changes mentioned above are used to implement the first part of #2769: template widgets now use a subscription for the template instead of refreshing on any state change. Because there might be multiple instances of the widget which start subscriptions, a single
CoroutineScope
is stored in the companion object to tie these subscriptions to - based on my understanding this should be fine but please let me know if it is not!As a result the
TemplateWidget
no longer implementsBaseWidgetProvider
, and some duplicate code had to be added.Screenshots
n/a, no visual changes
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#796
Any other notes
I opted to combine changes 1 and 2 in this PR, because without change 2 nothing will actually use the new subscription support. Change 2 is all included in commit 4bcf375 though.