Extend supported variables and improve parser errors

Sharp7.Rx.Tests/S7VariableAddressTests/MatchesType.cs

@@ -0,0 +1,40 @@
+using System.Reflection;
+using NUnit.Framework;
+using Sharp7.Rx.Extensions;
+using Sharp7.Rx.Interfaces;
+using Shouldly;
+
+namespace Sharp7.Rx.Tests.S7VariableAddressTests;
+
+[TestFixture]
+public class MatchesType
+{
+    static readonly IS7VariableNameParser parser = new S7VariableNameParser();
+
+    
+    public void Supported(Type type, string address)
+    {
+        Check(type, address, true);
+    }
+
+    public IEnumerable<TestCase> GetValid()
+    {
+        yield return new TestCase(typeof(bool), "DB0.DBx0.0");
+        yield return new TestCase(typeof(short), "DB0.INT0");
+        yield return new TestCase(typeof(int), "DB0.DINT0");
+        yield return new TestCase(typeof(long), "DB0.DUL0");
+        yield return new TestCase(typeof(ulong), "DB0.DUL0");
+    }
+
+
+    private static void Check(Type type, string address, bool expected)
+    {
+        //Arrange
+        var variableAddress = parser.Parse(address);
+
+        //Act
+        variableAddress.MatchesType(type).ShouldBe(expected);
+    }
+
+    public record TestCase(Type Type, string Address);
+}

Sharp7.Rx.Tests/S7VariableNameParserTests.cs

@@ -1,13 +1,14 @@
 using DeepEqual.Syntax;
 using NUnit.Framework;
 using Sharp7.Rx.Enums;
