Mirrors/PowerToys
1
0
Fork 0
mirror of https://github.com/microsoft/PowerToys.git synced 2025-12-15 03:07:56 +01:00
Code Issues Packages Projects Releases Wiki Activity

Refactor: use Polly, ConcurrentDictionary, unify VCP parsing

Browse Source 
- Replace custom retry logic with Polly.Core resilience pipelines for DDC/CI operations
- Remove LockedDictionary and RetryHelper; use ConcurrentDictionary for thread safety
- Add MonitorFeatureHelper to centralize VCP feature parsing
- Simplify monitor state management and update code for clarity
- Add Polly.Core dependency and update documentation
- Remove obsolete helper files
This commit is contained in:
Yu Leng
2025-12-11 10:52:51 +08:00
parent 75f57f53f2
commit 5f15229691
10 changed files with 237 additions and 293 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
Directory.Packages.props
View File

@@ -89,6 +89,7 @@
<PackageVersion Include="NLog.Extensions.Logging" Version="5.3.8" />
<PackageVersion Include="NLog.Schema" Version="5.2.8" />
<PackageVersion Include="OpenAI" Version="2.5.0" />
<PackageVersion Include="Polly.Core" Version="8.6.5" />
<PackageVersion Include="ReverseMarkdown" Version="4.1.0" />
<PackageVersion Include="RtfPipe" Version="2.0.7677.4303" />
<PackageVersion Include="ScipBe.Common.Office.OneNote" Version="3.0.1" />

1
NOTICE.md
View File

@@ -1587,6 +1587,7 @@ SOFTWARE.
- NLog.Extensions.Logging
- NLog.Schema
- OpenAI
- Polly.Core
- ReverseMarkdown
- ScipBe.Common.Office.OneNote
- SharpCompress

68
src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/DdcCiController.cs
View File

