Improve WriteToBuffer implementation and tests

Sharp7.Rx.Tests/S7ValueConverterTests/ReadFromBuffer.cs

@@ -24,23 +24,23 @@ internal class ReadFromBuffer : ConverterTestBase
     {
         yield return new ConverterTestCase(true, "DB0.DBx0.4", [0x1F]);
         yield return new ConverterTestCase(false, "DB0.DBx0.4", [0xEF]);
-        yield return new ConverterTestCase("ABCD", "DB0.string0.10", [0x04, 0x04, 0x41, 0x42, 0x43, 0x44]); // Length in address exceeds PLC string length
+        yield return new ConverterTestCase("ABCD", "DB0.string0.6", [0x04, 0x04, 0x41, 0x42, 0x43, 0x44, 0x00, 0x00]); // Length in address exceeds PLC string length
     }
 
     [TestCase((char) 18, "DB0.DBB0", new byte[] {0x12})]
-    [TestCase((ushort) 3532, "DB0.INT0", new byte[] {0xF2, 0x34})]
+    public void UnsupportedType<T>(T template, string address, byte[] data)
-    [TestCase(0.25, "DB0.D0", new byte[] {0x3E, 0x80, 0x00, 0x00})]
-    public void Invalid<T>(T template, string address, byte[] data)
     {
         //Arrange
         var variableAddress = Parser.Parse(address);
 
         //Act
-        Should.Throw<InvalidOperationException>(() => S7ValueConverter.ReadFromBuffer<T>(data, variableAddress));
+        Should.Throw<UnsupportedS7TypeException>(() => S7ValueConverter.ReadFromBuffer<T>(data, variableAddress));
     }
 
-    [TestCase(3532, "DB0.DINT0", new byte[] {0xF2, 0x34})]
+    [TestCase(123, "DB12.DINT3", new byte[] {0x01, 0x02, 0x03})]
-    public void Argument<T>(T template, string address, byte[] data)
+    [TestCase((short) 123, "DB12.INT3", new byte[] {0xF2})]
+    [TestCase("ABC", "DB0.string0.6", new byte[] {0x01, 0x02, 0x03})]
+    public void BufferTooSmall<T>(T template, string address, byte[] data)
     {
         //Arrange
         var variableAddress = Parser.Parse(address);

Sharp7.Rx.Tests/S7ValueConverterTests/WriteToBuffer.cs

@@ -1,13 +1,13 @@
 using NUnit.Framework;
-using Sharp7.Rx.Interfaces;
 using Shouldly;
 
 namespace Sharp7.Rx.Tests.S7ValueConverterTests;
 
 [TestFixture]
-internal class WriteToBuffer:ConverterTestBase
+internal class WriteToBuffer : ConverterTestBase
 {
     [TestCaseSource(nameof(GetValidTestCases))]
+    [TestCaseSource(nameof(GetAdditinalWriteTestCases))]
     public void Write(ConverterTestCase tc)
     {
         //Arrange
@@ -21,15 +21,33 @@ internal class WriteToBuffer:ConverterTestBase
         buffer.ShouldBe(tc.Data);
     }
 
+    public static IEnumerable<ConverterTestCase> GetAdditinalWriteTestCases()
+    {
+        yield return new ConverterTestCase("aaaaBCDE", "DB0.string0.4", [0x04, 0x04, 0x61, 0x61, 0x61, 0x61]); // Length in address exceeds PLC string length
+        yield return new ConverterTestCase("aaaaBCDE", "DB0.WString0.4", [0x00, 0x04, 0x00, 0x04, 0x00, 0x61, 0x00, 0x61, 0x00, 0x61, 0x00, 0x61]); // Length in address exceeds PLC string length
+    }
+
+    [TestCase(18, "DB0.DInt12", 3)]
+    [TestCase(0.25f, "DB0.Real1", 3)]
+    [TestCase("test", "DB0.String1.10", 9)]
+    public void BufferToSmall<T>(T input, string address, int bufferSize)
+    {
+        //Arrange
+        var variableAddress = Parser.Parse(address);
+        var buffer = new byte[bufferSize];
+
+        //Act
+        Should.Throw<ArgumentException>(() => S7ValueConverter.WriteToBuffer(buffer, input, variableAddress));
+    }
+
     [TestCase((char) 18, "DB0.DBB0")]
-    [TestCase(0.25, "DB0.D0")]
+    public void UnsupportedType<T>(T input, string address)
-    public void Invalid<T>(T input, string address)
     {
         //Arrange
         var variableAddress = Parser.Parse(address);
         var buffer = new byte[variableAddress.BufferLength];
 
         //Act
-        Should.Throw<InvalidOperationException>(() => S7ValueConverter.WriteToBuffer<T>(buffer, input, variableAddress));
+        Should.Throw<UnsupportedS7TypeException>(() => S7ValueConverter.WriteToBuffer(buffer, input, variableAddress));
     }
 }

Sharp7.Rx.Tests/S7VariableNameParserTests.cs

@@ -17,6 +17,7 @@ internal class S7VariableNameParserTests
     }
 
     [TestCase("DB506.Bit216", TestName = "Bit without Bit")]
