From 530072b70f0fc5c75d782438384231bbcc91bff6 Mon Sep 17 00:00:00 2001 From: mbalous Date: Wed, 21 Mar 2018 22:18:37 +0100 Subject: [PATCH 1/3] Improved XML function comments. --- S7.Net/PLC.cs | 72 ++++++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/S7.Net/PLC.cs b/S7.Net/PLC.cs index c5b9676..7fe5a14 100644 --- a/S7.Net/PLC.cs +++ b/S7.Net/PLC.cs @@ -17,31 +17,32 @@ namespace S7.Net public class Plc : IDisposable { private const int CONNECTION_TIMED_OUT_ERROR_CODE = 10060; - - private Socket _mSocket; //TCP connection to device + + //TCP connection to device + private Socket _mSocket; /// - /// Ip address of the plc + /// IP address of the PLC /// public string IP { get; private set; } /// - /// Cpu type of the plc + /// CPU type of the PLC /// public CpuType CPU { get; private set; } /// - /// Rack of the plc + /// Rack of the PLC /// public Int16 Rack { get; private set; } /// - /// Slot of the CPU of the plc + /// Slot of the CPU of the PLC /// public Int16 Slot { get; private set; } /// - /// Returns true if a connection to the plc can be established + /// Returns true if a connection to the PLC can be established /// public bool IsAvailable { @@ -61,7 +62,7 @@ namespace S7.Net /// - /// Checks if the socket is connected and polls the other peer (the plc) to see if it's connected. + /// Checks if the socket is connected and polls the other peer (the PLC) to see if it's connected. /// This is the variable that you should continously check to see if the communication is working /// See also: http://stackoverflow.com/questions/2661764/how-to-check-if-a-socket-is-connected-disconnected-in-c /// @@ -100,10 +101,10 @@ namespace S7.Net /// You need slot > 0 if you are connecting to external ethernet card (CP). /// For S7-300 and S7-400 the default is rack = 0 and slot = 2. /// - /// CpuType of the plc (select from the enum) - /// Ip address of the plc - /// rack of the plc, usually it's 0, but check in the hardware configuration of Step7 or TIA portal - /// slot of the CPU of the plc, usually it's 2 for S7300-S7400, 0 for S7-1200 and S7-1500. + /// CpuType of the PLC (select from the enum) + /// Ip address of the PLC + /// rack of the PLC, usually it's 0, but check in the hardware configuration of Step7 or TIA portal + /// slot of the CPU of the PLC, usually it's 2 for S7300-S7400, 0 for S7-1200 and S7-1500. /// If you use an external ethernet card, this must be set accordingly. public Plc(CpuType cpu, string ip, Int16 rack, Int16 slot) { @@ -146,7 +147,8 @@ namespace S7.Net } /// - /// Open a socket and connects to the plc, sending all the corrected package and returning if the connection was successful (ErroreCode.NoError) of it was wrong. + /// Open a and connects to the PLC, sending all the corrected package + /// and returning if the connection was successful () of it was wrong. /// /// Returns ErrorCode.NoError if the connection was successful, otherwise check the ErrorCode public ErrorCode Open() @@ -245,7 +247,7 @@ namespace S7.Net } /// - /// Disonnects from the plc and close the socket + /// Disonnects from the PLC and close the socket /// public void Close() { @@ -269,7 +271,7 @@ namespace S7.Net int cntBytes = dataItems.Sum(dataItem => VarTypeToByteLength(dataItem.VarType, dataItem.Count)); if (dataItems.Count > 20) throw new Exception("Too many vars requested"); - if (cntBytes > 222) throw new Exception("Too many bytes requested"); //todo, proper TDU check + split in multiple requests + if (cntBytes > 222) throw new Exception("Too many bytes requested"); // TODO: proper TDU check + split in multiple requests try { @@ -363,7 +365,7 @@ namespace S7.Net } /// - /// Reads a single variable from the plc, takes in input strings like "DB1.DBX0.0", "DB20.DBD200", "MB20", "T45", etc. + /// Reads a single variable from the PLC, takes in input strings like "DB1.DBX0.0", "DB20.DBD200", "MB20", "T45", etc. /// If the read was not successful, check LastErrorCode or LastErrorString. /// /// Input strings like "DB1.DBX0.0", "DB20.DBD200", "MB20", "T45", etc. @@ -533,7 +535,7 @@ namespace S7.Net } /// - /// Reads all the bytes needed to fill a class in C#, starting from a certain address, and set all the properties values to the value that are read from the plc. + /// Reads all the bytes needed to fill a class in C#, starting from a certain address, and set all the properties values to the value that are read from the PLC. /// This reads only properties, it doesn't read private variable or public variable without {get;set;} specified. /// /// Instance of the class that will store the values @@ -557,28 +559,28 @@ namespace S7.Net } /// - /// Reads all the bytes needed to fill a class in C#, starting from a certain address, and set all the properties values to the value that are read from the plc. + /// Reads all the bytes needed to fill a class in C#, starting from a certain address, and set all the properties values to the value that are read from the PLC. /// This reads only properties, it doesn't read private variable or public variable without {get;set;} specified. To instantiate the class defined by the generic /// type, the class needs a default constructor. /// /// The class that will be instantiated. Requires a default constructor /// Index of the DB; es.: 1 is for DB1 /// Start byte address. If you want to read DB1.DBW200, this is 200. - /// An instance of the class with the values read from the plc. If no data has been read, null will be returned + /// An instance of the class with the values read from the PLC. If no data has been read, null will be returned public T ReadClass(int db, int startByteAdr = 0) where T : class { return ReadClass(() => Activator.CreateInstance(), db, startByteAdr); } /// - /// Reads all the bytes needed to fill a class in C#, starting from a certain address, and set all the properties values to the value that are read from the plc. + /// Reads all the bytes needed to fill a class in C#, starting from a certain address, and set all the properties values to the value that are read from the PLC. /// This reads only properties, it doesn't read private variable or public variable without {get;set;} specified. /// /// The class that will be instantiated /// Function to instantiate the class /// Index of the DB; es.: 1 is for DB1 /// Start byte address. If you want to read DB1.DBW200, this is 200. - /// An instance of the class with the values read from the plc. If no data has been read, null will be returned + /// An instance of the class with the values read from the PLC. If no data has been read, null will be returned public T ReadClass(Func classFactory, int db, int startByteAdr = 0) where T : class { var instance = classFactory(); @@ -747,11 +749,11 @@ namespace S7.Net } /// - /// Writes a single variable from the plc, takes in input strings like "DB1.DBX0.0", "DB20.DBD200", "MB20", "T45", etc. - /// If the write was not successful, check LastErrorCode or LastErrorString. + /// Writes a single variable from the PLC, takes in input strings like "DB1.DBX0.0", "DB20.DBD200", "MB20", "T45", etc. + /// If the write was not successful, check or . /// /// Input strings like "DB1.DBX0.0", "DB20.DBD200", "MB20", "T45", etc. - /// Value to be written to the plc + /// Value to be written to the PLC /// NoError if it was successful, or the error is specified public ErrorCode Write(string variable, object value) { @@ -915,11 +917,11 @@ namespace S7.Net } /// - /// Writes a C# struct to a DB in the plc + /// Writes a C# struct to a DB in the PLC /// /// The struct to be written /// Db address - /// Start bytes on the plc + /// Start bytes on the PLC /// NoError if it was successful, or the error is specified public ErrorCode WriteStruct(object structValue, int db, int startByteAdr = 0) { @@ -929,11 +931,11 @@ namespace S7.Net } /// - /// Writes a C# class to a DB in the plc + /// Writes a C# class to a DB in the PLC /// /// The class to be written /// Db address - /// Start bytes on the plc + /// Start bytes on the PLC /// NoError if it was successful, or the error is specified public ErrorCode WriteClass(object classValue, int db, int startByteAdr = 0) { @@ -943,7 +945,7 @@ namespace S7.Net } /// - /// Sets the LastErrorCode to NoError and LastErrorString to String.Empty + /// Sets the to and to . /// public void ClearLastError() { @@ -952,7 +954,7 @@ namespace S7.Net } /// - /// Creates the header to read bytes from the plc + /// Creates the header to read bytes from the PLC /// /// /// @@ -974,7 +976,7 @@ namespace S7.Net } /// - /// Create the bytes-package to request data from the plc. You have to specify the memory type (dataType), + /// Create the bytes-package to request data from the PLC. You have to specify the memory type (dataType), /// the address of the memory, the address of the byte and the bytes count. /// /// MemoryType (DB, Timer, Counter, etc.) @@ -1057,7 +1059,7 @@ namespace S7.Net } /// - /// Writes up to 200 bytes to the plc and returns NoError if successful. You must specify the memory area type, memory are address, byte start address and bytes count. + /// Writes up to 200 bytes to the PLC and returns NoError if successful. You must specify the memory area type, memory are address, byte start address and bytes count. /// If the write was not successful, check LastErrorCode or LastErrorString. /// /// Data type of the memory area, can be DB, Timer, Counter, Merker(Memory), Input, Output. @@ -1234,11 +1236,11 @@ namespace S7.Net } /// - /// Given a S7 variable type (Bool, Word, DWord, etc.), it returns how many bytes to read. + /// Given a S7 (Bool, Word, DWord, etc.), it returns how many bytes to read. /// /// /// - /// + /// Byte lenght of variable private int VarTypeToByteLength(VarType varType, int varCount = 1) { switch (varType) @@ -1266,7 +1268,7 @@ namespace S7.Net #region IDisposable members /// - /// Releases all resources, disonnects from the plc and closes the socket + /// Releases all resources, disonnects from the PLC and closes the /// public void Dispose() { From dd71e1bf0b89631fecf8668ec8373343bdcb6683 Mon Sep 17 00:00:00 2001 From: mbalous Date: Wed, 21 Mar 2018 22:32:23 +0100 Subject: [PATCH 2/3] Checking constructor parameters. --- S7.Net/PLC.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/S7.Net/PLC.cs b/S7.Net/PLC.cs index 7fe5a14..72907a4 100644 --- a/S7.Net/PLC.cs +++ b/S7.Net/PLC.cs @@ -108,8 +108,14 @@ namespace S7.Net /// If you use an external ethernet card, this must be set accordingly. public Plc(CpuType cpu, string ip, Int16 rack, Int16 slot) { - IP = ip; + if (!Enum.IsDefined(typeof(CpuType), cpu)) + throw new InvalidEnumArgumentException(nameof(cpu), (int) cpu, typeof(CpuType)); + + if (string.IsNullOrEmpty(ip)) + throw new ArgumentException("IP address must valid.", nameof(ip)); + CPU = cpu; + IP = ip; Rack = rack; Slot = slot; } From a99ea469ce23e5d3cfff7886bf558bc8a6a687a7 Mon Sep 17 00:00:00 2001 From: mbalous Date: Wed, 21 Mar 2018 22:40:25 +0100 Subject: [PATCH 3/3] Correct exceptions. Constructor parameter checking. --- S7.Net/PLC.cs | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/S7.Net/PLC.cs b/S7.Net/PLC.cs index 72907a4..febc87d 100644 --- a/S7.Net/PLC.cs +++ b/S7.Net/PLC.cs @@ -1,6 +1,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.ComponentModel; using System.Linq; using System.Net; using System.Net.NetworkInformation; @@ -245,7 +246,7 @@ namespace S7.Net catch(Exception exc) { LastErrorCode = ErrorCode.ConnectionError; - LastErrorString = "Couldn't establish the connection to " + IP + ".\nMessage: " + exc.Message; + LastErrorString = string.Format("Couldn't establish the connection to {0}.\nMessage: {1}", IP, exc.Message); return ErrorCode.ConnectionError; } @@ -276,8 +277,10 @@ namespace S7.Net { int cntBytes = dataItems.Sum(dataItem => VarTypeToByteLength(dataItem.VarType, dataItem.Count)); - if (dataItems.Count > 20) throw new Exception("Too many vars requested"); - if (cntBytes > 222) throw new Exception("Too many bytes requested"); // TODO: proper TDU check + split in multiple requests + if (dataItems.Count > 20) + throw new Exception("Too many vars requested"); + if (cntBytes > 222) + throw new Exception("Too many bytes requested"); // TODO: proper TDU check + split in multiple requests try { @@ -295,7 +298,8 @@ namespace S7.Net byte[] bReceive = new byte[512]; int numReceived = _mSocket.Receive(bReceive, 512, SocketFlags.None); - if (bReceive[21] != 0xff) throw new Exception(ErrorCode.WrongNumberReceivedBytes.ToString()); + if (bReceive[21] != 0xff) + throw new Exception(ErrorCode.WrongNumberReceivedBytes.ToString()); int offset = 25; foreach (var dataItem in dataItems) @@ -493,7 +497,8 @@ namespace S7.Net } string txt2 = txt.Substring(1); - if (txt2.IndexOf(".") == -1) throw new Exception(); + if (txt2.IndexOf(".") == -1) + throw new Exception(); mByte = int.Parse(txt2.Substring(0, txt2.IndexOf("."))); mBit = int.Parse(txt2.Substring(txt2.IndexOf(".") + 1)); @@ -694,7 +699,7 @@ namespace S7.Net { var intValue = (int) value; if (intValue < 0 || intValue > 7) - throw new Exception(string.Format("Addressing Error: You can only reference bitwise locations 0-7. Address {0} is invalid", bitAdr)); + throw new ArgumentOutOfRangeException(string.Format("Addressing Error: You can only reference bitwise locations 0-7. Address {0} is invalid", bitAdr), nameof(bitAdr)); bitValue = intValue == 1; } @@ -917,7 +922,7 @@ namespace S7.Net catch(Exception exc) { LastErrorCode = ErrorCode.WrongVarFormat; - LastErrorString = "The variable'" + variable + "' could not be parsed. Please check the syntax and try again.\nException: " + exc.Message; + LastErrorString = string.Format("The variable'{0}' could not be parsed. Please check the syntax and try again.\nException: {1}", variable, exc.Message); return LastErrorCode; } } @@ -1145,7 +1150,7 @@ namespace S7.Net package.Add(Types.Word.ToByteArray((ushort)varCount)); package.Add(Types.Word.ToByteArray((ushort)(db))); package.Add((byte)dataType); - var overflow = (int)(startByteAdr * 8 / 0xffffU); // handles words with address bigger than 8191 + int overflow = (int)(startByteAdr * 8 / 0xffffU); // handles words with address bigger than 8191 package.Add((byte)overflow); package.Add(Types.Word.ToByteArray((ushort)(startByteAdr * 8 + bitAdr))); package.Add(new byte[] { 0, 0x03 }); //ending 0x03 is used for writing a sinlge bit @@ -1182,7 +1187,8 @@ namespace S7.Net /// private object ParseBytes(VarType varType, byte[] bytes, int varCount, byte bitAdr = 0) { - if (bytes == null) return null; + if (bytes == null) + return null; switch (varType) { @@ -1230,12 +1236,16 @@ namespace S7.Net return Types.Counter.ToArray(bytes); case VarType.Bit: if (varCount == 1) + { if (bitAdr > 7) return null; else return Types.Bit.FromByte(bytes[0], bitAdr); + } else + { return Types.Bit.ToBitArray(bytes); + } default: return null; }