fix #39 Write Single Register (FC=06) value validation not allowing u…

…signed int16 values to be used
aldas · Sep 17, 2019 · 3d0076e · 3d0076e
1 parent 0d1e8fc
commit 3d0076e
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 12 deletions.
diff --git a/src/Packet/ModbusFunction/WriteSingleRegisterRequest.php b/src/Packet/ModbusFunction/WriteSingleRegisterRequest.php
@@ -32,10 +32,11 @@ public function __construct(int $startAddress, int $value, int $unitId = 0, int
     public function validate()
     {
         parent::validate();
-        if ((null !== $this->value) && (($this->value >= Types::MIN_VALUE_INT16) && ($this->value <= Types::MAX_VALUE_INT16))) {
+        // value is 2 bytes in packet so it must be set and in range of uint16 (0 - 65535) or int16 (-32768 - +32767)
+        if ((null !== $this->value) && (($this->value >= Types::MIN_VALUE_INT16) && ($this->value <= Types::MAX_VALUE_UINT16))) {
             return;
         }
-        throw new InvalidArgumentException("value is not set or out of range (int16): {$this->value}");
+        throw new InvalidArgumentException("value is not set or out of range (u)int16: {$this->value}");
     }
 
     public function getFunctionCode(): int
@@ -61,4 +62,4 @@ protected function getLengthInternal(): int
     {
         return parent::getLengthInternal() + 2; // value size (2 bytes)
     }
-}
+}
diff --git a/tests/unit/Packet/ModbusFunction/WriteSingleRegisterRequestTest.php b/tests/unit/Packet/ModbusFunction/WriteSingleRegisterRequestTest.php
@@ -1,8 +1,10 @@
 <?php
+
 namespace Tests\Packet\ModbusFunction;
 
-use ModbusTcpClient\Packet\ModbusPacket;
+use ModbusTcpClient\Exception\InvalidArgumentException;
 use ModbusTcpClient\Packet\ModbusFunction\WriteSingleRegisterRequest;
+use ModbusTcpClient\Packet\ModbusPacket;
 use PHPUnit\Framework\TestCase;
 
 class WriteSingleRegisterRequestTest extends TestCase
@@ -15,16 +17,30 @@ public function testPacketToString()
         );
     }
 
+    public function validationFailureProvider(): array
+    {
+        return [
+            'validation fails over max uint16"' => [65536, 'value is not set or out of range (u)int16: 65536'],
+            'validation fails below min int16"' => [-32769, 'value is not set or out of range (u)int16: -32769'],
+            'validation success at min int16"' => [-32768, ''],
+            'validation success at max uint16"' => [65535, ''],
+            'validation success at 65534 (between ok range)"' => [65534, ''],
+        ];
+    }
+
     /**
-     * @expectedException \ModbusTcpClient\Exception\InvalidArgumentException
-     * @expectedExceptionMessage value is not set or out of range (int16): 213213123
+     * @dataProvider validationFailureProvider
      */
-    public function testValueValidationException()
+    public function testValueValidationException($value, $expectedMessage)
     {
-        $this->assertEquals(
-            "\x00\x01\x00\x00\x00\x06\x11\x06\x00\x6B\x01\x01",
-            (new WriteSingleRegisterRequest(107, 213213123, 17, 1))->__toString()
-        );
+        try {
+            $r = new WriteSingleRegisterRequest(107, $value, 17, 1);
+            $this->assertEquals($value, $r->getValue());
+        } catch (\Exception $e) {
+            $this->expectException(InvalidArgumentException::class);
+            $this->expectExceptionMessage($expectedMessage);
+            throw $e;
+        }
     }
 
     public function testValueValidationValidForNegative1()
@@ -50,4 +66,4 @@ public function testPacketProperties()
         $this->assertEquals(17, $header->getUnitId());
     }
 
-}
+}