@@ -8,6 +8,8 @@ using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using ManagedCommon;
using Polly;
using Polly.Retry;
using PowerDisplay.Common.Interfaces;
using PowerDisplay.Common.Models;
using PowerDisplay.Common.Utils;
@@ -45,6 +47,42 @@ namespace PowerDisplay.Common.Drivers.DDC
/// </summary>
private const int RetryDelayMs = 100;
/// <summary>
/// Retry pipeline for getting capabilities string length (3 retries).
/// </summary>
private static readonly ResiliencePipeline<uint> CapabilitiesLengthRetryPipeline =
new ResiliencePipelineBuilder<uint>()
.AddRetry(new RetryStrategyOptions<uint>
{
MaxRetryAttempts = 2, // 2 retries = 3 total attempts
Delay = TimeSpan.FromMilliseconds(RetryDelayMs),
ShouldHandle = new PredicateBuilder<uint>().HandleResult(len => len == 0),
OnRetry = static args =>
{
Logger.LogWarning($"[Retry] GetCapabilitiesStringLength returned invalid result on attempt {args.AttemptNumber + 1}, retrying...");
return default;
},
})
.Build();
/// <summary>
/// Retry pipeline for getting capabilities string (5 retries).
/// </summary>
private static readonly ResiliencePipeline<string?> CapabilitiesStringRetryPipeline =
new ResiliencePipelineBuilder<string?>()
.AddRetry(new RetryStrategyOptions<string?>
{
MaxRetryAttempts = 4, // 4 retries = 5 total attempts
Delay = TimeSpan.FromMilliseconds(RetryDelayMs),
ShouldHandle = new PredicateBuilder<string?>().HandleResult(static str => string.IsNullOrEmpty(str)),
OnRetry = static args =>
{
Logger.LogWarning($"[Retry] GetCapabilitiesString returned invalid result on attempt {args.AttemptNumber + 1}, retrying...");
return default;
},
})
.Build();
private readonly PhysicalMonitorHandleManager _handleManager = new();
private readonly MonitorDiscoveryHelper _discoveryHelper;
@@ -142,39 +180,33 @@ namespace PowerDisplay.Common.Drivers.DDC
try
{
// Step 1: Get capabilities string length with retry
var length = RetryHelper.ExecuteWithRetry(
() =>
var length = CapabilitiesLengthRetryPipeline.Execute(() =>
{
if (GetCapabilitiesStringLength(monitor.Handle, out uint len) && len > 0)
{
if (GetCapabilitiesStringLength(monitor.Handle, out uint len) && len > 0)
{
return len;
}
return len;
}
return 0u;
},
len => len > 0,
maxRetries: 3,
delayMs: RetryDelayMs,
operationName: "GetCapabilitiesStringLength");
return 0u;
});
if (length == 0)
{
Logger.LogWarning("[Retry] GetCapabilitiesStringLength failed after 3 attempts");
return string.Empty;
}
// Step 2: Get actual capabilities string with retry
var capsString = RetryHelper.ExecuteWithRetry(
() => TryGetCapabilitiesString(monitor.Handle, length),
str => !string.IsNullOrEmpty(str),
maxRetries: 5,
delayMs: RetryDelayMs,
operationName: "GetCapabilitiesString");
var capsString = CapabilitiesStringRetryPipeline.Execute(
() => TryGetCapabilitiesString(monitor.Handle, length));
if (!string.IsNullOrEmpty(capsString))
{
Logger.LogDebug($"Got capabilities string (length: {capsString.Length})");
return capsString;
}
Logger.LogWarning("[Retry] GetCapabilitiesString failed after 5 attempts");
}
catch (Exception ex)
{

37
src/modules/powerdisplay/PowerDisplay.Lib/Drivers/DDC/PhysicalMonitorHandleManager.cs
View File

@@ -3,9 +3,10 @@
// See the LICENSE file in the project root for more information.
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using ManagedCommon;
using PowerDisplay.Common.Utils;
using static PowerDisplay.Common.Drivers.PInvoke;
namespace PowerDisplay.Common.Drivers.DDC
@@ -16,7 +17,8 @@ namespace PowerDisplay.Common.Drivers.DDC
public partial class PhysicalMonitorHandleManager : IDisposable
{
// Mapping: monitorId -> physical handle (thread-safe)
private readonly LockedDictionary<string, IntPtr> _monitorIdToHandleMap = new();
private readonly ConcurrentDictionary<string, IntPtr> _monitorIdToHandleMap = new();
private readonly object _handleLock = new();
private bool _disposed;
/// <summary>
@@ -24,28 +26,28 @@ namespace PowerDisplay.Common.Drivers.DDC
/// </summary>
public void UpdateHandleMap(Dictionary<string, IntPtr> newHandleMap)
{
_monitorIdToHandleMap.ExecuteWithLock(dict =>
// Lock to ensure atomic update (cleanup + replace)
lock (_handleLock)
{
// Clean up unused handles before updating
CleanupUnusedHandles(dict, newHandleMap);
CleanupUnusedHandles(newHandleMap);
// Update the device key map
dict.Clear();
_monitorIdToHandleMap.Clear();
foreach (var kvp in newHandleMap)
{
dict[kvp.Key] = kvp.Value;
_monitorIdToHandleMap[kvp.Key] = kvp.Value;
}
});
}
}
/// <summary>
/// Clean up handles that are no longer in use.
/// Called within ExecuteWithLock context with the internal dictionary.
/// Optimized to O(n) using HashSet lookup instead of O(n*m) nested loops.
/// Called within lock context. Optimized to O(n) using HashSet lookup.
/// </summary>
private void CleanupUnusedHandles(Dictionary<string, IntPtr> currentHandles, Dictionary<string, IntPtr> newHandles)
private void CleanupUnusedHandles(Dictionary<string, IntPtr> newHandles)
{
if (currentHandles.Count == 0)
if (_monitorIdToHandleMap.IsEmpty)
{
return;
}
@@ -54,14 +56,9 @@ namespace PowerDisplay.Common.Drivers.DDC
var reusedHandles = new HashSet<IntPtr>(newHandles.Values);
// Find handles to destroy: in old map but not reused (O(n) with O(1) lookup)
var handlesToDestroy = new List<IntPtr>();
foreach (var oldHandle in currentHandles.Values)
{
if (oldHandle != IntPtr.Zero && !reusedHandles.Contains(oldHandle))
{
handlesToDestroy.Add(oldHandle);
}
}
var handlesToDestroy = _monitorIdToHandleMap.Values
.Where(h => h != IntPtr.Zero && !reusedHandles.Contains(h))
.ToList();
// Destroy unused handles
foreach (var handle in handlesToDestroy)
@@ -86,7 +83,7 @@ namespace PowerDisplay.Common.Drivers.DDC
}
// Release all physical monitor handles - get snapshot to avoid holding lock during cleanup
var handles = _monitorIdToHandleMap.GetValuesSnapshot();
var handles = _monitorIdToHandleMap.Values.ToList();
foreach (var handle in handles)
{
if (handle != IntPtr.Zero)

1
src/modules/powerdisplay/PowerDisplay.Lib/PowerDisplay.Lib.csproj
View File

@@ -23,6 +23,7 @@
<SuppressTrimAnalysisWarnings>false</SuppressTrimAnalysisWarnings>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Polly.Core" />
<PackageReference Include="WmiLight" />
<PackageReference Include="System.Collections.Immutable" />
</ItemGroup>

116
src/modules/powerdisplay/PowerDisplay.Lib/Services/MonitorStateManager.cs
View File

@@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.IO;
using System.Text.Json;
@@ -23,7 +24,8 @@ namespace PowerDisplay.Common.Services
public partial class MonitorStateManager : IDisposable
{
private readonly string _stateFilePath;
private readonly LockedDictionary<string, MonitorState> _states = new();
private readonly ConcurrentDictionary<string, MonitorState> _states = new();
private readonly object _statesLock = new();
private readonly SimpleDebouncer _saveDebouncer;
private bool _disposed;
@@ -83,39 +85,35 @@ namespace PowerDisplay.Common.Services
return;
}
bool shouldSave = _states.ExecuteWithLock(dict =>
var state = _states.GetOrAdd(hardwareId, _ => new MonitorState());
// Update the specific property
bool shouldSave = true;
switch (property)
{
// Get or create state entry using HardwareId
if (!dict.TryGetValue(hardwareId, out var state))
{
state = new MonitorState();
dict[hardwareId] = state;
}
// Update the specific property
switch (property)
{
case "Brightness":
state.Brightness = value;
break;
case "ColorTemperature":
state.ColorTemperatureVcp = value;
break;
case "Contrast":
state.Contrast = value;
break;
case "Volume":
state.Volume = value;
break;
default:
Logger.LogWarning($"Unknown property: {property}");
return false;
}
case "Brightness":
state.Brightness = value;
break;
case "ColorTemperature":
state.ColorTemperatureVcp = value;
break;
case "Contrast":
state.Contrast = value;
break;
case "Volume":
state.Volume = value;
break;
default:
Logger.LogWarning($"Unknown property: {property}");
shouldSave = false;
break;
}
if (shouldSave)
{
// Mark dirty for flush on dispose
_isDirty = true;
return true;
});
}
// Schedule debounced save (SimpleDebouncer handles cancellation of previous calls)
if (shouldSave)
@@ -141,7 +139,7 @@ namespace PowerDisplay.Common.Services
return null;
}
if (_states.TryGetValue(hardwareId, out var state) && state != null)
if (_states.TryGetValue(hardwareId, out var state))
{
return (state.Brightness, state.ColorTemperatureVcp, state.Contrast, state.Volume);
}
@@ -167,23 +165,20 @@ namespace PowerDisplay.Common.Services
if (stateFile?.Monitors != null)
{
_states.ExecuteWithLock(dict =>
foreach (var kvp in stateFile.Monitors)
{
foreach (var kvp in stateFile.Monitors)
{
var monitorKey = kvp.Key; // Should be HardwareId (e.g., "GSM5C6D")
var entry = kvp.Value;
var monitorKey = kvp.Key; // Should be HardwareId (e.g., "GSM5C6D")
var entry = kvp.Value;
dict[monitorKey] = new MonitorState
{
Brightness = entry.Brightness,
ColorTemperatureVcp = entry.ColorTemperatureVcp,
Contrast = entry.Contrast,
Volume = entry.Volume,
CapabilitiesRaw = entry.CapabilitiesRaw,
};
}
});
_states[monitorKey] = new MonitorState
{
Brightness = entry.Brightness,
ColorTemperatureVcp = entry.ColorTemperatureVcp,
Contrast = entry.Contrast,
Volume = entry.Volume,
CapabilitiesRaw = entry.CapabilitiesRaw,
};
}
Logger.LogInfo($"[State] Loaded state for {stateFile.Monitors.Count} monitors from {_stateFilePath}");
Logger.LogInfo($"[State] Monitor keys in state file: {string.Join(", ", stateFile.Monitors.Keys)}");
@@ -258,24 +253,21 @@ namespace PowerDisplay.Common.Services
LastUpdated = now,
};
_states.ExecuteWithLock(dict =>
foreach (var kvp in _states)
{
foreach (var kvp in dict)
{
var monitorId = kvp.Key;
var state = kvp.Value;
var monitorId = kvp.Key;
var state = kvp.Value;
stateFile.Monitors[monitorId] = new MonitorStateEntry
{
Brightness = state.Brightness,
ColorTemperatureVcp = state.ColorTemperatureVcp,
Contrast = state.Contrast,
Volume = state.Volume,
CapabilitiesRaw = state.CapabilitiesRaw,
LastUpdated = now,
};
}
});
stateFile.Monitors[monitorId] = new MonitorStateEntry
{
Brightness = state.Brightness,
ColorTemperatureVcp = state.ColorTemperatureVcp,
Contrast = state.Contrast,
Volume = state.Volume,
CapabilitiesRaw = state.CapabilitiesRaw,
LastUpdated = now,
};
}
var json = JsonSerializer.Serialize(stateFile, ProfileSerializationContext.Default.MonitorStateFile);
return (json, stateFile.Monitors.Count);

