Skip to content

Allow settings auto favorite from automotive#6718

Draft
cddu33 wants to merge 66 commits intohome-assistant:mainfrom
cddu33:feature/add-favorite-automotive
Draft

Allow settings auto favorite from automotive#6718
cddu33 wants to merge 66 commits intohome-assistant:mainfrom
cddu33:feature/add-favorite-automotive

Conversation

@cddu33
Copy link
Copy Markdown

@cddu33 cddu33 commented Apr 16, 2026

Summary

I have a Renault Scenic with Android Automotive, but Debug Mode is secured. Therefore, I can't install the minimal version of Home Assistant.

I created a 'favorites' mode for the full release. Users can add favorites on the home screen only when the car is parked (not while driving).

Checklist

  • New or updated tests have been added to cover the changes following the testing guidelines.
  • The code follows the project's code style and best_practices.
  • The changes have been thoroughly tested, and edge cases have been considered.
  • Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

Screenshots

Capture d'écran 2026-04-16 215905 Capture d'écran 2026-04-16 215944

Copy link
Copy Markdown

@home-assistant home-assistant Bot left a comment

Choose a reason for hiding this comment

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

Hi @cddu33

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!

@cddu33
Copy link
Copy Markdown
Author

cddu33 commented Apr 16, 2026

Hi everyone, ,it's my firt PR for HA, .It's only for Android automotive :)

Copy link
Copy Markdown

@home-assistant home-assistant Bot left a comment

Choose a reason for hiding this comment

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

Hi @cddu33

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!

@cddu33 cddu33 marked this pull request as ready for review April 16, 2026 16:22
Copilot AI review requested due to automatic review settings April 16, 2026 16:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an in-car “Manage favorites” flow for Android Automotive (full flavor) so users can edit the Driving favorites list when the vehicle is not in distraction-optimized mode.

Changes:

  • Add a new Automotive screen to toggle favorites across supported entities, including simple paging
  • Add a “Manage favorites” header action on Automotive screens for the full flavor when parked
  • Refactor Automotive UX restriction handling in BaseVehicleScreen

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
common/src/main/res/values/strings.xml Adds strings for the new “Manage favorites” UI and paging labels
app/src/main/kotlin/io/homeassistant/companion/android/vehicle/ManageFavoritesVehicleScreen.kt New Automotive screen to add/remove favorites via toggles
app/src/main/kotlin/io/homeassistant/companion/android/vehicle/MainVehicleScreen.kt Adds header action to open favorites management when parked (full flavor)
app/src/main/kotlin/io/homeassistant/companion/android/vehicle/DomainListScreen.kt Adds the same header action on the domain list screen
app/src/main/kotlin/io/homeassistant/companion/android/vehicle/BaseVehicleScreen.kt Updates how UX restriction state is sourced and logs restriction changes
app/src/main/kotlin/io/homeassistant/companion/android/util/vehicle/TemplateComponents.kt Adds helper to create the “Manage favorites” header action
.idea/markdown.xml Adds IDE configuration file (should not be committed)
Files not reviewed (1)
  • .idea/markdown.xml: Language not supported

Comment thread common/src/main/res/values/strings.xml Outdated
cddu33 and others added 12 commits April 16, 2026 18:29
…/ManageFavoritesVehicleScreen.kt


Logging raw entityId can leak user-specific information (entity IDs often include user-defined names). Prefer removing these logs or wrapping the value with sensitive(...) as used elsewhere in the app.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
aa_manage_favorites_parked_only is added but not referenced anywhere in the PR. If it’s not needed, remove it to avoid unused resource warnings; if it is needed, wire it into the UI where the parked-only restriction is enforced.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…/DomainListScreen.kt


Kotlin style in this codebase generally uses trailing commas for multiline argument lists; add a trailing comma after prefsRepository to match surrounding calls and avoid ktlint churn.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…/MainVehicleScreen.kt


Kotlin style in this codebase generally uses trailing commas for multiline argument lists; add a trailing comma after prefsRepository to match surrounding calls and avoid ktlint churn.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…/ManageFavoritesVehicleScreen.kt


Sorting computes favorite membership with favoritesList.any { ... } for every entity, which is O(entities × favorites). Consider building a Set (or HashSet) of favorite entityIds for the current server once per update and using contains in both the sort and row-building logic.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…/ManageFavoritesVehicleScreen.kt


itemsPerPage always reserves space for both paging rows by doing listLimit - 2, even when only one (or neither) paging row is shown. This reduces the number of entities shown per page unnecessarily and can make paging feel off. Consider calculating the reserved rows based on hasPreviousPage/hasNextPage for the current page.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This reverts commit 03b9cc0.

Co-authored-by: Claude <noreply@anthropic.com>
@cddu33 cddu33 marked this pull request as ready for review May 5, 2026 19:25
@home-assistant home-assistant Bot requested a review from TimoPtr May 5, 2026 19:25
@cddu33
Copy link
Copy Markdown
Author

cddu33 commented May 5, 2026

Hi @TimoPtr i updated my PR , thanks for your feedback

Copy link
Copy Markdown

@github-advanced-security github-advanced-security AI left a comment

Choose a reason for hiding this comment

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

ktlint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

cddu33 and others added 3 commits May 7, 2026 10:09
…/ManageFavoritesVehicleScreen.kt

Co-authored-by: Timothy <6560631+TimoPtr@users.noreply.github.com>
…/ManageFavoritesVehicleScreen.kt

