[BREAKING] Improve logging

Apollo3zehn · Jul 1, 2024 · 1e0f7ed · 1e0f7ed
1 parent 1d5e16b
commit 1e0f7ed
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 41 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -9,7 +9,7 @@
             "type": "coreclr",
             "request": "launch",
             "preLaunchTask": "build",
-            "program": "${workspaceFolder}/artifacts/bin/SampleServerClientTcp/SampleServerClientTcp.dll",
+            "program": "${workspaceFolder}/artifacts/bin/SampleServerClientTcp/debug/SampleServerClientTcp.dll",
             "args": [],
             "cwd": "${workspaceFolder}/sample/SampleServerClientTcp",
             "console": "externalTerminal",

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## vNext - xxxx-xx-xx
+
+### Breaking changes
+- The constructors of `ModbusTcpClient` and `ModbusRtuClient` have been unified so that both accept an `ILogger`.
+
 ## v5.2.0 - 2024-04-23
 
 ### Features

diff --git a/src/FluentModbus/Server/ModbusRtuRequestHandler.cs b/src/FluentModbus/Server/ModbusRtuRequestHandler.cs
@@ -1,3 +1,5 @@
+using Microsoft.Extensions.Logging;
+
 namespace FluentModbus
 {
     internal class ModbusRtuRequestHandler : ModbusRequestHandler, IDisposable
@@ -6,12 +8,15 @@ internal class ModbusRtuRequestHandler : ModbusRequestHandler, IDisposable
 
         private IModbusRtuSerialPort _serialPort;
 
+        private readonly ILogger _logger;
+
         #endregion
 
         #region Constructors
 
-        public ModbusRtuRequestHandler(IModbusRtuSerialPort serialPort, ModbusRtuServer rtuServer) : base(rtuServer, 256)
+        public ModbusRtuRequestHandler(IModbusRtuSerialPort serialPort, ModbusRtuServer rtuServer, ILogger logger) : base(rtuServer, 256)
         {
+            _logger = logger;
             _serialPort = serialPort;
             _serialPort.Open();
 
@@ -47,8 +52,10 @@ internal override async Task ReceiveRequestAsync()
                         WriteResponse();
                 }
             }
-            catch
+            catch (Exception ex)
             {
+                _logger.LogDebug(ex, "The connection will be closed");
+
                 CancelToken();
             }
         }