93
src/modules/powerdisplay/PowerDisplay.Lib/Utils/LockedDictionary.cs
View File

@@ -1,93 +0,0 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
using System;
using System.Collections.Generic;
#pragma warning disable SA1649 // File name should match first type name (generic class name is valid)
namespace PowerDisplay.Common.Utils
{
/// <summary>
/// A thread-safe dictionary wrapper that provides atomic operations with minimal locking overhead.
/// Designed for scenarios where simple get/set operations are common but complex transactions are rare.
/// For complex multi-step transactions, use <see cref="ExecuteWithLock"/> to ensure atomicity.
/// </summary>
/// <typeparam name="TKey">The type of keys in the dictionary.</typeparam>
/// <typeparam name="TValue">The type of values in the dictionary.</typeparam>
public class LockedDictionary<TKey, TValue>
where TKey : notnull
{
private readonly Dictionary<TKey, TValue> _dictionary = new();
private readonly object _lock = new();
/// <summary>
/// Tries to get the value associated with the specified key.
/// </summary>
/// <param name="key">The key to locate.</param>
/// <param name="value">When this method returns, contains the value if found; otherwise, the default value.</param>
/// <returns>True if the key was found; otherwise, false.</returns>
public bool TryGetValue(TKey key, out TValue? value)
{
lock (_lock)
{
return _dictionary.TryGetValue(key, out value);
}
}
/// <summary>
/// Removes all key/value pairs from the dictionary.
/// </summary>
public void Clear()
{
lock (_lock)
{
_dictionary.Clear();
}
}
/// <summary>
/// Gets a snapshot of all values in the dictionary.
/// Returns a copy to ensure thread safety.
/// </summary>
/// <returns>A list containing copies of all values.</returns>
public List<TValue> GetValuesSnapshot()
{
lock (_lock)
{
return new List<TValue>(_dictionary.Values);
}
}
/// <summary>
/// Executes an action within the lock, providing the internal dictionary for complex operations.
/// Use this for multi-step transactions that need to be atomic.
/// WARNING: Do not store or return references to the dictionary outside the action.
/// </summary>
/// <param name="action">The action to execute with the dictionary.</param>
public void ExecuteWithLock(Action<Dictionary<TKey, TValue>> action)
{
lock (_lock)
{
action(_dictionary);
}
}
/// <summary>
/// Executes a function within the lock, providing the internal dictionary for complex operations.
/// Use this for multi-step transactions that need to be atomic and return a result.
/// WARNING: Do not store or return references to the dictionary outside the function.
/// </summary>
/// <typeparam name="TResult">The type of the result.</typeparam>
/// <param name="func">The function to execute with the dictionary.</param>
/// <returns>The result of the function.</returns>
public TResult ExecuteWithLock<TResult>(Func<Dictionary<TKey, TValue>, TResult> func)
{
lock (_lock)
{
return func(_dictionary);
}
}
}
}