+using Shouldly;
 
 namespace Sharp7.Rx.Tests;
 
 [TestFixture]
 internal class S7VariableNameParserTests
 {
-    [TestCaseSource(nameof(GetTestCases))]
+    [TestCaseSource(nameof(ValidTestCases))]
     public void Run(TestCase tc)
     {
         var parser = new S7VariableNameParser();
@@ -15,23 +16,71 @@ internal class S7VariableNameParserTests
         resp.ShouldDeepEqual(tc.Expected);
     }
 
-    public static IEnumerable<TestCase> GetTestCases()
+    [TestCase("DB506.Bit216", TestName = "Bit without Bit")]
+    [TestCase("DB506.String216", TestName = "String without Length")]
+    [TestCase("DB506.WString216", TestName = "WString without Length")]
+
+    [TestCase("DB506.Int216.1", TestName = "Int with Length")]
+    [TestCase("DB506.UInt216.1", TestName = "UInt with Length")]
+    [TestCase("DB506.DInt216.1", TestName = "DInt with Length")]
+    [TestCase("DB506.UDInt216.1", TestName = "UDInt with Length")]
+    [TestCase("DB506.LInt216.1", TestName = "LInt with Length")]
+    [TestCase("DB506.ULInt216.1", TestName = "ULInt with Length")]
+    [TestCase("DB506.Real216.1", TestName = "LReal with Length")]
+    [TestCase("DB506.LReal216.1", TestName = "LReal with Length")]
+
+    [TestCase("DB506.xx216", TestName = "Invalid type")]
+    [TestCase("DB506.216", TestName = "No type")]
+    [TestCase("DB506.Int216.", TestName = "Trailing dot")]
+    [TestCase("x506.Int216", TestName = "Wrong type")]
+    [TestCase("506.Int216", TestName = "No type")]
+    [TestCase("", TestName = "empty")]
+    [TestCase(" ", TestName = "space")]
+    [TestCase(" DB506.Int216", TestName = "leading space")]
+    [TestCase("DB506.Int216 ", TestName = "trailing space")]
+    [TestCase("DB.Int216 ", TestName = "No db")]
+    [TestCase("DB5061234.Int216.1", TestName = "DB too large")]
+    public void Invalid(string? input)
     {
+        var parser = new S7VariableNameParser();
+        Should.Throw<InvalidS7AddressException>(() => parser.Parse(input));
+    }
+
+    public static IEnumerable<TestCase> ValidTestCases()
+    {
+        yield return new TestCase("DB506.Bit216.2", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 1, Bit = 2, Type = DbType.Bit});
+
+        yield return new TestCase("DB506.String216.10", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 10, Type = DbType.String});
+        yield return new TestCase("DB506.WString216.10", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 10, Type = DbType.WString});
+
+        yield return new TestCase("DB506.Byte216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 1, Type = DbType.Byte});
+        yield return new TestCase("DB506.Byte216.100", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 100, Type = DbType.Byte});
+        yield return new TestCase("DB506.Int216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 2, Type = DbType.Int});
+        yield return new TestCase("DB506.UInt216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 2, Type = DbType.UInt});
+        yield return new TestCase("DB506.DInt216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 4, Type = DbType.DInt});
+        yield return new TestCase("DB506.UDInt216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 4, Type = DbType.UDInt});
+        yield return new TestCase("DB506.LInt216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 8, Type = DbType.LInt});
+        yield return new TestCase("DB506.ULInt216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 8, Type = DbType.ULInt});
+
+        yield return new TestCase("DB506.Real216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 4, Type = DbType.Single});
+        yield return new TestCase("DB506.LReal216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 8, Type = DbType.Double});
+
+
+        // Legacy
         yield return new TestCase("DB13.DBX3.1", new S7VariableAddress {Operand = Operand.Db, DbNr = 13, Start = 3, Length = 1, Bit = 1, Type = DbType.Bit});
         yield return new TestCase("Db403.X5.2", new S7VariableAddress {Operand = Operand.Db, DbNr = 403, Start = 5, Length = 1, Bit = 2, Type = DbType.Bit});
         yield return new TestCase("DB55DBX23.6", new S7VariableAddress {Operand = Operand.Db, DbNr = 55, Start = 23, Length = 1, Bit = 6, Type = DbType.Bit});
-        yield return new TestCase("DB1.S255", new S7VariableAddress {Operand = Operand.Db, DbNr = 1, Start = 255, Length = 0, Bit = 0, Type = DbType.String});
+        yield return new TestCase("DB1.S255.20", new S7VariableAddress {Operand = Operand.Db, DbNr = 1, Start = 255, Length = 20, Type = DbType.String});
-        yield return new TestCase("DB1.S255.20", new S7VariableAddress {Operand = Operand.Db, DbNr = 1, Start = 255, Length = 20, Bit = 0, Type = DbType.String});
+        yield return new TestCase("DB5.String887.20", new S7VariableAddress {Operand = Operand.Db, DbNr = 5, Start = 887, Length = 20, Type = DbType.String});
-        yield return new TestCase("DB5.String887.20", new S7VariableAddress {Operand = Operand.Db, DbNr = 5, Start = 887, Length = 20, Bit = 0, Type = DbType.String});
+        yield return new TestCase("DB506.B216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 1, Type = DbType.Byte});
-        yield return new TestCase("DB506.B216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 1, Bit = 0, Type = DbType.Byte});
+        yield return new TestCase("DB506.DBB216.5", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 5, Type = DbType.Byte});
-        yield return new TestCase("DB506.DBB216.5", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 5, Bit = 0, Type = DbType.Byte});
+        yield return new TestCase("DB506.D216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 4, Type = DbType.Single});
-        yield return new TestCase("DB506.D216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 4, Bit = 0, Type = DbType.Double});
+        yield return new TestCase("DB506.DINT216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 4, Type = DbType.DInt});
-        yield return new TestCase("DB506.DINT216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 4, Bit = 0, Type = DbType.DInteger});
+        yield return new TestCase("DB506.INT216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 2, Type = DbType.Int});
-        yield return new TestCase("DB506.INT216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 2, Bit = 0, Type = DbType.Integer});
+        yield return new TestCase("DB506.DBW216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 2, Type = DbType.Int});
-        yield return new TestCase("DB506.DBW216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 2, Bit = 0, Type = DbType.Integer});
+        yield return new TestCase("DB506.DUL216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 8, Type = DbType.ULInt});
-        yield return new TestCase("DB506.DUL216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 8, Bit = 0, Type = DbType.ULong});
+        yield return new TestCase("DB506.DULINT216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 8, Type = DbType.ULInt});
-        yield return new TestCase("DB506.DULINT216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 8, Bit = 0, Type = DbType.ULong});
+        yield return new TestCase("DB506.DULONG216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 8, Type = DbType.ULInt});
-        yield return new TestCase("DB506.DULONG216", new S7VariableAddress {Operand = Operand.Db, DbNr = 506, Start = 216, Length = 8, Bit = 0, Type = DbType.ULong});
     }
 
     public record TestCase(string Input, S7VariableAddress Expected)

Sharp7.Rx/Enums/DbType.cs

@@ -1,12 +1,52 @@
 namespace Sharp7.Rx.Enums;
 
+// see https://support.industry.siemens.com/cs/mdm/109747174?c=88343664523&lc=de-DE
 internal enum DbType
 {
     Bit,
+
+    /// <summary>
+    ///     ASCII string
+    /// </summary>
     String,
+
+    /// <summary>
+    ///     UTF16 string
+    /// </summary>
+    WString,
+
     Byte,
+
+    /// <summary>
+    ///     Int16
+    /// </summary>
+    Int,
+
+    /// <summary>
+    ///     UInt16
+    /// </summary>
+    UInt,
+
+    /// <summary>
+    ///     Int32
+    /// </summary>
+    DInt,
+
+    /// <summary>
+    ///     UInt32
+    /// </summary>
+    UDInt,
+
+    /// <summary>
+    ///     Int64
+    /// </summary>
+    LInt,
+
+    /// <summary>
+    ///     UInt64
+    /// </summary>
+    ULInt,
+
+    Single,
     Double,
-    Integer,
-    DInteger,
-    ULong
 }

Sharp7.Rx/Exceptions/S7Exception.cs

@@ -0,0 +1,27 @@
+namespace Sharp7.Rx;
+
+public abstract class S7Exception : Exception
+{
+    protected S7Exception(string message) : base(message)
+    {
+    }
+
+    protected S7Exception(string message, Exception innerException) : base(message, innerException)
+    {
+    }
+}
+
+public class InvalidS7AddressException : S7Exception
+{
+    public InvalidS7AddressException(string message, string input) : base(message)
+    {
+        Input = input;
+    }
+
+    public InvalidS7AddressException(string message, Exception innerException, string input) : base(message, innerException)
+    {
+        Input = input;
+    }
+
+    public string Input { get; private set; }
+}

Sharp7.Rx/Extensions/S7VariableExtensions.cs

@@ -0,0 +1,9 @@
+namespace Sharp7.Rx.Extensions;
+
+internal static class S7VariableAddressExtensions
+{
+    public static bool MatchesType(this S7VariableAddress address, Type type)
+    {
+        return false;
+    }
+}

Sharp7.Rx/Interfaces/IS7VariableNameParser.cs

@@ -1,4 +1,5 @@
-namespace Sharp7.Rx.Interfaces;
+#nullable enable
+namespace Sharp7.Rx.Interfaces;
 
 internal interface IS7VariableNameParser
 {

Sharp7.Rx/S7VariableAddress.cs

@@ -10,8 +10,13 @@ internal class S7VariableAddress
     public ushort DbNr { get; set; }
     public ushort Start { get; set; }
     public ushort Length { get; set; }
-    public byte Bit { get; set; }
+    public byte? Bit { get; set; }
     public DbType Type { get; set; }
 
-    public ushort BufferLength => Type == DbType.String ? (ushort)(Length + 2) : Length;
+    public ushort BufferLength => Type switch
+    {
+        DbType.String => (ushort) (Length + 2),
+        DbType.WString => (ushort) (Length * 2 + 4),
+        _ => Length
+    };
 }

Sharp7.Rx/S7VariableNameParser.cs

@@ -7,75 +7,142 @@ namespace Sharp7.Rx;
 
 internal class S7VariableNameParser : IS7VariableNameParser
 {
-    private static readonly Regex regex = new Regex(@"^(?<operand>db{1})(?<dbNr>\d{1,4})\.?(?<type>dbx|x|s|string|b|dbb|d|int|dbw|w|dint|dul|dulint|dulong|){1}(?<start>\d+)(\.(?<bitOrLength>\d+))?$", RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.CultureInvariant);
+    private static readonly Regex regex = new Regex(@"^(?<operand>db)(?<dbNo>\d+)\.?(?<type>[a-z]+)(?<start>\d+)(\.(?<bitOrLength>\d+))?$",
+                                                    RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.CultureInvariant);
 
     private static readonly IReadOnlyDictionary<string, DbType> types = new Dictionary<string, DbType>(StringComparer.OrdinalIgnoreCase)
     {
-        {"x", DbType.Bit},
+        {"bit", DbType.Bit},
-        {"dbx", DbType.Bit},
+
-        {"s", DbType.String},
         {"string", DbType.String},
+        {"wstring", DbType.WString},
+
+        {"byte", DbType.Byte},
+        {"int", DbType.Int},
+        {"uint", DbType.UInt},
+        {"dint", DbType.DInt},
+        {"udint", DbType.UDInt},
+        {"lint", DbType.LInt},
+        {"ulint", DbType.ULInt},
+
+        {"real", DbType.Single},
+        {"lreal", DbType.Double},
+
+        // used for legacy compatability
         {"b", DbType.Byte},
+        {"d", DbType.Single},
         {"dbb", DbType.Byte},
-        {"d", DbType.Double},
+        {"dbw", DbType.Int},
-        {"int", DbType.Integer},
+        {"dbx", DbType.Bit},
-        {"dint", DbType.DInteger},
+        {"dul", DbType.ULInt},
-        {"w", DbType.Integer},
+        {"dulint", DbType.ULInt},
-        {"dbw", DbType.Integer},
+        {"dulong", DbType.ULInt},
-        {"dul", DbType.ULong},
+        {"s", DbType.String},
-        {"dulint", DbType.ULong},
+        {"w", DbType.Int},
-        {"dulong", DbType.ULong}
+        {"x", DbType.Bit},
     };
 
     public S7VariableAddress Parse(string input)
     {
+        if (input == null)
+            throw new ArgumentNullException(nameof(input));
+
         var match = regex.Match(input);
-        if (match.Success)
+        if (!match.Success)
+            throw new InvalidS7AddressException($"Invalid S7 address: \"{input}\"", input);
+
+        var operand = (Operand) Enum.Parse(typeof(Operand), match.Groups["operand"].Value, true);
+
+        if (!ushort.TryParse(match.Groups["dbNo"].Value, NumberStyles.Integer, CultureInfo.InvariantCulture, out var dbNr))
+            throw new InvalidS7AddressException($"\"{match.Groups["dbNo"].Value}\" is an invalid DB number in \"{input}\"", input);
+
+        if (!ushort.TryParse(match.Groups["start"].Value, NumberStyles.Integer, CultureInfo.InvariantCulture, out var start))
+            throw new InvalidS7AddressException($"\"{match.Groups["start"].Value}\" is an invalid start bit in \"{input}\"", input);
+
+        if (!types.TryGetValue(match.Groups["type"].Value, out var type))
+            throw new InvalidS7AddressException($"\"{match.Groups["type"].Value}\" is an invalid type in \"{input}\"", input);
+
+        ushort length = type switch
         {
-            var operand = (Operand) Enum.Parse(typeof(Operand), match.Groups["operand"].Value, true);
+            DbType.Bit => 1,
-            var dbNr = ushort.Parse(match.Groups["dbNr"].Value, NumberStyles.Integer, CultureInfo.InvariantCulture);
-            var start = ushort.Parse(match.Groups["start"].Value, NumberStyles.Integer, CultureInfo.InvariantCulture);
-            if (!types.TryGetValue(match.Groups["type"].Value, out var type))
-                return null;
 
+            DbType.String => GetLength(),
+            DbType.WString => GetLength(),
 
-            var s7VariableAddress = new S7VariableAddress
+            DbType.Byte => GetLength(1),
-            {
-                Operand = operand,
-                DbNr = dbNr,
-                Start = start,
-                Type = type,
-            };
 
-            switch (type)
+            DbType.Int => 2,
-            {
+            DbType.DInt => 4,
-                case DbType.Bit:
+            DbType.ULInt => 8,
-                    s7VariableAddress.Length = 1;
+            DbType.UInt => 2,
-                    s7VariableAddress.Bit = byte.Parse(match.Groups["bitOrLength"].Value);
+            DbType.UDInt => 4,
-                    break;
+            DbType.LInt => 8,
-                case DbType.Byte:
-                    s7VariableAddress.Length = match.Groups["bitOrLength"].Success ? ushort.Parse(match.Groups["bitOrLength"].Value) : (ushort) 1;
-                    break;
-                case DbType.String:
-                    s7VariableAddress.Length = match.Groups["bitOrLength"].Success ? ushort.Parse(match.Groups["bitOrLength"].Value) : (ushort) 0;
-                    break;
-                case DbType.Integer:
-                    s7VariableAddress.Length = 2;
-                    break;
-                case DbType.DInteger:
-                    s7VariableAddress.Length = 4;
-                    break;
-                case DbType.ULong:
-                    s7VariableAddress.Length = 8;
-                    break;
-                case DbType.Double:
-                    s7VariableAddress.Length = 4;
-                    break;
-            }
 
-            return s7VariableAddress;
+            DbType.Single => 4,
+            DbType.Double => 8,
+            _ => throw new ArgumentOutOfRangeException($"DbType {type} is not supported")
+        };
+
+        switch (type)
+        {
+            case DbType.Bit:
+            case DbType.String:
+            case DbType.WString:
+            case DbType.Byte:
+                break;
+            case DbType.Int:
+            case DbType.UInt:
+            case DbType.DInt:
+            case DbType.UDInt:
+            case DbType.LInt:
+            case DbType.ULInt:
+            case DbType.Single:
+            case DbType.Double:
+            default:
+                if (match.Groups["bitOrLength"].Success)
+                    throw new InvalidS7AddressException($"{type} address must not have a length: \"{input}\"", input);
+                break;
         }
 
-        return null;
+        byte? bit = type == DbType.Bit ? GetBit() : null;
+
+
+        var s7VariableAddress = new S7VariableAddress
+        {
+            Operand = operand,
+            DbNr = dbNr,
+            Start = start,
+            Type = type,
+            Length = length,
+            Bit = bit
+        };
+
+        return s7VariableAddress;
+
+        ushort GetLength(ushort? defaultValue = null)
+        {
+            if (!match.Groups["bitOrLength"].Success)
+            {
+                if (defaultValue.HasValue)
+                    return defaultValue.Value;
+                throw new InvalidS7AddressException($"Variable of type {type} must have a length set \"{input}\"", input);
+            }
+
+            if (!ushort.TryParse(match.Groups["bitOrLength"].Value, out var result))
+                throw new InvalidS7AddressException($"\"{match.Groups["bitOrLength"].Value}\" is an invalid length in \"{input}\"", input);
+
+            return result;
+        }
+
+        byte GetBit()
+        {
+            if (!match.Groups["bitOrLength"].Success)
+                throw new InvalidS7AddressException($"Variable of type {type} must have a bit number set \"{input}\"", input);
+
+            if (!byte.TryParse(match.Groups["bitOrLength"].Value, out var result))
+                throw new InvalidS7AddressException($"\"{match.Groups["bitOrLength"].Value}\" is an invalid bit number in \"{input}\"", input);
+
+            return result;
+        }
     }
 }

Sharp7.Rx/Sharp7.Rx.csproj

@@ -42,4 +42,8 @@
     </EmbeddedResource>
   </ItemGroup>
 
+  <ItemGroup>
+    <Folder Include="Exceptions\" />
+  </ItemGroup>
+
 </Project>

Sharp7.Rx/Sharp7.Rx.csproj.DotSettings

@@ -0,0 +1,2 @@
+<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
+	<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=exceptions/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>

Sharp7.Rx/Sharp7Plc.cs

@@ -154,9 +154,15 @@ public class Sharp7Plc : IPlc
         if (address == null) throw new ArgumentException("Input variable name is not valid", "variableName");
 
         if (typeof(TValue) == typeof(bool))
+        {
             // Special handling for bools, which are written on a by-bit basis. Writing a complete byte would
             // overwrite other bits within this byte.
-            await s7Connector.WriteBit(address.Operand, address.Start, address.Bit, (bool) (object) value, address.DbNr, token);
+
+            if (address.Bit == null)
+                throw new InvalidOperationException("Address must have a Bit to write a bool.");
+
+            await s7Connector.WriteBit(address.Operand, address.Start, address.Bit.Value, (bool) (object) value, address.DbNr, token);
+        }
         else
         {
             // TODO: Use ArrayPool.Rent() once we drop Framwework support