Conversation
Add a ComboBox to SettingsForm that lets the user choose which Windows audio output device VolumeWatcherService monitors, instead of always using the system default. Changes: - IAppConfiguration: add AudioDeviceId property - AppConfiguration: implement AudioDeviceId (HomeAssistant:AudioDeviceId config key) - appsettings.json: add AudioDeviceId field (default empty = Windows default output) - VolumeWatcherService: use configured device if AudioDeviceId is set; fall back to Windows default if device not found (COMException), log a warning - SettingsManager: add audioDeviceId parameter to SaveSettings() (optional, default '') save/update AudioDeviceId in appsettings.json; include in change-detection for Reload - SettingsForm: add ComboBox populated with active render endpoints via MMDeviceEnumerator.EnumerateAudioEndPoints(); first item is always 'Default (Windows default output)'; device selection saved and restored correctly - SystemTrayService: pass audioDeviceId from SettingsForm to SaveSettings() Tests: - AppConfigurationTests: 3 new tests for AudioDeviceId (configured, empty, missing) - IAppConfigurationTests: 2 new tests for AudioDeviceId via interface - SettingsManagerTests: 3 new tests for SaveSettings() with audioDeviceId - TestConfigurationHelper: add audioDeviceId parameter to CreateConfigurationWithWebhook() Resolves #32
* fix: act on PR #34 Copilot review comments 1. IAppConfiguration.AudioDeviceId doc: clarify it is a Core Audio endpoint ID string (opaque), not a plain GUID 2. VolumeWatcherService: rename _defaultDevice -> _monitoredDevice to better reflect semantics after audio device selector change 3. VolumeWatcherService: extract device resolution into internal static ResolveMonitoredDevice() with delegate injection for testability 4. SettingsManager: omit/remove AudioDeviceId key from JSON when value is empty (null/empty = Windows default, key should not be written) 5. Tests: 6 new [Fact] tests for ResolveMonitoredDevice covering all branches (configured device, empty/null -> default, COMException fallback with warning, log kind assertions); no COM/Windows required for these tests 6. SettingsManagerTests: update assertions to expect AudioDeviceId key omitted when empty; add test for reverting to default (key removal) * fix: guard against null device.FriendlyName in ResolveMonitoredDevice * fix: act on PR #35 Copilot review comments 1. Rename test: SaveSettings_OmitsAudioDeviceId_WhenNotProvided (was 'Saves') 2. VolumeWatcherService.ResolveMonitoredDevice: update delegates to MMDevice? nullable; use Copilot-suggested null-safe pattern (device is not null guard before FriendlyName; COMException falls through to getDefaultDevice) 3. Update tests to remove stubDevice! null-forgiveness; update routing assertions to match new null-safe fallback semantics; add comments explaining why logInfo logging tests cannot assert 'configured'/'default' kind without a real COM MMDevice (integration-tested on Windows CI) * fix: act on PR #35 second Copilot review round 1. ResolveMonitoredDevice made generic (TDevice) with a getName delegate so tests can use string stubs instead of COM MMDevice — fully cross-platform 2. All ResolveMonitoredDevice tests rewritten using string-typed stubs: - routing tests now return real non-null values and assert correctly - LogsConfiguredKind and LogsDefaultKind now properly assert 'configured' and 'default' log kinds - 3 new tests: ReturnsConfiguredDevice, ReturnsDefaultDevice, ReturnsDefaultDevice_WhenConfiguredDeviceNotFound - no MMDevice or COM references in test file; NAudio using removed 3. Fix stale comment in ResolveMonitoredDevice_UsesGetDevice test (removed incorrect 'throw from getDefaultDevice' claim) 4. Tests use [Fact] not [WindowsFact] — intentional since no NAudio types are referenced; pure logic test via string stub * fix: clarify ResolveMonitoredDevice test region comment wording
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v0.5.0
What's new
Audio device selector (closes #32)
%APPDATA%config as an endpoint IDInternal improvements
ResolveMonitoredDeviceextracted as a genericinternal staticmethod with delegate injection for full unit testabilityAudioDeviceIdJSON key is omitted (not written as"") when switching back to Windows default