112
src/modules/powerdisplay/PowerDisplay.Lib/Utils/MonitorFeatureHelper.cs Normal file
View File

@@ -0,0 +1,112 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
using System;
using System.Collections.Generic;
using System.Globalization;
using PowerDisplay.Common.Drivers;
namespace PowerDisplay.Common.Utils
{
/// <summary>
/// Unified helper class for parsing monitor feature support from VCP capabilities.
/// This eliminates duplicate VCP parsing logic across PowerDisplay.exe and Settings.UI.
/// </summary>
public static class MonitorFeatureHelper
{
/// <summary>
/// Result of parsing monitor feature support from VCP capabilities
/// </summary>
public readonly struct FeatureSupportResult
{
public bool SupportsBrightness { get; init; }
public bool SupportsContrast { get; init; }
public bool SupportsColorTemperature { get; init; }
public bool SupportsVolume { get; init; }
public bool SupportsInputSource { get; init; }
public string CapabilitiesStatus { get; init; }
public static FeatureSupportResult Unavailable => new()
{
SupportsBrightness = false,
SupportsContrast = false,
SupportsColorTemperature = false,
SupportsVolume = false,
SupportsInputSource = false,
CapabilitiesStatus = "unavailable",
};
}
/// <summary>
/// Parse feature support from a list of VCP code strings.
/// This is the single source of truth for determining monitor capabilities.
/// </summary>
/// <param name="vcpCodes">List of VCP codes as strings (e.g., "0x10", "10", "0x12")</param>
/// <param name="capabilitiesRaw">Raw capabilities string, used to determine availability status</param>
/// <returns>Feature support result</returns>
public static FeatureSupportResult ParseFeatureSupport(IReadOnlyList<string>? vcpCodes, string? capabilitiesRaw)
{
// Check capabilities availability
if (string.IsNullOrEmpty(capabilitiesRaw))
{
return FeatureSupportResult.Unavailable;
}
// Convert all VCP codes to integers for comparison
var vcpCodeInts = ParseVcpCodesToIntegers(vcpCodes);
// Determine feature support based on VCP codes
return new FeatureSupportResult
{
SupportsBrightness = vcpCodeInts.Contains(NativeConstants.VcpCodeBrightness),
SupportsContrast = vcpCodeInts.Contains(NativeConstants.VcpCodeContrast),
SupportsColorTemperature = vcpCodeInts.Contains(NativeConstants.VcpCodeSelectColorPreset),
SupportsVolume = vcpCodeInts.Contains(NativeConstants.VcpCodeVolume),
SupportsInputSource = vcpCodeInts.Contains(NativeConstants.VcpCodeInputSource),
CapabilitiesStatus = "available",
};
}
/// <summary>
/// Parse VCP codes from string list to integer set
/// Handles both hex formats: "0x10" and "10"
/// </summary>
private static HashSet<int> ParseVcpCodesToIntegers(IReadOnlyList<string>? vcpCodes)
{
var result = new HashSet<int>();
if (vcpCodes == null)
{
return result;
}
foreach (var code in vcpCodes)
{
if (string.IsNullOrWhiteSpace(code))
{
continue;
}
// Remove "0x" prefix if present and parse as hex
var cleanCode = code.Trim();
if (cleanCode.StartsWith("0x", StringComparison.OrdinalIgnoreCase))
{
cleanCode = cleanCode.Substring(2);
}
if (int.TryParse(cleanCode, NumberStyles.HexNumber, CultureInfo.InvariantCulture, out int codeInt))
{
result.Add(codeInt);
}
}
return result;
}
}
}

