From 4e5e0961d813859248ae0d252823cec3a5a59e8d Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Tue, 2 Jun 2026 10:42:50 -0500 Subject: [PATCH] Extract PlotTrackingService from MainWindowViewModel Moves the plot series collection, color palette, retention constants, and the ~80-line TrackPlotPoints reconciliation (history + retained synthetic points + current value, de-duplicated by timestamp and pruned to the retention window) out of the view model into a focused, unit-testable collaborator. The view model keeps a thin PlotSeries pass-through and delegates Track/Reset/RefreshSeriesColor. Behavior is unchanged. Adds 8 tests covering selection add/remove, history+current merge, timestamp de-duplication, Fahrenheit conversion, reset, and pen-color application. 193 tests pass (was 185). Co-Authored-By: Claude Opus 4.8 --- .../ViewModels/PlotTrackingServiceTests.cs | 144 +++++++++++++++++ .../ViewModels/MainWindowViewModel.cs | 119 +------------- .../ViewModels/PlotTrackingService.cs | 150 ++++++++++++++++++ 3 files changed, 300 insertions(+), 113 deletions(-) create mode 100644 LibreHardwareMonitor.Windows.WinUI.Tests/ViewModels/PlotTrackingServiceTests.cs create mode 100644 LibreHardwareMonitor.Windows.WinUI/ViewModels/PlotTrackingService.cs diff --git a/LibreHardwareMonitor.Windows.WinUI.Tests/ViewModels/PlotTrackingServiceTests.cs b/LibreHardwareMonitor.Windows.WinUI.Tests/ViewModels/PlotTrackingServiceTests.cs new file mode 100644 index 0000000..1ac64e9 --- /dev/null +++ b/LibreHardwareMonitor.Windows.WinUI.Tests/ViewModels/PlotTrackingServiceTests.cs @@ -0,0 +1,144 @@ +// This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. +// If a copy of the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. +// Copyright (C) LibreHardwareMonitor and Contributors. + +using System; +using System.Linq; +using LibreHardwareMonitor.Hardware; +using LibreHardwareMonitor.Windows.WinUI.Services; +using LibreHardwareMonitor.Windows.WinUI.ViewModels; +using Moq; +using Windows.UI; +using Xunit; + +namespace LibreHardwareMonitor.Windows.WinUI.Tests.ViewModels; + +public class PlotTrackingServiceTests +{ + [Fact] + public void Track_AddsSeriesForPlottedSensor() + { + (PlotTrackingService service, SensorTreeItemViewModel root, _, Mock sensorMock) = Plotted(SensorType.Load, currentValue: 50f); + + service.Track(root, TemperatureUnit.Celsius); + + Assert.Single(service.Series); + Assert.Equal(sensorMock.Object.Identifier.ToString(), service.Series[0].SensorIdentifier); + } + + [Fact] + public void Track_IgnoresSensorsNotFlaggedForPlotting() + { + (PlotTrackingService service, SensorTreeItemViewModel root, SensorTreeItemViewModel sensorItem, _) = Plotted(SensorType.Load, currentValue: 50f); + sensorItem.Plot = false; + + service.Track(root, TemperatureUnit.Celsius); + + Assert.Empty(service.Series); + } + + [Fact] + public void Track_RemovesSeriesWhenSensorDeselected() + { + (PlotTrackingService service, SensorTreeItemViewModel root, SensorTreeItemViewModel sensorItem, _) = Plotted(SensorType.Load, currentValue: 50f); + + service.Track(root, TemperatureUnit.Celsius); + Assert.Single(service.Series); + + sensorItem.Plot = false; + service.Track(root, TemperatureUnit.Celsius); + Assert.Empty(service.Series); + } + + [Fact] + public void Track_IncludesStoredHistoryThenCurrentValue() + { + DateTime now = DateTime.UtcNow; + SensorValue[] history = [new SensorValue(10f, now.AddSeconds(-3)), new SensorValue(20f, now.AddSeconds(-2))]; + (PlotTrackingService service, SensorTreeItemViewModel root, _, _) = Plotted(SensorType.Load, currentValue: 30f, values: history); + + service.Track(root, TemperatureUnit.Celsius); + + double[] values = service.Series[0].Points.Select(point => point.Value).ToArray(); + Assert.Equal([10d, 20d, 30d], values); + } + + [Fact] + public void Track_DeduplicatesPointsWithSameTimestamp() + { + DateTime time = DateTime.UtcNow.AddSeconds(-5); + SensorValue[] history = [new SensorValue(10f, time), new SensorValue(99f, time)]; + (PlotTrackingService service, SensorTreeItemViewModel root, _, _) = Plotted(SensorType.Load, currentValue: null, values: history); + + service.Track(root, TemperatureUnit.Celsius); + + PlotPointViewModel point = Assert.Single(service.Series[0].Points); + Assert.Equal(99d, point.Value); // the last value sharing that timestamp wins + } + + [Fact] + public void Track_AppliesFahrenheitConversion() + { + (PlotTrackingService service, SensorTreeItemViewModel root, _, _) = Plotted(SensorType.Temperature, currentValue: 100f); + + service.Track(root, TemperatureUnit.Fahrenheit); + + PlotPointViewModel point = Assert.Single(service.Series[0].Points); + Assert.Equal(212d, point.Value, 1); // 100 °C == 212 °F + Assert.Equal("°F", service.Series[0].Unit); + } + + [Fact] + public void Reset_ClearsSeriesAndAllowsReTracking() + { + (PlotTrackingService service, SensorTreeItemViewModel root, _, _) = Plotted(SensorType.Load, currentValue: 50f); + service.Track(root, TemperatureUnit.Celsius); + + service.Reset(); + Assert.Empty(service.Series); + + service.Track(root, TemperatureUnit.Celsius); + Assert.Single(service.Series); + } + + [Fact] + public void RefreshSeriesColor_AppliesUserPenColor() + { + (PlotTrackingService service, SensorTreeItemViewModel root, SensorTreeItemViewModel sensorItem, _) = Plotted(SensorType.Load, currentValue: 50f); + service.Track(root, TemperatureUnit.Celsius); + + Color userColor = Color.FromArgb(255, 1, 2, 3); + sensorItem.PenColor = userColor; + service.RefreshSeriesColor(sensorItem); + + Assert.Equal(userColor, service.Series[0].Color); + } + + private static (PlotTrackingService Service, SensorTreeItemViewModel Root, SensorTreeItemViewModel SensorItem, Mock SensorMock) + Plotted(SensorType sensorType, float? currentValue, params SensorValue[] values) + { + var hardwareMock = new Mock(); + var sensorMock = new Mock(); + + sensorMock.Setup(s => s.Name).Returns("Sensor"); + sensorMock.Setup(s => s.SensorType).Returns(sensorType); + sensorMock.Setup(s => s.Index).Returns(0); + sensorMock.Setup(s => s.Identifier).Returns(new Identifier("cpu", "0", sensorType.ToString().ToLowerInvariant(), "0")); + sensorMock.Setup(s => s.Hardware).Returns(hardwareMock.Object); + sensorMock.Setup(s => s.Value).Returns(currentValue); + sensorMock.Setup(s => s.Values).Returns(values); + + hardwareMock.Setup(h => h.Name).Returns("CPU"); + hardwareMock.Setup(h => h.HardwareType).Returns(HardwareType.Cpu); + hardwareMock.Setup(h => h.Identifier).Returns(new Identifier("cpu", "0")); + hardwareMock.Setup(h => h.Sensors).Returns([sensorMock.Object]); + hardwareMock.Setup(h => h.SubHardware).Returns([]); + + var root = SensorTreeItemViewModel.CreateRoot("HOST"); + root.Children.Add(SensorTreeItemViewModel.FromHardware(hardwareMock.Object, AppSettings.LoadDefault())); + SensorTreeItemViewModel sensorItem = root.EnumerateSensors().First(); + sensorItem.Plot = true; + + return (new PlotTrackingService(), root, sensorItem, sensorMock); + } +} diff --git a/LibreHardwareMonitor.Windows.WinUI/ViewModels/MainWindowViewModel.cs b/LibreHardwareMonitor.Windows.WinUI/ViewModels/MainWindowViewModel.cs index 559998e..6c0a0fd 100644 --- a/LibreHardwareMonitor.Windows.WinUI/ViewModels/MainWindowViewModel.cs +++ b/LibreHardwareMonitor.Windows.WinUI/ViewModels/MainWindowViewModel.cs @@ -63,8 +63,6 @@ public sealed class MainWindowViewModel : ViewModelBase, IDisposable ]; private const int DefaultPlotTimeWindowIndex = 2; - private static readonly TimeSpan MaximumPlotPointRetention = TimeSpan.FromHours(24); - private static readonly TimeSpan MaximumSyntheticPlotPointRetention = TimeSpan.FromMinutes(5); private static readonly TimeSpan?[] PlotTimeWindows = [ @@ -83,22 +81,10 @@ public sealed class MainWindowViewModel : ViewModelBase, IDisposable TimeSpan.FromHours(24) ]; - private static readonly Color[] PlotColors = - [ - Color.FromArgb(255, 0x00, 0x78, 0xD4), - Color.FromArgb(255, 0xE8, 0x11, 0x23), - Color.FromArgb(255, 0x10, 0x7C, 0x10), - Color.FromArgb(255, 0xF7, 0x63, 0x0C), - Color.FromArgb(255, 0x88, 0x17, 0x98), - Color.FromArgb(255, 0x00, 0xB7, 0xC3), - Color.FromArgb(255, 0x49, 0x8B, 0x00), - Color.FromArgb(255, 0xA8, 0x00, 0x00) - ]; - private readonly HardwareMonitorService _hardwareMonitor; private readonly DispatcherQueue _dispatcherQueue; private readonly Logger _logger; - private readonly Dictionary _plotSeriesByIdentifier = new(); + private readonly PlotTrackingService _plotTracking = new(); private readonly RemoteWebServer _remoteWebServer; private readonly StartupService _startupService = new(); private readonly WinUiStartupTrace? _startupTrace; @@ -185,7 +171,7 @@ public sealed class MainWindowViewModel : ViewModelBase, IDisposable public Visibility MinColumnVisibility => ShowMinColumn ? Visibility.Visible : Visibility.Collapsed; - public ObservableCollection PlotSeries { get; } = []; + public ObservableCollection PlotSeries => _plotTracking.Series; public bool IsPlotWindowVisible => ShowPlot && PlotLocation == PlotLocation.Window; @@ -766,14 +752,13 @@ public sealed class MainWindowViewModel : ViewModelBase, IDisposable public void ResetPlot() { _hardwareMonitor.ClearSensorValues(); - _plotSeriesByIdentifier.Clear(); - PlotSeries.Clear(); + _plotTracking.Reset(); PlotInvalidated?.Invoke(this, EventArgs.Empty); } public void RefreshPlotSeries() { - TrackPlotPoints(); + _plotTracking.Track(RootItems.FirstOrDefault(), TemperatureUnit); PlotInvalidated?.Invoke(this, EventArgs.Empty); } @@ -848,10 +833,7 @@ public sealed class MainWindowViewModel : ViewModelBase, IDisposable return; item.PenColor = color; - string identifier = item.Sensor.Identifier.ToString(); - if (_plotSeriesByIdentifier.TryGetValue(identifier, out PlotSeriesViewModel? series)) - series.Color = GetPlotColor(item); - + _plotTracking.RefreshSeriesColor(item); PlotInvalidated?.Invoke(this, EventArgs.Empty); } @@ -898,7 +880,7 @@ public sealed class MainWindowViewModel : ViewModelBase, IDisposable SensorTreeItemViewModel? root = RootItems.FirstOrDefault(); root?.RefreshValues(); if (trackPlotPoints) - TrackPlotPoints(); + _plotTracking.Track(root, TemperatureUnit); if (logSensors) _logger.Log(); @@ -1043,90 +1025,6 @@ public sealed class MainWindowViewModel : ViewModelBase, IDisposable OnPropertyChanged(); } - private void TrackPlotPoints() - { - SensorTreeItemViewModel? root = RootItems.FirstOrDefault(); - if (root == null) - return; - - DateTime now = DateTime.UtcNow; - HashSet selectedIdentifiers = new(); - foreach (SensorTreeItemViewModel sensorItem in root.EnumerateSensors().Where(sensorItem => sensorItem.Plot && sensorItem.Sensor != null)) - { - ISensor sensor = sensorItem.Sensor!; - string identifier = sensor.Identifier.ToString(); - selectedIdentifiers.Add(identifier); - - if (!_plotSeriesByIdentifier.TryGetValue(identifier, out PlotSeriesViewModel? series)) - { - series = new PlotSeriesViewModel( - identifier, - sensor.Hardware.Name, - sensor.Name, - sensor.SensorType, - SensorFormatter.GetPlotUnit(sensor.SensorType, TemperatureUnit), - GetPlotColor(sensorItem)); - _plotSeriesByIdentifier[identifier] = series; - PlotSeries.Add(series); - } - else - { - series.UpdateMetadata(sensor.Hardware.Name, sensor.Name, sensor.SensorType, SensorFormatter.GetPlotUnit(sensor.SensorType, TemperatureUnit)); - - // Only honor an explicit user pen color for an existing series; keep the auto-assigned color stable. - // (Recomputing it from the live series count made existing lines shift/collide colors every tick.) - if (sensorItem.PenColor.HasValue) - series.Color = sensorItem.PenColor.Value; - } - - List points = []; - foreach (SensorValue sensorValue in sensor.Values.OrderBy(value => value.Time)) - { - double? displayedValue = SensorFormatter.GetPlotValue(sensor, sensorValue.Value, TemperatureUnit); - if (displayedValue is not { } pointValue || !double.IsFinite(pointValue)) - continue; - - points.Add(new PlotPointViewModel(sensorValue.Time, pointValue)); - } - - DateTime? latestHistoryTimestamp = points.Count > 0 ? points[^1].Timestamp : null; - foreach (PlotPointViewModel existingPoint in series.Points) - { - if (now - existingPoint.Timestamp > MaximumSyntheticPlotPointRetention) - continue; - - if (!latestHistoryTimestamp.HasValue || existingPoint.Timestamp > latestHistoryTimestamp.Value) - points.Add(existingPoint); - } - - double? currentValue = SensorFormatter.GetPlotValue(sensor, TemperatureUnit); - if (currentValue is { } currentPointValue && double.IsFinite(currentPointValue)) - points.Add(new PlotPointViewModel(now, currentPointValue)); - - DateTime cutoff = now - MaximumPlotPointRetention; - points = points - .Where(point => point.Timestamp >= cutoff) - .GroupBy(point => point.Timestamp.Ticks) - .Select(group => group.Last()) - .OrderBy(point => point.Timestamp) - .ToList(); - - if (points.Count == 0 && currentValue is { } fallbackPointValue && double.IsFinite(fallbackPointValue)) - { - points.Add(new PlotPointViewModel(now, fallbackPointValue)); - } - - series.ReplacePoints(points); - } - - foreach (string identifier in _plotSeriesByIdentifier.Keys.Where(identifier => !selectedIdentifiers.Contains(identifier)).ToArray()) - { - PlotSeriesViewModel series = _plotSeriesByIdentifier[identifier]; - _plotSeriesByIdentifier.Remove(identifier); - PlotSeries.Remove(series); - } - } - private void UpdateRoot() { MeasureStartup("MainWindowViewModel.RootItems.Clear", RootItems.Clear); @@ -1144,11 +1042,6 @@ public sealed class MainWindowViewModel : ViewModelBase, IDisposable StatusText = $"{hardwareCount} hardware devices, {sensorCount} sensors, update every {FormatInterval(UpdateInterval)}{webServerStatus}"; } - private Color GetPlotColor(SensorTreeItemViewModel sensorItem) - { - return sensorItem.PenColor ?? PlotColors[_plotSeriesByIdentifier.Count % PlotColors.Length]; - } - private void RestartWebServerIfRunning() { if (!RunWebServer) diff --git a/LibreHardwareMonitor.Windows.WinUI/ViewModels/PlotTrackingService.cs b/LibreHardwareMonitor.Windows.WinUI/ViewModels/PlotTrackingService.cs new file mode 100644 index 0000000..5345e9a --- /dev/null +++ b/LibreHardwareMonitor.Windows.WinUI/ViewModels/PlotTrackingService.cs @@ -0,0 +1,150 @@ +// This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. +// If a copy of the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. +// Copyright (C) LibreHardwareMonitor and Contributors. + +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Linq; +using LibreHardwareMonitor.Hardware; +using LibreHardwareMonitor.Windows.WinUI.Utilities; +using Windows.UI; + +namespace LibreHardwareMonitor.Windows.WinUI.ViewModels; + +/// +/// Owns the plot's series collection and reconciles it with the currently selected sensors on each update. Extracted +/// from so the (subtle) point-retention/merge logic can be unit-tested in isolation. +/// +internal sealed class PlotTrackingService +{ + private static readonly TimeSpan MaximumPlotPointRetention = TimeSpan.FromHours(24); + private static readonly TimeSpan MaximumSyntheticPlotPointRetention = TimeSpan.FromMinutes(5); + + private static readonly Color[] PlotColors = + [ + Color.FromArgb(255, 0x00, 0x78, 0xD4), + Color.FromArgb(255, 0xE8, 0x11, 0x23), + Color.FromArgb(255, 0x10, 0x7C, 0x10), + Color.FromArgb(255, 0xF7, 0x63, 0x0C), + Color.FromArgb(255, 0x88, 0x17, 0x98), + Color.FromArgb(255, 0x00, 0xB7, 0xC3), + Color.FromArgb(255, 0x49, 0x8B, 0x00), + Color.FromArgb(255, 0xA8, 0x00, 0x00) + ]; + + private readonly Dictionary _seriesByIdentifier = new(); + + /// The live series collection, bound by the plot view. + public ObservableCollection Series { get; } = []; + + /// + /// Reconciles with the sensors under that are flagged for plotting: + /// adds/updates a series per selected sensor (merging stored history, retained synthetic points, and the current + /// value, de-duplicated by timestamp and pruned to the retention window) and removes series no longer selected. + /// + public void Track(SensorTreeItemViewModel? root, TemperatureUnit temperatureUnit) + { + if (root == null) + return; + + DateTime now = DateTime.UtcNow; + HashSet selectedIdentifiers = new(); + foreach (SensorTreeItemViewModel sensorItem in root.EnumerateSensors().Where(sensorItem => sensorItem.Plot && sensorItem.Sensor != null)) + { + ISensor sensor = sensorItem.Sensor!; + string identifier = sensor.Identifier.ToString(); + selectedIdentifiers.Add(identifier); + + if (!_seriesByIdentifier.TryGetValue(identifier, out PlotSeriesViewModel? series)) + { + series = new PlotSeriesViewModel( + identifier, + sensor.Hardware.Name, + sensor.Name, + sensor.SensorType, + SensorFormatter.GetPlotUnit(sensor.SensorType, temperatureUnit), + GetPlotColor(sensorItem)); + _seriesByIdentifier[identifier] = series; + Series.Add(series); + } + else + { + series.UpdateMetadata(sensor.Hardware.Name, sensor.Name, sensor.SensorType, SensorFormatter.GetPlotUnit(sensor.SensorType, temperatureUnit)); + + // Only honor an explicit user pen color for an existing series; keep the auto-assigned color stable. + // (Recomputing it from the live series count made existing lines shift/collide colors every tick.) + if (sensorItem.PenColor.HasValue) + series.Color = sensorItem.PenColor.Value; + } + + List points = []; + foreach (SensorValue sensorValue in sensor.Values.OrderBy(value => value.Time)) + { + double? displayedValue = SensorFormatter.GetPlotValue(sensor, sensorValue.Value, temperatureUnit); + if (displayedValue is not { } pointValue || !double.IsFinite(pointValue)) + continue; + + points.Add(new PlotPointViewModel(sensorValue.Time, pointValue)); + } + + DateTime? latestHistoryTimestamp = points.Count > 0 ? points[^1].Timestamp : null; + foreach (PlotPointViewModel existingPoint in series.Points) + { + if (now - existingPoint.Timestamp > MaximumSyntheticPlotPointRetention) + continue; + + if (!latestHistoryTimestamp.HasValue || existingPoint.Timestamp > latestHistoryTimestamp.Value) + points.Add(existingPoint); + } + + double? currentValue = SensorFormatter.GetPlotValue(sensor, temperatureUnit); + if (currentValue is { } currentPointValue && double.IsFinite(currentPointValue)) + points.Add(new PlotPointViewModel(now, currentPointValue)); + + DateTime cutoff = now - MaximumPlotPointRetention; + points = points + .Where(point => point.Timestamp >= cutoff) + .GroupBy(point => point.Timestamp.Ticks) + .Select(group => group.Last()) + .OrderBy(point => point.Timestamp) + .ToList(); + + if (points.Count == 0 && currentValue is { } fallbackPointValue && double.IsFinite(fallbackPointValue)) + { + points.Add(new PlotPointViewModel(now, fallbackPointValue)); + } + + series.ReplacePoints(points); + } + + foreach (string identifier in _seriesByIdentifier.Keys.Where(identifier => !selectedIdentifiers.Contains(identifier)).ToArray()) + { + PlotSeriesViewModel series = _seriesByIdentifier[identifier]; + _seriesByIdentifier.Remove(identifier); + Series.Remove(series); + } + } + + /// Clears all tracked series (used when resetting the plot). + public void Reset() + { + _seriesByIdentifier.Clear(); + Series.Clear(); + } + + /// Re-applies the color for an existing series after its sensor's pen color changed. + public void RefreshSeriesColor(SensorTreeItemViewModel sensorItem) + { + if (sensorItem.Sensor == null) + return; + + if (_seriesByIdentifier.TryGetValue(sensorItem.Sensor.Identifier.ToString(), out PlotSeriesViewModel? series)) + series.Color = GetPlotColor(sensorItem); + } + + private Color GetPlotColor(SensorTreeItemViewModel sensorItem) + { + return sensorItem.PenColor ?? PlotColors[_seriesByIdentifier.Count % PlotColors.Length]; + } +}