diff --git a/App/Dialogs/ConnectDialog.cs b/App/Dialogs/ConnectDialog.cs index dd35868..a22b078 100644 --- a/App/Dialogs/ConnectDialog.cs +++ b/App/Dialogs/ConnectDialog.cs @@ -159,14 +159,7 @@ public ConnectDialog( }; // Default button highlighted with amber - var defaultButtonScheme = new ColorScheme - { - Normal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - Focus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - HotNormal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - HotFocus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - Disabled = new Terminal.Gui.Attribute(theme.MutedText, theme.Background) - }; + var defaultButtonScheme = ThemeStyler.CreateAccentButtonScheme(theme); var connectButton = new Button { diff --git a/App/Dialogs/OpenConfigDialog.cs b/App/Dialogs/OpenConfigDialog.cs index 564307b..062a869 100644 --- a/App/Dialogs/OpenConfigDialog.cs +++ b/App/Dialogs/OpenConfigDialog.cs @@ -72,14 +72,7 @@ public OpenConfigDialog() Y = Pos.AnchorEnd(1), Text = $"{theme.ButtonPrefix}Open{theme.ButtonSuffix}", IsDefault = true, - ColorScheme = new ColorScheme - { - Normal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - Focus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - HotNormal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - HotFocus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - Disabled = new Terminal.Gui.Attribute(theme.MutedText, theme.Background) - } + ColorScheme = ThemeStyler.CreateAccentButtonScheme(theme) }; openButton.Accepting += (_, _) => Confirm(); diff --git a/App/Dialogs/PasswordPromptDialog.cs b/App/Dialogs/PasswordPromptDialog.cs index 168d408..cb8a14b 100644 --- a/App/Dialogs/PasswordPromptDialog.cs +++ b/App/Dialogs/PasswordPromptDialog.cs @@ -49,14 +49,7 @@ public PasswordPromptDialog(string username, string endpoint) Secret = true }; - var defaultButtonScheme = new ColorScheme - { - Normal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - Focus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - HotNormal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - HotFocus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - Disabled = new Terminal.Gui.Attribute(theme.MutedText, theme.Background) - }; + var defaultButtonScheme = ThemeStyler.CreateAccentButtonScheme(theme); var okButton = new Button { diff --git a/App/Dialogs/SaveConfigDialog.cs b/App/Dialogs/SaveConfigDialog.cs index 6718def..171b2ad 100644 --- a/App/Dialogs/SaveConfigDialog.cs +++ b/App/Dialogs/SaveConfigDialog.cs @@ -120,14 +120,7 @@ public SaveConfigDialog(string defaultDirectory, string defaultFilename) }; // Buttons - var defaultButtonScheme = new ColorScheme - { - Normal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - Focus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - HotNormal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - HotFocus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - Disabled = new Terminal.Gui.Attribute(theme.MutedText, theme.Background) - }; + var defaultButtonScheme = ThemeStyler.CreateAccentButtonScheme(theme); var saveButton = new Button { diff --git a/App/Dialogs/SaveRecordingDialog.cs b/App/Dialogs/SaveRecordingDialog.cs index 2c26c23..b0752ef 100644 --- a/App/Dialogs/SaveRecordingDialog.cs +++ b/App/Dialogs/SaveRecordingDialog.cs @@ -100,14 +100,7 @@ public SaveRecordingDialog(string defaultDirectory, string defaultFilename) }; // Buttons - var defaultButtonScheme = new ColorScheme - { - Normal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - Focus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - HotNormal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - HotFocus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - Disabled = new Terminal.Gui.Attribute(theme.MutedText, theme.Background) - }; + var defaultButtonScheme = ThemeStyler.CreateAccentButtonScheme(theme); var saveButton = new Button { diff --git a/App/Dialogs/WriteValueDialog.cs b/App/Dialogs/WriteValueDialog.cs index 76b0ca6..e59e2ac 100644 --- a/App/Dialogs/WriteValueDialog.cs +++ b/App/Dialogs/WriteValueDialog.cs @@ -131,14 +131,7 @@ public WriteValueDialog(NodeId nodeId, string nodeName, BuiltInType dataType, st // Default button highlighted with amber - var defaultButtonScheme = new ColorScheme - { - Normal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - Focus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - HotNormal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - HotFocus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - Disabled = new Terminal.Gui.Attribute(theme.MutedText, theme.Background) - }; + var defaultButtonScheme = ThemeStyler.CreateAccentButtonScheme(theme); var writeButton = new Button { diff --git a/App/FocusManager.cs b/App/FocusManager.cs index 70a2849..8e76f63 100644 --- a/App/FocusManager.cs +++ b/App/FocusManager.cs @@ -79,7 +79,7 @@ private bool PollFocus() /// /// Sets focus to the pane at the specified index. /// - public void FocusPane(int index) + private void FocusPane(int index) { if (index >= 0 && index < _panes.Length) { @@ -96,15 +96,6 @@ public void FocusNext() FocusPane(GetNextIndex(currentIndex, _panes.Length)); } - /// - /// Cycles focus to the previous pane. - /// - public void FocusPrevious() - { - var currentIndex = _currentPane != null ? Array.IndexOf(_panes, _currentPane) : 0; - FocusPane(GetPreviousIndex(currentIndex, _panes.Length)); - } - /// /// Computes the index of the next pane in tab order, wrapping around to the /// first pane after the last. Pure helper extracted for testability. @@ -113,13 +104,4 @@ internal static int GetNextIndex(int currentIndex, int paneCount) { return (currentIndex + 1) % paneCount; } - - /// - /// Computes the index of the previous pane in tab order, wrapping around to the - /// last pane before the first. Pure helper extracted for testability. - /// - internal static int GetPreviousIndex(int currentIndex, int paneCount) - { - return (currentIndex - 1 + paneCount) % paneCount; - } } diff --git a/App/MainWindow.cs b/App/MainWindow.cs index d363a3e..1b134b8 100644 --- a/App/MainWindow.cs +++ b/App/MainWindow.cs @@ -88,14 +88,7 @@ public MainWindow() // Override global "Menu" ColorScheme BEFORE creating any views // This prevents StatusBar's blue background flash on first render var theme = ThemeManager.Current; - Colors.ColorSchemes["Menu"] = new ColorScheme - { - Normal = new Terminal.Gui.Attribute(theme.Foreground, theme.Background), - Focus = new Terminal.Gui.Attribute(theme.ForegroundBright, theme.Background), - HotNormal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - HotFocus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - Disabled = new Terminal.Gui.Attribute(theme.MutedText, theme.Background) - }; + Colors.ColorSchemes["Menu"] = ThemeStyler.CreateFlatBarScheme(theme); // Create theme toggle menu item _themeToggleItem = new MenuItem(GetThemeToggleTitle(), "", ToggleTheme); @@ -149,14 +142,7 @@ public MainWindow() }; // Also set ColorScheme directly on the StatusBar instance - _statusBar.ColorScheme = new ColorScheme - { - Normal = new Terminal.Gui.Attribute(theme.Foreground, theme.Background), - Focus = new Terminal.Gui.Attribute(theme.ForegroundBright, theme.Background), - HotNormal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - HotFocus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - Disabled = new Terminal.Gui.Attribute(theme.MutedText, theme.Background) - }; + _statusBar.ColorScheme = ThemeStyler.CreateFlatBarScheme(theme); // Connection status indicator (colored) - FAR RIGHT, overlaid on status bar row // We position it dynamically based on text width @@ -330,14 +316,7 @@ private void ApplyTheme() var theme = ThemeManager.Current; // Update global "Menu" ColorScheme (used by StatusBar) - Colors.ColorSchemes["Menu"] = new ColorScheme - { - Normal = new Terminal.Gui.Attribute(theme.Foreground, theme.Background), - Focus = new Terminal.Gui.Attribute(theme.ForegroundBright, theme.Background), - HotNormal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - HotFocus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - Disabled = new Terminal.Gui.Attribute(theme.MutedText, theme.Background) - }; + Colors.ColorSchemes["Menu"] = ThemeStyler.CreateFlatBarScheme(theme); // Apply main window styling - double-line for emphasis ColorScheme = theme.MainColorScheme; @@ -354,14 +333,7 @@ private void ApplyTheme() // Apply clean status bar styling (no blue background) // Must set ColorScheme AND call SetNeedsDisplay to override Terminal.Gui defaults - var cleanStatusBarScheme = new ColorScheme - { - Normal = new Terminal.Gui.Attribute(theme.Foreground, theme.Background), - Focus = new Terminal.Gui.Attribute(theme.ForegroundBright, theme.Background), - HotNormal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), - HotFocus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), - Disabled = new Terminal.Gui.Attribute(theme.MutedText, theme.Background) - }; + var cleanStatusBarScheme = ThemeStyler.CreateFlatBarScheme(theme); _statusBar.ColorScheme = cleanStatusBarScheme; _statusBar.SetNeedsLayout(); @@ -626,7 +598,7 @@ private async Task WriteToAddressSpaceNodeAsync(BrowsedNode node) if (Opc.Ua.StatusCode.IsGood(attrs[1].StatusCode) && attrs[1].Value is Opc.Ua.NodeId dataTypeNodeId) { - (builtInType, dataTypeName) = SubscriptionManager.ResolveDataType(dataTypeNodeId); + (builtInType, dataTypeName) = DataTypeResolver.Resolve(dataTypeNodeId); } } @@ -1251,15 +1223,16 @@ private async Task LoadConfigurationAsync(string filePath) // old server while (or after) the new connection is attempted. await DisconnectAsync(); - // Honor the config's security and sampling settings (the connect dialog has - // no UI for these, so the config file is their only source). + // Honor the config's security and subscription settings (the connect dialog + // has no UI for these, so the config file is their only source). var connected = await _connectionManager.ConnectAsync( config.Server.EndpointUrl, config.Settings.PublishingIntervalMs, credentials, config.Server.SecurityMode, config.Server.SecurityPolicy, - config.Settings.SamplingIntervalMs); + config.Settings.SamplingIntervalMs, + config.Settings.QueueSize); if (connected) { diff --git a/App/Themes/ThemeStyler.cs b/App/Themes/ThemeStyler.cs index 6bfdd84..c5e430a 100644 --- a/App/Themes/ThemeStyler.cs +++ b/App/Themes/ThemeStyler.cs @@ -9,50 +9,39 @@ namespace Opcilloscope.App.Themes; public static class ThemeStyler { /// - /// Applies full theme styling to a view including borders, colors, and spacing. + /// Applies full theme styling to a frame including borders, colors, and spacing. /// Note: Does NOT override BorderStyle - caller should set that explicitly to control /// whether a view uses emphasized (double-line) or secondary (single-line) borders. /// - public static void ApplyTo(View view, AppTheme? theme = null) + public static void ApplyToFrame(FrameView frame, AppTheme? theme = null) { theme ??= ThemeManager.Current; // Apply color scheme - view.ColorScheme = theme.MainColorScheme; + frame.ColorScheme = theme.MainColorScheme; // Note: BorderStyle is NOT set here - callers should set it explicitly // to control emphasized vs secondary border styling // Configure border colors - use BorderColorScheme for consistent grey borders // that don't change to amber/yellow when focused (avoids terminal inconsistencies) - if (view.Border != null) + if (frame.Border != null) { - view.Border.ColorScheme = theme.BorderColorScheme; + frame.Border.ColorScheme = theme.BorderColorScheme; } // Apply margin and padding from theme - if (view.Margin != null) + if (frame.Margin != null) { - view.Margin.Thickness = theme.MarginThickness; + frame.Margin.Thickness = theme.MarginThickness; } - if (view.Padding != null) + if (frame.Padding != null) { - view.Padding.Thickness = theme.PaddingThickness; + frame.Padding.Thickness = theme.PaddingThickness; } } - /// - /// Applies styling to a FrameView with themed borders. - /// - public static void ApplyToFrame(FrameView frame, AppTheme? theme = null) - { - theme ??= ThemeManager.Current; - - // Apply base styling - ApplyTo(frame, theme); - } - /// /// Applies dialog-specific styling. /// @@ -69,15 +58,6 @@ public static void ApplyToDialog(Dialog dialog, AppTheme? theme = null) } } - /// - /// Applies styling to a button. - /// - public static void ApplyToButton(Button button, AppTheme? theme = null) - { - theme ??= ThemeManager.Current; - button.ColorScheme = theme.ButtonColorScheme; - } - /// /// Applies menu bar styling. /// @@ -88,50 +68,35 @@ public static void ApplyToMenuBar(MenuBar menuBar, AppTheme? theme = null) } /// - /// Applies status bar styling. - /// - public static void ApplyToStatusBar(StatusBar statusBar, AppTheme? theme = null) - { - theme ??= ThemeManager.Current; - statusBar.ColorScheme = theme.MenuColorScheme; - } - - /// - /// Creates a styled label with theme colors. - /// - public static Label CreateLabel(string text, AppTheme? theme = null) - { - theme ??= ThemeManager.Current; - return new Label - { - Text = text, - ColorScheme = theme.MainColorScheme - }; - } - - /// - /// Creates a styled TextField with theme colors. + /// Creates the accent-colored scheme used to highlight a dialog's default button. /// - public static TextField CreateTextField(string text = "", AppTheme? theme = null) + public static ColorScheme CreateAccentButtonScheme(AppTheme? theme = null) { theme ??= ThemeManager.Current; - return new TextField + return new ColorScheme { - Text = text, - ColorScheme = theme.MainColorScheme + Normal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), + Focus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), + HotNormal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), + HotFocus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), + Disabled = new Terminal.Gui.Attribute(theme.MutedText, theme.Background) }; } /// - /// Creates a styled button with theme decorations. + /// Creates the flat menu/status bar scheme. Unlike MenuColorScheme this keeps the + /// theme background on focus, avoiding inverted highlight flashes on the bars. /// - public static Button CreateButton(string text, AppTheme? theme = null) + public static ColorScheme CreateFlatBarScheme(AppTheme? theme = null) { theme ??= ThemeManager.Current; - return new Button + return new ColorScheme { - Text = text, - ColorScheme = theme.ButtonColorScheme + Normal = new Terminal.Gui.Attribute(theme.Foreground, theme.Background), + Focus = new Terminal.Gui.Attribute(theme.ForegroundBright, theme.Background), + HotNormal = new Terminal.Gui.Attribute(theme.Accent, theme.Background), + HotFocus = new Terminal.Gui.Attribute(theme.AccentBright, theme.Background), + Disabled = new Terminal.Gui.Attribute(theme.MutedText, theme.Background) }; } } diff --git a/CLAUDE.md b/CLAUDE.md index c373786..be78f2b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,12 +77,15 @@ Opcilloscope/ │ ├── FocusManager.cs # Keyboard focus navigation between panes │ ├── Views/ │ │ ├── AddressSpaceView.cs # TreeView for OPC UA address space navigation +│ │ ├── BrailleCanvas.cs # High-resolution braille-character drawing canvas │ │ ├── MonitoredVariablesView.cs # TableView for subscribed variables with selection │ │ ├── NodeDetailsView.cs # Node attribute display panel │ │ ├── LogView.cs # Application log display │ │ └── ScopeView.cs # Real-time multi-signal oscilloscope view │ ├── Dialogs/ │ │ ├── ConnectDialog.cs # Server connection dialog with publishing interval +│ │ ├── OpenConfigDialog.cs # Open configuration file dialog with recent files +│ │ ├── PasswordPromptDialog.cs # Password prompt for authenticated connections │ │ ├── WriteValueDialog.cs # Write value to node dialog │ │ ├── ScopeDialog.cs # Multi-signal scope dialog (up to 5 signals) │ │ ├── SaveConfigDialog.cs # Save configuration file dialog @@ -109,7 +112,9 @@ Opcilloscope/ │ ├── OpcUa/ │ ├── OpcUaClientWrapper.cs # OPC Foundation Session wrapper with connection management +│ ├── ConnectionCredentials.cs # Username/password credentials for authenticated sessions │ ├── ConnectionManager.cs # Connection lifecycle orchestration (connect/disconnect/reconnect) +│ ├── DataTypeResolver.cs # Built-in OPC UA data type name/BuiltInType lookup │ ├── NodeBrowser.cs # Address space navigation and browsing │ ├── SubscriptionManager.cs # OPC UA Subscription with MonitoredVariables │ └── Models/ @@ -135,21 +140,29 @@ Opcilloscope/ │ └── Opcilloscope.Tests/ # Unit and integration tests (xUnit) │ ├── Opcilloscope.Tests.csproj │ ├── Infrastructure/ +│ │ ├── TestModuleInitializer.cs # Test assembly initialization │ │ └── TestServerFixture.cs # xUnit fixture with IAsyncLifetime │ ├── Integration/ │ │ ├── OpcUaIntegrationTests.cs +│ │ ├── AuthenticationIntegrationTests.cs │ │ ├── ConnectionManagerIntegrationTests.cs │ │ ├── ErrorHandlingIntegrationTests.cs │ │ ├── NodeBrowserIntegrationTests.cs -│ │ └── SubscriptionManagerIntegrationTests.cs +│ │ ├── ReconnectIntegrationTests.cs +│ │ ├── SubscriptionManagerIntegrationTests.cs +│ │ └── WriteIntegrationTests.cs │ ├── App/ │ │ ├── ThemeManagerTests.cs │ │ ├── AppThemeTests.cs +│ │ ├── FocusManagerTests.cs +│ │ ├── Views/ +│ │ │ └── BrailleCanvasTests.cs │ │ └── Keybindings/ │ │ ├── KeybindingTests.cs │ │ └── KeybindingManagerTests.cs │ ├── Configuration/ -│ │ └── ConfigurationServiceTests.cs +│ │ ├── ConfigurationServiceTests.cs +│ │ └── RecentFilesManagerTests.cs │ ├── OpcUa/ │ │ ├── SubscriptionManagerTests.cs │ │ ├── NodeAttributesTests.cs @@ -159,13 +172,11 @@ Opcilloscope/ │ └── Utilities/ │ ├── LoggerTests.cs │ ├── CsvRecordingManagerTests.cs +│ ├── NodeAttributeFormatterTests.cs │ ├── OpcValueConverterTests.cs │ └── ConnectionIdentifierTests.cs │ -├── docs/ -│ ├── MARKETING_DESCRIPTION.md # Product marketing copy -│ ├── UAT-CHECKLIST.md # User acceptance testing guide -│ └── plan.md # Development plan +├── docs/ # Screenshots and promotional images │ └── .github/workflows/ ├── ci.yml # Build and test on push/PR diff --git a/Configuration/ConfigurationService.cs b/Configuration/ConfigurationService.cs index b72b434..eba004e 100644 --- a/Configuration/ConfigurationService.cs +++ b/Configuration/ConfigurationService.cs @@ -440,62 +440,6 @@ public static string GenerateDefaultFilename(string? connectionUrl) return $"{identifier}{ConfigFileExtension}"; } - /// - /// Sanitizes a URL to be used as part of a filename. - /// Replaces invalid filename characters with underscores. - /// - /// The URL to sanitize. - /// A filename-safe string derived from the URL. - public static string SanitizeUrlForFilename(string url) - { - if (string.IsNullOrEmpty(url)) - return "unknown"; - - // Remove protocol prefix - var sanitized = url; - if (sanitized.StartsWith("opc.tcp://", StringComparison.OrdinalIgnoreCase)) - sanitized = sanitized.Substring(10); - else if (sanitized.StartsWith("opc.https://", StringComparison.OrdinalIgnoreCase)) - sanitized = sanitized.Substring(12); - else if (sanitized.StartsWith("https://", StringComparison.OrdinalIgnoreCase)) - sanitized = sanitized.Substring(8); - else if (sanitized.StartsWith("http://", StringComparison.OrdinalIgnoreCase)) - sanitized = sanitized.Substring(7); - - // Replace invalid filename characters with underscores - var invalidChars = Path.GetInvalidFileNameChars(); - foreach (var c in invalidChars) - { - sanitized = sanitized.Replace(c, '_'); - } - - // Also replace common URL special characters - sanitized = sanitized - .Replace(':', '_') - .Replace('/', '_') - .Replace('\\', '_') - .Replace('?', '_') - .Replace('&', '_') - .Replace('=', '_'); - - // Remove consecutive underscores - while (sanitized.Contains("__")) - { - sanitized = sanitized.Replace("__", "_"); - } - - // Trim underscores from start and end - sanitized = sanitized.Trim('_'); - - // Limit length to avoid overly long filenames - if (sanitized.Length > 50) - { - sanitized = sanitized.Substring(0, 50).TrimEnd('_'); - } - - return string.IsNullOrEmpty(sanitized) ? "unknown" : sanitized; - } - /// /// Ensures a filename has the correct .cfg extension. /// diff --git a/Configuration/Models/OpcilloscopeConfig.cs b/Configuration/Models/OpcilloscopeConfig.cs index 50715d9..92c56f9 100644 --- a/Configuration/Models/OpcilloscopeConfig.cs +++ b/Configuration/Models/OpcilloscopeConfig.cs @@ -24,23 +24,16 @@ public class ServerConfig /// /// Requested message security mode (for example: None, Sign, SignAndEncrypt). - /// - /// Warning: As of version 1.0, this value is not currently used by the - /// connection logic. It is included in the configuration model to - /// document intended security settings and to support future versions - /// of Opcilloscope that negotiate security based on this value. - /// + /// Used during endpoint selection when connecting; when None, an unsecured + /// endpoint is selected. /// public string SecurityMode { get; set; } = "None"; /// /// Requested security policy URI or shorthand (for example: /// Basic256Sha256 or the full policy URI). - /// - /// Warning: As of version 1.0, this value is not currently used by the - /// connection logic. It is provided for forward compatibility and - /// documentation of the desired security policy. - /// + /// Used during endpoint selection when connecting; honored when a + /// matching endpoint exists on the server. /// public string? SecurityPolicy { get; set; } @@ -83,20 +76,17 @@ public class SubscriptionSettings public int PublishingIntervalMs { get; set; } = 250; /// - /// Default sampling interval (in milliseconds) for monitored variables. - /// - /// Note: As of version 1.0, this setting is defined in the configuration model but is not yet - /// applied by the configuration loading logic. It is reserved for future use. - /// + /// Sampling interval (in milliseconds) applied to monitored variables. + /// Controls how often the server samples the underlying value; 0 means + /// "as fast as the server allows". + /// Valid range: 0-10000 ms (values outside this range will be clamped by SubscriptionManager). /// public int SamplingIntervalMs { get; set; } = 250; /// - /// Default queue size for monitored variables. - /// - /// Note: As of version 1.0, this setting is defined in the configuration model but is not yet - /// applied by the configuration loading logic. It is reserved for future use. - /// + /// Server-side notification queue size applied to monitored variables. + /// Values sampled between publishes are queued up to this depth (oldest discarded first). + /// Valid range: 1-1000 (values outside this range will be clamped by SubscriptionManager). /// public uint QueueSize { get; set; } = 10; } diff --git a/Configuration/RecentFilesManager.cs b/Configuration/RecentFilesManager.cs index 42b8e9c..5dc35d1 100644 --- a/Configuration/RecentFilesManager.cs +++ b/Configuration/RecentFilesManager.cs @@ -102,21 +102,6 @@ public IReadOnlyList GetExistingFiles() return _recentFiles.Where(File.Exists).ToList().AsReadOnly(); } - /// - /// Removes any files from the list that no longer exist on disk. - /// - public void CleanupMissingFiles() - { - var originalCount = _recentFiles.Count; - _recentFiles = _recentFiles.Where(File.Exists).ToList(); - - if (_recentFiles.Count != originalCount) - { - Save(); - FilesChanged?.Invoke(); - } - } - private void Load() { try diff --git a/OpcUa/ConnectionManager.cs b/OpcUa/ConnectionManager.cs index 2dd5871..b694159 100644 --- a/OpcUa/ConnectionManager.cs +++ b/OpcUa/ConnectionManager.cs @@ -17,10 +17,11 @@ public sealed class ConnectionManager : IDisposable private bool _disposed; private int _isReconnecting; - // Intervals from the most recent ConnectAsync, so subscription restoration - // after a reconnect does not silently fall back to the defaults. + // Subscription settings from the most recent ConnectAsync, so subscription + // restoration after a reconnect does not silently fall back to the defaults. private int _publishingInterval = 250; private int _samplingInterval = 250; + private uint _queueSize = 10; // Stored event handler references for proper unsubscription private Action? _valueChangedHandler; @@ -120,7 +121,8 @@ public ConnectionManager(Logger logger, bool? allowInsecure = null) /// Authentication credentials (defaults to anonymous). /// Requested message security mode (e.g. None, Sign, SignAndEncrypt). When null/None, an unsecured endpoint is selected. /// Requested security policy URI or shorthand (e.g. Basic256Sha256). Honored when a matching endpoint exists. - /// Sampling interval in milliseconds for monitored items. + /// Sampling interval in milliseconds for monitored items (0 = as fast as the server allows). + /// Server-side notification queue size for monitored items. /// True if connection succeeded, false otherwise. public async Task ConnectAsync( string endpoint, @@ -128,7 +130,8 @@ public async Task ConnectAsync( ConnectionCredentials? credentials = null, string? securityMode = null, string? securityPolicy = null, - int samplingInterval = 250) + int samplingInterval = 250, + uint queueSize = 10) { // Async teardown: the synchronous Disconnect() blocks on the OPC UA close // round-trip (up to the transport timeout against a dead server), which froze @@ -139,6 +142,7 @@ public async Task ConnectAsync( _credentials = credentials ?? ConnectionCredentials.Anonymous; _publishingInterval = publishingInterval; _samplingInterval = samplingInterval; + _queueSize = queueSize; StateChanged?.Invoke(ConnectionState.Connecting); try @@ -338,6 +342,7 @@ private async Task InitializeSubscriptionAsync() _subscriptionManager = new SubscriptionManager(_client, _logger); _subscriptionManager.PublishingInterval = _publishingInterval; _subscriptionManager.SamplingInterval = _samplingInterval; + _subscriptionManager.QueueSize = _queueSize; var initialized = await _subscriptionManager.InitializeAsync(); // Store handler references for proper unsubscription diff --git a/OpcUa/DataTypeResolver.cs b/OpcUa/DataTypeResolver.cs new file mode 100644 index 0000000..d387b6f --- /dev/null +++ b/OpcUa/DataTypeResolver.cs @@ -0,0 +1,76 @@ +using Opc.Ua; + +namespace Opcilloscope.OpcUa; + +/// +/// Shared lookup for OPC UA built-in data type names and BuiltInType resolution. +/// +internal static class DataTypeResolver +{ + // Built-in OPC UA data types: the numeric NodeId identifier in namespace 0 + // matches the BuiltInType enum value (Boolean=1 ... DiagnosticInfo=25). + private static readonly Dictionary BuiltInNames = new() + { + { 1, "Boolean" }, + { 2, "SByte" }, + { 3, "Byte" }, + { 4, "Int16" }, + { 5, "UInt16" }, + { 6, "Int32" }, + { 7, "UInt32" }, + { 8, "Int64" }, + { 9, "UInt64" }, + { 10, "Float" }, + { 11, "Double" }, + { 12, "String" }, + { 13, "DateTime" }, + { 14, "Guid" }, + { 15, "ByteString" }, + { 16, "XmlElement" }, + { 17, "NodeId" }, + { 18, "ExpandedNodeId" }, + { 19, "StatusCode" }, + { 20, "QualifiedName" }, + { 21, "LocalizedText" }, + { 22, "ExtensionObject" }, + { 23, "DataValue" }, + { 24, "Variant" }, + { 25, "DiagnosticInfo" }, + }; + + /// + /// Gets the display name of a built-in data type, or null if the NodeId + /// does not refer to a built-in type. + /// + public static bool TryGetBuiltInName(NodeId dataTypeNodeId, out string? name) + { + if (dataTypeNodeId.NamespaceIndex == 0 + && dataTypeNodeId.IdType == IdType.Numeric + && dataTypeNodeId.Identifier is uint id + && BuiltInNames.TryGetValue(id, out name)) + { + return true; + } + + name = null; + return false; + } + + /// + /// Resolves a data type NodeId to a BuiltInType (for value conversion) and a + /// display name. Only primitive types (Boolean through ByteString) map to a + /// concrete BuiltInType; everything else is treated as Variant so string + /// input is passed through unchanged when writing values. + /// + public static (BuiltInType Type, string Name) Resolve(NodeId dataTypeNodeId) + { + if (TryGetBuiltInName(dataTypeNodeId, out var name) + && dataTypeNodeId.Identifier is uint id) + { + var type = id <= (uint)BuiltInType.ByteString ? (BuiltInType)id : BuiltInType.Variant; + return (type, name!); + } + + return (BuiltInType.Variant, dataTypeNodeId.ToString()); + } +} diff --git a/OpcUa/NodeBrowser.cs b/OpcUa/NodeBrowser.cs index 67565d2..a8e104d 100644 --- a/OpcUa/NodeBrowser.cs +++ b/OpcUa/NodeBrowser.cs @@ -17,36 +17,6 @@ public class NodeBrowser // data type names for multiple variables in parallel. private readonly ConcurrentDictionary _dataTypeCache = new(); - // Common data type NodeIds - private static readonly Dictionary BuiltInDataTypes = new() - { - { 1, "Boolean" }, - { 2, "SByte" }, - { 3, "Byte" }, - { 4, "Int16" }, - { 5, "UInt16" }, - { 6, "Int32" }, - { 7, "UInt32" }, - { 8, "Int64" }, - { 9, "UInt64" }, - { 10, "Float" }, - { 11, "Double" }, - { 12, "String" }, - { 13, "DateTime" }, - { 14, "Guid" }, - { 15, "ByteString" }, - { 16, "XmlElement" }, - { 17, "NodeId" }, - { 18, "ExpandedNodeId" }, - { 19, "StatusCode" }, - { 20, "QualifiedName" }, - { 21, "LocalizedText" }, - { 22, "ExtensionObject" }, - { 23, "DataValue" }, - { 24, "Variant" }, - { 25, "DiagnosticInfo" }, - }; - public NodeBrowser(OpcUaClientWrapper client, Logger logger) { _client = client; @@ -142,13 +112,9 @@ public async Task> GetChildrenAsync(BrowsedNode parent) var attrs = await _client.ReadAttributesAsync(nodeId, Attributes.DataType); if (attrs.Count > 0 && attrs[0].Value is NodeId dataTypeId) { - // Check built-in types first - already a dictionary lookup, no caching needed - if (dataTypeId.NamespaceIndex == 0 && dataTypeId.IdType == IdType.Numeric) - { - var id = (uint)dataTypeId.Identifier; - if (BuiltInDataTypes.TryGetValue(id, out var builtIn)) - return builtIn; - } + // Check built-in types first - resolved without a network call, no caching needed + if (DataTypeResolver.TryGetBuiltInName(dataTypeId, out var builtIn)) + return builtIn; var key = dataTypeId.ToString(); if (_dataTypeCache.TryGetValue(key, out var cached)) @@ -393,12 +359,8 @@ private static string FormatValue(object? value) private async Task GetDataTypeNameByIdAsync(NodeId dataTypeId) { - if (dataTypeId.NamespaceIndex == 0 && dataTypeId.IdType == IdType.Numeric) - { - var id = (uint)dataTypeId.Identifier; - if (BuiltInDataTypes.TryGetValue(id, out var name)) - return name; - } + if (DataTypeResolver.TryGetBuiltInName(dataTypeId, out var builtInName)) + return builtInName; try { diff --git a/OpcUa/SubscriptionManager.cs b/OpcUa/SubscriptionManager.cs index e39b0bf..86bc77f 100644 --- a/OpcUa/SubscriptionManager.cs +++ b/OpcUa/SubscriptionManager.cs @@ -22,6 +22,7 @@ public class SubscriptionManager : IDisposable, IAsyncDisposable private uint _nextClientHandle = 1; private int _publishingInterval = 250; private int _samplingInterval = 250; + private uint _queueSize = 10; private bool _isInitialized; private readonly object _lock = new(); @@ -56,6 +57,15 @@ public int SamplingInterval set => _samplingInterval = Math.Max(0, Math.Min(60000, value)); } + /// + /// Server-side notification queue size applied to newly created monitored items. + /// + public uint QueueSize + { + get => _queueSize; + set => _queueSize = Math.Max(1, Math.Min(1000, value)); + } + public IReadOnlyCollection MonitoredVariables { get @@ -143,7 +153,7 @@ public async Task InitializeAsync() StartNodeId = nodeId, AttributeId = Attributes.Value, SamplingInterval = _samplingInterval, - QueueSize = 10, + QueueSize = _queueSize, DiscardOldest = true }; @@ -282,7 +292,7 @@ private async Task ReadNodeAttributesAsync(MonitoredNode item) // DataType - this is a NodeId that we need to resolve if (StatusCode.IsGood(results[1].StatusCode) && results[1].Value is NodeId dataTypeNodeId) { - var (builtInType, typeName) = ResolveDataType(dataTypeNodeId); + var (builtInType, typeName) = DataTypeResolver.Resolve(dataTypeNodeId); item.DataType = builtInType; item.DataTypeName = typeName; } @@ -297,49 +307,6 @@ private async Task ReadNodeAttributesAsync(MonitoredNode item) } } - internal static (BuiltInType, string) ResolveDataType(NodeId dataTypeNodeId) - { - // Compare against standard OPC UA DataType NodeIds explicitly for clarity and maintainability - if (dataTypeNodeId.NamespaceIndex == 0 && dataTypeNodeId.IdType == IdType.Numeric) - { - if (dataTypeNodeId.Equals(DataTypeIds.Boolean)) - return (BuiltInType.Boolean, "Boolean"); - if (dataTypeNodeId.Equals(DataTypeIds.SByte)) - return (BuiltInType.SByte, "SByte"); - if (dataTypeNodeId.Equals(DataTypeIds.Byte)) - return (BuiltInType.Byte, "Byte"); - if (dataTypeNodeId.Equals(DataTypeIds.Int16)) - return (BuiltInType.Int16, "Int16"); - if (dataTypeNodeId.Equals(DataTypeIds.UInt16)) - return (BuiltInType.UInt16, "UInt16"); - if (dataTypeNodeId.Equals(DataTypeIds.Int32)) - return (BuiltInType.Int32, "Int32"); - if (dataTypeNodeId.Equals(DataTypeIds.UInt32)) - return (BuiltInType.UInt32, "UInt32"); - if (dataTypeNodeId.Equals(DataTypeIds.Int64)) - return (BuiltInType.Int64, "Int64"); - if (dataTypeNodeId.Equals(DataTypeIds.UInt64)) - return (BuiltInType.UInt64, "UInt64"); - if (dataTypeNodeId.Equals(DataTypeIds.Float)) - return (BuiltInType.Float, "Float"); - if (dataTypeNodeId.Equals(DataTypeIds.Double)) - return (BuiltInType.Double, "Double"); - if (dataTypeNodeId.Equals(DataTypeIds.String)) - return (BuiltInType.String, "String"); - if (dataTypeNodeId.Equals(DataTypeIds.DateTime)) - return (BuiltInType.DateTime, "DateTime"); - if (dataTypeNodeId.Equals(DataTypeIds.Guid)) - return (BuiltInType.Guid, "Guid"); - if (dataTypeNodeId.Equals(DataTypeIds.ByteString)) - return (BuiltInType.ByteString, "ByteString"); - - // Fallback for other namespace 0 numeric types - return (BuiltInType.Variant, dataTypeNodeId.ToString()); - } - - return (BuiltInType.Variant, dataTypeNodeId.ToString()); - } - public async Task RemoveNodeAsync(uint clientHandle) { MonitoredNode? variable; @@ -381,21 +348,6 @@ public async Task RemoveNodeAsync(uint clientHandle) return true; } - public async Task RemoveNodeByNodeIdAsync(NodeId nodeId) - { - MonitoredNode? variable; - lock (_lock) - { - variable = _monitoredVariables.Values.FirstOrDefault(m => m.NodeId.EqualsNodeId(nodeId)); - } - - if (variable != null) - { - return await RemoveNodeAsync(variable.ClientHandle); - } - return false; - } - private void ProcessValueChange(MonitoredNode variable, DataValue dataValue) { var oldValue = variable.Value; @@ -605,7 +557,7 @@ public async Task RecreateSubscriptionsAsync() StartNodeId = nodeId, AttributeId = Attributes.Value, SamplingInterval = _samplingInterval, - QueueSize = 10, + QueueSize = _queueSize, DiscardOldest = true }; diff --git a/Tests/Opcilloscope.Tests/App/FocusManagerTests.cs b/Tests/Opcilloscope.Tests/App/FocusManagerTests.cs index 3c9632d..8357856 100644 --- a/Tests/Opcilloscope.Tests/App/FocusManagerTests.cs +++ b/Tests/Opcilloscope.Tests/App/FocusManagerTests.cs @@ -3,8 +3,8 @@ namespace Opcilloscope.Tests.App; /// -/// Pure unit tests for the FocusManager next/prev index ordering logic. -/// These exercise the extracted index helpers directly and do not start the +/// Pure unit tests for the FocusManager next index ordering logic. +/// These exercise the extracted index helper directly and do not start the /// Terminal.Gui application loop. /// public class FocusManagerTests @@ -56,63 +56,4 @@ public void GetNextIndex_FullCycle_VisitsEveryPaneInOrder() // One more step wraps back to the start. Assert.Equal(0, FocusManager.GetNextIndex(index, paneCount)); } - - // ── GetPreviousIndex ── - - [Theory] - [InlineData(2, 3, 1)] - [InlineData(1, 3, 0)] - [InlineData(0, 3, 2)] // wrap around before the first pane - public void GetPreviousIndex_RetreatsAndWrapsAround(int currentIndex, int paneCount, int expected) - { - var result = FocusManager.GetPreviousIndex(currentIndex, paneCount); - - Assert.Equal(expected, result); - } - - [Fact] - public void GetPreviousIndex_FromNoSelection_ReturnsLastPane() - { - // FocusPrevious treats "no current pane" as index 0 so the first Previous wraps to the last pane. - var result = FocusManager.GetPreviousIndex(0, 3); - - Assert.Equal(2, result); - } - - [Fact] - public void GetPreviousIndex_SinglePane_StaysOnPaneZero() - { - var result = FocusManager.GetPreviousIndex(0, 1); - - Assert.Equal(0, result); - } - - [Fact] - public void GetPreviousIndex_FullCycle_VisitsEveryPaneInReverseOrder() - { - const int paneCount = 4; - var visited = new List(); - var index = 0; - - // Starting from "no selection" (index 0) the first step wraps to the last pane. - for (var i = 0; i < paneCount; i++) - { - index = FocusManager.GetPreviousIndex(index, paneCount); - visited.Add(index); - } - - Assert.Equal(new[] { 3, 2, 1, 0 }, visited); - } - - [Fact] - public void NextThenPrevious_ReturnsToOriginalPane() - { - const int paneCount = 5; - const int start = 2; - - var next = FocusManager.GetNextIndex(start, paneCount); - var back = FocusManager.GetPreviousIndex(next, paneCount); - - Assert.Equal(start, back); - } } diff --git a/Tests/Opcilloscope.Tests/Configuration/ConfigurationServiceTests.cs b/Tests/Opcilloscope.Tests/Configuration/ConfigurationServiceTests.cs index 3f4bb91..522bb42 100644 --- a/Tests/Opcilloscope.Tests/Configuration/ConfigurationServiceTests.cs +++ b/Tests/Opcilloscope.Tests/Configuration/ConfigurationServiceTests.cs @@ -439,89 +439,6 @@ public void GetDefaultConfigDirectory_CreatesDirectory() #region Filename Generation Tests - [Fact] - public void SanitizeUrlForFilename_RemovesOpcTcpProtocol() - { - // Act - var result = ConfigurationService.SanitizeUrlForFilename("opc.tcp://localhost:4840"); - - // Assert - Assert.Equal("localhost_4840", result); - } - - [Fact] - public void SanitizeUrlForFilename_HandlesIpAddress() - { - // Act - var result = ConfigurationService.SanitizeUrlForFilename("opc.tcp://192.168.1.100:4840"); - - // Assert - Assert.Equal("192.168.1.100_4840", result); - } - - [Fact] - public void SanitizeUrlForFilename_RemovesHttpsProtocol() - { - // Act - var result = ConfigurationService.SanitizeUrlForFilename("opc.https://server.example.com:443"); - - // Assert - Assert.Equal("server.example.com_443", result); - } - - [Fact] - public void SanitizeUrlForFilename_HandlesPathComponents() - { - // Act - var result = ConfigurationService.SanitizeUrlForFilename("opc.tcp://server:4840/UA/MyServer"); - - // Assert - Assert.Equal("server_4840_UA_MyServer", result); - } - - [Fact] - public void SanitizeUrlForFilename_RemovesConsecutiveUnderscores() - { - // Act - var result = ConfigurationService.SanitizeUrlForFilename("opc.tcp://server:4840//path"); - - // Assert - Assert.DoesNotContain("__", result); - } - - [Fact] - public void SanitizeUrlForFilename_TruncatesLongUrls() - { - // Arrange - var longUrl = "opc.tcp://" + new string('a', 100) + ".example.com:4840"; - - // Act - var result = ConfigurationService.SanitizeUrlForFilename(longUrl); - - // Assert - Assert.True(result.Length <= 50, $"Result should be <= 50 chars, was {result.Length}"); - } - - [Fact] - public void SanitizeUrlForFilename_HandlesEmptyString() - { - // Act - var result = ConfigurationService.SanitizeUrlForFilename(""); - - // Assert - Assert.Equal("unknown", result); - } - - [Fact] - public void SanitizeUrlForFilename_HandlesNull() - { - // Act - var result = ConfigurationService.SanitizeUrlForFilename(null!); - - // Assert - Assert.Equal("unknown", result); - } - [Fact] public void GenerateDefaultFilename_WithConnectionUrl_ContainsUrlAndTimestamp() { diff --git a/Tests/Opcilloscope.Tests/Configuration/RecentFilesManagerTests.cs b/Tests/Opcilloscope.Tests/Configuration/RecentFilesManagerTests.cs index aaa6758..e76a1c1 100644 --- a/Tests/Opcilloscope.Tests/Configuration/RecentFilesManagerTests.cs +++ b/Tests/Opcilloscope.Tests/Configuration/RecentFilesManagerTests.cs @@ -168,23 +168,4 @@ public void GetExistingFiles_ReturnsOnlyFilesOnDisk() Assert.Single(result); Assert.Equal(Path.GetFullPath(existing), result[0]); } - - [Fact] - public void CleanupMissingFiles_RemovesMissingAndPersists() - { - var manager = CreateManager(); - var existing = MakePath("exists.cfg"); - File.WriteAllText(existing, "{}"); - manager.Add(existing); - manager.Add(MakePath("missing.cfg")); - - manager.CleanupMissingFiles(); - - Assert.Single(manager.Files); - Assert.Equal(Path.GetFullPath(existing), manager.Files[0]); - - // Cleanup must be persisted. - var reloaded = CreateManager(); - Assert.Single(reloaded.Files); - } } diff --git a/Tests/Opcilloscope.Tests/Integration/ConnectionManagerIntegrationTests.cs b/Tests/Opcilloscope.Tests/Integration/ConnectionManagerIntegrationTests.cs index b88db2b..659c40f 100644 --- a/Tests/Opcilloscope.Tests/Integration/ConnectionManagerIntegrationTests.cs +++ b/Tests/Opcilloscope.Tests/Integration/ConnectionManagerIntegrationTests.cs @@ -63,6 +63,24 @@ public async Task ConnectAsync_ValidEndpoint_InitializesSubscriptionManager() Assert.NotNull(_connectionManager.SubscriptionManager); } + [Fact] + public async Task ConnectAsync_AppliesSubscriptionSettings() + { + // Act + await _connectionManager!.ConnectAsync( + _fixture.EndpointUrl, + publishingInterval: 500, + samplingInterval: 1000, + queueSize: 25); + + // Assert - settings flow through to the subscription manager + var subscriptionManager = _connectionManager.SubscriptionManager; + Assert.NotNull(subscriptionManager); + Assert.Equal(500, subscriptionManager.PublishingInterval); + Assert.Equal(1000, subscriptionManager.SamplingInterval); + Assert.Equal(25u, subscriptionManager.QueueSize); + } + [Fact] public async Task ConnectAsync_FiresStateChangedEvents() { diff --git a/Tests/Opcilloscope.Tests/Integration/ErrorHandlingIntegrationTests.cs b/Tests/Opcilloscope.Tests/Integration/ErrorHandlingIntegrationTests.cs index d274305..735c113 100644 --- a/Tests/Opcilloscope.Tests/Integration/ErrorHandlingIntegrationTests.cs +++ b/Tests/Opcilloscope.Tests/Integration/ErrorHandlingIntegrationTests.cs @@ -113,21 +113,6 @@ public async Task SubscriptionManager_RemoveNodeAsync_InvalidHandle_ReturnsFalse Assert.False(result); } - [Fact] - public async Task SubscriptionManager_RemoveNodeByNodeIdAsync_NonExistent_ReturnsFalse() - { - // Arrange - using var subscriptionManager = new SubscriptionManager(Client!, _logger); - await subscriptionManager.InitializeAsync(); - var nonExistentNodeId = new NodeId("NonExistent", 99); - - // Act - var result = await subscriptionManager.RemoveNodeByNodeIdAsync(nonExistentNodeId); - - // Assert - Assert.False(result); - } - [Fact] public async Task ConnectionManager_ConnectAsync_InvalidHost_DoesNotThrow() { diff --git a/Tests/Opcilloscope.Tests/Integration/SubscriptionManagerIntegrationTests.cs b/Tests/Opcilloscope.Tests/Integration/SubscriptionManagerIntegrationTests.cs index fff5e80..b551797 100644 --- a/Tests/Opcilloscope.Tests/Integration/SubscriptionManagerIntegrationTests.cs +++ b/Tests/Opcilloscope.Tests/Integration/SubscriptionManagerIntegrationTests.cs @@ -193,29 +193,6 @@ public async Task RemoveNodeAsync_FiresVariableRemovedEvent() Assert.Equal(node.ClientHandle, removedHandle); } - [Fact] - public async Task RemoveNodeByNodeIdAsync_UnsubscribesFromNode() - { - // Arrange - using var subscriptionManager = new SubscriptionManager(Client!, _logger); - await subscriptionManager.InitializeAsync(); - var nodeId = new NodeId("Counter", (ushort)GetNamespaceIndex()); - var node = await subscriptionManager.AddNodeAsync(nodeId, "Counter"); - - // Skip test if subscription failed (server may not support the node) - if (node == null) - { - return; - } - - // Act - var result = await subscriptionManager.RemoveNodeByNodeIdAsync(nodeId); - - // Assert - Assert.True(result); - Assert.Empty(subscriptionManager.MonitoredVariables); - } - [Fact] public async Task ValueChanged_ReceivesUpdates_WhenValueChanges() { @@ -308,6 +285,41 @@ public async Task PublishingInterval_ClampsToMaximum() Assert.Equal(10000, subscriptionManager.PublishingInterval); } + [Fact] + public async Task SamplingInterval_AppliedToNewMonitoredItems() + { + // Arrange + using var subscriptionManager = new SubscriptionManager(Client!, _logger); + subscriptionManager.SamplingInterval = 1000; + subscriptionManager.QueueSize = 25; + await subscriptionManager.InitializeAsync(); + var nodeId = new NodeId("Counter", (ushort)GetNamespaceIndex()); + + // Act + var node = await subscriptionManager.AddNodeAsync(nodeId, "Counter"); + + // Assert + Assert.NotNull(node); + Assert.Equal(1000, subscriptionManager.SamplingInterval); + Assert.Equal(25u, subscriptionManager.QueueSize); + } + + // SamplingInterval clamping is covered by the unit tests in + // SubscriptionManagerTests (0-60000 range). + + [Theory] + [InlineData(0u, 1u)] // zero clamps to minimum of 1 + [InlineData(10u, 10u)] + [InlineData(5000u, 1000u)] // above maximum clamps to 1000 + public void QueueSize_ClampsToValidRange(uint input, uint expected) + { + using var subscriptionManager = new SubscriptionManager(Client!, _logger); + + subscriptionManager.QueueSize = input; + + Assert.Equal(expected, subscriptionManager.QueueSize); + } + [Fact] public async Task AddNodeAsync_InvalidNodeId_ReturnsNull() { diff --git a/Tests/Opcilloscope.Tests/Utilities/ConnectionIdentifierTests.cs b/Tests/Opcilloscope.Tests/Utilities/ConnectionIdentifierTests.cs index 9f42160..cb76b83 100644 --- a/Tests/Opcilloscope.Tests/Utilities/ConnectionIdentifierTests.cs +++ b/Tests/Opcilloscope.Tests/Utilities/ConnectionIdentifierTests.cs @@ -253,6 +253,38 @@ public void Generate_WithBracketedIpv6_ProducesCleanIdentifier() Assert.DoesNotContain(":", result); } + [Theory] + [InlineData("opc.tcp://localhost:4840", "localhost_4840")] + [InlineData("opc.tcp://192.168.1.100:4840", "192.168.1.100_4840")] + [InlineData("opc.https://server.example.com:443", "server.example.com_443")] + [InlineData("opc.tcp://server:4840/UA/MyServer", "server_4840_UA_MyServer")] + [InlineData("", "unknown")] + [InlineData(null, "unknown")] + public void SanitizeUrlForFilename_ProducesFilenameSafeName(string? url, string expected) + { + var result = ConnectionIdentifier.SanitizeUrlForFilename(url); + + Assert.Equal(expected, result); + } + + [Fact] + public void SanitizeUrlForFilename_RemovesConsecutiveUnderscores() + { + var result = ConnectionIdentifier.SanitizeUrlForFilename("opc.tcp://server:4840//path"); + + Assert.DoesNotContain("__", result); + } + + [Fact] + public void SanitizeUrlForFilename_TruncatesLongUrls() + { + var longUrl = "opc.tcp://" + new string('a', 100) + ".example.com:4840"; + + var result = ConnectionIdentifier.SanitizeUrlForFilename(longUrl); + + Assert.True(result.Length <= 50, $"Result should be <= 50 chars, was {result.Length}"); + } + [Fact] public void LimitLength_WithShortIdentifier_ReturnsUnchanged() { diff --git a/Utilities/ConnectionIdentifier.cs b/Utilities/ConnectionIdentifier.cs index b1eeecc..1d204d4 100644 --- a/Utilities/ConnectionIdentifier.cs +++ b/Utilities/ConnectionIdentifier.cs @@ -161,6 +161,21 @@ private static string RemoveProtocolPrefix(string url) return url; } + /// + /// Sanitizes a full URL to be used as part of a filename. + /// Strips the protocol prefix and replaces invalid filename characters with underscores. + /// + /// The URL to sanitize. + /// A filename-safe string derived from the URL. + public static string SanitizeUrlForFilename(string? url) + { + if (string.IsNullOrEmpty(url)) + return "unknown"; + + var sanitized = SanitizeComponent(RemoveProtocolPrefix(url)); + return LimitLength(sanitized); + } + /// /// Limits the length of an identifier to avoid overly long filenames. /// diff --git a/Utilities/CsvRecordingManager.cs b/Utilities/CsvRecordingManager.cs index 064b072..a191f2d 100644 --- a/Utilities/CsvRecordingManager.cs +++ b/Utilities/CsvRecordingManager.cs @@ -97,7 +97,7 @@ public static string GenerateDefaultRecordingFilename(string? connectionUrl, int if (!string.IsNullOrEmpty(connectionUrl)) { - baseName = SanitizeUrlForFilename(connectionUrl); + baseName = ConnectionIdentifier.SanitizeUrlForFilename(connectionUrl); } else { @@ -107,62 +107,6 @@ public static string GenerateDefaultRecordingFilename(string? connectionUrl, int return $"{baseName}_{variableCount}vars_{timestamp}{RecordingFileExtension}"; } - /// - /// Sanitizes a URL to be used as part of a filename. - /// Replaces invalid filename characters with underscores. - /// - /// The URL to sanitize. - /// A filename-safe string derived from the URL. - public static string SanitizeUrlForFilename(string url) - { - if (string.IsNullOrEmpty(url)) - return "unknown"; - - // Remove protocol prefix - var sanitized = url; - if (sanitized.StartsWith("opc.tcp://", StringComparison.OrdinalIgnoreCase)) - sanitized = sanitized.Substring(10); - else if (sanitized.StartsWith("opc.https://", StringComparison.OrdinalIgnoreCase)) - sanitized = sanitized.Substring(12); - else if (sanitized.StartsWith("https://", StringComparison.OrdinalIgnoreCase)) - sanitized = sanitized.Substring(8); - else if (sanitized.StartsWith("http://", StringComparison.OrdinalIgnoreCase)) - sanitized = sanitized.Substring(7); - - // Replace invalid filename characters with underscores - var invalidChars = Path.GetInvalidFileNameChars(); - foreach (var c in invalidChars) - { - sanitized = sanitized.Replace(c, '_'); - } - - // Also replace common URL special characters - sanitized = sanitized - .Replace(':', '_') - .Replace('/', '_') - .Replace('\\', '_') - .Replace('?', '_') - .Replace('&', '_') - .Replace('=', '_'); - - // Remove consecutive underscores - while (sanitized.Contains("__")) - { - sanitized = sanitized.Replace("__", "_"); - } - - // Trim underscores from start and end - sanitized = sanitized.Trim('_'); - - // Limit length to avoid overly long filenames - if (sanitized.Length > 50) - { - sanitized = sanitized.Substring(0, 50).TrimEnd('_'); - } - - return string.IsNullOrEmpty(sanitized) ? "unknown" : sanitized; - } - /// /// Ensures a filename has the correct .csv extension. /// diff --git a/Utilities/TaskExtensions.cs b/Utilities/TaskExtensions.cs index 2e57439..41021c1 100644 --- a/Utilities/TaskExtensions.cs +++ b/Utilities/TaskExtensions.cs @@ -32,25 +32,4 @@ public static async void FireAndForget(this Task task, Logger? logger = null, st System.Diagnostics.Debug.WriteLine($"{message}\n{ex}"); } } - - /// - /// Safely executes a task without awaiting, using a custom error handler. - /// - /// The task to execute. - /// Action to execute when an error occurs. - public static async void FireAndForget(this Task task, Action onError) - { - try - { - await task.ConfigureAwait(false); - } - catch (OperationCanceledException) - { - // Cancellation is expected - } - catch (Exception ex) - { - onError(ex); - } - } } diff --git a/Utilities/UiThread.cs b/Utilities/UiThread.cs index a0d805b..d01433e 100644 --- a/Utilities/UiThread.cs +++ b/Utilities/UiThread.cs @@ -14,16 +14,4 @@ public static void Run(Action action) { Application.Invoke(action); } - - /// - /// Executes an action on the UI thread after a delay. - /// - public static void RunDelayed(Action action, TimeSpan delay) - { - Application.AddTimeout(delay, () => - { - action(); - return false; // Don't repeat - }); - } }