diff --git a/src/FluentModbus/Server/ModbusRtuServer.cs b/src/FluentModbus/Server/ModbusRtuServer.cs
@@ -1,4 +1,6 @@
 using System.IO.Ports;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
 
 namespace FluentModbus
 {
@@ -18,37 +20,43 @@ public class ModbusRtuServer : ModbusServer
         /// <summary>
         /// Creates a Modbus RTU server with support for holding registers (read and write, 16 bit), input registers (read-only, 16 bit), coils (read and write, 1 bit) and discete inputs (read-only, 1 bit).
         /// </summary>
+        /// <param name="isAsynchronous">Enables or disables the asynchronous operation, where each client request is processed immediately using a locking mechanism. Use synchronuous operation to avoid locks in the hosting application. See the <see href="https://github.com/Apollo3zehn/FluentModbus">documentation</see> for more details.</param>
         /// <param name="unitIdentifier">The unique Modbus RTU unit identifier (1..247).</param>
-        public ModbusRtuServer(byte unitIdentifier) : this(unitIdentifier, true)
+        public ModbusRtuServer(byte unitIdentifier, bool isAsynchronous = true) 
+            : this(logger: NullLogger.Instance, unitIdentifier, isAsynchronous)
         {
             //
         }
 
         /// <summary>
         /// Creates a Modbus RTU server with support for holding registers (read and write, 16 bit), input registers (read-only, 16 bit), coils (read and write, 1 bit) and discete inputs (read-only, 1 bit).
         /// </summary>
+        /// <param name="logger">The logger to use.</param>
         /// <param name="isAsynchronous">Enables or disables the asynchronous operation, where each client request is processed immediately using a locking mechanism. Use synchronuous operation to avoid locks in the hosting application. See the <see href="https://github.com/Apollo3zehn/FluentModbus">documentation</see> for more details.</param>
         /// <param name="unitIdentifier">The unique Modbus RTU unit identifier (1..247).</param>
-        public ModbusRtuServer(byte unitIdentifier, bool isAsynchronous) : base(isAsynchronous)
+        public ModbusRtuServer(ILogger logger, byte unitIdentifier, bool isAsynchronous = true) : base(isAsynchronous, logger)
         {
             AddUnit(unitIdentifier);
         }
 
         /// <summary>
         /// Creates a multi-unit Modbus RTU server with support for holding registers (read and write, 16 bit), input registers (read-only, 16 bit), coils (read and write, 1 bit) and discete inputs (read-only, 1 bit).
         /// </summary>
+        /// <param name="isAsynchronous">Enables or disables the asynchronous operation, where each client request is processed immediately using a locking mechanism. Use synchronuous operation to avoid locks in the hosting application. See the <see href="https://github.com/Apollo3zehn/FluentModbus">documentation</see> for more details.</param>
         /// <param name="unitIdentifiers">The unique Modbus RTU unit identifiers (1..247).</param>
-        public ModbusRtuServer(IEnumerable<byte> unitIdentifiers) : this(unitIdentifiers, true)
+        public ModbusRtuServer(IEnumerable<byte> unitIdentifiers, bool isAsynchronous = true) 
+            : this(logger: NullLogger.Instance, unitIdentifiers, isAsynchronous)
         {
             //
         }
 
         /// <summary>
         /// Creates a multi-unit Modbus RTU server with support for holding registers (read and write, 16 bit), input registers (read-only, 16 bit), coils (read and write, 1 bit) and discete inputs (read-only, 1 bit).
         /// </summary>
+        /// <param name="logger">The logger to use.</param>
         /// <param name="isAsynchronous">Enables or disables the asynchronous operation, where each client request is processed immediately using a locking mechanism. Use synchronuous operation to avoid locks in the hosting application. See the <see href="https://github.com/Apollo3zehn/FluentModbus">documentation</see> for more details.</param>
         /// <param name="unitIdentifiers">The unique Modbus RTU unit identifiers (1..247).</param>
-        public ModbusRtuServer(IEnumerable<byte> unitIdentifiers, bool isAsynchronous) : base(isAsynchronous)
+        public ModbusRtuServer(ILogger logger, IEnumerable<byte> unitIdentifiers, bool isAsynchronous = true) : base(isAsynchronous, logger)
         {
             foreach (var unitIdentifier in unitIdentifiers)
             {
@@ -147,7 +155,7 @@ public void Start(IModbusRtuSerialPort serialPort)
             base.StopProcessing();
             base.StartProcessing();
 
-            RequestHandler = new ModbusRtuRequestHandler(serialPort, this);
+            RequestHandler = new ModbusRtuRequestHandler(serialPort, this, Logger);
 
             // remove clients asynchronously
             /* https://stackoverflow.com/questions/2782802/can-net-task-instances-go-out-of-scope-during-run */

diff --git a/src/FluentModbus/Server/ModbusServer.cs b/src/FluentModbus/Server/ModbusServer.cs
@@ -1,5 +1,6 @@
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
+using Microsoft.Extensions.Logging;
 
 [assembly: InternalsVisibleTo("FluentModbus.Tests")]
 
@@ -79,10 +80,12 @@ public abstract class ModbusServer : IDisposable
         /// Initializes a new instance of the <see cref="ModbusServer"/>.
         /// </summary>
         /// <param name="isAsynchronous">A boolean which indicates if the server responds to client requests asynchronously (immediately) or synchronously (regularly at fixed events).</param>
-        protected ModbusServer(bool isAsynchronous)
+        /// <param name="logger">The logger to use.</param>
+        protected ModbusServer(bool isAsynchronous, ILogger logger)
         {
             Lock = this;
             IsAsynchronous = isAsynchronous;
+            Logger = logger;
 
             MaxInputRegisterAddress = ushort.MaxValue;
             MaxHoldingRegisterAddress = ushort.MaxValue;
@@ -153,6 +156,11 @@ protected ModbusServer(bool isAsynchronous)
         /// </summary>
         public bool AlwaysRaiseChangedEvent { get; set; } = false;
 
+        /// <summary>
+        /// Gets the logger.
+        /// </summary>
+        protected ILogger Logger { get; }
+
         internal bool IsSingleZeroUnitMode => UnitIdentifiers.Count == 1 && UnitIdentifiers[0] == 0;
 
         private protected CancellationTokenSource CTS { get; private set; } = new CancellationTokenSource();

diff --git a/src/FluentModbus/Server/ModbusTcpRequestHandler.cs b/src/FluentModbus/Server/ModbusTcpRequestHandler.cs
@@ -1,5 +1,6 @@
 using System.Net;
 using System.Net.Sockets;
+using Microsoft.Extensions.Logging;
 
 namespace FluentModbus
 {
@@ -14,13 +15,16 @@ internal class ModbusTcpRequestHandler : ModbusRequestHandler, IDisposable
         private ushort _protocolIdentifier;
         private ushort _bytesFollowing;
 
+        private readonly ILogger _logger;
+
         #endregion
 
         #region Constructors
 
-        public ModbusTcpRequestHandler(TcpClient tcpClient, ModbusTcpServer tcpServer)
+        public ModbusTcpRequestHandler(TcpClient tcpClient, ModbusTcpServer tcpServer, ILogger logger)
             : base(tcpServer, 260)
         {
+            _logger = logger;
             _tcpClient = tcpClient;
             _networkStream = tcpClient.GetStream();
 
@@ -59,8 +63,10 @@ internal override async Task ReceiveRequestAsync()
                         WriteResponse();
                 }
             }
-            catch
+            catch (Exception ex)
             {
+                _logger.LogDebug(ex, "The connection will be closed");
+
                 CancelToken();
             }
         }

diff --git a/src/FluentModbus/Server/ModbusTcpServer.cs b/src/FluentModbus/Server/ModbusTcpServer.cs
@@ -19,40 +19,22 @@ public class ModbusTcpServer : ModbusServer
 
         #region Constructors
 
-        /// <summary>
-        /// Creates a Modbus TCP server with support for holding registers (read and write, 16 bit), input registers (read-only, 16 bit), coils (read and write, 1 bit) and discete inputs (read-only, 1 bit).
-        /// </summary>
-        public ModbusTcpServer() : this(NullLogger.Instance, true)
-        {
-            //
-        }
-
-        /// <summary>
-        /// Creates a Modbus TCP server with support for holding registers (read and write, 16 bit), input registers (read-only, 16 bit), coils (read and write, 1 bit) and discete inputs (read-only, 1 bit).
-        /// </summary>
-        /// <param name="logger">A logger instance to provide runtime information.</param>
-        public ModbusTcpServer(ILogger logger) : this(logger, true)
-        {
-            //
-        }
-
         /// <summary>
         /// Creates a Modbus TCP server with support for holding registers (read and write, 16 bit), input registers (read-only, 16 bit), coils (read and write, 1 bit) and discete inputs (read-only, 1 bit).
         /// </summary>
         /// <param name="isAsynchronous">Enables or disables the asynchronous operation, where each client request is processed immediately using a locking mechanism. Use synchronuous operation to avoid locks in the hosting application. See the <see href="https://github.com/Apollo3zehn/FluentModbus">documentation</see> for more details.</param>
-        public ModbusTcpServer(bool isAsynchronous) : this(NullLogger.Instance, isAsynchronous)
+        public ModbusTcpServer(bool isAsynchronous = true) : base(isAsynchronous, logger: NullLogger.Instance)
         {
-            //
+            AddUnit(0);
         }
 
         /// <summary>
         /// Creates a Modbus TCP server with support for holding registers (read and write, 16 bit), input registers (read-only, 16 bit), coils (read and write, 1 bit) and discete inputs (read-only, 1 bit).
         /// </summary>
-        /// <param name="logger">A logger instance to provide runtime information.</param>
+        /// <param name="logger">The logger to use.</param>
         /// <param name="isAsynchronous">Enables or disables the asynchronous operation, where each client request is processed immediately using a locking mechanism. Use synchronuous operation to avoid locks in the hosting application. See the <see href="https://github.com/Apollo3zehn/FluentModbus">documentation</see> for more details.</param>
-        public ModbusTcpServer(ILogger logger, bool isAsynchronous) : base(isAsynchronous)
+        public ModbusTcpServer(ILogger logger, bool isAsynchronous = true) : base(isAsynchronous, logger)
         {
-            Logger = logger;
             AddUnit(0);
         }
 
@@ -63,7 +45,7 @@ public ModbusTcpServer(ILogger logger, bool isAsynchronous) : base(isAsynchronou
         /// <summary>
         /// Gets or sets the timeout for each client connection. When the client does not send any request within the specified period of time, the connection will be reset. Default is 1 minute.
         /// </summary>
-        public TimeSpan ConnectionTimeout { get; set; } = ModbusTcpServer.DefaultConnectionTimeout;
+        public TimeSpan ConnectionTimeout { get; set; } = DefaultConnectionTimeout;
 
         /// <summary>
         /// Gets or sets the maximum number of concurrent client connections. A value of zero means there is no limit.
@@ -79,8 +61,6 @@ public ModbusTcpServer(ILogger logger, bool isAsynchronous) : base(isAsynchronou
 
         internal List<ModbusTcpRequestHandler> RequestHandlers { get; private set; } = new List<ModbusTcpRequestHandler>();
 
-        private ILogger Logger { get; }
-
         #endregion
 
         #region Methods
@@ -133,7 +113,7 @@ public void Start(ITcpClientProvider tcpClientProvider, bool leaveOpen = false)
                     // There are no default timeouts (SendTimeout and ReceiveTimeout = 0), 
                     // use ConnectionTimeout instead.
                     var tcpClient = await _tcpClientProvider.AcceptTcpClientAsync();
-                    var requestHandler = new ModbusTcpRequestHandler(tcpClient, this);
+                    var requestHandler = new ModbusTcpRequestHandler(tcpClient, this, Logger);
 
                     lock (Lock)
                     {
@@ -170,12 +150,16 @@ public void Start(ITcpClientProvider tcpClientProvider, bool leaveOpen = false)
                                 // This should be the only cause but since "ReceiveRequestAsync" is never
                                 // awaited, the actual cause may be different.
                                 requestHandler.CancellationToken.IsCancellationRequested ||
-                                // or there was not request received within the specified timeout
+                                // or there was no request received within the specified timeout
                                 requestHandler.LastRequest.Elapsed > ConnectionTimeout)
                             {
                                 try
                                 {
-                                    Logger.LogInformation($"Connection {requestHandler.DisplayName} timed out.");
+                                    if (requestHandler.CancellationToken.IsCancellationRequested)
+                                        Logger.LogInformation($"Connection {requestHandler.DisplayName} was closed due to an error");
+
+                                    else
+                                        Logger.LogInformation($"Connection {requestHandler.DisplayName} timed out");
 
                                     // remove request handler
                                     RequestHandlers.Remove(requestHandler);
@@ -207,7 +191,7 @@ public void Start(TcpClient tcpClient)
 
             RequestHandlers = new List<ModbusTcpRequestHandler>()
             {
-                new ModbusTcpRequestHandler(tcpClient, this)
+                new ModbusTcpRequestHandler(tcpClient, this, Logger)
             };
         }
 

diff --git a/version.json b/version.json
@@ -1,4 +1,4 @@
 {
-    "version": "5.2.0",
+    "version": "vNext",
     "suffix": ""
 }