From 2432653e4aa8dbdadf44b513df0aecedd749d657 Mon Sep 17 00:00:00 2001 From: Frank Robijn Date: Tue, 17 Sep 2024 10:17:00 +0200 Subject: [PATCH 01/12] .editorconfig --- .editorconfig | 213 +++++++++++++++++++++++++++++++++++++++++ spelling_exclusion.dic | 1 + 2 files changed, 214 insertions(+) create mode 100644 .editorconfig create mode 100644 spelling_exclusion.dic diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..a5a9d5f --- /dev/null +++ b/.editorconfig @@ -0,0 +1,213 @@ +# EditorConfig for Visual Studio 2022: https://learn.microsoft.com/en-us/visualstudio/ide/create-portable-custom-editor-options?view=vs-2022 + +# This is a top-most .editorconfig file +root = true + +#===================================================== +# +# nanoFramework specific settings +# +# +#===================================================== +[*] +# Generic EditorConfig settings +end_of_line = crlf +charset = utf-8-bom + +# Visual Studio spell checker +spelling_languages = en-us +spelling_checkable_types = strings,identifiers,comments +spelling_error_severity = information +spelling_exclusion_path = spelling_exclusion.dic + +#===================================================== +# +# Settings copied from the .NET runtime +# +# https://github.com/dotnet/runtime +# +#===================================================== +# Default settings: +# A newline ending every file +# Use 4 spaces as indentation +insert_final_newline = true +indent_style = space +indent_size = 4 +trim_trailing_whitespace = true + +# Generated code +[*{_AssemblyInfo.cs,.notsupported.cs,AsmOffsets.cs}] +generated_code = true + +# C# files +[*.cs] +# New line preferences +csharp_new_line_before_open_brace = all +csharp_new_line_before_else = true +csharp_new_line_before_catch = true +csharp_new_line_before_finally = true +csharp_new_line_before_members_in_object_initializers = true +csharp_new_line_before_members_in_anonymous_types = true +csharp_new_line_between_query_expression_clauses = true + +# Indentation preferences +csharp_indent_block_contents = true +csharp_indent_braces = false +csharp_indent_case_contents = true +csharp_indent_case_contents_when_block = false +csharp_indent_switch_labels = true +csharp_indent_labels = one_less_than_current + +# Modifier preferences +csharp_preferred_modifier_order = public,private,protected,internal,file,static,extern,new,virtual,abstract,sealed,override,readonly,unsafe,required,volatile,async:suggestion + +# avoid this. unless absolutely necessary +dotnet_style_qualification_for_field = false:suggestion +dotnet_style_qualification_for_property = false:suggestion +dotnet_style_qualification_for_method = false:suggestion +dotnet_style_qualification_for_event = false:suggestion + +# Types: use keywords instead of BCL types, and permit var only when the type is clear +csharp_style_var_for_built_in_types = false:suggestion +csharp_style_var_when_type_is_apparent = false:none +csharp_style_var_elsewhere = false:suggestion +dotnet_style_predefined_type_for_locals_parameters_members = true:suggestion +dotnet_style_predefined_type_for_member_access = true:suggestion + +# name all constant fields using PascalCase +dotnet_naming_rule.constant_fields_should_be_pascal_case.severity = suggestion +dotnet_naming_rule.constant_fields_should_be_pascal_case.symbols = constant_fields +dotnet_naming_rule.constant_fields_should_be_pascal_case.style = pascal_case_style +dotnet_naming_symbols.constant_fields.applicable_kinds = field +dotnet_naming_symbols.constant_fields.required_modifiers = const +dotnet_naming_style.pascal_case_style.capitalization = pascal_case + +# static fields should have s_ prefix +dotnet_naming_rule.static_fields_should_have_prefix.severity = suggestion +dotnet_naming_rule.static_fields_should_have_prefix.symbols = static_fields +dotnet_naming_rule.static_fields_should_have_prefix.style = static_prefix_style +dotnet_naming_symbols.static_fields.applicable_kinds = field +dotnet_naming_symbols.static_fields.required_modifiers = static +dotnet_naming_symbols.static_fields.applicable_accessibilities = private, internal, private_protected +dotnet_naming_style.static_prefix_style.required_prefix = s_ +dotnet_naming_style.static_prefix_style.capitalization = camel_case + +# internal and private fields should be _camelCase +dotnet_naming_rule.camel_case_for_private_internal_fields.severity = suggestion +dotnet_naming_rule.camel_case_for_private_internal_fields.symbols = private_internal_fields +dotnet_naming_rule.camel_case_for_private_internal_fields.style = camel_case_underscore_style +dotnet_naming_symbols.private_internal_fields.applicable_kinds = field +dotnet_naming_symbols.private_internal_fields.applicable_accessibilities = private, internal +dotnet_naming_style.camel_case_underscore_style.required_prefix = _ +dotnet_naming_style.camel_case_underscore_style.capitalization = camel_case + +# Code style defaults +csharp_using_directive_placement = outside_namespace:suggestion +dotnet_sort_system_directives_first = true +csharp_prefer_braces = true:silent +csharp_preserve_single_line_blocks = true:none +csharp_preserve_single_line_statements = false:none +csharp_prefer_static_local_function = true:suggestion +csharp_prefer_simple_using_statement = false:none +csharp_style_prefer_switch_expression = true:suggestion +dotnet_style_readonly_field = true:suggestion + +# Expression-level preferences +dotnet_style_object_initializer = true:suggestion +dotnet_style_collection_initializer = true:suggestion +dotnet_style_prefer_collection_expression = when_types_exactly_match +dotnet_style_explicit_tuple_names = true:suggestion +dotnet_style_coalesce_expression = true:suggestion +dotnet_style_null_propagation = true:suggestion +dotnet_style_prefer_is_null_check_over_reference_equality_method = true:suggestion +dotnet_style_prefer_inferred_tuple_names = true:suggestion +dotnet_style_prefer_inferred_anonymous_type_member_names = true:suggestion +dotnet_style_prefer_auto_properties = true:suggestion +dotnet_style_prefer_conditional_expression_over_assignment = true:silent +dotnet_style_prefer_conditional_expression_over_return = true:silent +csharp_prefer_simple_default_expression = true:suggestion + +# Expression-bodied members +csharp_style_expression_bodied_methods = true:silent +csharp_style_expression_bodied_constructors = true:silent +csharp_style_expression_bodied_operators = true:silent +csharp_style_expression_bodied_properties = true:silent +csharp_style_expression_bodied_indexers = true:silent +csharp_style_expression_bodied_accessors = true:silent +csharp_style_expression_bodied_lambdas = true:silent +csharp_style_expression_bodied_local_functions = true:silent + +# Pattern matching +csharp_style_pattern_matching_over_is_with_cast_check = true:suggestion +csharp_style_pattern_matching_over_as_with_null_check = true:suggestion +csharp_style_inlined_variable_declaration = true:suggestion + +# Null checking preferences +csharp_style_throw_expression = true:suggestion +csharp_style_conditional_delegate_call = true:suggestion + +# Other features +csharp_style_prefer_index_operator = false:none +csharp_style_prefer_range_operator = false:none +csharp_style_pattern_local_over_anonymous_function = false:none + +# Space preferences +csharp_space_after_cast = false +csharp_space_after_colon_in_inheritance_clause = true +csharp_space_after_comma = true +csharp_space_after_dot = false +csharp_space_after_keywords_in_control_flow_statements = true +csharp_space_after_semicolon_in_for_statement = true +csharp_space_around_binary_operators = before_and_after +csharp_space_around_declaration_statements = do_not_ignore +csharp_space_before_colon_in_inheritance_clause = true +csharp_space_before_comma = false +csharp_space_before_dot = false +csharp_space_before_open_square_brackets = false +csharp_space_before_semicolon_in_for_statement = false +csharp_space_between_empty_square_brackets = false +csharp_space_between_method_call_empty_parameter_list_parentheses = false +csharp_space_between_method_call_name_and_opening_parenthesis = false +csharp_space_between_method_call_parameter_list_parentheses = false +csharp_space_between_method_declaration_empty_parameter_list_parentheses = false +csharp_space_between_method_declaration_name_and_open_parenthesis = false +csharp_space_between_method_declaration_parameter_list_parentheses = false +csharp_space_between_parentheses = false +csharp_space_between_square_brackets = false + +# License header +file_header_template = Licensed to the .NET Foundation under one or more agreements.\nThe .NET Foundation licenses this file to you under the MIT license. + +# C++ Files +[*.{cpp,h,in}] +curly_bracket_next_line = true +indent_brace_style = Allman + +# Xml project files +[*.{csproj,vbproj,vcxproj,vcxproj.filters,proj,nativeproj,locproj}] +indent_size = 2 + +[*.{csproj,vbproj,proj,nativeproj,locproj}] +charset = utf-8 + +# Xml build files +[*.builds] +indent_size = 2 + +# Xml files +[*.{xml,stylecop,resx,ruleset}] +indent_size = 2 + +# Xml config files +[*.{props,targets,config,nuspec}] +indent_size = 2 + +# YAML config files +[*.{yml,yaml}] +indent_size = 2 + +# Shell scripts +[*.sh] +end_of_line = lf +[*.{cmd,bat}] +end_of_line = crlf \ No newline at end of file diff --git a/spelling_exclusion.dic b/spelling_exclusion.dic new file mode 100644 index 0000000..8c0e1f8 --- /dev/null +++ b/spelling_exclusion.dic @@ -0,0 +1 @@ +nano From 58b3a3b70c1fe1512c0db16d587f9f4179074f08 Mon Sep 17 00:00:00 2001 From: Frank Robijn Date: Tue, 17 Sep 2024 16:40:34 +0200 Subject: [PATCH 02/12] Local locks for global NanoFrameworkDevices collection because of discovery of devices is asynchronous (and because of the use of multiple async PortManagers in the test platform v3): - Make sure all list operations use a lock of the collection - PortTcpIpManager._networkDevices made static as it acts as a complement to the NanoFrameworkDevices collection - if there are multiple, the device list management does not work properly for a non-static _networkDevices. System-wide exclusive access to a device via system-wide mutex, required to avoid competition between e.g.,Device Explorer auto-discovery, test platform etc. If this is used, it is possible for the nanoFramework tools to wait for a device to become available instead of throwing an exception. If an exception is thrown, that is an indication of a real problem, not a competition with other nanoFramework tools. Not sure if that also means the WaitAndRetry in the PortSerialManager should be removed. Use of System-wide exclusive access in the serial DeviceWatcher; async part of that operation moved from PortSerialManager to DeviceWatcher. Also updated PortTcpIpManager+DeviceWatcher. Serial ports: DeviceWatcher now has a static method to find all serial ports, incl exclusion list, so that all nanoFramework devices use the same method. Also to be used in the test platform v3. Port**Manager returns the added or already registered device for PostBase.AddDevice. Makes use in test platform code easier. Fixed OnDeviceEnumerationCompleted logic if both AddDevice and a device watcher are used. The OnDeviceEnumerationCompleted is now (indirectly) fired by the DeviceWatcher. Also fixed at least one statement that would result in an exception if NanoNetworkDevices have been discovered before NanoSerialDevices. Signed-off-by: Frank Robijn --- .../NanoFrameworkDevices.cs | 13 +- .../NFDevice/GlobalExclusiveDeviceAccess.cs | 93 ++++++ .../PortCompositeDeviceManager.cs | 29 +- .../PortDefinitions/PortBase.cs | 12 +- .../PortSerial/DeviceWatcher.cs | 125 +++++++- .../PortSerial/PortSerialManager.cs | 301 +++++++++--------- .../PortTcpIp/DeviceWatcher.cs | 22 +- .../PortTcpIp/PortTcpIpManager.cs | 225 ++++++++----- ...Framework.Tools.DebugLibrary.Net.projitems | 1 + 9 files changed, 534 insertions(+), 287 deletions(-) create mode 100644 nanoFramework.Tools.DebugLibrary.Shared/NFDevice/GlobalExclusiveDeviceAccess.cs diff --git a/nanoFramework.Tools.DebugLibrary.Shared/DeviceConfiguration/NanoFrameworkDevices.cs b/nanoFramework.Tools.DebugLibrary.Shared/DeviceConfiguration/NanoFrameworkDevices.cs index 985eddf..961b3e1 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/DeviceConfiguration/NanoFrameworkDevices.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/DeviceConfiguration/NanoFrameworkDevices.cs @@ -1,4 +1,7 @@ -using System; +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; using System.Collections.ObjectModel; namespace nanoFramework.Tools.Debugger @@ -10,7 +13,13 @@ public class NanoFrameworkDevices : ObservableCollection public static NanoFrameworkDevices Instance { - get { return _instance.Value; } + get + { + lock (typeof(NanoFrameworkDevices)) + { + return _instance.Value; + } + } } private NanoFrameworkDevices() { } diff --git a/nanoFramework.Tools.DebugLibrary.Shared/NFDevice/GlobalExclusiveDeviceAccess.cs b/nanoFramework.Tools.DebugLibrary.Shared/NFDevice/GlobalExclusiveDeviceAccess.cs new file mode 100644 index 0000000..8c51e93 --- /dev/null +++ b/nanoFramework.Tools.DebugLibrary.Shared/NFDevice/GlobalExclusiveDeviceAccess.cs @@ -0,0 +1,93 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Threading; +using nanoFramework.Tools.Debugger.PortTcpIp; + +namespace nanoFramework.Tools.Debugger.NFDevice +{ + /// + /// Code that wants to access a device should use this system-wide exclusive access while + /// communicating to a device to prevent that another nanoFramework tool also wants to + /// communicate with the device. + /// + public static class GlobalExclusiveDeviceAccess + { + #region Fields + /// + /// Base name for the system-wide mutex that controls access to a device connected to a COM port. + /// + private const string MutexBaseName = "276545121198496AADD346A60F14EF8D_"; + #endregion + + #region Methods + /// + /// Communicate with a serial device and ensure the code to be executed as exclusive access to the device. + /// + /// The serial port the device is connected to. + /// Code to execute while having exclusive access to the device + /// Maximum time in milliseconds to wait for exclusive access + /// Cancellation token that can be cancelled to stop/abort running the . + /// This method does not stop/abort execution of after it has been started. + /// Indicates whether the has been executed. Returns false if exclusive access + /// cannot be obtained within , or if was cancelled + /// before the has been started. + public static bool CommunicateWithDevice(string serialPort, Action communication, int millisecondsTimeout = Timeout.Infinite, CancellationToken? cancellationToken = null) + { + return DoCommunicateWithDevice(serialPort, communication, millisecondsTimeout, cancellationToken); + } + + /// + /// Communicate with a device accessible via the network and ensure the code to be executed as exclusive access to the device. + /// + /// The network address the device is connected to. + /// Code to execute while having exclusive access to the device + /// Maximum time in milliseconds to wait for exclusive access + /// Cancellation token that can be cancelled to stop/abort running the . + /// This method does not stop/abort execution of after it has been started. + /// Indicates whether the has been executed. Returns false if exclusive access + /// cannot be obtained within , or if was cancelled + /// before the has been started. + public static bool CommunicateWithDevice(NetworkDeviceInformation address, Action communication, int millisecondsTimeout = Timeout.Infinite, CancellationToken? cancellationToken = null) + { + return DoCommunicateWithDevice($"{address.Host}:{address.Port}", communication, millisecondsTimeout, cancellationToken); + } + #endregion + + #region Implementation + private static bool DoCommunicateWithDevice(string connectionKey, Action communication, int millisecondsTimeout, CancellationToken? cancellationToken) + { + var waitHandles = new List(); + var mutex = new Mutex(false, $"{MutexBaseName}_{connectionKey}"); + waitHandles.Add(mutex); + + CancellationTokenSource timeOutToken = null; + if (millisecondsTimeout > 0 && millisecondsTimeout != Timeout.Infinite) + { + timeOutToken = new CancellationTokenSource(millisecondsTimeout); + waitHandles.Add(timeOutToken.Token.WaitHandle); + } + if (cancellationToken.HasValue) + { + waitHandles.Add(cancellationToken.Value.WaitHandle); + } + try + { + if (WaitHandle.WaitAny(waitHandles.ToArray()) == 0) + { + communication(); + return true; + } + } + finally + { + mutex.ReleaseMutex(); + timeOutToken?.Dispose(); + } + return false; + } + #endregion + } +} diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs index 0b163c1..3378e34 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs @@ -1,10 +1,9 @@ -// -// Copyright (c) .NET Foundation and Contributors -// See LICENSE file in the project root for full license information. -// +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. using System; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; @@ -52,21 +51,30 @@ private void OnLogMessageAvailable(object sender, StringEventArgs e) private void OnPortDeviceEnumerationCompleted(object sender, EventArgs e) { - DeviceEnumerationCompleted?.Invoke(this, EventArgs.Empty); + IsDevicesEnumerationComplete = (from p in _ports + where p.IsDevicesEnumerationComplete + select p).Any(); + if (IsDevicesEnumerationComplete) + { + DeviceEnumerationCompleted?.Invoke(this, EventArgs.Empty); + } } /// /// This API is not available in - public override void AddDevice(string deviceId) + public override NanoDeviceBase AddDevice(string deviceId) { - _ports.ForEach(p => - { - p.AddDevice(deviceId); - }); + // None of the Port*Manager has a check whether deviceId matches the ID handled by the manager. + throw new NotImplementedException(); + //_ports.ForEach(p => + //{ + // p.AddDevice(deviceId); + //}); } public override void StartDeviceWatchers() { + IsDevicesEnumerationComplete = false; _ports.ForEach(p => p.StartDeviceWatchers()); } @@ -77,6 +85,7 @@ public override void StopDeviceWatchers() public override void ReScanDevices() { + IsDevicesEnumerationComplete = false; Task.Run(() => { _ports.ForEach(p => p.ReScanDevices()); diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs index 330cbd6..2e9072f 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs @@ -1,7 +1,5 @@ -// -// Copyright (c) .NET Foundation and Contributors -// See LICENSE file in the project root for full license information. -// +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. using System; using System.Collections.Generic; @@ -55,10 +53,10 @@ public string PersistName public NanoFrameworkDevices NanoFrameworkDevices { get; protected set; } /// - /// Adds a new device to list of NanoFrameworkDevices. + /// Adds a new device to list of NanoFrameworkDevices. /// - /// The serial port name where the device is connected. - public abstract void AddDevice(string deviceId); + /// The unique ID (based on the connection properties) of the device. + public abstract NanoDeviceBase AddDevice(string deviceId); /// /// Starts the device watchers. diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs index a01d5e4..3436883 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs @@ -1,11 +1,17 @@ -using Microsoft.Win32; +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + using System; using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Linq; using System.Runtime.InteropServices; using System.Text.RegularExpressions; using System.Threading; +using System.Threading.Tasks; +using Microsoft.Win32; +using nanoFramework.Tools.Debugger.NFDevice; namespace nanoFramework.Tools.Debugger.PortSerial { @@ -43,6 +49,18 @@ public class DeviceWatcher : IDisposable /// public event EventDeviceRemoved Removed; + /// + /// Represents a delegate method that is used to handle the AllNewDevicesAdded event. + /// + /// The object that raised the event. + /// Number of devices added + public delegate void EventAllNewDevicesAdded(object sender, int numDevicesAdded); + + /// + /// Raised when all newly discovered devices have been added + /// + public event EventAllNewDevicesAdded AllNewDevicesAdded; + /// /// Gets or sets the status of the device watcher. /// @@ -60,13 +78,14 @@ public DeviceWatcher(PortSerialManager owner) /// /// Starts the device watcher. /// - public void Start() + public void Start(ICollection portExclusionList = null) { if (!_started) { + var exclusionList = portExclusionList?.ToList(); _threadWatch = new Thread(() => { - StartWatcher(); + StartWatcher(exclusionList); }) { IsBackground = true, @@ -77,7 +96,7 @@ public void Start() } } - private void StartWatcher() + private void StartWatcher(ICollection portExclusionList) { _ownerManager.OnLogMessageAvailable($"PortSerial device watcher started @ Thread {_threadWatch.ManagedThreadId} [ProcessID: {Process.GetCurrentProcess().Id}]"); @@ -91,7 +110,7 @@ private void StartWatcher() { try { - var ports = GetPortNames(); + var ports = GetPortNames(portExclusionList); // check for ports that departed List portsToRemove = new(); @@ -115,14 +134,32 @@ private void StartWatcher() } // process ports that have arrived + var tasks = new List(); foreach (var port in ports) { if (!_ports.Contains(port)) { _ports.Add(port); - Added?.Invoke(this, port); + if (Added is not null) + { + if (PortSerialManager.GetRegisteredDevice(port) is null) + { + tasks.Add(Task.Run(() => + { + GlobalExclusiveDeviceAccess.CommunicateWithDevice( + port, + () => Added.Invoke(this, port) + ); + })); + } + } } } + if (tasks.Count > 0) + { + Task.WaitAll(tasks.ToArray()); + } + AllNewDevicesAdded?.Invoke(this, tasks.Count); Thread.Sleep(200); } @@ -146,22 +183,26 @@ private void StartWatcher() /// Gets the list of serial ports. /// /// The list of serial ports. - public List GetPortNames() + public static List GetPortNames(ICollection exclusionList = null) { - return RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? GetPortNames_Linux() - : RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? GetPortNames_OSX() - : RuntimeInformation.IsOSPlatform(OSPlatform.Create("FREEBSD")) ? GetPortNames_FreeBSD() - : GetPortNames_Windows(); + return RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? GetPortNames_Linux(exclusionList) + : RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? GetPortNames_OSX(exclusionList) + : RuntimeInformation.IsOSPlatform(OSPlatform.Create("FREEBSD")) ? GetPortNames_FreeBSD(exclusionList) + : GetPortNames_Windows(exclusionList); } - private List GetPortNames_Linux() + private static List GetPortNames_Linux(ICollection exclusionList) { List ports = new List(); string[] ttys = System.IO.Directory.GetFiles("/dev/", "tty*"); foreach (string dev in ttys) { + if (exclusionList?.Contains(dev) ?? false) + { + continue; + } if (dev.StartsWith("/dev/ttyS") || dev.StartsWith("/dev/ttyUSB") || dev.StartsWith("/dev/ttyACM") @@ -176,12 +217,17 @@ private List GetPortNames_Linux() return ports; } - private List GetPortNames_OSX() + private static List GetPortNames_OSX(ICollection exclusionList) { List ports = new List(); foreach (string name in Directory.GetFiles("/dev", "tty.usbserial*")) { + if (exclusionList?.Contains(name) ?? false) + { + continue; + } + // We don't want Bluetooth ports if (name.ToLower().Contains("bluetooth")) { @@ -198,6 +244,11 @@ private List GetPortNames_OSX() foreach (string name in Directory.GetFiles("/dev", "cu.usbserial*")) { + if (exclusionList?.Contains(name) ?? false) + { + continue; + } + // We don't want Bluetooth ports if (name.ToLower().Contains("bluetooth")) { @@ -213,12 +264,16 @@ private List GetPortNames_OSX() return ports; } - private List GetPortNames_FreeBSD() + private static List GetPortNames_FreeBSD(ICollection exclusionList) { List ports = new List(); foreach (string name in Directory.GetFiles("/dev", "ttyd*")) { + if (exclusionList?.Contains(name) ?? false) + { + continue; + } if (!name.EndsWith(".init", StringComparison.Ordinal) && !name.EndsWith(".lock", StringComparison.Ordinal)) { ports.Add(name); @@ -227,6 +282,10 @@ private List GetPortNames_FreeBSD() foreach (string name in Directory.GetFiles("/dev", "cuau*")) { + if (exclusionList?.Contains(name) ?? false) + { + continue; + } if (!name.EndsWith(".init", StringComparison.Ordinal) && !name.EndsWith(".lock", StringComparison.Ordinal)) { ports.Add(name); @@ -236,7 +295,7 @@ private List GetPortNames_FreeBSD() return ports; } - private List GetPortNames_Windows() + private static List GetPortNames_Windows(ICollection exclusionList) { const string FindFullPathPattern = @"\\\\\?\\([\w]*)#([\w&]*)#([\w&]*)"; const string RegExPattern = @"\\Device\\([a-zA-Z]*)(\d)"; @@ -271,7 +330,7 @@ private List GetPortNames_Windows() if (portKeyInfo != null) { string portName = (string)allPorts.GetValue(port); - if (portName != null) + if (portName != null && !(exclusionList?.Contains(portName) ?? false)) { portNames.Add(portName); } @@ -283,10 +342,44 @@ private List GetPortNames_Windows() string portName = (string)allPorts.GetValue(port); if (portName != null) { + if (exclusionList?.Contains(portName) ?? false) + { + continue; + } + + // discard known system and other rogue devices // Get the full qualified name of the device string deviceFullPath = (string)deviceFullPaths.GetValue(portName); if (deviceFullPath != null) { + // make it upper case for comparison + var deviceFULLPATH = deviceFullPath.ToUpperInvariant(); + + if ( + deviceFULLPATH.StartsWith(@"\\?\ACPI") || + + // reported in https://github.com/nanoframework/Home/issues/332 + // COM ports from Broadcom 20702 Bluetooth adapter + deviceFULLPATH.Contains(@"VID_0A5C+PID_21E1") || + + // reported in https://nanoframework.slack.com/archives/C4MGGBH1P/p1531660736000055?thread_ts=1531659631.000021&cid=C4MGGBH1P + // COM ports from Broadcom 20702 Bluetooth adapter + deviceFULLPATH.Contains(@"VID&00010057_PID&0023") || + + // reported in Discord channel + deviceFULLPATH.Contains(@"VID&0001009E_PID&400A") || + + // this seems to cover virtual COM ports from Bluetooth devices + deviceFULLPATH.Contains("BTHENUM") || + + // this seems to cover virtual COM ports by ELTIMA + deviceFULLPATH.Contains("EVSERIAL") + ) + { + // don't even bother with this one + continue; + } + var devicePathDetail = Regex.Match(deviceFullPath.Replace("+", "&"), FindFullPathPattern); if ((devicePathDetail.Success) && (devicePathDetail.Groups.Count == 4)) { diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs index af9b38a..aaa1a12 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs @@ -1,11 +1,6 @@ -// -// Copyright (c) .NET Foundation and Contributors -// See LICENSE file in the project root for full license information. -// +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. -using Microsoft.Win32; -using nanoFramework.Tools.Debugger.WireProtocol; -using Polly; using System; using System.Collections.Concurrent; using System.Collections.Generic; @@ -14,6 +9,8 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using nanoFramework.Tools.Debugger.WireProtocol; +using Polly; namespace nanoFramework.Tools.Debugger.PortSerial { @@ -27,10 +24,6 @@ public partial class PortSerialManager : PortBase // counter of device watchers completed private int _deviceWatchersCompletedCount = 0; - // counter of device watchers completed - private int _newDevicesCount = 0; - private readonly object _newDevicesCountLock = new object(); - private readonly Random _delay = new Random(DateTime.Now.Millisecond); private readonly ConcurrentDictionary _devicesCache = new ConcurrentDictionary(); @@ -66,8 +59,6 @@ public PortSerialManager(bool startDeviceWatchers = true, List portExclu public override void ReScanDevices() { - _newDevicesCount = 0; - // need to reset this here to have intimidate effect IsDevicesEnumerationComplete = false; @@ -101,6 +92,7 @@ private void InitializeDeviceWatchers() { _deviceWatcher.Added += OnDeviceAdded; _deviceWatcher.Removed += OnDeviceRemoved; + _deviceWatcher.AllNewDevicesAdded += ProcessDeviceEnumerationComplete; } public void StartSerialDeviceWatchers() @@ -116,11 +108,10 @@ private void StartDeviceWatchersInternal() { // Start all device watchers - _deviceWatcher.Start(); + _deviceWatcher.Start(PortExclusionList); _watchersStarted = true; - _deviceWatchersCompletedCount = 0; IsDevicesEnumerationComplete = false; } @@ -146,8 +137,13 @@ private void StopDeviceWatchersInternal() private void NanoFrameworkDevicesRemoveAllSerial() { + List devicesToRemove; + // also clear nanoFramework devices list - var devicesToRemove = NanoFrameworkDevices.Select(nanoDevice => ((NanoDevice)nanoDevice).DeviceId).ToList(); + lock (NanoFrameworkDevices) + { + devicesToRemove = NanoFrameworkDevices.Select(nanoDevice => ((NanoDevice)nanoDevice).DeviceId).ToList(); + } foreach (var deviceId in devicesToRemove) { @@ -158,122 +154,126 @@ private void NanoFrameworkDevicesRemoveAllSerial() #endregion #region Methods to manage device list add, remove, etc + /// + /// Get the device that communicates via the serial port, provided it has been added to the + /// list of known devices. + /// + /// + /// + public static NanoDeviceBase GetRegisteredDevice(string portName) + { + if (!string.IsNullOrWhiteSpace(portName)) + { + var devices = NanoFrameworkDevices.Instance; + lock (devices) + { + return devices.FirstOrDefault(d => (d as NanoDevice)?.DeviceId == portName); + } + } + return null; + } /// /// Adds a new device to list of NanoFrameworkDevices. /// /// The serial port name where the device is connected. - public override void AddDevice(string deviceId) + public override NanoDeviceBase AddDevice(string deviceId) { - AddDeviceToListAsync(deviceId); + return AddDeviceToListAsync(deviceId); } /// /// Creates a and adds it to the list of devices. /// /// The AQS used to find this device - private void AddDeviceToListAsync(string deviceId) + private NanoDeviceBase AddDeviceToListAsync(string deviceId) { // search the nanoFramework device list for a device with a matching interface ID var nanoFrameworkDeviceMatch = FindNanoFrameworkDevice(deviceId); // Add the device if it's new - if (nanoFrameworkDeviceMatch == null) + if (nanoFrameworkDeviceMatch is null) { OnLogMessageAvailable(NanoDevicesEventSource.Log.CandidateDevice(deviceId)); - if (nanoFrameworkDeviceMatch == null) - { - // Create a new element for this device and... - var newNanoFrameworkDevice = new NanoDevice(); - newNanoFrameworkDevice.DeviceId = deviceId; - newNanoFrameworkDevice.ConnectionPort = new PortSerial(this, newNanoFrameworkDevice); - newNanoFrameworkDevice.Transport = TransportType.Serial; + // Create a new element for this device and... + var newNanoFrameworkDevice = new NanoDevice(); + newNanoFrameworkDevice.DeviceId = deviceId; + newNanoFrameworkDevice.ConnectionPort = new PortSerial(this, newNanoFrameworkDevice); + newNanoFrameworkDevice.Transport = TransportType.Serial; - var connectResult = newNanoFrameworkDevice.ConnectionPort.ConnectDevice(); + var connectResult = newNanoFrameworkDevice.ConnectionPort.ConnectDevice(); - if (connectResult == ConnectPortResult.Unauthorized) + if (connectResult == ConnectPortResult.Unauthorized) + { + OnLogMessageAvailable(NanoDevicesEventSource.Log.UnauthorizedAccessToDevice(deviceId)); + } + else if (connectResult == ConnectPortResult.Connected) + { + if (CheckValidNanoFrameworkSerialDevice(newNanoFrameworkDevice)) { - OnLogMessageAvailable(NanoDevicesEventSource.Log.UnauthorizedAccessToDevice(deviceId)); + //add device to the collection + NanoFrameworkDeviceAdd(newNanoFrameworkDevice); + + OnLogMessageAvailable(NanoDevicesEventSource.Log.ValidDevice($"{newNanoFrameworkDevice.Description}")); + nanoFrameworkDeviceMatch = newNanoFrameworkDevice; } - else if (connectResult == ConnectPortResult.Connected) + else { - if (CheckValidNanoFrameworkSerialDevice(newNanoFrameworkDevice)) - { - //add device to the collection - NanoFrameworkDeviceAdd(newNanoFrameworkDevice); + // disconnect + newNanoFrameworkDevice.Disconnect(); - OnLogMessageAvailable(NanoDevicesEventSource.Log.ValidDevice($"{newNanoFrameworkDevice.Description}")); - } - else - { - // disconnect - newNanoFrameworkDevice.Disconnect(); + // devices powered by the USB cable and that feature a serial converter (like an FTDI chip) + // are still booting when the USB enumeration event raises + // so need to give them enough time for the boot sequence to complete before trying to communicate with them - // devices powered by the USB cable and that feature a serial converter (like an FTDI chip) - // are still booting when the USB enumeration event raises - // so need to give them enough time for the boot sequence to complete before trying to communicate with them + // Failing to connect to debugger engine on first attempt occurs frequently on dual USB devices like ESP32 WROVER KIT. + // Seems to be something related with both devices using the same USB endpoint + // Another reason is that an ESP32 takes around 3 seconds to complete the boot sequence and launch the CLR. + // Until then the device will look non responsive or invalid to the detection mechanism that we're using. + // A nice workaround for this seems to be adding an extra random wait so the comms are not simultaneous. - // Failing to connect to debugger engine on first attempt occurs frequently on dual USB devices like ESP32 WROVER KIT. - // Seems to be something related with both devices using the same USB endpoint - // Another reason is that an ESP32 takes around 3 seconds to complete the boot sequence and launch the CLR. - // Until then the device will look non responsive or invalid to the detection mechanism that we're using. - // A nice workaround for this seems to be adding an extra random wait so the comms are not simultaneous. - - int delay; - lock (_delay) - { - delay = _delay.Next(200, 600); - } + int delay; + lock (_delay) + { + delay = _delay.Next(200, 600); + } - Thread.Sleep(BootTime + delay); + Thread.Sleep(BootTime + delay); - OnLogMessageAvailable(NanoDevicesEventSource.Log.CheckingValidDevice($" {newNanoFrameworkDevice.DeviceId} *** 2nd attempt ***")); + OnLogMessageAvailable(NanoDevicesEventSource.Log.CheckingValidDevice($" {newNanoFrameworkDevice.DeviceId} *** 2nd attempt ***")); - connectResult = newNanoFrameworkDevice.ConnectionPort.ConnectDevice(); + connectResult = newNanoFrameworkDevice.ConnectionPort.ConnectDevice(); - if (connectResult == ConnectPortResult.Unauthorized) - { - OnLogMessageAvailable(NanoDevicesEventSource.Log.UnauthorizedAccessToDevice(deviceId)); - } - else if (connectResult == ConnectPortResult.Connected) + if (connectResult == ConnectPortResult.Unauthorized) + { + OnLogMessageAvailable(NanoDevicesEventSource.Log.UnauthorizedAccessToDevice(deviceId)); + } + else if (connectResult == ConnectPortResult.Connected) + { + if (CheckValidNanoFrameworkSerialDevice(newNanoFrameworkDevice, true)) { - if (CheckValidNanoFrameworkSerialDevice(newNanoFrameworkDevice, true)) - { - NanoFrameworkDeviceAdd(newNanoFrameworkDevice); + NanoFrameworkDeviceAdd(newNanoFrameworkDevice); - OnLogMessageAvailable(NanoDevicesEventSource.Log.ValidDevice($"{newNanoFrameworkDevice.Description}")); - } - else - { - OnLogMessageAvailable(NanoDevicesEventSource.Log.QuitDevice(deviceId)); - } + OnLogMessageAvailable(NanoDevicesEventSource.Log.ValidDevice($"{newNanoFrameworkDevice.Description}")); } else { OnLogMessageAvailable(NanoDevicesEventSource.Log.QuitDevice(deviceId)); } } - } - else - { - OnLogMessageAvailable(NanoDevicesEventSource.Log.QuitDevice(deviceId)); - } - - // subtract devices count - lock (_newDevicesCountLock) - { - _newDevicesCount--; - } - - - // check if we are done processing arriving devices - if (_newDevicesCount == 0) - { - ProcessDeviceEnumerationComplete(); + else + { + OnLogMessageAvailable(NanoDevicesEventSource.Log.QuitDevice(deviceId)); + } } } + else + { + OnLogMessageAvailable(NanoDevicesEventSource.Log.QuitDevice(deviceId)); + } } + return nanoFrameworkDeviceMatch; } /// @@ -282,16 +282,23 @@ private void AddDeviceToListAsync(string deviceId) /// new NanoSerialDevice private void NanoFrameworkDeviceAdd(NanoDevice newNanoFrameworkDevice) { - if (newNanoFrameworkDevice != null && NanoFrameworkDevices.OfType>().Count(i => i.DeviceId == newNanoFrameworkDevice.DeviceId) == 0) + lock (NanoFrameworkDevices) { - //add device to the collection - NanoFrameworkDevices.Add(newNanoFrameworkDevice); + if (newNanoFrameworkDevice != null && NanoFrameworkDevices.OfType>().Count(i => i.DeviceId == newNanoFrameworkDevice.DeviceId) == 0) + { + //add device to the collection + NanoFrameworkDevices.Add(newNanoFrameworkDevice); + } } } public override void DisposeDevice(string instanceId) { - var deviceToDispose = NanoFrameworkDevices.FirstOrDefault(nanoDevice => ((NanoDevice)nanoDevice).DeviceId == instanceId); + NanoDeviceBase deviceToDispose; + lock (NanoFrameworkDevices) + { + deviceToDispose = NanoFrameworkDevices.FirstOrDefault(nanoDevice => ((NanoDevice)nanoDevice).DeviceId == instanceId); + } if (deviceToDispose != null) { @@ -306,15 +313,28 @@ private void RemoveDeviceFromList(string deviceId) { OnLogMessageAvailable(NanoDevicesEventSource.Log.DeviceDeparture(deviceId)); - // get devices and remove them from collection - NanoFrameworkDevices.OfType>() - .Where(i => i.DeviceId == deviceId).ToList() - .ForEach(RemoveNanoFrameworkDevices); + List> devices; + lock (NanoFrameworkDevices) + { + // get devices + devices = NanoFrameworkDevices.OfType>() + .Where(i => i.DeviceId == deviceId).ToList(); + } + + // remove them from collection + devices.ForEach(RemoveNanoFrameworkDevices); } private void RemoveNanoFrameworkDevices(NanoDevice device) { - NanoFrameworkDevices.Remove(device); + if (device is null) + { + return; + } + lock (NanoFrameworkDevices) + { + NanoFrameworkDevices.Remove(device); + } device?.DebugEngine?.StopProcessing(); device?.DebugEngine?.Dispose(); } @@ -323,7 +343,10 @@ private NanoDeviceBase FindNanoFrameworkDevice(string deviceId) { if (deviceId != null) { - return NanoFrameworkDevices.FirstOrDefault(d => (d as NanoDevice).DeviceId == deviceId); + lock (NanoFrameworkDevices.Instance) + { + return NanoFrameworkDevices.FirstOrDefault(d => (d as NanoDevice)?.DeviceId == deviceId); + } } return null; @@ -333,7 +356,7 @@ private NanoDeviceBase FindNanoFrameworkDevice(string deviceId) /// Remove the device from the device list /// /// - /// + /// private void OnDeviceRemoved(object sender, string serialPort) { RemoveDeviceFromList(serialPort); @@ -344,7 +367,7 @@ private void OnDeviceRemoved(object sender, string serialPort) /// This function will add the device to the listOfDevices /// /// - /// + /// private void OnDeviceAdded(object sender, string serialPort) { // check against exclusion list @@ -354,62 +377,12 @@ private void OnDeviceAdded(object sender, string serialPort) return; } - // discard known system and other rogue devices - RegistryKey portKeyInfo = Registry.LocalMachine.OpenSubKey(@"SYSTEM\CurrentControlSet\Control\COM Name Arbiter\Devices"); - if (portKeyInfo != null) - { - var portInfo = (string)portKeyInfo.GetValue(serialPort); - - if (portInfo != null) - { - Debug.WriteLine($"{nameof(OnDeviceAdded)}: port {serialPort}, portinfo: {portInfo}"); - - // make it upper case for comparison - portInfo = portInfo.ToUpperInvariant(); - - if ( - portInfo.StartsWith(@"\\?\ACPI") || - - // reported in https://github.com/nanoframework/Home/issues/332 - // COM ports from Broadcom 20702 Bluetooth adapter - portInfo.Contains(@"VID_0A5C+PID_21E1") || - - // reported in https://nanoframework.slack.com/archives/C4MGGBH1P/p1531660736000055?thread_ts=1531659631.000021&cid=C4MGGBH1P - // COM ports from Broadcom 20702 Bluetooth adapter - portInfo.Contains(@"VID&00010057_PID&0023") || - - // reported in Discord channel - portInfo.Contains(@"VID&0001009E_PID&400A") || - - // this seems to cover virtual COM ports from Bluetooth devices - portInfo.Contains("BTHENUM") || - - // this seems to cover virtual COM ports by ELTIMA - portInfo.Contains("EVSERIAL") - ) - { - OnLogMessageAvailable(NanoDevicesEventSource.Log.DroppingDeviceToExclude(serialPort)); - - // don't even bother with this one - return; - } - } - } - OnLogMessageAvailable(NanoDevicesEventSource.Log.DeviceArrival(serialPort)); - lock (_newDevicesCountLock) - { - _newDevicesCount++; - } - - Task.Run(() => - { - Policy.Handle() - .WaitAndRetry(10, retryCount => TimeSpan.FromMilliseconds((retryCount * retryCount) * 25), - onRetry: (exception, delay, retryCount, context) => LogRetry(exception, delay, retryCount, context)) - .Execute(() => AddDeviceToListAsync(serialPort)); - }); + Policy.Handle() + .WaitAndRetry(10, retryCount => TimeSpan.FromMilliseconds((retryCount * retryCount) * 25), + onRetry: (exception, delay, retryCount, context) => LogRetry(exception, delay, retryCount, context)) + .Execute(() => AddDeviceToListAsync(serialPort)); } private void LogRetry(Exception exception, TimeSpan delay, object retryCount, object context) @@ -425,12 +398,22 @@ private void LogRetry(Exception exception, TimeSpan delay, object retryCount, ob #region Handlers and events for Device Enumeration Complete - private void ProcessDeviceEnumerationComplete() + private void ProcessDeviceEnumerationComplete(object sender, int numDevicesAdded) { - OnLogMessageAvailable(NanoDevicesEventSource.Log.SerialDeviceEnumerationCompleted(NanoFrameworkDevices.Count)); + int count; + lock (NanoFrameworkDevices) + { + if (IsDevicesEnumerationComplete && numDevicesAdded == 0) + { + // Nothing has changed + return; + } + // all watchers have completed enumeration + IsDevicesEnumerationComplete = true; - // all watchers have completed enumeration - IsDevicesEnumerationComplete = true; + count = NanoFrameworkDevices.OfType>().Count(); + } + OnLogMessageAvailable(NanoDevicesEventSource.Log.SerialDeviceEnumerationCompleted(count)); // fire event that Serial enumeration is complete OnDeviceEnumerationCompleted(); diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/DeviceWatcher.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/DeviceWatcher.cs index 205f234..cf347f8 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/DeviceWatcher.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/DeviceWatcher.cs @@ -1,7 +1,5 @@ -// -// Copyright (c) .NET Foundation and Contributors -// See LICENSE file in the project root for full license information. -// +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. using System; using System.Diagnostics; @@ -9,6 +7,8 @@ using System.Net.Sockets; using System.Text; using System.Threading; +using System.Threading.Tasks; +using nanoFramework.Tools.Debugger.NFDevice; namespace nanoFramework.Tools.Debugger.PortTcpIp { @@ -148,7 +148,19 @@ private void ProcessDiscoveryMessage(string message) switch (command) { case CommandDeviceStart: - Added?.Invoke(this, new NetworkDeviceInformation(host, port)); + if (Added is not null) + { + var info = new NetworkDeviceInformation(host, port); + if (PortTcpIpManager.GetRegisteredDevice(info) is null) + { + Task.Run(() => + GlobalExclusiveDeviceAccess.CommunicateWithDevice(info, () => + { + Added.Invoke(this, info); + }) + ); + } + } break; case CommandDeviceStop: diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs index 0227fe2..2000457 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs @@ -1,10 +1,6 @@ -// -// Copyright (c) .NET Foundation and Contributors -// See LICENSE file in the project root for full license information. -// +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. -using nanoFramework.Tools.Debugger.WireProtocol; -using Polly; using System; using System.Collections.Concurrent; using System.Collections.Generic; @@ -12,6 +8,8 @@ using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; +using nanoFramework.Tools.Debugger.WireProtocol; +using Polly; namespace nanoFramework.Tools.Debugger.PortTcpIp { @@ -27,13 +25,14 @@ public class PortTcpIpManager : PortBase // Network device watchers started flag private bool _watchersStarted = false; - // counter of device watchers completed - private int _newDevicesCount = 0; - /// - /// Internal list with the actual nF Network devices + /// Internal list with the actual nF Network devices. + /// This must be a static list as NanoFrameworkDevices is also global. + /// Take care that all items of _networkDevices are in the NanoFrameworkDevices, + /// and that there are no devices in NanoFrameworkDevices that should be present + /// in _networkDevices but are not (and use NanoFrameworkDevices for locks). /// - private readonly List _networkDevices = new List(); + private static readonly List _networkDevices = new List(); private IEnumerable> _networkNanoFrameworkDevices => NanoFrameworkDevices.Cast>(); @@ -126,21 +125,38 @@ private void StopDeviceWatchersInternal() } // Clear the list of devices so we don't have potentially disconnected devices around - ClearDeviceEntries(); - // also clear nanoFramework devices list - var devicesToRemove = _networkNanoFrameworkDevices.Select(nanoDevice => nanoDevice.DeviceId).ToList(); + List devicesToRemove; + lock (NanoFrameworkDevices) + { + devicesToRemove = _networkNanoFrameworkDevices.Select(nanoDevice => nanoDevice.DeviceId).ToList(); + } foreach (var deviceId in devicesToRemove) { // get device... - var device = FindNanoFrameworkDevice(deviceId); - - // ... and remove it from collection - NanoFrameworkDevices.Remove(device); - - device?.DebugEngine?.StopProcessing(); - device?.DebugEngine?.Stop(true); + NanoDeviceBase device; + lock (NanoFrameworkDevices) + { + var deviceEntry = FindDevice(deviceId); + if (deviceEntry is null) + { + // this is not a TcpIp-connected device and is managed by another PortManager + continue; + } + // ... and remove it from collection + _networkDevices.Remove(deviceEntry); + + device = FindNanoFrameworkDevice(deviceId); + if (device is null) + { + continue; + } + // ... and remove it from collection + NanoFrameworkDevices.Remove(device); + } + device.DebugEngine?.StopProcessing(); + device.DebugEngine?.Stop(true); } _watchersStarted = false; @@ -150,69 +166,91 @@ private void StopDeviceWatchersInternal() #region Methods to manage device list add, remove, etc + /// + /// Get the device that communicates via the network port, provided it has been added to the + /// list of known devices. + /// + /// + /// + public static NanoDeviceBase GetRegisteredDevice(NetworkDeviceInformation networkDevice) + { + if (networkDevice is not null) + { + var devices = NanoFrameworkDevices.Instance; + lock (devices) + { + return devices.FirstOrDefault(d => (d as NanoDevice)?.DeviceId == networkDevice.DeviceId); + } + } + return null; + } /// /// Creates a DeviceListEntry for a device and adds it to the list of devices /// - private void AddDeviceToListAsync(NetworkDeviceInformation networkDevice) + private (NanoDeviceBase device, bool isNew) AddDeviceToListAsync(NetworkDeviceInformation networkDevice) { - // search the device list for a device with a matching interface ID - var networkMatch = FindDevice(networkDevice.DeviceId); - - // Add the device if it's new - if (networkMatch != null) return; + bool isNew = false; - OnLogMessageAvailable(NanoDevicesEventSource.Log.CandidateDevice(networkDevice.DeviceId)); + // search the device list for a device with a matching interface ID + NetworkDeviceInformation networkMatch; + NanoDeviceBase nanoFrameworkDeviceMatch; + lock (NanoFrameworkDevices) + { + networkMatch = FindDevice(networkDevice.DeviceId); - // search the nanoFramework device list for a device with a matching interface ID - var nanoFrameworkDeviceMatch = FindNanoFrameworkDevice(networkDevice.DeviceId); + // search the nanoFramework device list for a device with a matching interface ID + nanoFrameworkDeviceMatch = FindNanoFrameworkDevice(networkDevice.DeviceId); + } - if (nanoFrameworkDeviceMatch != null) return; + // Add the device if it's new + if (networkMatch is null && nanoFrameworkDeviceMatch is null) + { + OnLogMessageAvailable(NanoDevicesEventSource.Log.CandidateDevice(networkDevice.DeviceId)); - // Create a new element for this device and... - var newNanoFrameworkDevice = new NanoDevice(); - newNanoFrameworkDevice.DeviceId = networkDevice.DeviceId; - newNanoFrameworkDevice.ConnectionPort = new PortTcpIp(this, newNanoFrameworkDevice, networkDevice); - newNanoFrameworkDevice.Transport = TransportType.TcpIp; + // Create a new element for this device and... + var newNanoFrameworkDevice = new NanoDevice(); + newNanoFrameworkDevice.DeviceId = networkDevice.DeviceId; + newNanoFrameworkDevice.ConnectionPort = new PortTcpIp(this, newNanoFrameworkDevice, networkDevice); + newNanoFrameworkDevice.Transport = TransportType.TcpIp; - var connectResult = newNanoFrameworkDevice.ConnectionPort.ConnectDevice(); + var connectResult = newNanoFrameworkDevice.ConnectionPort.ConnectDevice(); - if (connectResult == ConnectPortResult.Unauthorized) - { - OnLogMessageAvailable( - NanoDevicesEventSource.Log.UnauthorizedAccessToDevice(networkDevice.DeviceId)); - } - else if (connectResult == ConnectPortResult.Connected) - { - if (CheckValidNanoFrameworkNetworkDevice(newNanoFrameworkDevice)) + if (connectResult == ConnectPortResult.Unauthorized) { - //add device to the collection - NanoFrameworkDevices.Add(newNanoFrameworkDevice); - - _networkDevices.Add(networkDevice); - OnLogMessageAvailable( - NanoDevicesEventSource.Log.ValidDevice($"{newNanoFrameworkDevice.Description}")); + NanoDevicesEventSource.Log.UnauthorizedAccessToDevice(networkDevice.DeviceId)); + } + else if (connectResult == ConnectPortResult.Connected) + { + if (CheckValidNanoFrameworkNetworkDevice(newNanoFrameworkDevice)) + { + lock (NanoFrameworkDevices) + { + //add device to the collection + NanoFrameworkDevices.Add(newNanoFrameworkDevice); + _networkDevices.Add(networkDevice); + } + + OnLogMessageAvailable( + NanoDevicesEventSource.Log.ValidDevice($"{newNanoFrameworkDevice.Description}")); + + nanoFrameworkDeviceMatch = newNanoFrameworkDevice; + isNew = true; + } + else + { + // disconnect + newNanoFrameworkDevice.Disconnect(); + } } else { - // disconnect - newNanoFrameworkDevice.Disconnect(); + OnLogMessageAvailable(NanoDevicesEventSource.Log.QuitDevice(networkDevice.DeviceId)); } } - else - { - OnLogMessageAvailable(NanoDevicesEventSource.Log.QuitDevice(networkDevice.DeviceId)); - } - // subtract devices count - _newDevicesCount--; - - // check if we are done processing arriving devices - if (_newDevicesCount == 0) - { - ProcessDeviceEnumerationComplete(); - } + return (nanoFrameworkDeviceMatch, isNew); } public override void DisposeDevice(string instanceId) @@ -226,32 +264,35 @@ public override void DisposeDevice(string instanceId) private void RemoveDeviceFromList(NetworkDeviceInformation networkDevice) { // Removes the device entry from the internal list; therefore the UI - var deviceEntry = FindDevice(networkDevice.DeviceId); - OnLogMessageAvailable(NanoDevicesEventSource.Log.DeviceDeparture(networkDevice.DeviceId)); - _networkDevices.Remove(deviceEntry); - // get device... - var device = FindNanoFrameworkDevice(networkDevice.DeviceId); + NanoDeviceBase device; + lock (NanoFrameworkDevices) + { + var deviceEntry = FindDevice(networkDevice.DeviceId); + if (deviceEntry != null) + { + _networkDevices.Remove(deviceEntry); + } - // ... and remove it from collection - NanoFrameworkDevices.Remove(device); + device = FindNanoFrameworkDevice(networkDevice.DeviceId); + if (device != null) + { + // ... and remove it from collection + NanoFrameworkDevices.Remove(device); + } + } device?.DebugEngine?.StopProcessing(); device?.DebugEngine?.Dispose(); } - private void ClearDeviceEntries() - { - _networkDevices.Clear(); - } - /// /// Searches through the existing list of devices for the first DeviceListEntry that has /// the specified device Id. /// - internal NetworkDeviceInformation FindDevice(string deviceId) => + private NetworkDeviceInformation FindDevice(string deviceId) => _networkDevices.FirstOrDefault(d => d.DeviceId == deviceId); private NanoDeviceBase FindNanoFrameworkDevice(string deviceId) => @@ -272,9 +313,12 @@ private void OnDeviceAdded(object sender, NetworkDeviceInformation networkDevice { OnLogMessageAvailable(NanoDevicesEventSource.Log.DeviceArrival(networkDevice.DeviceId)); - _newDevicesCount++; + var (_, isNew) = AddDeviceToListAsync(networkDevice); - Task.Run(() => { AddDeviceToListAsync(networkDevice); }); + if (isNew && !IsDevicesEnumerationComplete) + { + ProcessDeviceEnumerationComplete(); + } } #endregion @@ -284,11 +328,16 @@ private void OnDeviceAdded(object sender, NetworkDeviceInformation networkDevice private void ProcessDeviceEnumerationComplete() { - OnLogMessageAvailable( - NanoDevicesEventSource.Log.SerialDeviceEnumerationCompleted(NanoFrameworkDevices.Count)); + int count; + lock (NanoFrameworkDevices) + { + IsDevicesEnumerationComplete = true; + count = NanoFrameworkDevices.OfType>().Count(); + } - // all watchers have completed enumeration - IsDevicesEnumerationComplete = true; + // TODO: count are not serial devices + OnLogMessageAvailable( + NanoDevicesEventSource.Log.SerialDeviceEnumerationCompleted(count)); // fire event that Network enumeration is complete OnDeviceEnumerationCompleted(); @@ -494,7 +543,7 @@ internal void OnLogMessageAvailable(string message) LogMessageAvailable?.Invoke(this, new StringEventArgs(message)); } - public override void AddDevice(string deviceId) + public override NanoDeviceBase AddDevice(string deviceId) { // expected format is "tcpip://{Host}:{Port}" @@ -504,9 +553,9 @@ public override void AddDevice(string deviceId) throw new ArgumentException("Invalid tcpip format."); } - AddDeviceToListAsync(new NetworkDeviceInformation( + return AddDeviceToListAsync(new NetworkDeviceInformation( match.Groups["host"].Value, - int.Parse(match.Groups["port"].Value))); + int.Parse(match.Groups["port"].Value))).device; } /// @@ -517,4 +566,4 @@ public override void AddDevice(string deviceId) #endregion } -} \ No newline at end of file +} diff --git a/nanoFramework.Tools.DebugLibrary.Shared/nanoFramework.Tools.DebugLibrary.Net.projitems b/nanoFramework.Tools.DebugLibrary.Shared/nanoFramework.Tools.DebugLibrary.Net.projitems index 7c5cb25..d95bcc7 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/nanoFramework.Tools.DebugLibrary.Net.projitems +++ b/nanoFramework.Tools.DebugLibrary.Shared/nanoFramework.Tools.DebugLibrary.Net.projitems @@ -67,6 +67,7 @@ + From 29d3197a51145724789f51600826d09e951ecec4 Mon Sep 17 00:00:00 2001 From: Frank Robijn Date: Tue, 17 Sep 2024 22:24:31 +0200 Subject: [PATCH 03/12] Hopefully locking of NanoFrameworkDevices was flagged by sonarcloud because it was not read-only. SonarCloud may well be right that the way NanoFrameworkDevices is used in this library is questionable, but changing is more work. As it is now (readonly object instance), NanoFrameworkDevices is a valid lock-target as the instance never changes. Also added an exception handling in GlobalExclusiveDeviceAccess in case of killed processes - change was not committed previously. --- .../PortDefinitions/PortBase.cs | 12 ++-- .../NFDevice/GlobalExclusiveDeviceAccess.cs | 58 +++++++++++-------- .../PortCompositeDeviceManager.cs | 2 - .../PortDefinitions/PortBase.cs | 2 +- .../PortSerial/DeviceWatcher.cs | 3 +- .../PortSerial/PortSerialManager.cs | 1 - .../PortTcpIp/DeviceWatcher.cs | 8 ++- .../PortTcpIp/PortTcpIpManager.cs | 2 - 8 files changed, 50 insertions(+), 38 deletions(-) diff --git a/nanoFramework.Tools.DebugLibrary.Net/PortDefinitions/PortBase.cs b/nanoFramework.Tools.DebugLibrary.Net/PortDefinitions/PortBase.cs index 8a63ed7..b4a9cd1 100644 --- a/nanoFramework.Tools.DebugLibrary.Net/PortDefinitions/PortBase.cs +++ b/nanoFramework.Tools.DebugLibrary.Net/PortDefinitions/PortBase.cs @@ -1,18 +1,20 @@ -// -// Copyright (c) .NET Foundation and Contributors -// See LICENSE file in the project root for full license information. -// +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using nanoFramework.Tools.Debugger.PortComposite; using nanoFramework.Tools.Debugger.PortSerial; using nanoFramework.Tools.Debugger.PortTcpIp; -using System.Collections.Generic; namespace nanoFramework.Tools.Debugger { // write intellisense documentation for this class public abstract partial class PortBase : PortMessageBase { + protected PortBase() + { + NanoFrameworkDevices = NanoFrameworkDevices.Instance; + } #region creating serial instances diff --git a/nanoFramework.Tools.DebugLibrary.Shared/NFDevice/GlobalExclusiveDeviceAccess.cs b/nanoFramework.Tools.DebugLibrary.Shared/NFDevice/GlobalExclusiveDeviceAccess.cs index 8c51e93..e9b3abd 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/NFDevice/GlobalExclusiveDeviceAccess.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/NFDevice/GlobalExclusiveDeviceAccess.cs @@ -59,32 +59,44 @@ public static bool CommunicateWithDevice(NetworkDeviceInformation address, Actio #region Implementation private static bool DoCommunicateWithDevice(string connectionKey, Action communication, int millisecondsTimeout, CancellationToken? cancellationToken) { - var waitHandles = new List(); - var mutex = new Mutex(false, $"{MutexBaseName}_{connectionKey}"); - waitHandles.Add(mutex); - - CancellationTokenSource timeOutToken = null; - if (millisecondsTimeout > 0 && millisecondsTimeout != Timeout.Infinite) - { - timeOutToken = new CancellationTokenSource(millisecondsTimeout); - waitHandles.Add(timeOutToken.Token.WaitHandle); - } - if (cancellationToken.HasValue) - { - waitHandles.Add(cancellationToken.Value.WaitHandle); - } - try + for (var retry = true; retry;) { - if (WaitHandle.WaitAny(waitHandles.ToArray()) == 0) + retry = false; + + var waitHandles = new List(); + var mutex = new Mutex(false, $"{MutexBaseName}_{connectionKey}"); + waitHandles.Add(mutex); + + CancellationTokenSource timeOutToken = null; + if (millisecondsTimeout > 0 && millisecondsTimeout != Timeout.Infinite) { - communication(); - return true; + timeOutToken = new CancellationTokenSource(millisecondsTimeout); + waitHandles.Add(timeOutToken.Token.WaitHandle); + } + if (cancellationToken.HasValue) + { + waitHandles.Add(cancellationToken.Value.WaitHandle); + } + try + { + if (WaitHandle.WaitAny(waitHandles.ToArray()) == 0) + { + communication(); + return true; + } + } + catch (AbandonedMutexException) + { + // While this process is waiting on a mutex, the process that owned the mutex has been terminated + // without properly releasing the mutex. + // Try again, if this is the only remaining process it will re-create the mutex and get exclusive access. + retry = true; + } + finally + { + mutex.ReleaseMutex(); + timeOutToken?.Dispose(); } - } - finally - { - mutex.ReleaseMutex(); - timeOutToken?.Dispose(); } return false; } diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs index 3378e34..15752bc 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs @@ -19,8 +19,6 @@ public PortCompositeDeviceManager( IEnumerable ports, bool startDeviceWatchers = true) { - NanoFrameworkDevices = NanoFrameworkDevices.Instance; - _ports.AddRange(ports); SubscribeToPortEvents(); diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs index 2e9072f..957fb9e 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs @@ -50,7 +50,7 @@ public string PersistName /// public bool IsDevicesEnumerationComplete { get; internal set; } = false; - public NanoFrameworkDevices NanoFrameworkDevices { get; protected set; } + public NanoFrameworkDevices NanoFrameworkDevices { get; } /// /// Adds a new device to list of NanoFrameworkDevices. diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs index 3436883..1927cce 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs @@ -144,8 +144,9 @@ private void StartWatcher(ICollection portExclusionList) { if (PortSerialManager.GetRegisteredDevice(port) is null) { - tasks.Add(Task.Run(() => + tasks.Add(Task.Run(async () => { + await Task.Yield(); // Force true async running GlobalExclusiveDeviceAccess.CommunicateWithDevice( port, () => Added.Invoke(this, port) diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs index aaa1a12..d2a2ba9 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs @@ -35,7 +35,6 @@ public partial class PortSerialManager : PortBase /// public PortSerialManager(bool startDeviceWatchers = true, List portExclusionList = null, int bootTime = 3000) { - NanoFrameworkDevices = NanoFrameworkDevices.Instance; _deviceWatcher = new(this); BootTime = bootTime; diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/DeviceWatcher.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/DeviceWatcher.cs index cf347f8..260b4d5 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/DeviceWatcher.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/DeviceWatcher.cs @@ -153,12 +153,14 @@ private void ProcessDiscoveryMessage(string message) var info = new NetworkDeviceInformation(host, port); if (PortTcpIpManager.GetRegisteredDevice(info) is null) { - Task.Run(() => + Task.Run(async () => + { + await Task.Yield(); // Force true async running GlobalExclusiveDeviceAccess.CommunicateWithDevice(info, () => { Added.Invoke(this, info); - }) - ); + }); + }); } } break; diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs index 2000457..96c1b58 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs @@ -47,8 +47,6 @@ public PortTcpIpManager(bool startDeviceWatchers = true, int discoveryPort = Dis { _deviceWatcher = new DeviceWatcher(this, discoveryPort); - NanoFrameworkDevices = NanoFrameworkDevices.Instance; - Task.Factory.StartNew(() => { InitializeDeviceWatchers(); From 7e7e6832ced85db470348af8a13ec548d2a95e98 Mon Sep 17 00:00:00 2001 From: Frank Robijn Date: Tue, 17 Sep 2024 22:39:21 +0200 Subject: [PATCH 04/12] Might have misread the locking issue. Fixed this one. --- .../DeviceConfiguration/NanoFrameworkDevices.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nanoFramework.Tools.DebugLibrary.Shared/DeviceConfiguration/NanoFrameworkDevices.cs b/nanoFramework.Tools.DebugLibrary.Shared/DeviceConfiguration/NanoFrameworkDevices.cs index 961b3e1..d16d696 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/DeviceConfiguration/NanoFrameworkDevices.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/DeviceConfiguration/NanoFrameworkDevices.cs @@ -15,7 +15,7 @@ public static NanoFrameworkDevices Instance { get { - lock (typeof(NanoFrameworkDevices)) + lock (_instance) { return _instance.Value; } From 463de39987b0c60eb4b6f31aad39dfd2c5fce4b3 Mon Sep 17 00:00:00 2001 From: Frank Robijn Date: Sat, 21 Sep 2024 13:40:42 +0200 Subject: [PATCH 05/12] Some corrections: - Before the changes in this PR, the PortSerialManager.OnDeviceEnumerationCompleted event was postponed if a new device was detected before the addition of previously detected devices was completed. In the modified version that was no longer the case. Behaviour is restored in this version. - The DeviceWatcher.GetPortNames was changed to a static method, as it apparently is not used by any code in nanoFramework. As it is a public library, this may break the code of someone else. Original method reinstated and new static method made internal. The public version of the method is made part of the PortSerialManager, as external code is most likely to interact with the PortSerialManager than with DeviceWatcher. Addition: - It is allowed to change the PortExclusionList after the DeviceWatcher has been started. Made sure to add locks in the right places. --- .../PortSerial/DeviceWatcher.cs | 55 +++++++++++++------ .../PortSerial/PortSerialManager.cs | 24 +++++++- 2 files changed, 58 insertions(+), 21 deletions(-) diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs index 1927cce..dad52c6 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; -using System.Linq; using System.Runtime.InteropServices; using System.Text.RegularExpressions; using System.Threading; @@ -53,8 +52,7 @@ public class DeviceWatcher : IDisposable /// Represents a delegate method that is used to handle the AllNewDevicesAdded event. /// /// The object that raised the event. - /// Number of devices added - public delegate void EventAllNewDevicesAdded(object sender, int numDevicesAdded); + public delegate void EventAllNewDevicesAdded(object sender); /// /// Raised when all newly discovered devices have been added @@ -78,14 +76,15 @@ public DeviceWatcher(PortSerialManager owner) /// /// Starts the device watcher. /// + /// The collection of serial ports to ignore when searching for devices. + /// Changes in the collection after the start of the device watcher are taken into account. public void Start(ICollection portExclusionList = null) { if (!_started) { - var exclusionList = portExclusionList?.ToList(); _threadWatch = new Thread(() => { - StartWatcher(exclusionList); + StartWatcher(portExclusionList ?? []); }) { IsBackground = true, @@ -100,17 +99,24 @@ private void StartWatcher(ICollection portExclusionList) { _ownerManager.OnLogMessageAvailable($"PortSerial device watcher started @ Thread {_threadWatch.ManagedThreadId} [ProcessID: {Process.GetCurrentProcess().Id}]"); - _ports = new List(); + _ports = []; _started = true; + int newPortsDetected = 0; + var newPortsDetectedLock = new object(); + Status = DeviceWatcherStatus.Started; while (_started) { try { - var ports = GetPortNames(portExclusionList); + List ports; + lock (portExclusionList) + { + ports = GetPortNames(portExclusionList); + } // check for ports that departed List portsToRemove = new(); @@ -134,7 +140,6 @@ private void StartWatcher(ICollection portExclusionList) } // process ports that have arrived - var tasks = new List(); foreach (var port in ports) { if (!_ports.Contains(port)) @@ -144,24 +149,30 @@ private void StartWatcher(ICollection portExclusionList) { if (PortSerialManager.GetRegisteredDevice(port) is null) { - tasks.Add(Task.Run(async () => + lock (newPortsDetectedLock) + { + newPortsDetected++; + } + + Task.Run(async () => { await Task.Yield(); // Force true async running GlobalExclusiveDeviceAccess.CommunicateWithDevice( port, () => Added.Invoke(this, port) ); - })); + lock (newPortsDetectedLock) + { + if (--newPortsDetected == 0) + { + AllNewDevicesAdded?.Invoke(this); + }; + } + }); } } } } - if (tasks.Count > 0) - { - Task.WaitAll(tasks.ToArray()); - } - AllNewDevicesAdded?.Invoke(this, tasks.Count); - Thread.Sleep(200); } #if DEBUG @@ -183,8 +194,16 @@ private void StartWatcher(ICollection portExclusionList) /// /// Gets the list of serial ports. /// - /// The list of serial ports. - public static List GetPortNames(ICollection exclusionList = null) + public List GetPortNames() + { + return GetPortNames(null); + } + + /// + /// Gets the list of serial ports. + /// + /// The list of serial ports to exclude from the result. + internal static List GetPortNames(ICollection exclusionList = null) { return RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? GetPortNames_Linux(exclusionList) diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs index d2a2ba9..d3659ab 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs @@ -33,6 +33,10 @@ public partial class PortSerialManager : PortBase /// /// Creates an Serial debug client /// + /// Indicates whether to start the device watcher. + /// The collection of serial ports to ignore when searching for devices. + /// Changes in the collection after the start of the device watcher are taken into account. + /// public PortSerialManager(bool startDeviceWatchers = true, List portExclusionList = null, int bootTime = 3000) { _deviceWatcher = new(this); @@ -153,6 +157,15 @@ private void NanoFrameworkDevicesRemoveAllSerial() #endregion #region Methods to manage device list add, remove, etc + /// + /// Gets the list of serial ports. + /// + /// The list of serial ports to exclude from the result. + public static List GetPortNames(ICollection exclusionList = null) + { + return DeviceWatcher.GetPortNames(exclusionList); + } + /// /// Get the device that communicates via the serial port, provided it has been added to the /// list of known devices. @@ -370,7 +383,12 @@ private void OnDeviceRemoved(object sender, string serialPort) private void OnDeviceAdded(object sender, string serialPort) { // check against exclusion list - if (PortExclusionList.Contains(serialPort)) + bool exclude; + lock (PortExclusionList) + { + exclude = PortExclusionList.Contains(serialPort); + } + if (exclude) { OnLogMessageAvailable(NanoDevicesEventSource.Log.DroppingDeviceToExclude(serialPort)); return; @@ -397,12 +415,12 @@ private void LogRetry(Exception exception, TimeSpan delay, object retryCount, ob #region Handlers and events for Device Enumeration Complete - private void ProcessDeviceEnumerationComplete(object sender, int numDevicesAdded) + private void ProcessDeviceEnumerationComplete(object sender) { int count; lock (NanoFrameworkDevices) { - if (IsDevicesEnumerationComplete && numDevicesAdded == 0) + if (IsDevicesEnumerationComplete) { // Nothing has changed return; From 500f6bbf30fed80fcd0d72cc3b002123de5ee0b1 Mon Sep 17 00:00:00 2001 From: Frank Robijn Date: Tue, 24 Sep 2024 20:21:24 +0200 Subject: [PATCH 06/12] Changes as a result of review comments --- .../PortCompositeDeviceManager.cs | 14 +- .../PortDefinitions/PortBase.cs | 10 +- .../PortSerial/DeviceWatcher.cs | 161 ++++++++---------- .../PortSerial/PortSerialManager.cs | 19 ++- .../PortTcpIp/PortTcpIpManager.cs | 9 +- 5 files changed, 119 insertions(+), 94 deletions(-) diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs index 15752bc..b1d5cbf 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs @@ -60,7 +60,19 @@ where p.IsDevicesEnumerationComplete /// /// This API is not available in - public override NanoDeviceBase AddDevice(string deviceId) + public override void AddDevice(string deviceId) + { + // None of the Port*Manager has a check whether deviceId matches the ID handled by the manager. + throw new NotImplementedException(); + //_ports.ForEach(p => + //{ + // p.AddDevice(deviceId); + //}); + } + + /// + /// This API is not available in + public override NanoDeviceBase AddAndReturnDevice(string deviceId) { // None of the Port*Manager has a check whether deviceId matches the ID handled by the manager. throw new NotImplementedException(); diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs index 957fb9e..7a521fa 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs @@ -56,7 +56,15 @@ public string PersistName /// Adds a new device to list of NanoFrameworkDevices. /// /// The unique ID (based on the connection properties) of the device. - public abstract NanoDeviceBase AddDevice(string deviceId); + public abstract void AddDevice(string deviceId); + + /// + /// Adds a new device to list of NanoFrameworkDevices and returns the device that has been added. + /// + /// The unique ID (based on the connection properties) of the device. + /// The device with the unique ID that is added or (if it was already discovered before) retrieved + /// from the list of devices. Returns null if no device has been added. + public abstract NanoDeviceBase AddAndReturnDevice(string deviceId); /// /// Starts the device watchers. diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs index dad52c6..81e2713 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Linq; using System.Runtime.InteropServices; using System.Text.RegularExpressions; using System.Threading; @@ -76,15 +77,15 @@ public DeviceWatcher(PortSerialManager owner) /// /// Starts the device watcher. /// - /// The collection of serial ports to ignore when searching for devices. + /// The collection of serial ports to ignore when searching for devices. /// Changes in the collection after the start of the device watcher are taken into account. - public void Start(ICollection portExclusionList = null) + public void Start(ICollection portsToExclude = null) { if (!_started) { _threadWatch = new Thread(() => { - StartWatcher(portExclusionList ?? []); + StartWatcher(portsToExclude ?? []); }) { IsBackground = true, @@ -95,7 +96,7 @@ public void Start(ICollection portExclusionList = null) } } - private void StartWatcher(ICollection portExclusionList) + private void StartWatcher(ICollection portsToExclude) { _ownerManager.OnLogMessageAvailable($"PortSerial device watcher started @ Thread {_threadWatch.ManagedThreadId} [ProcessID: {Process.GetCurrentProcess().Id}]"); @@ -112,10 +113,12 @@ private void StartWatcher(ICollection portExclusionList) { try { - List ports; - lock (portExclusionList) + var ports = new List(); + lock (portsToExclude) { - ports = GetPortNames(portExclusionList); + ports.AddRange(from p in DoGetPortNames() + where !portsToExclude.Contains(p) + select p); } // check for ports that departed @@ -196,33 +199,29 @@ private void StartWatcher(ICollection portExclusionList) /// public List GetPortNames() { - return GetPortNames(null); + return DoGetPortNames(); } /// /// Gets the list of serial ports. /// - /// The list of serial ports to exclude from the result. - internal static List GetPortNames(ICollection exclusionList = null) + /// The list of serial ports that may be connected to a nanoDevice. + internal static List DoGetPortNames() { - return RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? GetPortNames_Linux(exclusionList) - : RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? GetPortNames_OSX(exclusionList) - : RuntimeInformation.IsOSPlatform(OSPlatform.Create("FREEBSD")) ? GetPortNames_FreeBSD(exclusionList) - : GetPortNames_Windows(exclusionList); + return RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? GetPortNames_Linux() + : RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? GetPortNames_OSX() + : RuntimeInformation.IsOSPlatform(OSPlatform.Create("FREEBSD")) ? GetPortNames_FreeBSD() + : GetPortNames_Windows(); } - private static List GetPortNames_Linux(ICollection exclusionList) + private static List GetPortNames_Linux() { List ports = new List(); string[] ttys = System.IO.Directory.GetFiles("/dev/", "tty*"); foreach (string dev in ttys) { - if (exclusionList?.Contains(dev) ?? false) - { - continue; - } if (dev.StartsWith("/dev/ttyS") || dev.StartsWith("/dev/ttyUSB") || dev.StartsWith("/dev/ttyACM") @@ -237,17 +236,12 @@ private static List GetPortNames_Linux(ICollection exclusionList return ports; } - private static List GetPortNames_OSX(ICollection exclusionList) + private static List GetPortNames_OSX() { List ports = new List(); foreach (string name in Directory.GetFiles("/dev", "tty.usbserial*")) { - if (exclusionList?.Contains(name) ?? false) - { - continue; - } - // We don't want Bluetooth ports if (name.ToLower().Contains("bluetooth")) { @@ -264,11 +258,6 @@ private static List GetPortNames_OSX(ICollection exclusionList) foreach (string name in Directory.GetFiles("/dev", "cu.usbserial*")) { - if (exclusionList?.Contains(name) ?? false) - { - continue; - } - // We don't want Bluetooth ports if (name.ToLower().Contains("bluetooth")) { @@ -284,16 +273,12 @@ private static List GetPortNames_OSX(ICollection exclusionList) return ports; } - private static List GetPortNames_FreeBSD(ICollection exclusionList) + private static List GetPortNames_FreeBSD() { List ports = new List(); foreach (string name in Directory.GetFiles("/dev", "ttyd*")) { - if (exclusionList?.Contains(name) ?? false) - { - continue; - } if (!name.EndsWith(".init", StringComparison.Ordinal) && !name.EndsWith(".lock", StringComparison.Ordinal)) { ports.Add(name); @@ -302,10 +287,6 @@ private static List GetPortNames_FreeBSD(ICollection exclusionLi foreach (string name in Directory.GetFiles("/dev", "cuau*")) { - if (exclusionList?.Contains(name) ?? false) - { - continue; - } if (!name.EndsWith(".init", StringComparison.Ordinal) && !name.EndsWith(".lock", StringComparison.Ordinal)) { ports.Add(name); @@ -315,13 +296,49 @@ private static List GetPortNames_FreeBSD(ICollection exclusionLi return ports; } - private static List GetPortNames_Windows(ICollection exclusionList) + private static List GetPortNames_Windows() { const string FindFullPathPattern = @"\\\\\?\\([\w]*)#([\w&]*)#([\w&]*)"; const string RegExPattern = @"\\Device\\([a-zA-Z]*)(\d)"; List portNames = new List(); try { + // discard known system and other rogue devices + bool IsSpecialPort(string deviceFullPath) + { + if (deviceFullPath is not null) + { + // make it upper case for comparison + string deviceFULLPATH = deviceFullPath.ToUpperInvariant(); + + if ( + deviceFULLPATH.StartsWith(@"\\?\ACPI") || + + // reported in https://github.com/nanoframework/Home/issues/332 + // COM ports from Broadcom 20702 Bluetooth adapter + deviceFULLPATH.Contains(@"VID_0A5C+PID_21E1") || + + // reported in https://nanoframework.slack.com/archives/C4MGGBH1P/p1531660736000055?thread_ts=1531659631.000021&cid=C4MGGBH1P + // COM ports from Broadcom 20702 Bluetooth adapter + deviceFULLPATH.Contains(@"VID&00010057_PID&0023") || + + // reported in Discord channel + deviceFULLPATH.Contains(@"VID&0001009E_PID&400A") || + + // this seems to cover virtual COM ports from Bluetooth devices + deviceFULLPATH.Contains("BTHENUM") || + + // this seems to cover virtual COM ports by ELTIMA + deviceFULLPATH.Contains("EVSERIAL") + ) + { + // don't even bother with this one + return true; + } + } + return false; + } + // Gets the list of supposed open ports RegistryKey allPorts = Registry.LocalMachine.OpenSubKey(@"HARDWARE\DEVICEMAP\SERIALCOMM"); RegistryKey deviceFullPaths = Registry.LocalMachine.OpenSubKey(@"SYSTEM\CurrentControlSet\Control\COM Name Arbiter\Devices"); @@ -350,7 +367,8 @@ private static List GetPortNames_Windows(ICollection exclusionLi if (portKeyInfo != null) { string portName = (string)allPorts.GetValue(port); - if (portName != null && !(exclusionList?.Contains(portName) ?? false)) + if (portName != null + && !IsSpecialPort((string)deviceFullPaths.GetValue(portName))) { portNames.Add(portName); } @@ -360,63 +378,32 @@ private static List GetPortNames_Windows(ICollection exclusionLi else { string portName = (string)allPorts.GetValue(port); - if (portName != null) + string deviceFullPath = (string)deviceFullPaths.GetValue(portName); + if (deviceFullPath != null) { - if (exclusionList?.Contains(portName) ?? false) + if (IsSpecialPort(deviceFullPath)) { + // don't even bother with this one continue; } - // discard known system and other rogue devices // Get the full qualified name of the device - string deviceFullPath = (string)deviceFullPaths.GetValue(portName); - if (deviceFullPath != null) + var devicePathDetail = Regex.Match(deviceFullPath.Replace("+", "&"), FindFullPathPattern); + if ((devicePathDetail.Success) && (devicePathDetail.Groups.Count == 4)) { - // make it upper case for comparison - var deviceFULLPATH = deviceFullPath.ToUpperInvariant(); + string devicePath = deviceFullPath.Split('#')[1]; - if ( - deviceFULLPATH.StartsWith(@"\\?\ACPI") || - - // reported in https://github.com/nanoframework/Home/issues/332 - // COM ports from Broadcom 20702 Bluetooth adapter - deviceFULLPATH.Contains(@"VID_0A5C+PID_21E1") || - - // reported in https://nanoframework.slack.com/archives/C4MGGBH1P/p1531660736000055?thread_ts=1531659631.000021&cid=C4MGGBH1P - // COM ports from Broadcom 20702 Bluetooth adapter - deviceFULLPATH.Contains(@"VID&00010057_PID&0023") || - - // reported in Discord channel - deviceFULLPATH.Contains(@"VID&0001009E_PID&400A") || - - // this seems to cover virtual COM ports from Bluetooth devices - deviceFULLPATH.Contains("BTHENUM") || - - // this seems to cover virtual COM ports by ELTIMA - deviceFULLPATH.Contains("EVSERIAL") - ) + RegistryKey device = Registry.LocalMachine.OpenSubKey($"SYSTEM\\CurrentControlSet\\Enum\\{devicePathDetail.Groups[1]}\\{devicePath}\\{devicePathDetail.Groups[3]}"); + if (device != null) { - // don't even bother with this one - continue; - } - - var devicePathDetail = Regex.Match(deviceFullPath.Replace("+", "&"), FindFullPathPattern); - if ((devicePathDetail.Success) && (devicePathDetail.Groups.Count == 4)) - { - string devicePath = deviceFullPath.Split('#')[1]; - - RegistryKey device = Registry.LocalMachine.OpenSubKey($"SYSTEM\\CurrentControlSet\\Enum\\{devicePathDetail.Groups[1]}\\{devicePath}\\{devicePathDetail.Groups[3]}"); - if (device != null) + string service = (string)device.GetValue("Service"); + if (service != null) { - string service = (string)device.GetValue("Service"); - if (service != null) + activePorts = Registry.LocalMachine.OpenSubKey($"SYSTEM\\CurrentControlSet\\Services\\{service}\\Enum"); + if (activePorts != null) { - activePorts = Registry.LocalMachine.OpenSubKey($"SYSTEM\\CurrentControlSet\\Services\\{service}\\Enum"); - if (activePorts != null) - { - // If the device is still plugged, it should appear as valid here, if not present, it means, the device has been disconnected - portNames.Add(portName); - } + // If the device is still plugged, it should appear as valid here, if not present, it means, the device has been disconnected + portNames.Add(portName); } } } diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs index d3659ab..8562bfd 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs @@ -160,10 +160,10 @@ private void NanoFrameworkDevicesRemoveAllSerial() /// /// Gets the list of serial ports. /// - /// The list of serial ports to exclude from the result. - public static List GetPortNames(ICollection exclusionList = null) + /// The list of serial ports that may be connected to a nanoDevice. + public static List GetPortNames() { - return DeviceWatcher.GetPortNames(exclusionList); + return DeviceWatcher.DoGetPortNames(); } /// @@ -189,7 +189,18 @@ public static NanoDeviceBase GetRegisteredDevice(string portName) /// Adds a new device to list of NanoFrameworkDevices. /// /// The serial port name where the device is connected. - public override NanoDeviceBase AddDevice(string deviceId) + public override void AddDevice(string deviceId) + { + AddDeviceToListAsync(deviceId); + } + + /// + /// Adds a new device to list of NanoFrameworkDevices. + /// + /// The serial port name where the device is connected. + /// The device with the unique ID that is added or (if it was already discovered before) retrieved + /// from the list of devices. Returns null if no device has been added. + public override NanoDeviceBase AddAndReturnDevice(string deviceId) { return AddDeviceToListAsync(deviceId); } diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs index 96c1b58..2f6dd90 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs @@ -541,7 +541,14 @@ internal void OnLogMessageAvailable(string message) LogMessageAvailable?.Invoke(this, new StringEventArgs(message)); } - public override NanoDeviceBase AddDevice(string deviceId) + /// + public override void AddDevice(string deviceId) + { + AddAndReturnDevice(deviceId); + } + + /// + public override NanoDeviceBase AddAndReturnDevice(string deviceId) { // expected format is "tcpip://{Host}:{Port}" From 3faf5751a03fa3631017d82487d8fb365255b17a Mon Sep 17 00:00:00 2001 From: Frank Robijn Date: Wed, 25 Sep 2024 07:48:59 +0200 Subject: [PATCH 07/12] Use per-port lock for wire protocol communication rather than a system-wide lock. --- .../PortDefinitions/IPort.cs | 12 ++- .../PortTcpIp/PortTcpIp.cs | 10 +-- .../WireProtocol/Engine.cs | 82 ++++++++++--------- 3 files changed, 56 insertions(+), 48 deletions(-) diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/IPort.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/IPort.cs index 53a4e81..0a90788 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/IPort.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/IPort.cs @@ -1,7 +1,5 @@ -// -// Copyright (c) .NET Foundation and Contributors -// See LICENSE file in the project root for full license information. -// +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. using System; @@ -9,6 +7,12 @@ namespace nanoFramework.Tools.Debugger { public interface IPort { + /// + /// Gets the Instance ID of the port that is unique among all ports + /// (regardless of the type of port). + /// + string InstanceId { get; } + int AvailableBytes { get; } int SendBuffer(byte[] buffer); diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIp.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIp.cs index 6677e5f..071f6fb 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIp.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIp.cs @@ -1,7 +1,5 @@ -// -// Copyright (c) .NET Foundation and Contributors -// See LICENSE file in the project root for full license information. -// +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. using System; using System.Diagnostics; @@ -19,7 +17,7 @@ public class PortTcpIp : PortMessageBase, IPort private NanoNetworkDevice NanoNetworkDevice => NanoDevice.Device; - private string InstanceId => NanoDevice.DeviceId; + public string InstanceId => NanoDevice.DeviceId; public override event EventHandler LogMessageAvailable; @@ -140,4 +138,4 @@ private void OnLogMessageAvailable(string message) LogMessageAvailable?.Invoke(this, new StringEventArgs(message)); } } -} \ No newline at end of file +} diff --git a/nanoFramework.Tools.DebugLibrary.Shared/WireProtocol/Engine.cs b/nanoFramework.Tools.DebugLibrary.Shared/WireProtocol/Engine.cs index 858deae..ecee39d 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/WireProtocol/Engine.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/WireProtocol/Engine.cs @@ -1,12 +1,6 @@ -// -// Copyright (c) .NET Foundation and Contributors -// Portions Copyright (c) Microsoft Corporation. All rights reserved. -// See LICENSE file in the project root for full license information. -// +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. -using nanoFramework.Tools.Debugger.Extensions; -using nanoFramework.Tools.Debugger.WireProtocol; -using Polly; using System; using System.Collections; using System.Collections.Generic; @@ -16,6 +10,9 @@ using System.Text; using System.Threading; using System.Threading.Tasks; +using nanoFramework.Tools.Debugger.Extensions; +using nanoFramework.Tools.Debugger.WireProtocol; +using Polly; namespace nanoFramework.Tools.Debugger { @@ -77,6 +74,14 @@ public partial class Engine : IDisposable, IControllerHostLocal internal Engine(NanoDeviceBase device) { + lock (_syncReqLockForPort) + { + if (!_syncReqLockForPort.TryGetValue(device.ConnectionPort.InstanceId, out _syncReqLock)) + { + _syncReqLockForPort[device.ConnectionPort.InstanceId] = _syncReqLock = new SemaphoreSlim(1, 1); + } + } + InitializeLocal(device); // default to false @@ -619,9 +624,10 @@ public void Dispose() /// /// Global lock object for synchronizing message request. This ensures there is only one - /// outstanding request at any point of time. + /// outstanding request per device at any point of time. /// - private static readonly SemaphoreSlim _syncReqLock = new SemaphoreSlim(1, 1); + private readonly static Dictionary _syncReqLockForPort = []; + private readonly SemaphoreSlim _syncReqLock; private IncomingMessage PerformSyncRequest( uint command, @@ -821,42 +827,42 @@ public bool ProcessMessage(IncomingMessage message, bool isReply) switch (bp.Cmd) { case Commands.c_Monitor_Ping: + { + // signal that a monitor ping was received + _pingEvent.Set(); + + Commands.Monitor_Ping.Reply cmdReply = new Commands.Monitor_Ping.Reply { - // signal that a monitor ping was received - _pingEvent.Set(); + Source = Commands.Monitor_Ping.c_Ping_Source_Host, + Flags = (StopDebuggerOnConnect ? Commands.Monitor_Ping.c_Ping_DbgFlag_Stop : 0) + }; - Commands.Monitor_Ping.Reply cmdReply = new Commands.Monitor_Ping.Reply - { - Source = Commands.Monitor_Ping.c_Ping_Source_Host, - Flags = (StopDebuggerOnConnect ? Commands.Monitor_Ping.c_Ping_DbgFlag_Stop : 0) - }; - - PerformRequestAsync( - new OutgoingMessage( - _controlller.GetNextSequenceId(), - message, - _controlller.CreateConverter(), - Flags.c_NonCritical, - cmdReply) - ); - - return true; - } + PerformRequestAsync( + new OutgoingMessage( + _controlller.GetNextSequenceId(), + message, + _controlller.CreateConverter(), + Flags.c_NonCritical, + cmdReply) + ); - case Commands.c_Monitor_Message: - { - Commands.Monitor_Message payload = message.Payload as Commands.Monitor_Message; + return true; + } - Debug.Assert(payload != null); + case Commands.c_Monitor_Message: + { + Commands.Monitor_Message payload = message.Payload as Commands.Monitor_Message; - if (payload != null) - { - Task.Factory.StartNew(() => _eventMessage?.Invoke(message, payload.ToString())); - } + Debug.Assert(payload != null); - return true; + if (payload != null) + { + Task.Factory.StartNew(() => _eventMessage?.Invoke(message, payload.ToString())); } + return true; + } + case Commands.c_Debugging_Messaging_Query: Debug.Assert(message.Payload != null); Task.Factory.StartNew(() => RpcReceiveQuery(message, (Commands.Debugging_Messaging_Query)message.Payload)); From 5bf0262fea854657ad464448df236f2e255e3f6d Mon Sep 17 00:00:00 2001 From: josesimoes Date: Wed, 25 Sep 2024 12:52:03 +0100 Subject: [PATCH 08/12] Minor fixes and improvements in comments --- .../PortSerial/PortSerialManager.cs | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs index 8562bfd..f086e15 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs @@ -157,36 +157,30 @@ private void NanoFrameworkDevicesRemoveAllSerial() #endregion #region Methods to manage device list add, remove, etc - /// - /// Gets the list of serial ports. - /// - /// The list of serial ports that may be connected to a nanoDevice. - public static List GetPortNames() - { - return DeviceWatcher.DoGetPortNames(); - } /// /// Get the device that communicates via the serial port, provided it has been added to the /// list of known devices. /// - /// - /// + /// The port name of the device to get. + /// The that communicates via the serial port, or if the device is not found. public static NanoDeviceBase GetRegisteredDevice(string portName) { if (!string.IsNullOrWhiteSpace(portName)) { var devices = NanoFrameworkDevices.Instance; + lock (devices) { return devices.FirstOrDefault(d => (d as NanoDevice)?.DeviceId == portName); } } + return null; } /// - /// Adds a new device to list of NanoFrameworkDevices. + /// Adds a new device to list of . /// /// The serial port name where the device is connected. public override void AddDevice(string deviceId) @@ -199,14 +193,14 @@ public override void AddDevice(string deviceId) /// /// The serial port name where the device is connected. /// The device with the unique ID that is added or (if it was already discovered before) retrieved - /// from the list of devices. Returns null if no device has been added. + /// from the list of devices. Returns if no device has been added. public override NanoDeviceBase AddAndReturnDevice(string deviceId) { return AddDeviceToListAsync(deviceId); } /// - /// Creates a and adds it to the list of devices. + /// Creates a and adds it to the list of devices. /// /// The AQS used to find this device private NanoDeviceBase AddDeviceToListAsync(string deviceId) @@ -300,9 +294,9 @@ private NanoDeviceBase AddDeviceToListAsync(string deviceId) } /// - /// add device to the collection (if new) + /// Adds a device to the collection (if new). /// - /// new NanoSerialDevice + /// The new private void NanoFrameworkDeviceAdd(NanoDevice newNanoFrameworkDevice) { lock (NanoFrameworkDevices) From 839145b686ad082933864f30e2ae7bfdea421bbf Mon Sep 17 00:00:00 2001 From: Frank Robijn Date: Wed, 25 Sep 2024 14:07:27 +0200 Subject: [PATCH 09/12] Next round of comments --- .../NFDevice/GlobalExclusiveDeviceAccess.cs | 7 ++++-- .../PortCompositeDeviceManager.cs | 23 ++++--------------- .../PortDefinitions/PortBase.cs | 8 +------ .../PortSerial/DeviceWatcher.cs | 6 +++-- .../PortSerial/PortSerialManager.cs | 13 +++-------- .../PortTcpIp/PortTcpIpManager.cs | 12 ++++------ 6 files changed, 21 insertions(+), 48 deletions(-) diff --git a/nanoFramework.Tools.DebugLibrary.Shared/NFDevice/GlobalExclusiveDeviceAccess.cs b/nanoFramework.Tools.DebugLibrary.Shared/NFDevice/GlobalExclusiveDeviceAccess.cs index e9b3abd..ccb76b2 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/NFDevice/GlobalExclusiveDeviceAccess.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/NFDevice/GlobalExclusiveDeviceAccess.cs @@ -59,12 +59,12 @@ public static bool CommunicateWithDevice(NetworkDeviceInformation address, Actio #region Implementation private static bool DoCommunicateWithDevice(string connectionKey, Action communication, int millisecondsTimeout, CancellationToken? cancellationToken) { - for (var retry = true; retry;) + for (bool retry = true; retry;) { retry = false; var waitHandles = new List(); - var mutex = new Mutex(false, $"{MutexBaseName}_{connectionKey}"); + var mutex = new Mutex(false, $"{MutexBaseName}{connectionKey}"); waitHandles.Add(mutex); CancellationTokenSource timeOutToken = null; @@ -73,10 +73,12 @@ private static bool DoCommunicateWithDevice(string connectionKey, Action communi timeOutToken = new CancellationTokenSource(millisecondsTimeout); waitHandles.Add(timeOutToken.Token.WaitHandle); } + if (cancellationToken.HasValue) { waitHandles.Add(cancellationToken.Value.WaitHandle); } + try { if (WaitHandle.WaitAny(waitHandles.ToArray()) == 0) @@ -98,6 +100,7 @@ private static bool DoCommunicateWithDevice(string connectionKey, Action communi timeOutToken?.Dispose(); } } + return false; } #endregion diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs index b1d5cbf..8e861ce 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortComposite/PortCompositeDeviceManager.cs @@ -59,27 +59,12 @@ where p.IsDevicesEnumerationComplete } /// - /// This API is not available in - public override void AddDevice(string deviceId) + /// This API is not available in PortCompositeDeviceManager. + public override NanoDeviceBase AddDevice(string deviceId) { - // None of the Port*Manager has a check whether deviceId matches the ID handled by the manager. + // None of the Port*Manager has a check whether deviceId matches the ID handled by the manager, + // so we don't know how to add a device here. throw new NotImplementedException(); - //_ports.ForEach(p => - //{ - // p.AddDevice(deviceId); - //}); - } - - /// - /// This API is not available in - public override NanoDeviceBase AddAndReturnDevice(string deviceId) - { - // None of the Port*Manager has a check whether deviceId matches the ID handled by the manager. - throw new NotImplementedException(); - //_ports.ForEach(p => - //{ - // p.AddDevice(deviceId); - //}); } public override void StartDeviceWatchers() diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs index 7a521fa..6b7dfcb 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs @@ -56,15 +56,9 @@ public string PersistName /// Adds a new device to list of NanoFrameworkDevices. /// /// The unique ID (based on the connection properties) of the device. - public abstract void AddDevice(string deviceId); - - /// - /// Adds a new device to list of NanoFrameworkDevices and returns the device that has been added. - /// - /// The unique ID (based on the connection properties) of the device. /// The device with the unique ID that is added or (if it was already discovered before) retrieved /// from the list of devices. Returns null if no device has been added. - public abstract NanoDeviceBase AddAndReturnDevice(string deviceId); + public abstract NanoDeviceBase AddDevice(string deviceId); /// /// Starts the device watchers. diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs index 81e2713..9d31090 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs @@ -159,7 +159,8 @@ private void StartWatcher(ICollection portsToExclude) Task.Run(async () => { - await Task.Yield(); // Force true async running + // Force true async running + await Task.Yield(); GlobalExclusiveDeviceAccess.CommunicateWithDevice( port, () => Added.Invoke(this, port) @@ -169,8 +170,9 @@ private void StartWatcher(ICollection portsToExclude) if (--newPortsDetected == 0) { AllNewDevicesAdded?.Invoke(this); - }; + } } + }); } } diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs index 8562bfd..6c2e195 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs @@ -185,22 +185,13 @@ public static NanoDeviceBase GetRegisteredDevice(string portName) return null; } - /// - /// Adds a new device to list of NanoFrameworkDevices. - /// - /// The serial port name where the device is connected. - public override void AddDevice(string deviceId) - { - AddDeviceToListAsync(deviceId); - } - /// /// Adds a new device to list of NanoFrameworkDevices. /// /// The serial port name where the device is connected. /// The device with the unique ID that is added or (if it was already discovered before) retrieved /// from the list of devices. Returns null if no device has been added. - public override NanoDeviceBase AddAndReturnDevice(string deviceId) + public override NanoDeviceBase AddDevice(string deviceId) { return AddDeviceToListAsync(deviceId); } @@ -354,6 +345,7 @@ private void RemoveNanoFrameworkDevices(NanoDevice device) { return; } + lock (NanoFrameworkDevices) { NanoFrameworkDevices.Remove(device); @@ -441,6 +433,7 @@ private void ProcessDeviceEnumerationComplete(object sender) count = NanoFrameworkDevices.OfType>().Count(); } + OnLogMessageAvailable(NanoDevicesEventSource.Log.SerialDeviceEnumerationCompleted(count)); // fire event that Serial enumeration is complete diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs index 2f6dd90..4d735ba 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs @@ -142,6 +142,7 @@ private void StopDeviceWatchersInternal() // this is not a TcpIp-connected device and is managed by another PortManager continue; } + // ... and remove it from collection _networkDevices.Remove(deviceEntry); @@ -150,6 +151,7 @@ private void StopDeviceWatchersInternal() { continue; } + // ... and remove it from collection NanoFrameworkDevices.Remove(device); } @@ -168,7 +170,7 @@ private void StopDeviceWatchersInternal() /// Get the device that communicates via the network port, provided it has been added to the /// list of known devices. /// - /// + /// The name of the network device. /// public static NanoDeviceBase GetRegisteredDevice(NetworkDeviceInformation networkDevice) { @@ -542,13 +544,7 @@ internal void OnLogMessageAvailable(string message) } /// - public override void AddDevice(string deviceId) - { - AddAndReturnDevice(deviceId); - } - - /// - public override NanoDeviceBase AddAndReturnDevice(string deviceId) + public override NanoDeviceBase AddDevice(string deviceId) { // expected format is "tcpip://{Host}:{Port}" From bc708a7ba89f960bd25493c51d8112a4badb5386 Mon Sep 17 00:00:00 2001 From: Frank Robijn Date: Wed, 25 Sep 2024 14:24:15 +0200 Subject: [PATCH 10/12] Merged changes (auto-merge failed because of a wrong codepage???) --- .../PortDefinitions/PortBase.cs | 2 +- .../PortSerial/PortSerialManager.cs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs index 6b7dfcb..4e480ae 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortDefinitions/PortBase.cs @@ -57,7 +57,7 @@ public string PersistName /// /// The unique ID (based on the connection properties) of the device. /// The device with the unique ID that is added or (if it was already discovered before) retrieved - /// from the list of devices. Returns null if no device has been added. + /// from the list of devices. Returns if no device has been added. public abstract NanoDeviceBase AddDevice(string deviceId); /// diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs index 6c2e195..213f9a4 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs @@ -170,8 +170,8 @@ public static List GetPortNames() /// Get the device that communicates via the serial port, provided it has been added to the /// list of known devices. /// - /// - /// + /// The port name of the device to get. + /// The that communicates via the serial port, or if the device is not found. public static NanoDeviceBase GetRegisteredDevice(string portName) { if (!string.IsNullOrWhiteSpace(portName)) @@ -190,14 +190,14 @@ public static NanoDeviceBase GetRegisteredDevice(string portName) /// /// The serial port name where the device is connected. /// The device with the unique ID that is added or (if it was already discovered before) retrieved - /// from the list of devices. Returns null if no device has been added. + /// from the list of devices. Returns if no device has been added. public override NanoDeviceBase AddDevice(string deviceId) { return AddDeviceToListAsync(deviceId); } /// - /// Creates a and adds it to the list of devices. + /// Creates a and adds it to the list of devices. /// /// The AQS used to find this device private NanoDeviceBase AddDeviceToListAsync(string deviceId) @@ -291,9 +291,9 @@ private NanoDeviceBase AddDeviceToListAsync(string deviceId) } /// - /// add device to the collection (if new) + /// Adds a device to the collection (if new). /// - /// new NanoSerialDevice + /// The new private void NanoFrameworkDeviceAdd(NanoDevice newNanoFrameworkDevice) { lock (NanoFrameworkDevices) From 1df5b19bd9ced8dfe608f253fef0316fe38dc01a Mon Sep 17 00:00:00 2001 From: Frank Robijn Date: Thu, 26 Sep 2024 14:17:04 +0200 Subject: [PATCH 11/12] GetPortNames made static --- .../PortSerial/DeviceWatcher.cs | 13 ++----------- .../PortSerial/PortSerialManager.cs | 9 --------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs index 9d31090..69d7d92 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/DeviceWatcher.cs @@ -116,7 +116,7 @@ private void StartWatcher(ICollection portsToExclude) var ports = new List(); lock (portsToExclude) { - ports.AddRange(from p in DoGetPortNames() + ports.AddRange(from p in GetPortNames() where !portsToExclude.Contains(p) select p); } @@ -196,21 +196,12 @@ private void StartWatcher(ICollection portsToExclude) Status = DeviceWatcherStatus.Stopped; } - /// - /// Gets the list of serial ports. - /// - public List GetPortNames() - { - return DoGetPortNames(); - } - /// /// Gets the list of serial ports. /// /// The list of serial ports that may be connected to a nanoDevice. - internal static List DoGetPortNames() + public static List GetPortNames() { - return RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? GetPortNames_Linux() : RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? GetPortNames_OSX() : RuntimeInformation.IsOSPlatform(OSPlatform.Create("FREEBSD")) ? GetPortNames_FreeBSD() diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs index 213f9a4..fe10bd4 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs @@ -157,15 +157,6 @@ private void NanoFrameworkDevicesRemoveAllSerial() #endregion #region Methods to manage device list add, remove, etc - /// - /// Gets the list of serial ports. - /// - /// The list of serial ports that may be connected to a nanoDevice. - public static List GetPortNames() - { - return DeviceWatcher.DoGetPortNames(); - } - /// /// Get the device that communicates via the serial port, provided it has been added to the /// list of known devices. From dc2fe1614ccd5f3ee9c7a3494eda817af395d402 Mon Sep 17 00:00:00 2001 From: josesimoes Date: Fri, 27 Sep 2024 03:35:45 +0100 Subject: [PATCH 12/12] Port in now closed on device removal --- .../PortSerial/PortSerialManager.cs | 9 +++++++-- .../PortTcpIp/PortTcpIpManager.cs | 4 ++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs index fe10bd4..f511057 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortSerial/PortSerialManager.cs @@ -341,8 +341,13 @@ private void RemoveNanoFrameworkDevices(NanoDevice device) { NanoFrameworkDevices.Remove(device); } - device?.DebugEngine?.StopProcessing(); - device?.DebugEngine?.Dispose(); + + // get rid of debug engine, if that was created + device.DebugEngine?.StopProcessing(); + device.DebugEngine?.Dispose(); + + // disconnect device in order to free port + device.Disconnect(true); } private NanoDeviceBase FindNanoFrameworkDevice(string deviceId) diff --git a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs index 4d735ba..0402e72 100644 --- a/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs +++ b/nanoFramework.Tools.DebugLibrary.Shared/PortTcpIp/PortTcpIpManager.cs @@ -284,8 +284,12 @@ private void RemoveDeviceFromList(NetworkDeviceInformation networkDevice) } } + // get rid of debug engine, if that was created device?.DebugEngine?.StopProcessing(); device?.DebugEngine?.Dispose(); + + // disconnect device + device?.Disconnect(true); } ///