Only subscribe to supported entities on Wear OS home screen

[dshokouhi]

Summary

It has been observed by some users
that the app will crash due to the high rate of updates that can come in from some systems.

To combat this lets switch from registering for all state changes to only of those in the supported domain list. This should cut down on the noise from other entities which we never show on the home screen.

Screenshots

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#

Any other notes

[jpelgrom]

Switching to only subscribing to supported entities is a good improvement and should reduce network traffic (for now...). The current implementation has some issues however:

• On a cold start the entity registry will probably not be loaded by the time the app subscribes, which means no state updates come in until the app is paused + resumed.
• If the entity registry changes while the app is open, the list of entities it subscribes to might need to change. The app is already listening to entity registry changes, it just needs to potentially cancel/update the existing subscription.

Because the subscription is supposed to be tied to the activity lifecycle I think the best way to handle this is by adding a function to notify the activity to cancel the subscription/job, if any, and start again (viewmodel -> presenter -> view/activity).

• Any changes made while the app is paused are not reflected, as the initial states are only updated when the app is started.

To fix this a check can be added to reload the entity registry and then the states for those entities when resumed based on the loadingState.

[dshokouhi]

• On a cold start the entity registry will probably not be loaded by the time the app subscribes, which means no state updates come in until the app is paused + resumed.

I am still under the impression that all users who open the app on cold start end up having the screen timeout on them which means the app is paused and resumes once the user turns on the screen. I dont think you can see this in the emulator as there the screen never times out.

• If the entity registry changes while the app is open, the list of entities it subscribes to might need to change. The app is already listening to entity registry changes, it just needs to potentially cancel/update the existing subscription.

isnt this an issue today already before these code changes?

• Any changes made while the app is paused are not reflected, as the initial states are only updated when the app is started.

isn't this also existing behavior?

To fix this a check can be added to reload the entity registry and then the states for those entities when resumed based on the loadingState.

on a under powered device I fear the user may be in a loop or will suffer from more lag waiting for the states to load, if they have a low time out and a slow device they may never see it working as desired.

Personally I dont think we need to keep the websockets and states completely up to date all the time, users will not be spending too much time in the app and in the apps current state it simply doesnt load for some installations. I think the trade offs here are ok given that most users will indeed have the screen time out on them while they are waiting. The default time out of 15 seconds and some users opt for 10 seconds means the screen turns off very frequently, at least 2 times faster than a phone.

Edit: another consideration is that by adding more external calls to the home screen means potential for more battery drain. We are now making 8 additional calls each time the screen is turned on and off. By adding more checks to keep things upto date we can potentially use more battery. This is kind of why I htink the home screen should only show the favorite entities so we don't do too much and users can still quickly access things. Already the app is slow and sluggish during the cold start, adding more checks I fear will add to that sluggishness trying to keep things up to date.

[jpelgrom]

I am still under the impression that all users who open the app on cold start end up having the screen timeout on them which means the app is paused and resumes once the user turns on the screen.

While this might be true right now, I still feel like this should be considered as the load time might be reduced in the future because of more optimizations and/or faster devices (or in other cases where it doesn't timeout, like on the emulator as you mentioned). Saying "it will work out most of the time" isn't a great argument.

isnt this an issue today already before these code changes?

isn't this also existing behavior?

You're right, this started happening when the app switched to unsubscribing when paused.

on a under powered device I fear the user may be in a loop or will suffer from more lag waiting for the states to load, if they have a low time out and a slow device they may never see it working as desired.

While performance is a valid concern incorrect data is worse in my opinion. If the app was paused for a few seconds it might be acceptable but if it's been more than a few minutes or hours it is not. On non-initial loads it could be loaded and collected async before being pushed?

Already the app is slow and sluggish during the cold start, adding more checks I fear will add to that sluggishness trying to keep things up to date.

But is this due to network calls or something else? Last time I checked it was mainly because of recomposing the UI, which updating states shouldn't add much to as most states aren't visible in the UI at all times.

[dshokouhi]

Because the subscription is supposed to be tied to the activity lifecycle I think the best way to handle this is by adding a function to notify the activity to cancel the subscription/job, if any, and start again (viewmodel -> presenter -> view/activity).

so there is one issue with this approach I think. Because we are using the activity lifecycle, the usage of repeatOnLifecycle() is only valid in the activities onCreate the system tells us incorrect usage if we try to use it elsewhere. So I am not sure if we will be able to restart that job without letting the activity to handle it. For example in my previous PR I initially tried to add the code in onResume and was met with the follow warning.

To fix this a check can be added to reload the entity registry and then the states for those entities when resumed based on the loadingState.

My only issue here is that updating states (even before we had compose) would take 5 seconds to load just the data, adding in a entity registry update each time too will add to that. Then considering how many times a screen may turn on and off during an interaction with the app we could be making a lot of calls there because these updates need to happen in the onResume method.

I will try to see the performance impact of just doing a state update based on the loadingState to see if it adds any lag. If its acceptable that would at least fix the out of sync state issue :)

[dshokouhi]

I will try to see the performance impact of just doing a state update based on the loadingState to see if it adds any lag. If its acceptable that would at least fix the out of sync state issue :)

I only noticed a very small lag and the updates come in a few seconds later

[dshokouhi]

in the interest of keeping everything up to date I will also just make sure to update the registries :)

[jpelgrom]

I think I found a reasonable way to fix the state subscription on initial load / without changes without requiring the app to pause: allowing the activity to observe the list of entity IDs, to restart the job when changed. What do you think?

// MainViewModel
// - Add a hot flow that can hold the list of entity IDs
private val _supportedEntities = MutableStateFlow(emptyList<String>())
val supportedEntities = _supportedEntities.asStateFlow()

// - In updateUI() after the entity registry has loaded
_supportedEntities.value = entityRegistry.orEmpty().map { it.entityId }.filter { it.split(".")[0] in supportedDomains() }

// - In entityUpdates(), supportedEntities.value can be used
// - In entityRegistryUpdates(), _supportedEntities.value should be updated like in updateUI() after the registry has changed


// HomeActivity
// - Add a variable to reference the job
private var entityUpdateJob: Job? = null

// - launch { mainViewModel.entityUpdates() } in onCreate() will have to collect the flow
mainViewModel.supportedEntities.collect {
    if (entityUpdateJob?.isActive == true) entityUpdateJob?.cancel()
    entityUpdateJob = launch { mainViewModel.entityUpdates() }
}

[dshokouhi]

Thank you that seems to have done the trick and state changes now work on initial load!

[jpelgrom]

It seems to work as expected, doesn't remove any functionality and removes a lot of unnecessary updates.

Really nice to finally have this!