99
src/modules/powerdisplay/PowerDisplay.Lib/Utils/RetryHelper.cs
View File

@@ -1,99 +0,0 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
using System;
using System.Threading;
using ManagedCommon;
namespace PowerDisplay.Common.Utils
{
/// <summary>
/// Helper class for executing operations with retry logic.
/// Provides a unified retry pattern for DDC/CI operations that may fail intermittently.
/// </summary>
public static class RetryHelper
{
/// <summary>
/// Default delay between retry attempts in milliseconds.
/// </summary>
public const int DefaultRetryDelayMs = 100;
/// <summary>
/// Default maximum number of retry attempts.
/// </summary>
public const int DefaultMaxRetries = 3;
/// <summary>
/// Executes a synchronous operation with retry logic.
/// </summary>
/// <typeparam name="T">The return type of the operation.</typeparam>
/// <param name="operation">The operation to execute.</param>
/// <param name="isValid">Predicate to determine if the result is valid.</param>
/// <param name="maxRetries">Maximum number of retry attempts (default: 3).</param>
/// <param name="delayMs">Delay between retries in milliseconds (default: 100).</param>
/// <param name="operationName">Optional name for logging purposes.</param>
/// <returns>The result of the operation, or default if all retries failed.</returns>
public static T? ExecuteWithRetry<T>(
Func<T?> operation,
Func<T?, bool> isValid,
int maxRetries = DefaultMaxRetries,
int delayMs = DefaultRetryDelayMs,
string? operationName = null)
{
for (int attempt = 0; attempt < maxRetries; attempt++)
{
try
{
var result = operation();
if (isValid(result))
{
if (attempt > 0 && !string.IsNullOrEmpty(operationName))
{
Logger.LogDebug($"[Retry] {operationName} succeeded on attempt {attempt + 1}");
}
return result;
}
if (attempt < maxRetries - 1)
{
if (!string.IsNullOrEmpty(operationName))
{
Logger.LogWarning($"[Retry] {operationName} returned invalid result on attempt {attempt + 1}, retrying...");
}
Thread.Sleep(delayMs);
}
}
catch (Exception ex)
{
if (attempt < maxRetries - 1)
{
if (!string.IsNullOrEmpty(operationName))
{
Logger.LogWarning($"[Retry] {operationName} failed on attempt {attempt + 1}: {ex.Message}, retrying...");
}
Thread.Sleep(delayMs);
}
else
{
if (!string.IsNullOrEmpty(operationName))
{
Logger.LogWarning($"[Retry] {operationName} failed after {maxRetries} attempts: {ex.Message}");
}
}
}
}
if (!string.IsNullOrEmpty(operationName))
{
Logger.LogWarning($"[Retry] {operationName} failed after {maxRetries} attempts");
}
return default;
}
}
}

2
src/settings-ui/Settings.UI/ViewModels/PowerDisplayViewModel.cs
View File

@@ -565,7 +565,7 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
private ObservableCollection<PowerDisplayProfile> _profiles = new ObservableCollection<PowerDisplayProfile>();
/// <summary>
/// Collection of available profiles (for button display)
/// Gets or sets collection of available profiles (for button display)
/// </summary>
public ObservableCollection<PowerDisplayProfile> Profiles
{