From d027b6939a08e91117abb8411b20a98aeb09f828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20M=C3=BCller?= Date: Tue, 28 May 2024 21:04:09 +0200 Subject: [PATCH] Fixing an issue with missing internal client on TCP (caused by AddressFamily.Unknown in default constructor) --- .../Utils/AsyncQueue.cs | 4 +- AMWD.Protocols.Modbus.Proxy/ModbusRtuProxy.cs | 18 ++--- AMWD.Protocols.Modbus.Proxy/ModbusTcpProxy.cs | 34 +++++----- .../ModbusSerialConnection.cs | 14 ++-- .../Utils/SerialPortWrapper.cs | 6 +- .../Extensions/StreamExtensions.cs | 2 +- .../ModbusTcpConnection.cs | 67 +++++++++++++------ AMWD.Protocols.Modbus.Tcp/ModbusTcpServer.cs | 14 ++-- .../Utils/TcpClientWrapper.cs | 4 +- .../Utils/TcpClientWrapperFactory.cs | 26 +++++++ .../Tcp/ModbusTcpConnectionTest.cs | 51 ++++++++------ AMWD.Protocols.Modbus.sln | 2 +- CHANGELOG.md | 4 ++ 13 files changed, 156 insertions(+), 90 deletions(-) create mode 100644 AMWD.Protocols.Modbus.Tcp/Utils/TcpClientWrapperFactory.cs diff --git a/AMWD.Protocols.Modbus.Common/Utils/AsyncQueue.cs b/AMWD.Protocols.Modbus.Common/Utils/AsyncQueue.cs index 836e028..6bf73d2 100644 --- a/AMWD.Protocols.Modbus.Common/Utils/AsyncQueue.cs +++ b/AMWD.Protocols.Modbus.Common/Utils/AsyncQueue.cs @@ -45,7 +45,7 @@ namespace System.Collections.Generic internalDequeueTcs = ResetToken(ref _dequeueTcs); } - await WaitAsync(internalDequeueTcs, cancellationToken).ConfigureAwait(false); + await WaitAsync(internalDequeueTcs, cancellationToken); } } @@ -113,7 +113,7 @@ namespace System.Collections.Generic { if (await Task.WhenAny(tcs.Task, Task.Delay(-1, cancellationToken)) == tcs.Task) { - await tcs.Task.ConfigureAwait(false); + await tcs.Task; return; } diff --git a/AMWD.Protocols.Modbus.Proxy/ModbusRtuProxy.cs b/AMWD.Protocols.Modbus.Proxy/ModbusRtuProxy.cs index 12e8cf9..e6dee55 100644 --- a/AMWD.Protocols.Modbus.Proxy/ModbusRtuProxy.cs +++ b/AMWD.Protocols.Modbus.Proxy/ModbusRtuProxy.cs @@ -309,7 +309,7 @@ namespace AMWD.Protocols.Modbus.Proxy responseBytes.AddRange(requestBytes.Take(2)); try { - var coils = await Client.ReadCoilsAsync(unitId, firstAddress, count, cancellationToken).ConfigureAwait(false); + var coils = await Client.ReadCoilsAsync(unitId, firstAddress, count, cancellationToken); byte[] values = new byte[(int)Math.Ceiling(coils.Count / 8.0)]; for (int i = 0; i < coils.Count; i++) @@ -349,7 +349,7 @@ namespace AMWD.Protocols.Modbus.Proxy responseBytes.AddRange(requestBytes.Take(2)); try { - var discreteInputs = await Client.ReadDiscreteInputsAsync(unitId, firstAddress, count, cancellationToken).ConfigureAwait(false); + var discreteInputs = await Client.ReadDiscreteInputsAsync(unitId, firstAddress, count, cancellationToken); byte[] values = new byte[(int)Math.Ceiling(discreteInputs.Count / 8.0)]; for (int i = 0; i < discreteInputs.Count; i++) @@ -389,7 +389,7 @@ namespace AMWD.Protocols.Modbus.Proxy responseBytes.AddRange(requestBytes.Take(2)); try { - var holdingRegisters = await Client.ReadHoldingRegistersAsync(unitId, firstAddress, count, cancellationToken).ConfigureAwait(false); + var holdingRegisters = await Client.ReadHoldingRegistersAsync(unitId, firstAddress, count, cancellationToken); byte[] values = new byte[holdingRegisters.Count * 2]; for (int i = 0; i < holdingRegisters.Count; i++) @@ -424,7 +424,7 @@ namespace AMWD.Protocols.Modbus.Proxy responseBytes.AddRange(requestBytes.Take(2)); try { - var inputRegisters = await Client.ReadInputRegistersAsync(unitId, firstAddress, count, cancellationToken).ConfigureAwait(false); + var inputRegisters = await Client.ReadInputRegistersAsync(unitId, firstAddress, count, cancellationToken); byte[] values = new byte[count * 2]; for (int i = 0; i < count; i++) @@ -474,7 +474,7 @@ namespace AMWD.Protocols.Modbus.Proxy LowByte = requestBytes[5], }; - bool isSuccess = await Client.WriteSingleCoilAsync(requestBytes[0], coil, cancellationToken).ConfigureAwait(false); + bool isSuccess = await Client.WriteSingleCoilAsync(requestBytes[0], coil, cancellationToken); if (isSuccess) { // Response is an echo of the request @@ -514,7 +514,7 @@ namespace AMWD.Protocols.Modbus.Proxy LowByte = requestBytes[5] }; - bool isSuccess = await Client.WriteSingleHoldingRegisterAsync(requestBytes[0], register, cancellationToken).ConfigureAwait(false); + bool isSuccess = await Client.WriteSingleHoldingRegisterAsync(requestBytes[0], register, cancellationToken); if (isSuccess) { // Response is an echo of the request @@ -576,7 +576,7 @@ namespace AMWD.Protocols.Modbus.Proxy }); } - bool isSuccess = await Client.WriteMultipleCoilsAsync(requestBytes[0], coils, cancellationToken).ConfigureAwait(false); + bool isSuccess = await Client.WriteMultipleCoilsAsync(requestBytes[0], coils, cancellationToken); if (isSuccess) { // Response is an echo of the request @@ -634,7 +634,7 @@ namespace AMWD.Protocols.Modbus.Proxy LowByte = requestBytes[baseOffset + i * 2 + 1] }); - bool isSuccess = await Client.WriteMultipleHoldingRegistersAsync(requestBytes[0], list, cancellationToken).ConfigureAwait(false); + bool isSuccess = await Client.WriteMultipleHoldingRegistersAsync(requestBytes[0], list, cancellationToken); if (isSuccess) { // Response is an echo of the request @@ -693,7 +693,7 @@ namespace AMWD.Protocols.Modbus.Proxy try { - var res = await Client.ReadDeviceIdentificationAsync(requestBytes[6], category, firstObject, cancellationToken).ConfigureAwait(false); + var res = await Client.ReadDeviceIdentificationAsync(requestBytes[6], category, firstObject, cancellationToken); var bodyBytes = new List(); diff --git a/AMWD.Protocols.Modbus.Proxy/ModbusTcpProxy.cs b/AMWD.Protocols.Modbus.Proxy/ModbusTcpProxy.cs index 2b50c4e..d258960 100644 --- a/AMWD.Protocols.Modbus.Proxy/ModbusTcpProxy.cs +++ b/AMWD.Protocols.Modbus.Proxy/ModbusTcpProxy.cs @@ -203,11 +203,11 @@ namespace AMWD.Protocols.Modbus.Proxy try { #if NET8_0_OR_GREATER - var client = await _listener.AcceptTcpClientAsync(cancellationToken).ConfigureAwait(false); + var client = await _listener.AcceptTcpClientAsync(cancellationToken); #else - var client = await _listener.AcceptTcpClientAsync().ConfigureAwait(false); + var client = await _listener.AcceptTcpClientAsync(); #endif - await _clientListLock.WaitAsync(cancellationToken).ConfigureAwait(false); + await _clientListLock.WaitAsync(cancellationToken); try { _clients.Add(client); @@ -237,20 +237,20 @@ namespace AMWD.Protocols.Modbus.Proxy using (var cts = new CancellationTokenSource(ReadWriteTimeout)) using (cancellationToken.Register(cts.Cancel)) { - byte[] headerBytes = await stream.ReadExpectedBytesAsync(6, cts.Token).ConfigureAwait(false); + byte[] headerBytes = await stream.ReadExpectedBytesAsync(6, cts.Token); requestBytes.AddRange(headerBytes); byte[] followingCountBytes = headerBytes.Skip(4).Take(2).ToArray(); followingCountBytes.SwapBigEndian(); int followingCount = BitConverter.ToUInt16(followingCountBytes, 0); - byte[] bodyBytes = await stream.ReadExpectedBytesAsync(followingCount, cts.Token).ConfigureAwait(false); + byte[] bodyBytes = await stream.ReadExpectedBytesAsync(followingCount, cts.Token); requestBytes.AddRange(bodyBytes); } - byte[] responseBytes = await HandleRequestAsync([.. requestBytes], cancellationToken).ConfigureAwait(false); + byte[] responseBytes = await HandleRequestAsync([.. requestBytes], cancellationToken); if (responseBytes != null) - await stream.WriteAsync(responseBytes, 0, responseBytes.Length, cancellationToken).ConfigureAwait(false); + await stream.WriteAsync(responseBytes, 0, responseBytes.Length, cancellationToken); } } catch @@ -259,7 +259,7 @@ namespace AMWD.Protocols.Modbus.Proxy } finally { - await _clientListLock.WaitAsync(cancellationToken).ConfigureAwait(false); + await _clientListLock.WaitAsync(cancellationToken); try { _clients.Remove(client); @@ -334,7 +334,7 @@ namespace AMWD.Protocols.Modbus.Proxy responseBytes.AddRange(requestBytes.Take(8)); try { - var coils = await Client.ReadCoilsAsync(unitId, firstAddress, count, cancellationToken).ConfigureAwait(false); + var coils = await Client.ReadCoilsAsync(unitId, firstAddress, count, cancellationToken); byte[] values = new byte[(int)Math.Ceiling(coils.Count / 8.0)]; for (int i = 0; i < coils.Count; i++) @@ -373,7 +373,7 @@ namespace AMWD.Protocols.Modbus.Proxy responseBytes.AddRange(requestBytes.Take(8)); try { - var discreteInputs = await Client.ReadDiscreteInputsAsync(unitId, firstAddress, count, cancellationToken).ConfigureAwait(false); + var discreteInputs = await Client.ReadDiscreteInputsAsync(unitId, firstAddress, count, cancellationToken); byte[] values = new byte[(int)Math.Ceiling(discreteInputs.Count / 8.0)]; for (int i = 0; i < discreteInputs.Count; i++) @@ -412,7 +412,7 @@ namespace AMWD.Protocols.Modbus.Proxy responseBytes.AddRange(requestBytes.Take(8)); try { - var holdingRegisters = await Client.ReadHoldingRegistersAsync(unitId, firstAddress, count, cancellationToken).ConfigureAwait(false); + var holdingRegisters = await Client.ReadHoldingRegistersAsync(unitId, firstAddress, count, cancellationToken); byte[] values = new byte[holdingRegisters.Count * 2]; for (int i = 0; i < holdingRegisters.Count; i++) @@ -446,7 +446,7 @@ namespace AMWD.Protocols.Modbus.Proxy responseBytes.AddRange(requestBytes.Take(8)); try { - var inputRegisters = await Client.ReadInputRegistersAsync(unitId, firstAddress, count, cancellationToken).ConfigureAwait(false); + var inputRegisters = await Client.ReadInputRegistersAsync(unitId, firstAddress, count, cancellationToken); byte[] values = new byte[count * 2]; for (int i = 0; i < count; i++) @@ -493,7 +493,7 @@ namespace AMWD.Protocols.Modbus.Proxy LowByte = requestBytes[11], }; - bool isSuccess = await Client.WriteSingleCoilAsync(requestBytes[6], coil, cancellationToken).ConfigureAwait(false); + bool isSuccess = await Client.WriteSingleCoilAsync(requestBytes[6], coil, cancellationToken); if (isSuccess) { // Response is an echo of the request @@ -533,7 +533,7 @@ namespace AMWD.Protocols.Modbus.Proxy LowByte = requestBytes[11] }; - bool isSuccess = await Client.WriteSingleHoldingRegisterAsync(requestBytes[6], register, cancellationToken).ConfigureAwait(false); + bool isSuccess = await Client.WriteSingleHoldingRegisterAsync(requestBytes[6], register, cancellationToken); if (isSuccess) { // Response is an echo of the request @@ -592,7 +592,7 @@ namespace AMWD.Protocols.Modbus.Proxy }); } - bool isSuccess = await Client.WriteMultipleCoilsAsync(requestBytes[6], coils, cancellationToken).ConfigureAwait(false); + bool isSuccess = await Client.WriteMultipleCoilsAsync(requestBytes[6], coils, cancellationToken); if (isSuccess) { // Response is an echo of the request @@ -647,7 +647,7 @@ namespace AMWD.Protocols.Modbus.Proxy LowByte = requestBytes[baseOffset + i * 2 + 1] }); - bool isSuccess = await Client.WriteMultipleHoldingRegistersAsync(requestBytes[6], list, cancellationToken).ConfigureAwait(false); + bool isSuccess = await Client.WriteMultipleHoldingRegistersAsync(requestBytes[6], list, cancellationToken); if (isSuccess) { // Response is an echo of the request @@ -699,7 +699,7 @@ namespace AMWD.Protocols.Modbus.Proxy try { - var res = await Client.ReadDeviceIdentificationAsync(requestBytes[6], category, firstObject, cancellationToken).ConfigureAwait(false); + var res = await Client.ReadDeviceIdentificationAsync(requestBytes[6], category, firstObject, cancellationToken); var bodyBytes = new List(); diff --git a/AMWD.Protocols.Modbus.Serial/ModbusSerialConnection.cs b/AMWD.Protocols.Modbus.Serial/ModbusSerialConnection.cs index 6c3e62c..8d54951 100644 --- a/AMWD.Protocols.Modbus.Serial/ModbusSerialConnection.cs +++ b/AMWD.Protocols.Modbus.Serial/ModbusSerialConnection.cs @@ -259,7 +259,7 @@ namespace AMWD.Protocols.Modbus.Serial try { // Get next request to process - var item = await _requestQueue.DequeueAsync(cancellationToken).ConfigureAwait(false); + var item = await _requestQueue.DequeueAsync(cancellationToken); // Remove registration => already removed from queue item.CancellationTokenRegistration.Dispose(); @@ -267,13 +267,13 @@ namespace AMWD.Protocols.Modbus.Serial // Build combined cancellation token using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, item.CancellationTokenSource.Token); // Wait for exclusive access - await _portLock.WaitAsync(linkedCts.Token).ConfigureAwait(false); + await _portLock.WaitAsync(linkedCts.Token); try { // Ensure connection is up - await AssertConnection(linkedCts.Token).ConfigureAwait(false); + await AssertConnection(linkedCts.Token); - await _serialPort.WriteAsync(item.Request, linkedCts.Token).ConfigureAwait(false); + await _serialPort.WriteAsync(item.Request, linkedCts.Token); linkedCts.Token.ThrowIfCancellationRequested(); @@ -282,7 +282,7 @@ namespace AMWD.Protocols.Modbus.Serial do { - int readCount = await _serialPort.ReadAsync(buffer, 0, buffer.Length, linkedCts.Token).ConfigureAwait(false); + int readCount = await _serialPort.ReadAsync(buffer, 0, buffer.Length, linkedCts.Token); if (readCount < 1) throw new EndOfStreamException(); @@ -313,7 +313,7 @@ namespace AMWD.Protocols.Modbus.Serial _portLock.Release(); _idleTimer.Change(IdleTimeout, Timeout.InfiniteTimeSpan); - await Task.Delay(InterRequestDelay, cancellationToken).ConfigureAwait(false); + await Task.Delay(InterRequestDelay, cancellationToken); } } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) @@ -370,7 +370,7 @@ namespace AMWD.Protocols.Modbus.Serial try { - await Task.Delay(TimeSpan.FromSeconds(delay), cancellationToken).ConfigureAwait(false); + await Task.Delay(TimeSpan.FromSeconds(delay), cancellationToken); } catch { /* keep it quiet */ } diff --git a/AMWD.Protocols.Modbus.Serial/Utils/SerialPortWrapper.cs b/AMWD.Protocols.Modbus.Serial/Utils/SerialPortWrapper.cs index b1157c8..a631043 100644 --- a/AMWD.Protocols.Modbus.Serial/Utils/SerialPortWrapper.cs +++ b/AMWD.Protocols.Modbus.Serial/Utils/SerialPortWrapper.cs @@ -145,7 +145,7 @@ namespace AMWD.Protocols.Modbus.Serial.Utils try { - return await _serialPort.BaseStream.ReadAsync(buffer, offset, count, cts.Token).ConfigureAwait(false); + return await _serialPort.BaseStream.ReadAsync(buffer, offset, count, cts.Token); } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { @@ -195,9 +195,9 @@ namespace AMWD.Protocols.Modbus.Serial.Utils try { #if NET6_0_OR_GREATER - await _serialPort.BaseStream.WriteAsync(buffer, cts.Token).ConfigureAwait(false); + await _serialPort.BaseStream.WriteAsync(buffer, cts.Token); #else - await _serialPort.BaseStream.WriteAsync(buffer, 0, buffer.Length, cts.Token).ConfigureAwait(false); + await _serialPort.BaseStream.WriteAsync(buffer, 0, buffer.Length, cts.Token); #endif } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) diff --git a/AMWD.Protocols.Modbus.Tcp/Extensions/StreamExtensions.cs b/AMWD.Protocols.Modbus.Tcp/Extensions/StreamExtensions.cs index 545ce63..ad78b3c 100644 --- a/AMWD.Protocols.Modbus.Tcp/Extensions/StreamExtensions.cs +++ b/AMWD.Protocols.Modbus.Tcp/Extensions/StreamExtensions.cs @@ -11,7 +11,7 @@ namespace System.IO int offset = 0; do { - int count = await stream.ReadAsync(buffer, offset, expectedBytes - offset, cancellationToken).ConfigureAwait(false); + int count = await stream.ReadAsync(buffer, offset, expectedBytes - offset, cancellationToken); if (count < 1) throw new EndOfStreamException(); diff --git a/AMWD.Protocols.Modbus.Tcp/ModbusTcpConnection.cs b/AMWD.Protocols.Modbus.Tcp/ModbusTcpConnection.cs index a24ba6c..41a0ada 100644 --- a/AMWD.Protocols.Modbus.Tcp/ModbusTcpConnection.cs +++ b/AMWD.Protocols.Modbus.Tcp/ModbusTcpConnection.cs @@ -26,13 +26,17 @@ namespace AMWD.Protocols.Modbus.Tcp private bool _isDisposed; private readonly CancellationTokenSource _disposeCts = new(); + private readonly TcpClientWrapperFactory _tcpClientFactory = new(); private readonly SemaphoreSlim _clientLock = new(1, 1); - private readonly TcpClientWrapper _tcpClient = new(); + private TcpClientWrapper _tcpClient = null; private readonly Timer _idleTimer; private readonly Task _processingTask; private readonly AsyncQueue _requestQueue = new(); + private TimeSpan _readTimeout = TimeSpan.FromMilliseconds(1); + private TimeSpan _writeTimeout = TimeSpan.FromMilliseconds(1); + #endregion Fields /// @@ -58,15 +62,33 @@ namespace AMWD.Protocols.Modbus.Tcp /// public virtual TimeSpan ReadTimeout { - get => TimeSpan.FromMilliseconds(_tcpClient.ReceiveTimeout); - set => _tcpClient.ReceiveTimeout = (int)value.TotalMilliseconds; + get => _readTimeout; + set + { + if (value < TimeSpan.Zero) + throw new ArgumentOutOfRangeException(nameof(value)); + + _readTimeout = value; + + if (_tcpClient != null) + _tcpClient.ReceiveTimeout = (int)value.TotalMilliseconds; + } } /// public virtual TimeSpan WriteTimeout { - get => TimeSpan.FromMilliseconds(_tcpClient.SendTimeout); - set => _tcpClient.SendTimeout = (int)value.TotalMilliseconds; + get => _writeTimeout; + set + { + if (value < TimeSpan.Zero) + throw new ArgumentOutOfRangeException(nameof(value)); + + _writeTimeout = value; + + if (_tcpClient != null) + _tcpClient.SendTimeout = (int)value.TotalMilliseconds; + } } /// @@ -116,7 +138,6 @@ namespace AMWD.Protocols.Modbus.Tcp try { - _processingTask.Wait(); _processingTask.Dispose(); } catch @@ -124,7 +145,7 @@ namespace AMWD.Protocols.Modbus.Tcp OnIdleTimer(null); - _tcpClient.Dispose(); + _tcpClient?.Dispose(); _clientLock.Dispose(); while (_requestQueue.TryDequeue(out var item)) @@ -164,7 +185,7 @@ namespace AMWD.Protocols.Modbus.Tcp { Request = [.. request], ValidateResponseComplete = validateResponseComplete, - TaskCompletionSource = new(), + TaskCompletionSource = new TaskCompletionSource>(), CancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken) }; @@ -187,7 +208,7 @@ namespace AMWD.Protocols.Modbus.Tcp try { // Get next request to process - var item = await _requestQueue.DequeueAsync(cancellationToken).ConfigureAwait(false); + var item = await _requestQueue.DequeueAsync(cancellationToken); // Remove registration => already removed from queue item.CancellationTokenRegistration.Dispose(); @@ -195,19 +216,19 @@ namespace AMWD.Protocols.Modbus.Tcp // Build combined cancellation token using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, item.CancellationTokenSource.Token); // Wait for exclusive access - await _clientLock.WaitAsync(linkedCts.Token).ConfigureAwait(false); + await _clientLock.WaitAsync(linkedCts.Token); try { // Ensure connection is up - await AssertConnection(linkedCts.Token).ConfigureAwait(false); + await AssertConnection(linkedCts.Token); var stream = _tcpClient.GetStream(); - await stream.FlushAsync(linkedCts.Token).ConfigureAwait(false); + await stream.FlushAsync(linkedCts.Token); #if NET6_0_OR_GREATER - await stream.WriteAsync(item.Request, linkedCts.Token).ConfigureAwait(false); + await stream.WriteAsync(item.Request, linkedCts.Token); #else - await stream.WriteAsync(item.Request, 0, item.Request.Length, linkedCts.Token).ConfigureAwait(false); + await stream.WriteAsync(item.Request, 0, item.Request.Length, linkedCts.Token); #endif linkedCts.Token.ThrowIfCancellationRequested(); @@ -218,9 +239,9 @@ namespace AMWD.Protocols.Modbus.Tcp do { #if NET6_0_OR_GREATER - int readCount = await stream.ReadAsync(buffer, linkedCts.Token).ConfigureAwait(false); + int readCount = await stream.ReadAsync(buffer, linkedCts.Token); #else - int readCount = await stream.ReadAsync(buffer, 0, buffer.Length, linkedCts.Token).ConfigureAwait(false); + int readCount = await stream.ReadAsync(buffer, 0, buffer.Length, linkedCts.Token); #endif if (readCount < 1) throw new EndOfStreamException(); @@ -267,7 +288,7 @@ namespace AMWD.Protocols.Modbus.Tcp // Has to be called within _clientLock! private async Task AssertConnection(CancellationToken cancellationToken) { - if (_tcpClient.Connected) + if (_tcpClient?.Connected == true) return; int delay = 1; @@ -284,14 +305,16 @@ namespace AMWD.Protocols.Modbus.Tcp { foreach (var ipAddress in ipAddresses) { - _tcpClient.Close(); + _tcpClient?.Close(); + _tcpClient?.Dispose(); + _tcpClient = _tcpClientFactory.Create(ipAddress.AddressFamily, _readTimeout, _writeTimeout); #if NET6_0_OR_GREATER - using var connectTask = _tcpClient.ConnectAsync(ipAddress, Port, cancellationToken); + var connectTask = _tcpClient.ConnectAsync(ipAddress, Port, cancellationToken); #else - using var connectTask = _tcpClient.ConnectAsync(ipAddress, Port); + var connectTask = _tcpClient.ConnectAsync(ipAddress, Port); #endif - if (await Task.WhenAny(connectTask, Task.Delay(ReadTimeout, cancellationToken)) == connectTask) + if (await Task.WhenAny(connectTask, Task.Delay(_readTimeout, cancellationToken)) == connectTask) { await connectTask; if (_tcpClient.Connected) @@ -309,7 +332,7 @@ namespace AMWD.Protocols.Modbus.Tcp try { - await Task.Delay(TimeSpan.FromSeconds(delay), cancellationToken).ConfigureAwait(false); + await Task.Delay(TimeSpan.FromSeconds(delay), cancellationToken); } catch { /* keep it quiet */ } diff --git a/AMWD.Protocols.Modbus.Tcp/ModbusTcpServer.cs b/AMWD.Protocols.Modbus.Tcp/ModbusTcpServer.cs index 134412a..2e285e0 100644 --- a/AMWD.Protocols.Modbus.Tcp/ModbusTcpServer.cs +++ b/AMWD.Protocols.Modbus.Tcp/ModbusTcpServer.cs @@ -218,11 +218,11 @@ namespace AMWD.Protocols.Modbus.Tcp try { #if NET8_0_OR_GREATER - var client = await _listener.AcceptTcpClientAsync(cancellationToken).ConfigureAwait(false); + var client = await _listener.AcceptTcpClientAsync(cancellationToken); #else - var client = await _listener.AcceptTcpClientAsync().ConfigureAwait(false); + var client = await _listener.AcceptTcpClientAsync(); #endif - await _clientListLock.WaitAsync(cancellationToken).ConfigureAwait(false); + await _clientListLock.WaitAsync(cancellationToken); try { _clients.Add(client); @@ -252,20 +252,20 @@ namespace AMWD.Protocols.Modbus.Tcp using (var cts = new CancellationTokenSource(ReadWriteTimeout)) using (cancellationToken.Register(cts.Cancel)) { - byte[] headerBytes = await stream.ReadExpectedBytesAsync(6, cts.Token).ConfigureAwait(false); + byte[] headerBytes = await stream.ReadExpectedBytesAsync(6, cts.Token); requestBytes.AddRange(headerBytes); byte[] followingCountBytes = headerBytes.Skip(4).Take(2).ToArray(); followingCountBytes.SwapBigEndian(); int followingCount = BitConverter.ToUInt16(followingCountBytes, 0); - byte[] bodyBytes = await stream.ReadExpectedBytesAsync(followingCount, cts.Token).ConfigureAwait(false); + byte[] bodyBytes = await stream.ReadExpectedBytesAsync(followingCount, cts.Token); requestBytes.AddRange(bodyBytes); } byte[] responseBytes = HandleRequest([.. requestBytes]); if (responseBytes != null) - await stream.WriteAsync(responseBytes, 0, responseBytes.Length, cancellationToken).ConfigureAwait(false); + await stream.WriteAsync(responseBytes, 0, responseBytes.Length, cancellationToken); } } catch @@ -274,7 +274,7 @@ namespace AMWD.Protocols.Modbus.Tcp } finally { - await _clientListLock.WaitAsync(cancellationToken).ConfigureAwait(false); + await _clientListLock.WaitAsync(cancellationToken); try { _clients.Remove(client); diff --git a/AMWD.Protocols.Modbus.Tcp/Utils/TcpClientWrapper.cs b/AMWD.Protocols.Modbus.Tcp/Utils/TcpClientWrapper.cs index 674fa35..9bfa3a0 100644 --- a/AMWD.Protocols.Modbus.Tcp/Utils/TcpClientWrapper.cs +++ b/AMWD.Protocols.Modbus.Tcp/Utils/TcpClientWrapper.cs @@ -8,11 +8,11 @@ namespace AMWD.Protocols.Modbus.Tcp.Utils { /// [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] - internal class TcpClientWrapper : IDisposable + internal class TcpClientWrapper(AddressFamily addressFamily) : IDisposable { #region Fields - private readonly TcpClient _client = new(); + private readonly TcpClient _client = new(addressFamily); #endregion Fields diff --git a/AMWD.Protocols.Modbus.Tcp/Utils/TcpClientWrapperFactory.cs b/AMWD.Protocols.Modbus.Tcp/Utils/TcpClientWrapperFactory.cs new file mode 100644 index 0000000..2e396dd --- /dev/null +++ b/AMWD.Protocols.Modbus.Tcp/Utils/TcpClientWrapperFactory.cs @@ -0,0 +1,26 @@ +using System; +using System.Net.Sockets; + +namespace AMWD.Protocols.Modbus.Tcp.Utils +{ + [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] + internal class TcpClientWrapperFactory + { + /// + /// Creates a new instance of . + /// + /// The of the to use. + /// The read timeout. + /// The write timeout. + /// A new instance. + public virtual TcpClientWrapper Create(AddressFamily addressFamily, TimeSpan readTimeout, TimeSpan writeTimeout) + { + var client = new TcpClientWrapper(addressFamily) + { + ReceiveTimeout = (int)readTimeout.TotalMilliseconds, + SendTimeout = (int)writeTimeout.TotalMilliseconds + }; + return client; + } + } +} diff --git a/AMWD.Protocols.Modbus.Tests/Tcp/ModbusTcpConnectionTest.cs b/AMWD.Protocols.Modbus.Tests/Tcp/ModbusTcpConnectionTest.cs index 4e2ca66..8b987b2 100644 --- a/AMWD.Protocols.Modbus.Tests/Tcp/ModbusTcpConnectionTest.cs +++ b/AMWD.Protocols.Modbus.Tests/Tcp/ModbusTcpConnectionTest.cs @@ -1,6 +1,7 @@ using System.Collections.Generic; using System.IO; using System.Net; +using System.Net.Sockets; using System.Reflection; using System.Threading; using System.Threading.Tasks; @@ -16,6 +17,7 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp private readonly string _hostname = "127.0.0.1"; private Mock _tcpClientMock; + private Mock _tcpClientFactoryMock; private Mock _networkStreamMock; private bool _alwaysConnected; @@ -40,10 +42,19 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp } [TestMethod] - public void ShouldGetAndSetPropertiesOfBaseClient() + public async Task ShouldSetPropertiesOfBaseClient() { // Arrange + byte[] request = [1, 2, 3]; + byte[] expectedResponse = [9, 8, 7]; + var validation = new Func, bool>(_ => true); + _networkResponseQueue.Enqueue(expectedResponse); + var connection = GetTcpConnection(); + await connection.InvokeAsync(request, validation); + + _tcpClientMock.Invocations.Clear(); + _networkStreamMock.Invocations.Clear(); // Act connection.ReadTimeout = TimeSpan.FromSeconds(123); @@ -51,8 +62,8 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp // Assert - part 1 Assert.AreEqual("TCP", connection.Name); - Assert.AreEqual(1, connection.ReadTimeout.TotalSeconds); - Assert.AreEqual(1, connection.WriteTimeout.TotalSeconds); + Assert.AreEqual(123, connection.ReadTimeout.TotalSeconds); + Assert.AreEqual(456, connection.WriteTimeout.TotalSeconds); Assert.AreEqual(_hostname, connection.Hostname); Assert.AreEqual(502, connection.Port); @@ -61,9 +72,6 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp _tcpClientMock.VerifySet(c => c.ReceiveTimeout = 123000, Times.Once); _tcpClientMock.VerifySet(c => c.SendTimeout = 456000, Times.Once); - _tcpClientMock.VerifyGet(c => c.ReceiveTimeout, Times.Once); - _tcpClientMock.VerifyGet(c => c.SendTimeout, Times.Once); - _tcpClientMock.VerifyNoOtherCalls(); _networkStreamMock.VerifyNoOtherCalls(); } @@ -173,6 +181,7 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp CollectionAssert.AreEqual(expectedResponse, response.ToArray()); CollectionAssert.AreEqual(request, _networkRequestCallbacks.First()); + _tcpClientMock.Verify(c => c.ConnectAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); _tcpClientMock.Verify(c => c.Connected, Times.Once); _tcpClientMock.Verify(c => c.GetStream(), Times.Once); @@ -211,11 +220,10 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp CollectionAssert.AreEqual(expectedResponse, response.ToArray()); CollectionAssert.AreEqual(request, _networkRequestCallbacks.First()); - _tcpClientMock.VerifyGet(c => c.ReceiveTimeout, Times.Once); - _tcpClientMock.Verify(c => c.Connected, Times.Exactly(3)); _tcpClientMock.Verify(c => c.Close(), Times.Exactly(2)); - _tcpClientMock.Verify(c => c.ConnectAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + _tcpClientMock.Verify(c => c.Dispose(), Times.Once); + _tcpClientMock.Verify(c => c.ConnectAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); _tcpClientMock.Verify(c => c.GetStream(), Times.Once); _networkStreamMock.Verify(ns => ns.FlushAsync(It.IsAny()), Times.Once); @@ -289,11 +297,10 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp CollectionAssert.AreEqual(expectedResponse, response.ToArray()); CollectionAssert.AreEqual(request, _networkRequestCallbacks.First()); - _tcpClientMock.VerifyGet(c => c.ReceiveTimeout, Times.Once); - _tcpClientMock.Verify(c => c.Connected, Times.Exactly(3)); _tcpClientMock.Verify(c => c.Close(), Times.Once); - _tcpClientMock.Verify(c => c.ConnectAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + _tcpClientMock.Verify(c => c.Dispose(), Times.Once); + _tcpClientMock.Verify(c => c.ConnectAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); _tcpClientMock.Verify(c => c.GetStream(), Times.Once); _networkStreamMock.Verify(ns => ns.FlushAsync(It.IsAny()), Times.Once); @@ -329,11 +336,10 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp CollectionAssert.AreEqual(expectedResponse, response.ToArray()); CollectionAssert.AreEqual(request, _networkRequestCallbacks.First()); - _tcpClientMock.VerifyGet(c => c.ReceiveTimeout, Times.Exactly(2)); - _tcpClientMock.Verify(c => c.Connected, Times.Exactly(3)); _tcpClientMock.Verify(c => c.Close(), Times.Exactly(2)); - _tcpClientMock.Verify(c => c.ConnectAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); + _tcpClientMock.Verify(c => c.Dispose(), Times.Exactly(2)); + _tcpClientMock.Verify(c => c.ConnectAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(3)); _tcpClientMock.Verify(c => c.GetStream(), Times.Once); _networkStreamMock.Verify(ns => ns.FlushAsync(It.IsAny()), Times.Once); @@ -426,6 +432,7 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp CollectionAssert.AreEqual(expectedResponse, response.ToArray()); _tcpClientMock.Verify(c => c.Connected, Times.Once); + _tcpClientMock.Verify(c => c.ConnectAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); _tcpClientMock.Verify(c => c.GetStream(), Times.Once); _networkStreamMock.Verify(ns => ns.FlushAsync(It.IsAny()), Times.Once); @@ -475,6 +482,7 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp CollectionAssert.AreEqual(request, _networkRequestCallbacks.First()); _tcpClientMock.Verify(c => c.Connected, Times.Once); + _tcpClientMock.Verify(c => c.ConnectAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); _tcpClientMock.Verify(c => c.GetStream(), Times.Once); _tcpClientMock.Verify(c => c.Dispose(), Times.Once); @@ -508,7 +516,7 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp return ValueTask.FromResult(0); }); - _tcpClientMock = new Mock(); + _tcpClientMock = new Mock(AddressFamily.Unknown); _tcpClientMock.Setup(c => c.Connected).Returns(() => _alwaysConnected || _connectedQueue.Dequeue()); _tcpClientMock.Setup(c => c.ReceiveTimeout).Returns(() => _clientReceiveTimeout); _tcpClientMock.Setup(c => c.SendTimeout).Returns(() => _clientSendTimeout); @@ -521,6 +529,11 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp .Setup(c => c.GetStream()) .Returns(() => _networkStreamMock.Object); + _tcpClientFactoryMock = new Mock(); + _tcpClientFactoryMock + .Setup(c => c.Create(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(_tcpClientMock.Object); + var connection = new ModbusTcpConnection { Hostname = _hostname, @@ -528,9 +541,9 @@ namespace AMWD.Protocols.Modbus.Tests.Tcp }; // Replace real connection with mock - var connectionField = connection.GetType().GetField("_tcpClient", BindingFlags.NonPublic | BindingFlags.Instance); - (connectionField.GetValue(connection) as TcpClientWrapper)?.Dispose(); - connectionField.SetValue(connection, _tcpClientMock.Object); + var factoryField = connection.GetType().GetField("_tcpClientFactory", BindingFlags.NonPublic | BindingFlags.Instance); + (factoryField.GetValue(connection) as TcpClientWrapper)?.Dispose(); + factoryField.SetValue(connection, _tcpClientFactoryMock.Object); return connection; } diff --git a/AMWD.Protocols.Modbus.sln b/AMWD.Protocols.Modbus.sln index 728d0d8..f1cf8e3 100644 --- a/AMWD.Protocols.Modbus.sln +++ b/AMWD.Protocols.Modbus.sln @@ -35,7 +35,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "AMWD.Protocols.Modbus.Tcp", EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "AMWD.Protocols.Modbus.Serial", "AMWD.Protocols.Modbus.Serial\AMWD.Protocols.Modbus.Serial.csproj", "{D966826F-EE6C-4BC0-9185-C2A9A50FD586}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "AMWD.Protocols.Modbus.Proxy", "AMWD.Protocols.Modbus.Proxy\AMWD.Protocols.Modbus.Proxy.csproj", "{C30EBE45-E3B8-4997-95DF-8F94B31C8E1A}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "AMWD.Protocols.Modbus.Proxy", "AMWD.Protocols.Modbus.Proxy\AMWD.Protocols.Modbus.Proxy.csproj", "{C30EBE45-E3B8-4997-95DF-8F94B31C8E1A}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution diff --git a/CHANGELOG.md b/CHANGELOG.md index c42469b..9d74432 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Renamed `ModbusSerialServer` to `ModbusRtuServer` to clearify the protocol that is used. - Made `Protocol` property of `ModbusClientBase` non-abstract. +### Fixed + +- Issue with missing client on TCP connection when using default constructor (`AddressFamily.Unknown`) + ## [v0.2.0] (2024-04-02)