Use homescreen code paths for CarPlay where possible#1680
Conversation
Use the same queue building mechanisms as the main app rather than carplay specific ones. This removes the 250 track carplay cap too.
- CarPlay event callbacks can run while a frame is being drawn, so the _verifyCallable assert rejected their API calls and surfaced error toasts in debug builds - Release builds strip asserts and were unaffected
Update carplay library browsing calls to also unify with the main UI paths. This now means: - Offline only and favourites only browsing now transfers to carplay - List data stays cached until carplay rebuilds, so tapping deep into a long list plays the tapped track instead of re-fetching - A failed fetch cancels the navigation instead of showing an empty library, and the next tap retries cleanly
Update the carplay home rows to fetch through the same home screen presets as the main UI. - Offline Recently Added is sorted by date added instead of random download order - Recently Played shows your server-wide play history, and hides when offline since play dates only exist on the server
Shuffle all respects the only show favourites setting. Start radio is the same Surprise Me as the rest of the app, still falling back to shuffle when offline. Closes finamp-app#1586
|
I also have in here something I used for debug "Allow Jellyfin calls from CarPlay in debug builds" I am happy to remove this, or submit separately. |
|
@APIUM would you mind posting some screenshots or a short video of how it looks/works now, just for reference? |
Display is identical for this change, I have used these to make some UI changes to be submitted in a follow up PR |
|
Ah, so you just re-create the existing CarPlay content via the new home screen methods, got it. I just got confused by the mentions of home screen sections, and thought you already rendered the sections configured for the in-app home screen. This looks mergeable from my side. @Komodo5197 what do you think? Is this something you'd like to postpone until after the stable release? |
|
It doesn't look too crazy, but I haven't properly looked through everything yet. I noticed the providers are used via read(), including using the slice method on the page provider, and I want to think about whether we might be able to do things in a more reactive way. I'm also wondering about the changes to verifyCallable(), because I wouldn't have expected carplay to be acting during build(). The changes are very isolated from the main app though, which does increase my confidence for allowing this to be pushed before stable. If this is just a pure code improvement I might wait on it, but if it also has bug fixes it's probably not too crazy to try to get it in. |
b66740c to
07ebe88
Compare
Update the carplay library list loads to watch the paging provider and request pages as needed, rather than reading a slice from the notifier.
Thanks @Komodo5197 I’ve played around this and made some changes, it is more reactive but it’s slightly different to the normal path now. Let me know what you think. There are a fair few bug fixes in this, but they are all pretty minor. Up to you on risk v reward. I want this in as I’ve got some CarPlay feature updates that I’d like to add such as allowing viewing far more of the library, a better homescreen, and now playing controls such as radio, shuffle and favourite. I’ll get that up when we’re ok with this one. The fixes are (commit msg snippets):
|
3 participants
Changes
Replace as much of my custom CarPlay code for things like queue building with work from the new homescreen.
Related Issues