From 3f84ccc603506907aff00bebf8bed81da787d814 Mon Sep 17 00:00:00 2001 From: Yu Leng Date: Tue, 18 Nov 2025 01:42:10 +0800 Subject: [PATCH] Refactor and modernize codebase for maintainability Refactored code to improve performance, readability, and scalability: - Removed color temperature constants and obsolete VCP codes. - Converted `MonitorStateManager` methods to async for non-blocking I/O. - Added retry logic for physical monitor discovery in `DdcCiController`. - Simplified UI logic in `MainWindow.xaml.cs` by removing animations. - Streamlined `MainViewModel` initialization and reduced excessive logging. - Enhanced error handling during disposal and initialization processes. - Removed deprecated methods and unused features for cleaner code. - Consolidated repetitive code into reusable helper methods. - Replaced hardcoded UI constants with configurable values in `AppConstants`. These changes align the application with modern coding practices. --- .../Configuration/AppConstants.cs | 5 - .../Helpers/MonitorStateManager.cs | 14 +- .../Native/DDC/DdcCiController.cs | 157 ++++++++---- .../PowerDisplay/Native/NativeConstants.cs | 28 --- .../PowerDisplayXAML/MainWindow.xaml.cs | 121 +-------- .../PowerDisplay/ViewModels/MainViewModel.cs | 237 ++++++------------ 6 files changed, 201 insertions(+), 361 deletions(-) diff --git a/src/modules/powerdisplay/PowerDisplay/Configuration/AppConstants.cs b/src/modules/powerdisplay/PowerDisplay/Configuration/AppConstants.cs index 359335392b..17418bd536 100644 --- a/src/modules/powerdisplay/PowerDisplay/Configuration/AppConstants.cs +++ b/src/modules/powerdisplay/PowerDisplay/Configuration/AppConstants.cs @@ -35,11 +35,6 @@ namespace PowerDisplay.Configuration public const int MaxBrightness = 100; public const int DefaultBrightness = 50; - // Color Temperature (Kelvin) - public const int MinColorTemp = 2000; // Warm - public const int MaxColorTemp = 10000; // Cool - public const int DefaultColorTemp = 6500; // Neutral - // Contrast public const int MinContrast = 0; public const int MaxContrast = 100; diff --git a/src/modules/powerdisplay/PowerDisplay/Helpers/MonitorStateManager.cs b/src/modules/powerdisplay/PowerDisplay/Helpers/MonitorStateManager.cs index e9cb72969f..0e9a5a98b2 100644 --- a/src/modules/powerdisplay/PowerDisplay/Helpers/MonitorStateManager.cs +++ b/src/modules/powerdisplay/PowerDisplay/Helpers/MonitorStateManager.cs @@ -72,7 +72,7 @@ namespace PowerDisplay.Helpers /// /// Timer callback to save state when dirty /// - private void OnSaveTimerElapsed(object? state) + private async void OnSaveTimerElapsed(object? state) { bool shouldSave = false; lock (_lock) @@ -86,7 +86,7 @@ namespace PowerDisplay.Helpers if (shouldSave) { - SaveStateToDisk(); + await SaveStateToDiskAsync(); } } @@ -295,10 +295,10 @@ namespace PowerDisplay.Helpers } /// - /// Save current state to disk immediately. + /// Save current state to disk immediately (async). /// Called by timer after debounce period or on dispose to flush pending changes. /// - private void SaveStateToDisk() + private async Task SaveStateToDiskAsync() { try { @@ -334,9 +334,9 @@ namespace PowerDisplay.Helpers } } - // Write to disk + // Write to disk asynchronously var json = JsonSerializer.Serialize(stateFile, AppJsonContext.Default.MonitorStateFile); - File.WriteAllText(_stateFilePath, json); + await File.WriteAllTextAsync(_stateFilePath, json); Logger.LogDebug($"[State] Saved state for {stateFile.Monitors.Count} monitors"); } @@ -368,7 +368,7 @@ namespace PowerDisplay.Helpers if (wasDirty) { Logger.LogInfo("Flushing pending state changes before dispose"); - SaveStateToDisk(); + SaveStateToDiskAsync().GetAwaiter().GetResult(); } _saveTimer?.Dispose(); diff --git a/src/modules/powerdisplay/PowerDisplay/Native/DDC/DdcCiController.cs b/src/modules/powerdisplay/PowerDisplay/Native/DDC/DdcCiController.cs index 5ec803a6d0..e5107a450d 100644 --- a/src/modules/powerdisplay/PowerDisplay/Native/DDC/DdcCiController.cs +++ b/src/modules/powerdisplay/PowerDisplay/Native/DDC/DdcCiController.cs @@ -404,61 +404,12 @@ namespace PowerDisplay.Native.DDC continue; } - // Sometimes Windows returns NULL handles, so we implement retry logic - PHYSICAL_MONITOR[]? physicalMonitors = null; - const int maxRetries = 3; - const int retryDelayMs = 200; - - for (int attempt = 0; attempt < maxRetries; attempt++) - { - if (attempt > 0) - { - await Task.Delay(retryDelayMs, cancellationToken); - } - - physicalMonitors = _discoveryHelper.GetPhysicalMonitors(hMonitor); - - if (physicalMonitors == null || physicalMonitors.Length == 0) - { - if (attempt < maxRetries - 1) - { - Logger.LogWarning($"DDC: GetPhysicalMonitors returned null/empty on attempt {attempt + 1}, will retry"); - } - - continue; - } - - // Check if any handle is NULL - bool hasNullHandle = false; - for (int i = 0; i < physicalMonitors.Length; i++) - { - if (physicalMonitors[i].HPhysicalMonitor == IntPtr.Zero) - { - hasNullHandle = true; - Logger.LogWarning($"DDC: Physical monitor [{i}] has NULL handle on attempt {attempt + 1}"); - break; - } - } - - if (!hasNullHandle) - { - // Success! All handles are valid - break; - } - else if (attempt < maxRetries - 1) - { - Logger.LogWarning($"DDC: NULL handle detected, will retry (attempt {attempt + 1}/{maxRetries})"); - physicalMonitors = null; // Reset for retry - } - else - { - Logger.LogWarning($"DDC: NULL handle still present after {maxRetries} attempts, continuing anyway"); - } - } + // Get physical monitors with retry logic for NULL handle workaround + var physicalMonitors = await GetPhysicalMonitorsWithRetryAsync(hMonitor, cancellationToken); if (physicalMonitors == null || physicalMonitors.Length == 0) { - Logger.LogWarning($"DDC: Failed to get physical monitors for hMonitor 0x{hMonitor:X} after {maxRetries} attempts"); + Logger.LogWarning($"DDC: Failed to get physical monitors for hMonitor 0x{hMonitor:X} after retries"); continue; } @@ -544,6 +495,104 @@ namespace PowerDisplay.Native.DDC cancellationToken); } + /// + /// Get physical monitors with retry logic to handle Windows API occasionally returning NULL handles + /// + /// Handle to the monitor + /// Cancellation token + /// Array of physical monitors, or null if failed after retries + private async Task GetPhysicalMonitorsWithRetryAsync( + IntPtr hMonitor, + CancellationToken cancellationToken) + { + const int maxRetries = 3; + const int retryDelayMs = 200; + + for (int attempt = 0; attempt < maxRetries; attempt++) + { + if (attempt > 0) + { + await Task.Delay(retryDelayMs, cancellationToken); + } + + var monitors = _discoveryHelper.GetPhysicalMonitors(hMonitor); + + var validationResult = ValidatePhysicalMonitors(monitors, attempt, maxRetries); + + if (validationResult.IsValid) + { + return monitors; + } + + if (validationResult.ShouldRetry) + { + continue; + } + + // Last attempt failed, return what we have + return monitors; + } + + return null; + } + + /// + /// Validate physical monitors array for null handles + /// + /// Tuple indicating if valid and if should retry + private (bool IsValid, bool ShouldRetry) ValidatePhysicalMonitors( + PHYSICAL_MONITOR[]? monitors, + int attempt, + int maxRetries) + { + if (monitors == null || monitors.Length == 0) + { + if (attempt < maxRetries - 1) + { + Logger.LogWarning($"DDC: GetPhysicalMonitors returned null/empty on attempt {attempt + 1}, will retry"); + } + + return (false, true); + } + + bool hasNullHandle = HasAnyNullHandles(monitors, out int nullIndex); + + if (!hasNullHandle) + { + return (true, false); // Valid, don't retry + } + + if (attempt < maxRetries - 1) + { + Logger.LogWarning($"DDC: Physical monitor [{nullIndex}] has NULL handle on attempt {attempt + 1}, will retry"); + return (false, true); // Invalid, should retry + } + + Logger.LogWarning($"DDC: NULL handle still present after {maxRetries} attempts, continuing anyway"); + return (false, false); // Invalid but no more retries + } + + /// + /// Check if any physical monitor has a NULL handle + /// + /// Array of physical monitors to check + /// Output index of first NULL handle found, or -1 if none + /// True if any NULL handle found + private bool HasAnyNullHandles(PHYSICAL_MONITOR[] monitors, out int nullIndex) + { + for (int i = 0; i < monitors.Length; i++) + { + if (monitors[i].HPhysicalMonitor == IntPtr.Zero) + { + nullIndex = i; + return true; + } + } + + nullIndex = -1; + return false; + } + /// /// Generic method to get VCP feature value /// @@ -584,7 +633,7 @@ namespace PowerDisplay.Native.DDC value = Math.Clamp(value, min, max); return await Task.Run( - () => + async () => { if (monitor.Handle == IntPtr.Zero) { @@ -594,7 +643,7 @@ namespace PowerDisplay.Native.DDC try { // Get current value to determine range - var currentInfo = GetVcpFeatureAsync(monitor, vcpCode).Result; + var currentInfo = await GetVcpFeatureAsync(monitor, vcpCode); if (!currentInfo.IsValid) { return MonitorOperationResult.Failure($"Cannot read current value for VCP 0x{vcpCode:X2}"); diff --git a/src/modules/powerdisplay/PowerDisplay/Native/NativeConstants.cs b/src/modules/powerdisplay/PowerDisplay/Native/NativeConstants.cs index caf17052c6..9d796c3473 100644 --- a/src/modules/powerdisplay/PowerDisplay/Native/NativeConstants.cs +++ b/src/modules/powerdisplay/PowerDisplay/Native/NativeConstants.cs @@ -24,20 +24,6 @@ namespace PowerDisplay.Native /// public const byte VcpCodeContrast = 0x12; - /// - /// VCP code: Backlight control (0x13) - /// OBSOLETE: PowerDisplay now uses only VcpCodeBrightness (0x10). - /// - [Obsolete("Use VcpCodeBrightness (0x10) only. PowerDisplay no longer uses fallback codes.")] - public const byte VcpCodeBacklightControl = 0x13; - - /// - /// VCP code: White backlight level (0x6B) - /// OBSOLETE: PowerDisplay now uses only VcpCodeBrightness (0x10). - /// - [Obsolete("Use VcpCodeBrightness (0x10) only. PowerDisplay no longer uses fallback codes.")] - public const byte VcpCodeBacklightLevelWhite = 0x6B; - /// /// VCP code: Audio Speaker Volume (0x62) /// Standard VESA MCCS volume control for monitors with built-in speakers. @@ -49,20 +35,6 @@ namespace PowerDisplay.Native /// public const byte VcpCodeMute = 0x8D; - /// - /// VCP code: Color Temperature Request (0x0C) - /// OBSOLETE: Not widely supported. Use VcpCodeSelectColorPreset (0x14) instead. - /// - [Obsolete("Not widely supported. Use VcpCodeSelectColorPreset (0x14) instead.")] - public const byte VcpCodeColorTemperature = 0x0C; - - /// - /// VCP code: Color Temperature Increment (0x0B) - /// OBSOLETE: Not widely supported. Use VcpCodeSelectColorPreset (0x14) instead. - /// - [Obsolete("Not widely supported. Use VcpCodeSelectColorPreset (0x14) instead.")] - public const byte VcpCodeColorTemperatureIncrement = 0x0B; - /// /// VCP code: Gamma correction (0x72) /// diff --git a/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/MainWindow.xaml.cs b/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/MainWindow.xaml.cs index 1c96251fe3..269715013e 100644 --- a/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/MainWindow.xaml.cs +++ b/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/MainWindow.xaml.cs @@ -15,6 +15,7 @@ using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; using Microsoft.UI.Xaml.Media; using Microsoft.UI.Xaml.Media.Animation; +using PowerDisplay.Configuration; using PowerDisplay.Core; using PowerDisplay.Core.Interfaces; using PowerDisplay.Core.Models; @@ -430,16 +431,10 @@ namespace PowerDisplay } } - private async void OnRefreshClick(object sender, RoutedEventArgs e) + private void OnRefreshClick(object sender, RoutedEventArgs e) { try { - // Add button press animation - if (sender is Button button) - { - await AnimateButtonPress(button); - } - // Refresh monitor list if (_viewModel?.RefreshCommand?.CanExecute(null) == true) { @@ -459,16 +454,10 @@ namespace PowerDisplay } } - private async void OnLinkClick(object sender, RoutedEventArgs e) + private void OnLinkClick(object sender, RoutedEventArgs e) { try { - // Add button press animation - if (sender is Button button) - { - await AnimateButtonPress(button); - } - // Link all monitor brightness (synchronized adjustment) if (_viewModel != null && _viewModel.Monitors.Count > 0) { @@ -483,16 +472,10 @@ namespace PowerDisplay } } - private async void OnDisableClick(object sender, RoutedEventArgs e) + private void OnDisableClick(object sender, RoutedEventArgs e) { try { - // Add button press animation - if (sender is Button button) - { - await AnimateButtonPress(button); - } - // Disable/enable all monitor controls if (_viewModel != null) { @@ -512,72 +495,6 @@ namespace PowerDisplay } } - /// - /// Get internal monitor name, consistent with SettingsManager logic - /// - private async void OnTestClick(object sender, RoutedEventArgs e) - { - ContentDialog? dlg = null; - Core.MonitorManager? manager = null; - - try - { - // Test monitor discovery functionality - dlg = new ContentDialog - { - Title = "Monitor Detection Test", - Content = "Starting monitor detection...", - CloseButtonText = "Close", - XamlRoot = this.Content.XamlRoot, - }; - - _ = dlg.ShowAsync(); - - manager = new Core.MonitorManager(); - var monitors = await manager.DiscoverMonitorsAsync(default(System.Threading.CancellationToken)); - - string message = $"Found {monitors.Count} monitors:\n\n"; - foreach (var monitor in monitors) - { - message += $"• {monitor.Name}\n"; - message += $" Communication: {monitor.CommunicationMethod}\n"; - message += $" Brightness: {monitor.CurrentBrightness}%\n\n"; - } - - if (monitors.Count == 0) - { - message = "No monitors found.\n\n"; - message += "Possible reasons:\n"; - message += "• DDC/CI not supported\n"; - message += "• Driver issues\n"; - message += "• Permission issues\n"; - message += "• Cable doesn't support DDC/CI"; - } - - dlg.Content = message; - - // Don't dispose manager, use existing manager - // Initialize ViewModel and bind to root Grid refresh - if (monitors.Count > 0) - { - // Use existing refresh command - await _viewModel.RefreshMonitorsAsync(); - } - } - catch (Exception ex) - { - Logger.LogError($"OnTestClick failed: {ex}"); - if (dlg != null) - { - dlg.Content = $"Error: {ex.Message}\n\nType: {ex.GetType().Name}"; - } - } - finally - { - manager?.Dispose(); - } - } - /// /// Configure window properties (synchronous, no data dependency) /// @@ -593,7 +510,7 @@ namespace PowerDisplay if (_appWindow != null) { // Set initial window size - will be adjusted later based on content - _appWindow.Resize(new SizeInt32 { Width = 640, Height = 480 }); + _appWindow.Resize(new SizeInt32 { Width = AppConstants.UI.WindowWidth, Height = 480 }); // Position window at bottom right corner PositionWindowAtBottomRight(_appWindow); @@ -692,7 +609,7 @@ namespace PowerDisplay RootGrid.UpdateLayout(); // Get precise content height - var availableWidth = 640.0; + var availableWidth = (double)AppConstants.UI.WindowWidth; var contentHeight = GetContentHeight(availableWidth); // Account for display scaling @@ -700,16 +617,13 @@ namespace PowerDisplay var scaledHeight = (int)Math.Ceiling(contentHeight * scale); // Only set maximum height for scrollable content - scaledHeight = Math.Min(scaledHeight, 650); + scaledHeight = Math.Min(scaledHeight, AppConstants.UI.MaxWindowHeight); // Check if resize is needed var currentSize = _appWindow.Size; if (Math.Abs(currentSize.Height - scaledHeight) > 1) { - _appWindow.Resize(new SizeInt32 { Width = 640, Height = scaledHeight }); - - // Update clip region to match new window size - UpdateClipRegion(640, scaledHeight / scale); + _appWindow.Resize(new SizeInt32 { Width = AppConstants.UI.WindowWidth, Height = scaledHeight }); // Reposition to maintain bottom-right position PositionWindowAtBottomRight(_appWindow); @@ -735,12 +649,6 @@ namespace PowerDisplay return RootGrid.DesiredSize.Height + 4; // Small padding for fallback method } - private void UpdateClipRegion(double width, double height) - { - // Clip region removed to allow automatic sizing - // No longer needed as we removed the fixed clip from RootGrid - } - private void PositionWindowAtBottomRight(AppWindow appWindow) { try @@ -754,7 +662,7 @@ namespace PowerDisplay // Calculate bottom-right position, close to taskbar // WorkArea already excludes taskbar area, so use WorkArea bottom directly - int rightMargin = 10; // Small margin from right edge + int rightMargin = AppConstants.UI.WindowRightMargin; // Small margin from right edge int x = workArea.Width - windowSize.Width - rightMargin; int y = workArea.Height - windowSize.Height; // Close to taskbar top, no gap @@ -768,17 +676,6 @@ namespace PowerDisplay } } - /// - /// Animates button press for modern interaction feedback - /// - /// The button to animate - private async Task AnimateButtonPress(Button button) - { - // Button animation disabled to avoid compilation errors - // Using default button visual states instead - await Task.CompletedTask; - } - /// /// Slider ValueChanged event handler - does nothing during drag /// This allows the slider UI to update smoothly without triggering hardware operations diff --git a/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs b/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs index 18d0d0da3d..648cfc56cd 100644 --- a/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs +++ b/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs @@ -7,7 +7,6 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.ComponentModel; using System.Diagnostics.CodeAnalysis; -using System.IO; using System.Linq; using System.Runtime.CompilerServices; using System.Threading; @@ -152,28 +151,18 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable { StatusText = "Scanning monitors..."; IsScanning = true; - Logger.LogInfo("[InitializeAsync] Starting monitor discovery"); // Discover monitors var monitors = await _monitorManager.DiscoverMonitorsAsync(_cancellationTokenSource.Token); - Logger.LogInfo($"[InitializeAsync] Discovery completed, found {monitors.Count} monitors"); // Update UI on the dispatcher thread - Logger.LogInfo("[InitializeAsync] Calling TryEnqueue to update UI"); - bool enqueued = _dispatcherQueue.TryEnqueue(() => + _dispatcherQueue.TryEnqueue(() => { - Logger.LogInfo("[InitializeAsync] TryEnqueue lambda started"); try { - Logger.LogInfo("[InitializeAsync] Calling UpdateMonitorList"); UpdateMonitorList(monitors); - Logger.LogInfo("[InitializeAsync] UpdateMonitorList returned"); - IsScanning = false; - Logger.LogInfo("[InitializeAsync] IsScanning set to false"); - IsInitialized = true; - Logger.LogInfo("[InitializeAsync] IsInitialized set to true"); if (monitors.Count > 0) { @@ -183,23 +172,18 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable { StatusText = "No controllable monitors found"; } - - Logger.LogInfo($"[InitializeAsync] StatusText set to: {StatusText}"); - Logger.LogInfo("[InitializeAsync] TryEnqueue lambda completed successfully"); } catch (Exception lambdaEx) { - Logger.LogError($"[InitializeAsync] Exception in TryEnqueue lambda: {lambdaEx.Message}"); - Logger.LogError($"[InitializeAsync] Stack trace: {lambdaEx.StackTrace}"); + Logger.LogError($"[InitializeAsync] UI update failed: {lambdaEx.Message}"); IsScanning = false; - throw; + StatusText = $"UI update failed: {lambdaEx.Message}"; } }); - Logger.LogInfo($"[InitializeAsync] TryEnqueue returned: {enqueued}"); } catch (Exception ex) { - Logger.LogError($"[InitializeAsync] Exception in InitializeAsync: {ex.Message}"); + Logger.LogError($"[InitializeAsync] Monitor discovery failed: {ex.Message}"); _dispatcherQueue.TryEnqueue(() => { StatusText = $"Scan failed: {ex.Message}"; @@ -241,10 +225,7 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable private void UpdateMonitorList(IReadOnlyList monitors) { - Logger.LogInfo($"[UpdateMonitorList] Starting with {monitors.Count} monitors"); - Monitors.Clear(); - Logger.LogInfo("[UpdateMonitorList] Cleared Monitors collection"); // Load settings to check for hidden monitors var settings = _settingsUtils.GetSettingsOrDefault(PowerDisplaySettings.ModuleName); @@ -252,67 +233,45 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable settings.Properties.Monitors .Where(m => m.IsHidden) .Select(m => m.HardwareId)); - Logger.LogInfo($"[UpdateMonitorList] Loaded settings, found {hiddenMonitorIds.Count} hidden monitor IDs"); var colorTempTasks = new List(); - Logger.LogInfo("[UpdateMonitorList] Starting monitor loop"); foreach (var monitor in monitors) { - Logger.LogInfo($"[UpdateMonitorList] Processing monitor: {monitor.Name} (HardwareId: {monitor.HardwareId})"); - // Skip monitors that are marked as hidden in settings if (hiddenMonitorIds.Contains(monitor.HardwareId)) { - Logger.LogInfo($"[UpdateMonitorList] Skipping hidden monitor: {monitor.Name} (HardwareId: {monitor.HardwareId})"); continue; } - Logger.LogInfo($"[UpdateMonitorList] Creating MonitorViewModel for {monitor.Name}"); var vm = new MonitorViewModel(monitor, _monitorManager, this); Monitors.Add(vm); - Logger.LogInfo($"[UpdateMonitorList] Added MonitorViewModel, total count: {Monitors.Count}"); // Asynchronously initialize color temperature for DDC/CI monitors if (monitor.SupportsColorTemperature && monitor.CommunicationMethod == "DDC/CI") { - Logger.LogInfo($"[UpdateMonitorList] Initializing color temperature for {monitor.Name}"); var task = InitializeColorTemperatureSafeAsync(monitor.Id, vm); colorTempTasks.Add(task); - Logger.LogInfo($"[UpdateMonitorList] Color temp task added, total tasks: {colorTempTasks.Count}"); } } - Logger.LogInfo("[UpdateMonitorList] Monitor loop completed"); - - Logger.LogInfo("[UpdateMonitorList] Calling OnPropertyChanged(HasMonitors)"); OnPropertyChanged(nameof(HasMonitors)); - Logger.LogInfo("[UpdateMonitorList] Calling OnPropertyChanged(ShowNoMonitorsMessage)"); OnPropertyChanged(nameof(ShowNoMonitorsMessage)); - Logger.LogInfo("[UpdateMonitorList] OnPropertyChanged calls completed"); // Wait for color temperature initialization to complete before saving // This ensures we save the actual scanned values instead of defaults if (colorTempTasks.Count > 0) { - Logger.LogInfo($"[UpdateMonitorList] Waiting for {colorTempTasks.Count} color temperature tasks to complete before saving..."); - // Use fire-and-forget async method to avoid blocking UI thread _ = WaitForColorTempAndSaveAsync(colorTempTasks); } else { // No color temperature tasks, save immediately - Logger.LogInfo("[UpdateMonitorList] No color temperature tasks, calling SaveMonitorsToSettings immediately"); SaveMonitorsToSettings(); - Logger.LogInfo("[UpdateMonitorList] SaveMonitorsToSettings completed"); // Restore saved settings if enabled (async, don't block) - Logger.LogInfo("[UpdateMonitorList] About to call ReloadMonitorSettingsAsync"); _ = ReloadMonitorSettingsAsync(null); - Logger.LogInfo("[UpdateMonitorList] ReloadMonitorSettingsAsync invoked (fire-and-forget)"); } - - Logger.LogInfo("[UpdateMonitorList] Method returning"); } private async Task WaitForColorTempAndSaveAsync(List colorTempTasks) @@ -321,22 +280,17 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable { // Wait for all color temperature initialization tasks to complete await Task.WhenAll(colorTempTasks); - Logger.LogInfo("[WaitForColorTempAndSaveAsync] Color temperature tasks completed"); // Save monitor information to settings.json and reload settings // Must be done on UI thread since these methods access UI properties and observable collections - Logger.LogInfo("[WaitForColorTempAndSaveAsync] Dispatching save and reload to UI thread"); _dispatcherQueue.TryEnqueue(async () => { try { SaveMonitorsToSettings(); - Logger.LogInfo("[WaitForColorTempAndSaveAsync] SaveMonitorsToSettings completed with actual color temp values"); // Restore saved settings if enabled (async) - Logger.LogInfo("[WaitForColorTempAndSaveAsync] About to call ReloadMonitorSettingsAsync"); await ReloadMonitorSettingsAsync(null); // Tasks already completed, pass null - Logger.LogInfo("[WaitForColorTempAndSaveAsync] ReloadMonitorSettingsAsync completed"); } catch (Exception innerEx) { @@ -352,7 +306,6 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable _dispatcherQueue.TryEnqueue(() => { SaveMonitorsToSettings(); - Logger.LogInfo("[WaitForColorTempAndSaveAsync] SaveMonitorsToSettings completed (after error)"); }); } } @@ -605,37 +558,6 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable await monitorVm.SetColorTemperatureAsync(colorTemperature); } - /// - /// Apply Settings UI configuration changes (feature visibility toggles only) - /// OBSOLETE: Use ApplySettingsFromUI() instead - /// - [Obsolete("Use ApplySettingsFromUI() instead - this method only handles UI config, not hardware parameters")] - public void ApplySettingsUIConfiguration() - { - try - { - Logger.LogInfo("[Settings] Applying Settings UI configuration changes (feature visibility only)"); - - // Read current settings - var settings = _settingsUtils.GetSettingsOrDefault("PowerDisplay"); - - // Update feature visibility for each monitor (UI configuration only) - foreach (var monitorVm in Monitors) - { - ApplyFeatureVisibility(monitorVm, settings); - } - - // Trigger UI refresh for configuration changes - UIRefreshRequested?.Invoke(this, EventArgs.Empty); - - Logger.LogInfo($"[Settings] Settings UI configuration applied, monitor count: {settings.Properties.Monitors.Count}"); - } - catch (Exception ex) - { - Logger.LogError($"[Settings] Failed to apply Settings UI configuration: {ex.Message}"); - } - } - /// /// Reload monitor settings from configuration - ONLY called at startup /// @@ -653,14 +575,9 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable try { - Logger.LogInfo("[ReloadMonitorSettingsAsync] Setting IsLoading = true"); - // Set loading state to block UI interactions IsLoading = true; - Logger.LogInfo("[ReloadMonitorSettingsAsync] IsLoading set to true"); - StatusText = "Loading settings..."; - Logger.LogInfo("[ReloadMonitorSettingsAsync] StatusText updated"); // Read current settings var settings = _settingsUtils.GetSettingsOrDefault("PowerDisplay"); @@ -668,19 +585,14 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable if (settings.Properties.RestoreSettingsOnStartup) { // Restore saved settings from configuration file - Logger.LogInfo("[Startup] RestoreSettingsOnStartup enabled - applying saved settings"); - foreach (var monitorVm in Monitors) { var hardwareId = monitorVm.HardwareId; - Logger.LogInfo($"[Startup] Processing monitor: '{monitorVm.Name}', HardwareId: '{hardwareId}'"); // Find and apply corresponding saved settings from state file using stable HardwareId var savedState = _stateManager.GetMonitorParameters(hardwareId); if (savedState.HasValue) { - Logger.LogInfo($"[Startup] Restoring state for HardwareId '{hardwareId}': Brightness={savedState.Value.Brightness}, ColorTemp={savedState.Value.ColorTemperature}"); - // Validate and apply saved values (skip invalid values) // Use UpdatePropertySilently to avoid triggering hardware updates during initialization if (savedState.Value.Brightness >= monitorVm.MinBrightness && savedState.Value.Brightness <= monitorVm.MaxBrightness) @@ -689,7 +601,7 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable } else { - Logger.LogWarning($"[Startup] Invalid brightness value {savedState.Value.Brightness} for HardwareId '{hardwareId}', skipping"); + Logger.LogWarning($"[Startup] Invalid brightness value {savedState.Value.Brightness} for HardwareId '{hardwareId}'"); } // Color temperature: VCP 0x14 preset value (discrete values, no range check needed) @@ -707,10 +619,6 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable { monitorVm.UpdatePropertySilently(nameof(monitorVm.Contrast), savedState.Value.Contrast); } - else if (!monitorVm.ShowContrast) - { - Logger.LogInfo($"[Startup] Contrast not supported on HardwareId '{hardwareId}', skipping"); - } // Volume validation - only apply if hardware supports it if (monitorVm.ShowVolume && @@ -719,14 +627,6 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable { monitorVm.UpdatePropertySilently(nameof(monitorVm.Volume), savedState.Value.Volume); } - else if (!monitorVm.ShowVolume) - { - Logger.LogInfo($"[Startup] Volume not supported on HardwareId '{hardwareId}', skipping"); - } - } - else - { - Logger.LogInfo($"[Startup] No saved state for HardwareId '{hardwareId}' - keeping current hardware values"); } // Apply feature visibility settings using HardwareId @@ -738,27 +638,18 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable else { // Save current hardware values to configuration file - Logger.LogInfo("[Startup] RestoreSettingsOnStartup disabled - saving current hardware values"); - // Wait for color temperature initialization to complete (if any) if (colorTempInitTasks != null && colorTempInitTasks.Count > 0) { - Logger.LogInfo($"[Startup] Waiting for {colorTempInitTasks.Count} color temperature initialization tasks to complete..."); try { - Logger.LogInfo("[Startup] Calling Task.WhenAll on color temp tasks"); await Task.WhenAll(colorTempInitTasks); - Logger.LogInfo("[Startup] Task.WhenAll completed successfully"); } catch (Exception ex) { Logger.LogWarning($"[Startup] Some color temperature initializations failed: {ex.Message}"); } } - else - { - Logger.LogInfo("[Startup] No color temperature tasks to wait for"); - } foreach (var monitorVm in Monitors) { @@ -768,8 +659,6 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable SaveMonitorSettingDirect(monitorVm.HardwareId, "Contrast", monitorVm.Contrast); SaveMonitorSettingDirect(monitorVm.HardwareId, "Volume", monitorVm.Volume); - Logger.LogInfo($"[Startup] Saved current values for Hardware ID '{monitorVm.HardwareId}': Brightness={monitorVm.Brightness}, ColorTemp={monitorVm.ColorTemperature}"); - // Apply feature visibility settings ApplyFeatureVisibility(monitorVm, settings); } @@ -780,7 +669,7 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable } catch (Exception ex) { - Logger.LogError($"[ReloadMonitorSettingsAsync] Exception caught: {ex.Message}"); + Logger.LogError($"[ReloadMonitorSettingsAsync] Failed to reload settings: {ex.Message}"); Logger.LogError($"[ReloadMonitorSettingsAsync] Stack trace: {ex.StackTrace}"); StatusText = $"Failed to process settings: {ex.Message}"; } @@ -881,34 +770,8 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable foreach (var vm in Monitors) { - var monitorInfo = new Microsoft.PowerToys.Settings.UI.Library.MonitorInfo( - name: vm.Name, - internalName: vm.Id, - hardwareId: vm.HardwareId, - communicationMethod: vm.CommunicationMethod, - monitorType: vm.IsInternal ? "Internal" : "External", - currentBrightness: vm.Brightness, - colorTemperature: vm.ColorTemperature) - { - CapabilitiesRaw = vm.CapabilitiesRaw, - VcpCodes = vm.VcpCapabilitiesInfo?.SupportedVcpCodes - .OrderBy(kvp => kvp.Key) - .Select(kvp => $"0x{kvp.Key:X2}") - .ToList() ?? new List(), - VcpCodesFormatted = vm.VcpCapabilitiesInfo?.SupportedVcpCodes - .OrderBy(kvp => kvp.Key) - .Select(kvp => FormatVcpCodeForDisplay(kvp.Key, kvp.Value)) - .ToList() ?? new List(), - }; - - // Preserve user settings from existing monitor if available - if (existingMonitorSettings.TryGetValue(vm.HardwareId, out var existingMonitor)) - { - monitorInfo.IsHidden = existingMonitor.IsHidden; - monitorInfo.EnableContrast = existingMonitor.EnableContrast; - monitorInfo.EnableVolume = existingMonitor.EnableVolume; - } - + var monitorInfo = CreateMonitorInfo(vm); + ApplyPreservedUserSettings(monitorInfo, existingMonitorSettings); monitors.Add(monitorInfo); } @@ -942,6 +805,63 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable } } + /// + /// Create MonitorInfo object from MonitorViewModel + /// + private Microsoft.PowerToys.Settings.UI.Library.MonitorInfo CreateMonitorInfo(MonitorViewModel vm) + { + return new Microsoft.PowerToys.Settings.UI.Library.MonitorInfo( + name: vm.Name, + internalName: vm.Id, + hardwareId: vm.HardwareId, + communicationMethod: vm.CommunicationMethod, + monitorType: vm.IsInternal ? "Internal" : "External", + currentBrightness: vm.Brightness, + colorTemperature: vm.ColorTemperature) + { + CapabilitiesRaw = vm.CapabilitiesRaw, + VcpCodes = BuildVcpCodesList(vm), + VcpCodesFormatted = BuildFormattedVcpCodesList(vm), + }; + } + + /// + /// Build list of VCP codes in hex format + /// + private List BuildVcpCodesList(MonitorViewModel vm) + { + return vm.VcpCapabilitiesInfo?.SupportedVcpCodes + .OrderBy(kvp => kvp.Key) + .Select(kvp => $"0x{kvp.Key:X2}") + .ToList() ?? new List(); + } + + /// + /// Build list of formatted VCP codes with display info + /// + private List BuildFormattedVcpCodesList(MonitorViewModel vm) + { + return vm.VcpCapabilitiesInfo?.SupportedVcpCodes + .OrderBy(kvp => kvp.Key) + .Select(kvp => FormatVcpCodeForDisplay(kvp.Key, kvp.Value)) + .ToList() ?? new List(); + } + + /// + /// Apply preserved user settings from existing monitor settings + /// + private void ApplyPreservedUserSettings( + Microsoft.PowerToys.Settings.UI.Library.MonitorInfo monitorInfo, + Dictionary existingSettings) + { + if (existingSettings.TryGetValue(monitorInfo.HardwareId, out var existingMonitor)) + { + monitorInfo.IsHidden = existingMonitor.IsHidden; + monitorInfo.EnableContrast = existingMonitor.EnableContrast; + monitorInfo.EnableVolume = existingMonitor.EnableVolume; + } + } + /// /// Signal Settings UI that the monitor list has been refreshed /// @@ -1018,18 +938,25 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable // State is already persisted, no pending changes to wait for. // Quick cleanup of monitor view models - try + foreach (var vm in Monitors) { - foreach (var vm in Monitors) + try { vm?.Dispose(); } + catch (Exception ex) + { + Logger.LogDebug($"Error disposing monitor VM: {ex.Message}"); + } + } + try + { Monitors.Clear(); } - catch + catch (Exception ex) { - /* Ignore cleanup errors */ + Logger.LogDebug($"Error clearing Monitors collection: {ex.Message}"); } // Release monitor manager @@ -1037,9 +964,9 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable { _monitorManager?.Dispose(); } - catch + catch (Exception ex) { - /* Ignore cleanup errors */ + Logger.LogDebug($"Error disposing MonitorManager: {ex.Message}"); } // Release state manager @@ -1047,9 +974,9 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable { _stateManager?.Dispose(); } - catch + catch (Exception ex) { - /* Ignore cleanup errors */ + Logger.LogDebug($"Error disposing StateManager: {ex.Message}"); } // Finally release cancellation token @@ -1057,9 +984,9 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable { _cancellationTokenSource?.Dispose(); } - catch + catch (Exception ex) { - /* Ignore cleanup errors */ + Logger.LogDebug($"Error disposing CancellationTokenSource: {ex.Message}"); } GC.SuppressFinalize(this);