mirror of
https://github.com/microsoft/PowerToys.git
synced 2025-12-15 03:07:56 +01:00
Refactor controller selection and remove CanControlMonitorAsync
Refactored MonitorManager to use dedicated controller fields for O(1) lookup based on CommunicationMethod, replacing the async GetControllerForMonitorAsync with a synchronous method. Removed CanControlMonitorAsync from IMonitorController and all implementations, simplifying the interface and controller logic. Updated controller discovery and disposal logic accordingly. Improved performance and maintainability by eliminating unnecessary async checks and streamlining controller management.
This commit is contained in:
@@ -57,51 +57,13 @@ namespace PowerDisplay.Common.Drivers.DDC
|
||||
|
||||
public string Name => "DDC/CI Monitor Controller";
|
||||
|
||||
/// <summary>
|
||||
/// Check if the specified monitor can be controlled.
|
||||
/// Uses quick connection check since capabilities are already cached during discovery.
|
||||
/// </summary>
|
||||
public async Task<bool> CanControlMonitorAsync(Monitor monitor, CancellationToken cancellationToken = default)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(monitor);
|
||||
|
||||
return await Task.Run(
|
||||
() =>
|
||||
{
|
||||
var physicalHandle = GetPhysicalHandle(monitor);
|
||||
if (physicalHandle == IntPtr.Zero)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
// Capabilities are always cached during DiscoverMonitorsAsync Phase 2,
|
||||
// so we can use quick connection check instead of full validation
|
||||
return DdcCiNative.QuickConnectionCheck(physicalHandle);
|
||||
},
|
||||
cancellationToken);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Get monitor brightness using VCP code 0x10
|
||||
/// </summary>
|
||||
public async Task<BrightnessInfo> GetBrightnessAsync(Monitor monitor, CancellationToken cancellationToken = default)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(monitor);
|
||||
|
||||
return await Task.Run(
|
||||
() =>
|
||||
{
|
||||
var physicalHandle = GetPhysicalHandle(monitor);
|
||||
var result = GetBrightnessInfoCore(monitor.Id, physicalHandle);
|
||||
|
||||
if (!result.IsValid)
|
||||
{
|
||||
Logger.LogWarning($"[{monitor.Id}] Failed to read brightness");
|
||||
}
|
||||
|
||||
return result;
|
||||
},
|
||||
cancellationToken);
|
||||
return await GetVcpFeatureAsync(monitor, VcpCodeBrightness, "Brightness", cancellationToken);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -826,37 +788,6 @@ namespace PowerDisplay.Common.Drivers.DDC
|
||||
cancellationToken);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Core implementation for getting brightness information using VCP code 0x10.
|
||||
/// </summary>
|
||||
/// <param name="monitorId">Monitor ID for logging.</param>
|
||||
/// <param name="physicalHandle">Physical monitor handle.</param>
|
||||
/// <returns>BrightnessInfo with current, min, and max values, or Invalid if failed.</returns>
|
||||
private BrightnessInfo GetBrightnessInfoCore(string monitorId, IntPtr physicalHandle)
|
||||
{
|
||||
if (physicalHandle == IntPtr.Zero)
|
||||
{
|
||||
Logger.LogDebug($"[{monitorId}] Invalid physical handle");
|
||||
return BrightnessInfo.Invalid;
|
||||
}
|
||||
|
||||
if (DdcCiNative.TryGetVCPFeature(physicalHandle, VcpCodeBrightness, out uint current, out uint max))
|
||||
{
|
||||
Logger.LogDebug($"[{monitorId}] Brightness via VCP 0x10: {current}/{max}");
|
||||
return new BrightnessInfo((int)current, 0, (int)max);
|
||||
}
|
||||
|
||||
return BrightnessInfo.Invalid;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Get physical handle for monitor using stable deviceKey
|
||||
/// </summary>
|
||||
private IntPtr GetPhysicalHandle(Monitor monitor)
|
||||
{
|
||||
return _handleManager.GetPhysicalHandle(monitor);
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
Dispose(true);
|
||||
|
||||
@@ -136,47 +136,6 @@ namespace PowerDisplay.Common.Drivers.WMI
|
||||
|
||||
public string Name => "WMI Monitor Controller (WmiLight)";
|
||||
|
||||
/// <summary>
|
||||
/// Check if the specified monitor can be controlled.
|
||||
/// Verifies the specific monitor exists in WMI by filtering on InstanceName.
|
||||
/// </summary>
|
||||
public async Task<bool> CanControlMonitorAsync(Monitor monitor, CancellationToken cancellationToken = default)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(monitor);
|
||||
|
||||
if (monitor.CommunicationMethod != "WMI")
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
// If no InstanceName, we can't verify the specific monitor
|
||||
if (string.IsNullOrEmpty(monitor.InstanceName))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
return await Task.Run(
|
||||
() =>
|
||||
{
|
||||
try
|
||||
{
|
||||
using var connection = new WmiConnection(WmiNamespace);
|
||||
|
||||
// Filter by InstanceName to verify this specific monitor exists
|
||||
var escapedInstanceName = EscapeWmiString(monitor.InstanceName);
|
||||
var query = $"SELECT InstanceName FROM {BrightnessQueryClass} WHERE InstanceName = '{escapedInstanceName}'";
|
||||
var results = connection.CreateQuery(query).ToList();
|
||||
return results.Count > 0;
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Logger.LogWarning($"WMI CanControlMonitor check failed for '{monitor.InstanceName}': {ex.Message}");
|
||||
return false;
|
||||
}
|
||||
},
|
||||
cancellationToken);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Get monitor brightness
|
||||
/// </summary>
|
||||
|
||||
@@ -21,14 +21,6 @@ namespace PowerDisplay.Common.Interfaces
|
||||
/// </summary>
|
||||
string Name { get; }
|
||||
|
||||
/// <summary>
|
||||
/// Checks whether the specified monitor can be controlled
|
||||
/// </summary>
|
||||
/// <param name="monitor">Monitor object</param>
|
||||
/// <param name="cancellationToken">Cancellation token</param>
|
||||
/// <returns>Whether the monitor can be controlled</returns>
|
||||
Task<bool> CanControlMonitorAsync(Monitor monitor, CancellationToken cancellationToken = default);
|
||||
|
||||
/// <summary>
|
||||
/// Gets monitor brightness
|
||||
/// </summary>
|
||||
|
||||
@@ -27,9 +27,12 @@ namespace PowerDisplay.Helpers
|
||||
{
|
||||
private readonly List<Monitor> _monitors = new();
|
||||
private readonly Dictionary<string, Monitor> _monitorLookup = new();
|
||||
private readonly List<IMonitorController> _controllers = new();
|
||||
private readonly SemaphoreSlim _discoveryLock = new(1, 1);
|
||||
private readonly DisplayRotationService _rotationService = new();
|
||||
|
||||
// Controllers stored by type for O(1) lookup based on CommunicationMethod
|
||||
private DdcCiController? _ddcController;
|
||||
private WmiController? _wmiController;
|
||||
private bool _disposed;
|
||||
|
||||
public IReadOnlyList<Monitor> Monitors => _monitors.AsReadOnly();
|
||||
@@ -48,7 +51,7 @@ namespace PowerDisplay.Helpers
|
||||
try
|
||||
{
|
||||
// DDC/CI controller (external monitors)
|
||||
_controllers.Add(new DdcCiController());
|
||||
_ddcController = new DdcCiController();
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
@@ -61,7 +64,7 @@ namespace PowerDisplay.Helpers
|
||||
// First check if WMI is available
|
||||
if (WmiController.IsWmiAvailable())
|
||||
{
|
||||
_controllers.Add(new WmiController());
|
||||
_wmiController = new WmiController();
|
||||
}
|
||||
else
|
||||
{
|
||||
@@ -84,19 +87,18 @@ namespace PowerDisplay.Helpers
|
||||
|
||||
try
|
||||
{
|
||||
var discoveryResults = await DiscoverFromAllControllersAsync(cancellationToken);
|
||||
var discoveredMonitors = await DiscoverFromAllControllersAsync(cancellationToken);
|
||||
|
||||
// Update collections
|
||||
_monitors.Clear();
|
||||
_monitorLookup.Clear();
|
||||
|
||||
var newMonitors = discoveryResults
|
||||
.SelectMany(r => r.Monitors)
|
||||
var sortedMonitors = discoveredMonitors
|
||||
.OrderBy(m => m.MonitorNumber)
|
||||
.ToList();
|
||||
|
||||
_monitors.AddRange(newMonitors);
|
||||
foreach (var monitor in newMonitors)
|
||||
_monitors.AddRange(sortedMonitors);
|
||||
foreach (var monitor in sortedMonitors)
|
||||
{
|
||||
_monitorLookup[monitor.Id] = monitor;
|
||||
}
|
||||
@@ -112,25 +114,40 @@ namespace PowerDisplay.Helpers
|
||||
/// <summary>
|
||||
/// Discover monitors from all registered controllers in parallel.
|
||||
/// </summary>
|
||||
private async Task<List<(IMonitorController Controller, List<Monitor> Monitors)>> DiscoverFromAllControllersAsync(
|
||||
private async Task<List<Monitor>> DiscoverFromAllControllersAsync(CancellationToken cancellationToken)
|
||||
{
|
||||
var tasks = new List<Task<IEnumerable<Monitor>>>();
|
||||
|
||||
if (_ddcController != null)
|
||||
{
|
||||
tasks.Add(SafeDiscoverAsync(_ddcController, cancellationToken));
|
||||
}
|
||||
|
||||
if (_wmiController != null)
|
||||
{
|
||||
tasks.Add(SafeDiscoverAsync(_wmiController, cancellationToken));
|
||||
}
|
||||
|
||||
var results = await Task.WhenAll(tasks);
|
||||
return results.SelectMany(m => m).ToList();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Safely discover monitors from a controller, returning empty list on failure.
|
||||
/// </summary>
|
||||
private static async Task<IEnumerable<Monitor>> SafeDiscoverAsync(
|
||||
IMonitorController controller,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
var discoveryTasks = _controllers.Select(async controller =>
|
||||
try
|
||||
{
|
||||
try
|
||||
{
|
||||
var monitors = await controller.DiscoverMonitorsAsync(cancellationToken);
|
||||
return (Controller: controller, Monitors: monitors.ToList());
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Logger.LogWarning($"Controller {controller.Name} discovery failed: {ex.Message}");
|
||||
return (Controller: controller, Monitors: new List<Monitor>());
|
||||
}
|
||||
});
|
||||
|
||||
var results = await Task.WhenAll(discoveryTasks);
|
||||
return results.ToList();
|
||||
return await controller.DiscoverMonitorsAsync(cancellationToken);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Logger.LogWarning($"Controller {controller.Name} discovery failed: {ex.Message}");
|
||||
return Enumerable.Empty<Monitor>();
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -144,7 +161,7 @@ namespace PowerDisplay.Helpers
|
||||
return BrightnessInfo.Invalid;
|
||||
}
|
||||
|
||||
var controller = await GetControllerForMonitorAsync(monitor, cancellationToken);
|
||||
var controller = GetControllerForMonitor(monitor);
|
||||
if (controller == null)
|
||||
{
|
||||
return BrightnessInfo.Invalid;
|
||||
@@ -215,7 +232,7 @@ namespace PowerDisplay.Helpers
|
||||
return BrightnessInfo.Invalid;
|
||||
}
|
||||
|
||||
var controller = await GetControllerForMonitorAsync(monitor, cancellationToken);
|
||||
var controller = GetControllerForMonitor(monitor);
|
||||
if (controller == null)
|
||||
{
|
||||
return BrightnessInfo.Invalid;
|
||||
@@ -254,7 +271,7 @@ namespace PowerDisplay.Helpers
|
||||
return BrightnessInfo.Invalid;
|
||||
}
|
||||
|
||||
var controller = await GetControllerForMonitorAsync(monitor, cancellationToken);
|
||||
var controller = GetControllerForMonitor(monitor);
|
||||
if (controller == null)
|
||||
{
|
||||
return BrightnessInfo.Invalid;
|
||||
@@ -326,20 +343,17 @@ namespace PowerDisplay.Helpers
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Get controller for the monitor
|
||||
/// Get controller for the monitor based on CommunicationMethod.
|
||||
/// O(1) lookup - no async validation needed since controller type is determined at discovery.
|
||||
/// </summary>
|
||||
private async Task<IMonitorController?> GetControllerForMonitorAsync(Monitor monitor, CancellationToken cancellationToken = default)
|
||||
private IMonitorController? GetControllerForMonitor(Monitor monitor)
|
||||
{
|
||||
// WMI monitors use WmiController, DDC/CI monitors use DdcCiController
|
||||
foreach (var controller in _controllers)
|
||||
return monitor.CommunicationMethod switch
|
||||
{
|
||||
if (await controller.CanControlMonitorAsync(monitor, cancellationToken))
|
||||
{
|
||||
return controller;
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
"WMI" => _wmiController,
|
||||
"DDC/CI" => _ddcController,
|
||||
_ => null,
|
||||
};
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -360,7 +374,7 @@ namespace PowerDisplay.Helpers
|
||||
return MonitorOperationResult.Failure("Monitor not found");
|
||||
}
|
||||
|
||||
var controller = await GetControllerForMonitorAsync(monitor, cancellationToken);
|
||||
var controller = GetControllerForMonitor(monitor);
|
||||
if (controller == null)
|
||||
{
|
||||
Logger.LogError($"[MonitorManager] No controller available for monitor {monitorId}");
|
||||
@@ -403,13 +417,10 @@ namespace PowerDisplay.Helpers
|
||||
{
|
||||
_discoveryLock?.Dispose();
|
||||
|
||||
// Release all controllers
|
||||
foreach (var controller in _controllers)
|
||||
{
|
||||
controller?.Dispose();
|
||||
}
|
||||
// Release controllers
|
||||
_ddcController?.Dispose();
|
||||
_wmiController?.Dispose();
|
||||
|
||||
_controllers.Clear();
|
||||
_monitors.Clear();
|
||||
_monitorLookup.Clear();
|
||||
_disposed = true;
|
||||
|
||||
Reference in New Issue
Block a user