mirror of
https://github.com/microsoft/PowerToys.git
synced 2025-12-16 19:57:57 +01:00
Improve error handling, debouncing, and code cleanup
Enhanced exception handling in RelayCommand to improve robustness. Standardized slider debounce delays using a new constant `SliderDebounceDelayMs`. Improved resource management in SimpleDebouncer with proper disposal of CancellationTokenSource and added support for synchronous actions. Refactored event handling in App.xaml.cs for clarity and consistency. Removed redundant logging in MonitorStateManager and MainViewModel to reduce verbosity. Updated namespaces and dependencies for better organization. General code cleanup to improve readability and maintainability.
This commit is contained in:
@@ -4,6 +4,7 @@
|
||||
|
||||
using System;
|
||||
using System.Windows.Input;
|
||||
using ManagedCommon;
|
||||
|
||||
namespace PowerDisplay.Commands
|
||||
{
|
||||
@@ -23,9 +24,35 @@ namespace PowerDisplay.Commands
|
||||
|
||||
public event EventHandler? CanExecuteChanged;
|
||||
|
||||
public bool CanExecute(object? parameter) => _canExecute?.Invoke() ?? true;
|
||||
public bool CanExecute(object? parameter)
|
||||
{
|
||||
if (_canExecute == null)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
public void Execute(object? parameter) => _execute();
|
||||
try
|
||||
{
|
||||
return _canExecute.Invoke();
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Logger.LogError($"CanExecute failed: {ex.Message}");
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
public void Execute(object? parameter)
|
||||
{
|
||||
try
|
||||
{
|
||||
_execute();
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Logger.LogError($"Command execution failed: {ex.Message}");
|
||||
}
|
||||
}
|
||||
|
||||
public void RaiseCanExecuteChanged() => CanExecuteChanged?.Invoke(this, EventArgs.Empty);
|
||||
}
|
||||
@@ -47,9 +74,35 @@ namespace PowerDisplay.Commands
|
||||
|
||||
public event EventHandler? CanExecuteChanged;
|
||||
|
||||
public bool CanExecute(object? parameter) => _canExecute?.Invoke((T?)parameter) ?? true;
|
||||
public bool CanExecute(object? parameter)
|
||||
{
|
||||
if (_canExecute == null)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
public void Execute(object? parameter) => _execute((T?)parameter);
|
||||
try
|
||||
{
|
||||
return _canExecute.Invoke((T?)parameter);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Logger.LogError($"CanExecute<T> failed: {ex.Message}");
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
public void Execute(object? parameter)
|
||||
{
|
||||
try
|
||||
{
|
||||
_execute((T?)parameter);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Logger.LogError($"Command<T> execution failed: {ex.Message}");
|
||||
}
|
||||
}
|
||||
|
||||
public void RaiseCanExecuteChanged() => CanExecuteChanged?.Invoke(this, EventArgs.Empty);
|
||||
}
|
||||
|
||||
@@ -60,6 +60,11 @@ namespace PowerDisplay.Configuration
|
||||
public const int AnimationDelayMs = 100;
|
||||
public const int LayoutUpdateDelayMs = 50;
|
||||
public const int MonitorDiscoveryDelayMs = 200;
|
||||
|
||||
/// <summary>
|
||||
/// Debounce delay for slider controls in milliseconds
|
||||
/// </summary>
|
||||
public const int SliderDebounceDelayMs = 300;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -175,7 +175,8 @@ namespace PowerDisplay.Core
|
||||
});
|
||||
|
||||
var initializedMonitors = await Task.WhenAll(initTasks);
|
||||
newMonitors.AddRange(initializedMonitors.Where(m => m != null));
|
||||
var validMonitors = initializedMonitors.Where(m => m != null).Cast<Monitor>();
|
||||
newMonitors.AddRange(validMonitors);
|
||||
}
|
||||
|
||||
// Update monitor list
|
||||
|
||||
@@ -140,8 +140,6 @@ namespace PowerDisplay.Helpers
|
||||
|
||||
// Reset timer to debounce rapid updates (e.g., during slider drag)
|
||||
_saveTimer.Change(SaveDebounceMs, Timeout.Infinite);
|
||||
|
||||
Logger.LogTrace($"[State] Updated {property}={value} for monitor HardwareId='{hardwareId}', save scheduled");
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
|
||||
@@ -34,43 +34,9 @@ namespace PowerDisplay.Helpers
|
||||
/// Debounce an async action. Cancels previous invocation if still pending.
|
||||
/// </summary>
|
||||
/// <param name="action">Async action to execute after delay</param>
|
||||
public async void Debounce(Func<Task> action)
|
||||
public void Debounce(Func<Task> action)
|
||||
{
|
||||
if (_disposed)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
CancellationTokenSource cts;
|
||||
|
||||
lock (_lock)
|
||||
{
|
||||
// Cancel previous invocation
|
||||
_cts?.Cancel();
|
||||
_cts = new CancellationTokenSource();
|
||||
cts = _cts;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
// Wait for quiet period
|
||||
await Task.Delay(_delayMs, cts.Token);
|
||||
|
||||
// Execute action if not cancelled
|
||||
if (!cts.Token.IsCancellationRequested)
|
||||
{
|
||||
await action();
|
||||
}
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
// Expected when debouncing - a newer call cancelled this one
|
||||
Logger.LogTrace("Debounced action cancelled (replaced by newer call)");
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Logger.LogError($"Debounced action failed: {ex.Message}");
|
||||
}
|
||||
_ = DebounceAsync(action);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -78,13 +44,68 @@ namespace PowerDisplay.Helpers
|
||||
/// </summary>
|
||||
public void Debounce(Action action)
|
||||
{
|
||||
Debounce(() =>
|
||||
_ = DebounceAsync(() =>
|
||||
{
|
||||
action();
|
||||
return Task.CompletedTask;
|
||||
});
|
||||
}
|
||||
|
||||
private async Task DebounceAsync(Func<Task> action)
|
||||
{
|
||||
if (_disposed)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
CancellationTokenSource cts;
|
||||
CancellationTokenSource? oldCts = null;
|
||||
|
||||
lock (_lock)
|
||||
{
|
||||
// Store old CTS to dispose later
|
||||
oldCts = _cts;
|
||||
|
||||
// Create new CTS
|
||||
_cts = new CancellationTokenSource();
|
||||
cts = _cts;
|
||||
}
|
||||
|
||||
// Dispose old CTS outside the lock to avoid blocking
|
||||
if (oldCts != null)
|
||||
{
|
||||
try
|
||||
{
|
||||
oldCts.Cancel();
|
||||
oldCts.Dispose();
|
||||
}
|
||||
catch
|
||||
{
|
||||
// Ignore disposal errors
|
||||
}
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
// Wait for quiet period
|
||||
await Task.Delay(_delayMs, cts.Token).ConfigureAwait(false);
|
||||
|
||||
// Execute action if not cancelled
|
||||
if (!cts.Token.IsCancellationRequested)
|
||||
{
|
||||
await action().ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
// Expected when debouncing - a newer call cancelled this one
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Logger.LogError($"Debounced action failed: {ex.Message}");
|
||||
}
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
if (_disposed)
|
||||
|
||||
@@ -114,7 +114,8 @@ namespace PowerDisplay
|
||||
{
|
||||
Logger.LogError($"[EVENT] _mainWindow type mismatch, actual type: {_mainWindow?.GetType().Name}");
|
||||
}
|
||||
});
|
||||
},
|
||||
CancellationToken.None);
|
||||
|
||||
NativeEventWaiter.WaitForEventLoop(
|
||||
TogglePowerDisplayEvent,
|
||||
@@ -130,7 +131,8 @@ namespace PowerDisplay
|
||||
{
|
||||
Logger.LogError($"[EVENT] _mainWindow type mismatch for toggle");
|
||||
}
|
||||
});
|
||||
},
|
||||
CancellationToken.None);
|
||||
|
||||
NativeEventWaiter.WaitForEventLoop(
|
||||
TerminatePowerDisplayEvent,
|
||||
@@ -138,7 +140,8 @@ namespace PowerDisplay
|
||||
{
|
||||
Logger.LogInfo("Received terminate event - exiting immediately");
|
||||
Environment.Exit(0);
|
||||
});
|
||||
},
|
||||
CancellationToken.None);
|
||||
|
||||
// Note: PowerDisplay.exe should NOT listen to RefreshMonitorsEvent
|
||||
// That event is sent BY PowerDisplay TO Settings UI for one-way notification
|
||||
@@ -155,7 +158,8 @@ namespace PowerDisplay
|
||||
mainWindow.ViewModel.ApplySettingsFromUI();
|
||||
}
|
||||
});
|
||||
});
|
||||
},
|
||||
CancellationToken.None);
|
||||
|
||||
NativeEventWaiter.WaitForEventLoop(
|
||||
ApplyColorTemperatureEvent,
|
||||
@@ -169,7 +173,8 @@ namespace PowerDisplay
|
||||
mainWindow.ViewModel.ApplyColorTemperatureFromSettings();
|
||||
}
|
||||
});
|
||||
});
|
||||
},
|
||||
CancellationToken.None);
|
||||
|
||||
// Monitor Runner process (backup exit mechanism)
|
||||
if (_powerToysRunnerPid > 0)
|
||||
|
||||
@@ -772,8 +772,6 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable
|
||||
// This is thread-safe - _stateManager has internal locking
|
||||
// No UI thread operations, no ObservableCollection access
|
||||
_stateManager.UpdateMonitorParameter(hardwareId, property, value);
|
||||
|
||||
Logger.LogTrace($"[State] Queued setting change for HardwareId '{hardwareId}': {property}={value}");
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
|
||||
@@ -10,6 +10,7 @@ using System.Windows.Input;
|
||||
using ManagedCommon;
|
||||
using Microsoft.UI.Xaml;
|
||||
using PowerDisplay.Commands;
|
||||
using PowerDisplay.Configuration;
|
||||
using PowerDisplay.Core;
|
||||
using PowerDisplay.Core.Models;
|
||||
using PowerDisplay.Helpers;
|
||||
@@ -27,9 +28,9 @@ public partial class MonitorViewModel : INotifyPropertyChanged, IDisposable
|
||||
private readonly MainViewModel? _mainViewModel;
|
||||
|
||||
// Simple debouncers for each property (KISS principle - simpler than complex queue)
|
||||
private readonly SimpleDebouncer _brightnessDebouncer = new(300);
|
||||
private readonly SimpleDebouncer _contrastDebouncer = new(300);
|
||||
private readonly SimpleDebouncer _volumeDebouncer = new(300);
|
||||
private readonly SimpleDebouncer _brightnessDebouncer = new(AppConstants.UI.SliderDebounceDelayMs);
|
||||
private readonly SimpleDebouncer _contrastDebouncer = new(AppConstants.UI.SliderDebounceDelayMs);
|
||||
private readonly SimpleDebouncer _volumeDebouncer = new(AppConstants.UI.SliderDebounceDelayMs);
|
||||
|
||||
private int _brightness;
|
||||
private int _contrast;
|
||||
@@ -196,7 +197,6 @@ public partial class MonitorViewModel : INotifyPropertyChanged, IDisposable
|
||||
if (result.IsSuccess)
|
||||
{
|
||||
_mainViewModel?.SaveMonitorSettingDirect(_monitor.HardwareId, "Brightness", brightness);
|
||||
Logger.LogTrace($"[{HardwareId}] Brightness applied and saved");
|
||||
}
|
||||
else
|
||||
{
|
||||
@@ -223,7 +223,6 @@ public partial class MonitorViewModel : INotifyPropertyChanged, IDisposable
|
||||
if (result.IsSuccess)
|
||||
{
|
||||
_mainViewModel?.SaveMonitorSettingDirect(_monitor.HardwareId, "Contrast", contrast);
|
||||
Logger.LogTrace($"[{HardwareId}] Contrast applied and saved");
|
||||
}
|
||||
else
|
||||
{
|
||||
@@ -250,7 +249,6 @@ public partial class MonitorViewModel : INotifyPropertyChanged, IDisposable
|
||||
if (result.IsSuccess)
|
||||
{
|
||||
_mainViewModel?.SaveMonitorSettingDirect(_monitor.HardwareId, "Volume", volume);
|
||||
Logger.LogTrace($"[{HardwareId}] Volume applied and saved");
|
||||
}
|
||||
else
|
||||
{
|
||||
|
||||
@@ -73,8 +73,7 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
|
||||
{
|
||||
Logger.LogInfo("Received refresh monitors event from PowerDisplay.exe");
|
||||
ReloadMonitorsFromSettings();
|
||||
},
|
||||
_cancellationTokenSource.Token);
|
||||
});
|
||||
}
|
||||
|
||||
private void InitializeEnabledValue()
|
||||
|
||||
Reference in New Issue
Block a user