+    [TestCase("DB506.Bit216.8", TestName = "Bit to high")]
     [TestCase("DB506.String216", TestName = "String without Length")]
     [TestCase("DB506.WString216", TestName = "WString without Length")]
 

Sharp7.Rx/S7ValueConverter.cs

@@ -7,12 +7,105 @@ namespace Sharp7.Rx;
 
 internal static class S7ValueConverter
 {
-    private static readonly Dictionary<Type, Func<byte[], S7VariableAddress, object>> readFunctions = new()
+    private static readonly Dictionary<Type, WriteFunc> writeFunctions = new()
+    {
+        {
+            typeof(bool), (data, address, value) =>
+            {
+                var byteValue = (bool) value ? (byte) 1 : (byte) 0;
+                var shifted = (byte) (byteValue << address.Bit!);
+                data[0] = shifted;
+            }
+        },
+
+        {typeof(byte), (data, address, value) => data[0] = (byte) value},
+        {
+            typeof(byte[]), (data, address, value) =>
+            {
+                var source = (byte[]) value;
+
+                var length = Math.Min(Math.Min(source.Length, data.Length), address.Length);
+
+                source.AsSpan(0, length).CopyTo(data);
+            }
+        },
+
+        {typeof(short), (data, address, value) => BinaryPrimitives.WriteInt16BigEndian(data, (short) value)},
+        {typeof(ushort), (data, address, value) => BinaryPrimitives.WriteUInt16BigEndian(data, (ushort) value)},
+        {typeof(int), (data, address, value) => BinaryPrimitives.WriteInt32BigEndian(data, (int) value)},
+        {typeof(uint), (data, address, value) => BinaryPrimitives.WriteUInt32BigEndian(data, (uint) value)},
+        {typeof(long), (data, address, value) => BinaryPrimitives.WriteInt64BigEndian(data, (long) value)},
+        {typeof(ulong), (data, address, value) => BinaryPrimitives.WriteUInt64BigEndian(data, (ulong) value)},
+
+        {
+            typeof(float), (data, address, value) =>
+            {
+                var map = new UInt32SingleMap
+                {
+                    Single = (float) value
+                };
+
+                BinaryPrimitives.WriteUInt32BigEndian(data, map.UInt32);
+            }
+        },
+        {
+            typeof(double), (data, address, value) =>
+            {
+                var map = new UInt64DoubleMap
+                {
+                    Double = (double) value
+                };
+
+                BinaryPrimitives.WriteUInt64BigEndian(data, map.UInt64);
+            }
+        },
+
+        {
+            typeof(string), (data, address, value) =>
+            {
+                if (value is not string stringValue) throw new ArgumentException("Value must be of type string", nameof(value));
+
+                var length = Math.Min(address.Length, stringValue.Length);
+
+                switch (address.Type)
+                {
+                    case DbType.String:
+                        data[0] = (byte) address.Length;
+                        data[1] = (byte) length;
+
+                        // Todo: Serialize directly to Span, when upgrading to .net
+                        Encoding.ASCII.GetBytes(stringValue)
+                            .AsSpan(0, length)
+                            .CopyTo(data.Slice(2));
+                        return;
+                    case DbType.WString:
+                        BinaryPrimitives.WriteUInt16BigEndian(data, address.Length);
+                        BinaryPrimitives.WriteUInt16BigEndian(data.Slice(2), (ushort) length);
+
+                        // Todo: Serialize directly to Span, when upgrading to .net
+                        Encoding.BigEndianUnicode.GetBytes(stringValue)
+                            .AsSpan(0, length * 2)
+                            .CopyTo(data.Slice(4));
+                        return;
+                    case DbType.Byte:
+                        // Todo: Serialize directly to Span, when upgrading to .net
+                        Encoding.ASCII.GetBytes(stringValue)
+                            .AsSpan(0, length)
+                            .CopyTo(data);
+                        return;
+                    default:
+                        throw new DataTypeMissmatchException($"Cannot write string to {address.Type}", typeof(string), address);
+                }
+            }
+        }
+    };
+
+    private static readonly Dictionary<Type, ReadFunc> readFunctions = new()
     {
         {typeof(bool), (buffer, address) => (buffer[0] >> address.Bit & 1) > 0},
 
         {typeof(byte), (buffer, address) => buffer[0]},
-        {typeof(byte[]), (buffer, address) => buffer},
+        {typeof(byte[]), (buffer, address) => buffer.ToArray()},
 
         {typeof(short), (buffer, address) => BinaryPrimitives.ReadInt16BigEndian(buffer)},
         {typeof(ushort), (buffer, address) => BinaryPrimitives.ReadUInt16BigEndian(buffer)},
@@ -52,7 +145,7 @@ internal static class S7ValueConverter
                 {
                     DbType.String => ParseString(),
                     DbType.WString => ParseWString(),
-                    DbType.Byte => Encoding.ASCII.GetString(buffer),
+                    DbType.Byte => Encoding.ASCII.GetString(buffer.ToArray()),
                     _ => throw new DataTypeMissmatchException($"Cannot read string from {address.Type}", typeof(string), address)
                 };
 
@@ -74,7 +167,7 @@ internal static class S7ValueConverter
                     // https://support.industry.siemens.com/cs/mdm/109747174?c=94063855243&lc=de-DE
 
                     // the length of the string is two bytes per 
-                    var length = Math.Min(address.Length, BinaryPrimitives.ReadUInt16BigEndian(buffer.AsSpan(2,2))) * 2;
+                    var length = Math.Min(address.Length, BinaryPrimitives.ReadUInt16BigEndian(buffer.AsSpan(2, 2))) * 2;
 
                     return Encoding.BigEndianUnicode.GetString(buffer, 4, length);
                 }
@@ -86,6 +179,9 @@ internal static class S7ValueConverter
     {
         // Todo: Change to Span<byte> when switched to newer .net
 
+        if (buffer.Length < address.BufferLength)
+            throw new ArgumentException($"Buffer must be at least {address.BufferLength} bytes long for {address}", nameof(buffer));
+
         var type = typeof(TValue);
 
         if (!readFunctions.TryGetValue(type, out var readFunc))
@@ -98,79 +194,18 @@ internal static class S7ValueConverter
     public static void WriteToBuffer<TValue>(Span<byte> buffer, TValue value, S7VariableAddress address)
     {
         if (buffer.Length < address.BufferLength)
-            throw new ArgumentException($"buffer must be at least {address.BufferLength} bytes long for {address}", nameof(buffer));
+            throw new ArgumentException($"Buffer must be at least {address.BufferLength} bytes long for {address}", nameof(buffer));
 
-        if (typeof(TValue) == typeof(bool))
+        var type = typeof(TValue);
-        {
-            var byteValue = (bool) (object) value ? (byte) 1 : (byte) 0;
-            var shifted = (byte) (byteValue << address.Bit);
-            buffer[0] = shifted;
-        }
 
-        else if (typeof(TValue) == typeof(int))
+        if (!writeFunctions.TryGetValue(type, out var writeFunc))
-        {
+            throw new UnsupportedS7TypeException($"{type.Name} is not supported. {address}", type, address);
-            if (address.Length == 2)
-                BinaryPrimitives.WriteInt16BigEndian(buffer, (short) (int) (object) value);
-            else
-                BinaryPrimitives.WriteInt32BigEndian(buffer, (int) (object) value);
-        }
-        else if (typeof(TValue) == typeof(short))
-        {
-            if (address.Length == 2)
-                BinaryPrimitives.WriteInt16BigEndian(buffer, (short) (object) value);
-            else
-                BinaryPrimitives.WriteInt32BigEndian(buffer, (short) (object) value);
-        }
-        else if (typeof(TValue) == typeof(long))
-            BinaryPrimitives.WriteInt64BigEndian(buffer, (long) (object) value);
-        else if (typeof(TValue) == typeof(ulong))
-            BinaryPrimitives.WriteUInt64BigEndian(buffer, (ulong) (object) value);
-        else if (typeof(TValue) == typeof(byte))
-            buffer[0] = (byte) (object) value;
-        else if (typeof(TValue) == typeof(byte[]))
-        {
-            var source = (byte[]) (object) value;
 
-            var length = Math.Min(Math.Min(source.Length, buffer.Length), address.Length);
+        writeFunc(buffer, address, value);
-
-            source.AsSpan(0, length).CopyTo(buffer);
-        }
-        else if (typeof(TValue) == typeof(float))
-        {
-            var map = new UInt32SingleMap
-            {
-                Single = (float) (object) value
-            };
-
-            BinaryPrimitives.WriteUInt32BigEndian(buffer, map.UInt32);
-        }
-        else if (typeof(TValue) == typeof(string))
-        {
-            if (value is not string stringValue) throw new ArgumentException("Value must be of type string", nameof(value));
-
-            // Todo: Serialize directly to Span, when upgrading to .net
-            var stringBytes = Encoding.ASCII.GetBytes(stringValue);
-
-            var length = Math.Min(address.Length, stringValue.Length);
-
-            int stringOffset;
-            if (address.Type == DbType.String)
-            {
-                stringOffset = 2;
-                buffer[0] = (byte) address.Length;
-                buffer[1] = (byte) length;
-            }
-            else
-                stringOffset = 0;
-
-            stringBytes.AsSpan(0, length).CopyTo(buffer.Slice(stringOffset));
-        }
-        else
-        {
-            throw new InvalidOperationException($"type '{typeof(TValue)}' not supported.");
-        }
     }
 
+    delegate object ReadFunc(byte[] data, S7VariableAddress address);
+
     [StructLayout(LayoutKind.Explicit)]
     private struct UInt32SingleMap
     {
@@ -184,4 +219,6 @@ internal static class S7ValueConverter
         [FieldOffset(0)] public ulong UInt64;
         [FieldOffset(0)] public double Double;
     }
+
+    delegate void WriteFunc(Span<byte> data, S7VariableAddress address, object value);
 }