Use the Voice Preview Edition sounds for CarPlay Assist cues - #5696
Use the Voice Preview Edition sounds for CarPlay Assist cues#5696bgoncal wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
CarPlayAssistTonePlayer.Tone.resourceName currently lacks a return and will not compile (and one new test is duration-dependent/flaky).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates CarPlay Assist’s audible feedback cues to use bundled Home Assistant Voice Preview Edition sounds (FLAC) instead of synthesized tones, aligning CarPlay’s cue sounds with other Assist hardware while continuing to route audio through the shared Assist audio session.
Changes:
- Replace generated WAV tone synthesis with bundled Voice Preview Edition FLAC assets, keyed by tone type.
- Update CarPlay Assist session tone triggers (listening vs listening-stopped) and stop any cue playback before starting TTS playback.
- Add unit tests to validate bundled tone availability and tone-player completion/interrupt behavior, plus add attribution documentation for the sound assets.
File summaries
| File | Description |
|---|---|
| Tests/App/CarPlay/CarPlayAssistTonePlayer.test.swift | Adds unit tests for tone resource presence and tone-player completion/stop semantics. |
| Tests/App/CarPlay/CarPlayAssistSession.test.swift | Updates expectations to the new tone names/behavior. |
| Sources/CarPlay/Templates/QuickAccess/CarPlayAssistTonePlayer.swift | Switches tone playback to bundled FLAC files and introduces tone-to-resource mapping. |
| Sources/CarPlay/Templates/QuickAccess/CarPlayAssistSession.swift | Updates which cues are played and stops cues prior to TTS session reconfiguration/playback. |
| Sources/App/Resources/Sounds/Assist/LICENSE.md | Adds attribution/license documentation for the bundled sounds. |
| HomeAssistant.xcodeproj/project.pbxproj | Adds the new license file to the Resources synchronized group exceptions for the App target. |
Review details
- Files reviewed: 6/9 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Found 2 unused localization strings in the codebase. Click to see detailsTo remove them, run the |
There was a problem hiding this comment.
🔵 Needs a closer look
Several renamed tone concepts now read inconsistently with their usage (and an existing debug setting name), which will be confusing to maintain and should be reconciled before merging.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
Sources/CarPlay/Templates/QuickAccess/CarPlayAssistSession.swift:551
- The debug flag
playRecordingIndicatorTonenow gates a “listening started” sound (.listening). This makes the setting name and its localized label misleading. Consider renaming the setting (and localization key) to match the new semantics so debug UI/options remain understandable.
Sources/CarPlay/Templates/QuickAccess/CarPlayAssistSession.swift:555 playListeningStoppedTone()is used as the generic “processing started” cue (including the text-input path), so the name and underlyingTone.listeningStoppedread misleadingly when no listening actually occurred. Consider renaming this concept back to.processing(or similar) and mapping that semantic tone to the Voice PE file name, so call sites remain self-explanatory.
- Files reviewed: 6/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
A couple of in-code names/comments are now misleading after the tone rename (e.g., “recording indicator” vs “listening”), and should be aligned to avoid confusion.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
Sources/CarPlay/Templates/QuickAccess/CarPlayAssistSession.swift:550
playListeningToneIfNeeded()still gates onplayRecordingIndicatorTone, which is confusing now that the tone is explicitly named “listening” (andTone.startRecordingno longer exists). Consider aligning the debug setting name and/or the helper method name to avoid ambiguity about what the toggle controls.
Tests/App/CarPlay/CarPlayAssistSession.test.swift:248- The comment mentions the “recording indicator tone”, but the test now waits for the
.listeningtone. This makes the test comment inaccurate/misleading.
- Files reviewed: 6/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The tone player is configured to load/play .flac files via AVAudioPlayer, which is likely unsupported on iOS and would break cue playback at runtime.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
Sources/CarPlay/Templates/QuickAccess/CarPlayAssistTonePlayer.swift:41
AVAudioPlayergenerally doesn't support decoding FLAC on iOS (Apple’s docs list AAC/ALAC/MP3/linear PCM, but not FLAC). WithresourceExtension = "flac", tone playback is likely to fail at runtime (silent cues / immediate error path).
Consider re-encoding these cue files to a supported format (e.g. .m4a AAC or .caf/ALAC) and updating resourceExtension accordingly (and ensuring the new files are bundled).
- Files reviewed: 6/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The project file currently lists Sounds/Assist/LICENSE.md under a build-file exception set, which likely prevents shipping attribution in the app bundle alongside CC BY sound assets.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/9 changed files
- Comments generated: 1
- Review effort level: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5696 +/- ##
==========================================
+ Coverage 41.59% 41.98% +0.38%
==========================================
Files 1108 1108
Lines 77099 77346 +247
==========================================
+ Hits 32073 32470 +397
+ Misses 45026 44876 -150
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AI Policy
Select exactly one option that describes AI usage in this contribution:
Summary
CarPlay Assist played synthesized sine-wave chimes for its feedback tones. It now plays the Home Assistant Voice Preview Edition sounds (CC BY 4.0, Clayton Charles Tapp), so the car sounds like the rest of the Assist hardware:
wake_word_triggeredmute_switch_offmute_switch_onThe tone player loads the bundled Apple Lossless files instead of generating WAV data, still through the shared Assist audio session so the cues follow the car route and ignore the ring/silent switch. The listening-stopped cue is stopped when TTS playback starts so the response never layers over it and the session can be reconfigured without a player still running. The files were converted losslessly from FLAC to Apple Lossless and had their trailing silence trimmed, the audio is otherwise unchanged.
Screenshots
No visual change.
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#
Any other notes