Co-authored-by: Timothy <6560631+TimoPtr@users.noreply.github.com>
@TimoPtr TimoPtr marked this pull request as draft May 7, 2026 10:31
@TimoPtr
Copy link
Copy Markdown
Member

TimoPtr commented May 7, 2026

From claude

  1. Off-by-one waste on first/last pages → unnecessary pagination                                                                                                                                                                                                                                                 
   
  computePageSlice reserves space for both Previous and Next rows on every page (itemsPerPage = listLimit - 2), but page 0 has no Previous and the last page has no Next. So one slot is wasted at each end of the pagination, and lists that should fit on a single page get split.                               
                                                            
  Example with listLimit = 10, totalItems = 9:                                                                                                                                                                                                                                                                     
                                                            
  - itemsPerPage = 8                                                                                                                                                                                                                                                                                               
  - Page 0: 8 entities + Next = 9 rows
  - Page 1: Prev + 1 entity = 2 rows                                                                                                                                                                                                                                                                               
                                                                                                                                                                                                                                                                                                                   
  …even though all 9 entities would fit on a single page (9 ≤ 10). The user has to tap "Next" for one orphan entity.                                                                                                                                                                                               
                                                                                                                                                                                                                                                                                                                   
  Fix: compute the slot reservation per-page based on whether nav rows are actually rendered. Roughly:                                                                                                                                                                                                             
                                                            
  val capacity = listLimit - (if (page > 0) 1 else 0)  // reserve Prev                                                                                                                                                                                                                                             
  val tentativeTo = minOf(page * itemsPerPage(prevPages) + capacity, totalItems)                                                                                                                                                                                                                                   
  val needsNext = tentativeTo < totalItems                                                                                                                                                                                                                                                                         
  val from = page * itemsPerPage                                                                                                                                                                                                                                                                                   
  val to = minOf(from + capacity - if (needsNext) 1 else 0, totalItems)                                                                                                                                                                                                                                            
                                                                                                                                                                                                                                                                                                                   
  You'll want a few unit tests for boundaries (totalItems == listLimit, totalItems == listLimit + 1, last-page-just-fits).                                                                                                                                                                                         
                                                                                                                                                                                                                                                                                                                   
  2. Overflow when listLimit ≤ 3                                                                                                                                                                                                                                                                                   
                                                            
  itemsPerPage = (listLimit - 2).coerceAtLeast(1) clamps the items, but the total rows on middle pages is Prev + items + Next = items + 2. With listLimit = 2 or 3 and a middle page, you render 3 rows against a 2- or 3-row host limit. Latent bug — current AAOS hosts return 6+, but worth defending against.  
   
  3. subList reads entities twice without a snapshot                                                                                                                                                                                                                                                               
                                                            
  val pageSlice = computePageSlice(entities.size, page, listLimit)                                                                                                                                                                                                                                                 
  val pageEntities = if (isLoaded && pageSlice.fromIndex < entities.size) {                                                                                                                                                                                                                                        
      entities.subList(pageSlice.fromIndex, pageSlice.toIndex)  // ← entities may have changed
  } else { emptyList() }                                                                                                                                                                                                                                                                                           
                                                            
  entities is reassigned by the toggle handler (lifecycleScope.launch) and by the allEntities collector — and the logs from the previous turn show the collector is running on background threads. onGetTemplate runs on the Car App thread. So the size used to compute pageSlice and the size of entities at     
  subList time can differ.                                  
                                                                                                                                                                                                                                                                                                                   
  If entities.size shrinks between the two reads to below pageSlice.toIndex, subList throws IndexOutOfBoundsException. Snapshot once:                                                                                                                                                                              
   
  val snapshot = entities                                                                                                                                                                                                                                                                                          
  val pageSlice = computePageSlice(snapshot.size, page, listLimit)                                                                                                                                                                                                                                                 
  val pageEntities = if (isLoaded && pageSlice.fromIndex < snapshot.size) {
      snapshot.subList(pageSlice.fromIndex, pageSlice.toIndex)                                                                                                                                                                                                                                                     
  } else emptyList()                                        
                                                                                                                                                                                                                                                                                                                   
  4. page is never clamped against current size                                                                                                                                                                                                                                                                    
   
  Nothing recovers if page outlives a list shrink in a way that doesn't trip the listChanged reset (e.g., size shrinks while IDs at lower indices remain identical — your equality check is on the prefix of the list). The user can land on an empty page that only shows "Previous" and has to tap back manually.
   Cheap fix in onGetTemplate:                              
                                                                                                                                                                                                                                                                                                                   
  val maxPage = ((entities.size - 1) / itemsPerPage).coerceAtLeast(0)
  if (page > maxPage) page = maxPage

I think you need to spend sometime writing some tests on your pagination logic.

cddu33 and others added 3 commits May 7, 2026 20:03
…/ManageFavoritesVehicleScreen.kt

Co-authored-by: Timothy <6560631+TimoPtr@users.noreply.github.com>
- Wrap state into UIState data class protected by stateMutex
- Move prev/next pagination to header (addEndHeaderAction) to free list slots
- Add SearchFavoritesVehicleScreen (private) with SearchTemplate for inline filtering
- Fix computePageSlice: page clamped, no slot wasted since nav is in header
- Extract shared helpers: persistFavoriteToggle, compareByFavoriteThenName, etc.
- Resolve merge conflict with origin/feature/add-favorite-automotive
- Add strings: aa_search_entities, aa_search_no_results

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

6 participants