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 caching of favorites in Wear OS, resulting in prettier loading #2982

Merged
merged 11 commits into from
Nov 9, 2022

Conversation

mads03dk
Copy link
Contributor

Summary

Added local caching of the friendly_name and icon properties in the WearOS app, thus avoiding using the entity ID and a bookmark icon whilst waiting for the server to send the current states. Aimed to achieve a prettier visual while waiting for the connection to be established

Screenshots

Before, whilst waiting for a connection:
Screenshot_20221021_194139_android

Now, whilst waiting for a connection:
Screenshot_20221021_194142_android

Settings:
image

Any other notes

Only favorites are cached, since they're the first entities to be shown. Cached is updated every time the entity is received from the server or added/removed from the list of favorites. Some internals of the getIcon function has also been updated to allow direct usage of icon-strings

Please note;
This is my very first go at native app development (hereby Kotlin), so I expect some - if not all - of my changes to be suboptimal. I'm really open to suggestions and would love to get some constructive feedback, but don't go too harsh, haha!

@home-assistant
Copy link

Hi mads03dk

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!

@jpelgrom
Copy link
Member

Hi 👋 This is a really nice idea

For database changes please increase the version as you're changing something for it. Right now, upgrades of existing installs won't work. When increasing the version this also means that the existing JSON files that describe the database don't change, and you can probably use an AutoMigration instead of writing SQL yourself.

Copy link
Member

@jpelgrom jpelgrom left a comment

Choose a reason for hiding this comment

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

In addition to the more general comment about database changes above, a few more specific notes. If anything is unclear feel free to add a comment :)

mads03dk and others added 3 commits October 21, 2022 21:35
…nViewModel.kt

Co-authored-by: Joris Pelgröm <jpelgrom@users.noreply.github.com>
Revert previous changes to database and implement properly
mads03dk and others added 2 commits October 22, 2022 11:52
Co-authored-by: Joris Pelgröm <jpelgrom@users.noreply.github.com>
@jpelgrom
Copy link
Member

I've been thinking, do we need a clear cache button in the settings? The app handles adding/updating/deleting on load, when the favorites are edited and on logout.

mads03dk and others added 2 commits October 31, 2022 19:01
…ws/MainView.kt

Co-authored-by: Joris Pelgröm <jpelgrom@users.noreply.github.com>
Remove wrapper function to delete cached items
@jpelgrom
Copy link
Member

jpelgrom commented Nov 2, 2022

I've been thinking, do we need a clear cache button in the settings? The app handles adding/updating/deleting on load, when the favorites are edited and on logout.

@mads03dk What is your opinion on this ^ ?

@mads03dk
Copy link
Contributor Author

mads03dk commented Nov 2, 2022

Hmm. Since the cache is updated every time data is received, I also deem it unnecessary. If you feel the same way, let me know, and I will remove it :)

@jpelgrom
Copy link
Member

jpelgrom commented Nov 2, 2022

Yes it also seems unnecessary to me, so feel free to remove it :)

Copy link
Member

@jpelgrom jpelgrom left a comment

Choose a reason for hiding this comment

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

Thanks, nice first contribution!

@JBassett JBassett merged commit e01246a into home-assistant:master Nov 9, 2022
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.

5 participants