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 areas to entity list on Wear #2118

Merged
merged 12 commits into from
Jan 12, 2022

Conversation

jpelgrom
Copy link
Member

@jpelgrom jpelgrom commented Jan 5, 2022

Summary

Based on the work in #1970, this PR uses area information for entities to improve the main list shown on Wear OS. Should fix #1795.
I haven't worked on Wear OS or with Compose before, so happy to get any feedback from more frequent Wear OS contributors 🙂

Still to do:

  • Fix 'All entities' chip
  • Fix setting favorites + tiles from the watch settings
  • Sort area and domain chips/headers alphabetically, the current implementation with a map has no stable order
  • Listen for registry update events and refresh the list
  • Further tests for when all entities are in areas / no entities are in areas

Further possible improvements:

  • Find a nicer way to create the lists while still allowing Compose to detect the changes (not considering favorites and shortcuts, we need to be able to make combinations of areas with entities in them, entities by area, and entities by domain) Edit: I've now implemented this by creating main entities and areas lists, and nested lists grouping entities by area and domain. Any further filtering is done in Compose which should be quick and allow it to stay live.
  • The code for determining the area of an entity is now used in both the Wear entity list and (phone) device controls, this should probably be moved somewhere else so it can be shared Created a utility object in the common code of the app for this. The function would typically be placed in a ViewModel or repository but there is no shared VM and the WebSocketRepository doesn't follow a repository pattern and do caching/transforming.

Screenshots

Favorites is still at the top, directly below that the user will see their areas with entities:
ha-wear-1

Tapping on an area shows a list of all entities in the area:
ha-wear-2
(the first item is an input boolean, the second a light, ...)

Back on the main screen, below the areas the app shows chips for domains with entities not in any area (so 'More entities' refers to more than in the areas):
ha-wear-3

And finally at the bottom of the list is the chip for 'All entities', which shows all entities regardless of area status, and the settings:
ha-wear-4

If the user has no areas with supported entities in them, nothing is shown and below favorites the app shows domains like it currently does:
ha-wear-5

If the user has placed all the supported entities in an area, the app won't show any domains and just end with the all entities and settings chips.
ha-wear-6

Link to pull request in Documentation repository

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

Any other notes

@jpelgrom jpelgrom marked this pull request as draft January 5, 2022 23:07
@jpelgrom jpelgrom changed the title Use area information for entities for main list on Wear Add areas to entity list on Wear Jan 5, 2022
 - Change the approach to getting state lists to support any number of lists per type. By nesting a state list inside the list Compose should still be able to update on any changes. To display, loop through the items.
 - The main view model now filters the main/source entity list that is used everywhere by supported entities to prevent unusable domains from showing up.
 - Fix padding above all entities/settings to make sure that there is still padding even if there are no entities.
 - Temporary fix for favorites/tiles: hardcode the lists used. Should be updated to read items dynamically but I couldn't get the rememberSaveable to work in time.
 - Keep the updates in the EntityListView always live by allowing filtering the list in the view itself. Without these changes, selecting any domain from the main screen would give a static list because filtering creates a copy of the original.
 - Fix ktlint import ordering
 - Subscribe to events for registry updates and update any lists. Compose will take care of updating views if necessary, which basically makes this plug and play.
 - Load all the entities by domain when setting favorites or tiles, instead of using hardcoded references to specific domains. This is a proper fix for the updated list structure introduced in a previous commit.
 - Reuse saving the expanded states of lists from EntityListView because favorites and tiles can now also be any number of headers
 - For determining which tiles are toggleable we already access the implementation directly so remove todos and move names for those domains over to the implementation to keep everything in one place. The main view model will include a simple function to convert the string ID to an actual string.
 - Create a new observable list that can be used to determine the order in which to display the list or areas/entities. A mutableMapOf in Kotlin by default respects the insert order but it looks like the mutableStateMapOf doesn't follow that as the items are already inserted alphabetically.
 - Allow passing in a list order for the EntityListView to prevent entities from suddenly ending up in a different place on a different screen
 - Order the list of entities displayed in one area alphabetically by their friendly name instead of domain, as it would otherwise still use domains internally to sort but not show them which is not a good experience.
 - Create an util object in the common package for getting the area of an entity, as the same code was used both in the device controls and Wear main list.
@jpelgrom
Copy link
Member Author

jpelgrom commented Jan 7, 2022

OK, I think this is ready for testing and review now! Screenshots have also been updated.

Based on the emulator there isn't any noticeable performance impact aside from slightly longer initial loading, hopefully that is also true for real watches.

While I tried not to make any large architectural changes, working with 0-n areas and (not) displaying entities depending on whether you view them from an area list or somewhere else, you can't copy the approach of 6 fixed lists for domains. There are now two main lists, areas with entities and domains with entities, and depending on what is needed the app filters or loops their contents. I think this is the best approach when you also need to give Compose something to observe.
(Also pretty amazing how much lines of code this removes)

Copy link
Member

@dshokouhi dshokouhi left a comment

Choose a reason for hiding this comment

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

Tested the debug APK and it works well, thank you for taking this on!

@JBassett JBassett merged commit 4796284 into home-assistant:master Jan 12, 2022
@jpelgrom jpelgrom deleted the wear-areaslist branch January 12, 2022 07:46
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.

Area support for lights in WearOS app
4 participants