#171 Minor bug fix in BinaryPayloadBuilder.add_string method for python3

CHANGELOG.rst

@@ -3,6 +3,7 @@ Version 1.3.0.rc2
 ------------------------------------------------------------
 * Fix encoding problem for ReadDeviceInformationRequest method on python3
 * Fix problem with the usage of ord in python3 while cleaning up receive buffer
+* Fix struct unpack errors with BinaryPayloadDecoder on python3 - string vs bytestring error
 * Calculate expected response size for ReadWriteMultipleRegistersRequest
 * Enhancement for ModbusTcpClient, ModbusTcpClient can now accept connection timeout as one of the parameter
 * Misc updates

examples/common/modbus-payload.py

@@ -2,6 +2,8 @@
 '''
 Pymodbus Payload Building/Decoding Example
 --------------------------------------------------------------------------
+
+# Run modbus-payload-server.py or synchronous-server.py to check the behavior
 '''
 from pymodbus.constants import Endian
 from pymodbus.payload import BinaryPayloadDecoder
@@ -35,14 +37,14 @@
 # - an 8 bit int 0x12
 # - an 8 bit bitstring [0,1,0,1,1,0,1,0]
 #---------------------------------------------------------------------------# 
-builder = BinaryPayloadBuilder(endian=Endian.Little)
+builder = BinaryPayloadBuilder(endian=Endian.Big)
 builder.add_string('abcdefgh')
 builder.add_32bit_float(22.34)
 builder.add_16bit_uint(0x1234)
 builder.add_8bit_int(0x12)
 builder.add_bits([0,1,0,1,1,0,1,0])
 payload = builder.build()
-address = 0x01
+address = 0
 result  = client.write_registers(address, payload, skip_encode=True, unit=1)
 
 #---------------------------------------------------------------------------# 
@@ -58,10 +60,10 @@
 # - an 8 bit int 0x12
 # - an 8 bit bitstring [0,1,0,1,1,0,1,0]
 #---------------------------------------------------------------------------# 
-address = 0x01
+address = 0x00
 count   = 8
-result  = client.read_input_registers(address, count,  unit=1)
-decoder = BinaryPayloadDecoder.fromRegisters(result.registers, endian=Endian.Little)
+result  = client.read_holding_registers(address, count,  unit=1)
+decoder = BinaryPayloadDecoder.fromRegisters(result.registers, endian=Endian.Big)
 decoded = {
     'string': decoder.decode_string(8),
     'float': decoder.decode_32bit_float(),

examples/common/synchronous-server.py

@@ -105,7 +105,7 @@
 # run the server you want
 #---------------------------------------------------------------------------# 
 # Tcp:
-# StartTcpServer(context, identity=identity, address=("localhost", 5020))
+StartTcpServer(context, identity=identity, address=("localhost", 5020))
 
 # Udp:
 #StartUdpServer(context, identity=identity, address=("localhost", 502))
@@ -114,4 +114,4 @@
 #StartSerialServer(context, identity=identity, port='/dev/pts/3', timeout=1)
 
 # RTU:
-StartSerialServer(context, framer=ModbusRtuFramer, identity=identity, port='/dev/ptyp0', timeout=.005, baudrate=9600)
+# StartSerialServer(context, framer=ModbusRtuFramer, identity=identity, port='/dev/ptyp0', timeout=.005, baudrate=9600)

pymodbus/payload.py

@@ -10,6 +10,7 @@
 from pymodbus.constants import Endian
 from pymodbus.utilities import pack_bitstring
 from pymodbus.utilities import unpack_bitstring
+from pymodbus.utilities import make_byte_string
 from pymodbus.exceptions import ParameterException
 
 
@@ -30,7 +31,7 @@ class BinaryPayloadBuilder(IPayloadBuilder):
     def __init__(self, payload=None, endian=Endian.Little):
         ''' Initialize a new instance of the payload builder
 
-        :param payload: Raw payload data to initialize with
+        :param payload: Raw binary payload data to initialize with
         :param endian: The endianess of the payload
         '''
         self._payload = payload or []
@@ -175,6 +176,7 @@ def add_string(self, value):
 
         :param value: The value to add to the buffer
         '''
+        value = make_byte_string(value)
         fstring = self._endian + str(len(value)) + 's'
         self._payload.append(pack(fstring, value))
 
@@ -246,14 +248,16 @@ def decode_8bit_uint(self):
         self._pointer += 1
         fstring = self._endian + 'B'
         handle = self._payload[self._pointer - 1:self._pointer]
+        handle = make_byte_string(handle)
         return unpack(fstring, handle)[0]
 
     def decode_bits(self):
         ''' Decodes a byte worth of bits from the buffer
         '''
         self._pointer += 1
-        fstring = self._endian + 'B'
+        # fstring = self._endian + 'B'
         handle = self._payload[self._pointer - 1:self._pointer]
+        handle = make_byte_string(handle)
         return unpack_bitstring(handle)
 
     def decode_16bit_uint(self):
@@ -262,6 +266,7 @@ def decode_16bit_uint(self):
         self._pointer += 2
         fstring = self._endian + 'H'
         handle = self._payload[self._pointer - 2:self._pointer]
+        handle = make_byte_string(handle)
         return unpack(fstring, handle)[0]
 
     def decode_32bit_uint(self):
@@ -270,6 +275,7 @@ def decode_32bit_uint(self):
         self._pointer += 4
         fstring = self._endian + 'I'
         handle = self._payload[self._pointer - 4:self._pointer]
+        handle = make_byte_string(handle)
         return unpack(fstring, handle)[0]
 
     def decode_64bit_uint(self):
@@ -278,6 +284,7 @@ def decode_64bit_uint(self):
         self._pointer += 8
         fstring = self._endian + 'Q'
         handle = self._payload[self._pointer - 8:self._pointer]
+        handle = make_byte_string(handle)
         return unpack(fstring, handle)[0]
 
     def decode_8bit_int(self):
@@ -286,6 +293,7 @@ def decode_8bit_int(self):
         self._pointer += 1
         fstring = self._endian + 'b'
         handle = self._payload[self._pointer - 1:self._pointer]
+        handle = make_byte_string(handle)
         return unpack(fstring, handle)[0]
 
     def decode_16bit_int(self):
@@ -294,6 +302,7 @@ def decode_16bit_int(self):
         self._pointer += 2
         fstring = self._endian + 'h'
         handle = self._payload[self._pointer - 2:self._pointer]
+        handle = make_byte_string(handle)
         return unpack(fstring, handle)[0]
 
     def decode_32bit_int(self):
@@ -302,6 +311,7 @@ def decode_32bit_int(self):
         self._pointer += 4
         fstring = self._endian + 'i'
         handle = self._payload[self._pointer - 4:self._pointer]
+        handle = make_byte_string(handle)
         return unpack(fstring, handle)[0]
 
     def decode_64bit_int(self):
@@ -310,6 +320,7 @@ def decode_64bit_int(self):
         self._pointer += 8
         fstring = self._endian + 'q'
         handle = self._payload[self._pointer - 8:self._pointer]
+        handle = make_byte_string(handle)
         return unpack(fstring, handle)[0]
 
     def decode_32bit_float(self):
@@ -318,6 +329,7 @@ def decode_32bit_float(self):
         self._pointer += 4
         fstring = self._endian + 'f'
         handle = self._payload[self._pointer - 4:self._pointer]
+        handle = make_byte_string(handle)
         return unpack(fstring, handle)[0]
 
     def decode_64bit_float(self):
@@ -326,6 +338,7 @@ def decode_64bit_float(self):
         self._pointer += 8
         fstring = self._endian + 'd'
         handle = self._payload[self._pointer - 8:self._pointer]
+        handle = make_byte_string(handle)
         return unpack(fstring, handle)[0]
 
     def decode_string(self, size=1):

pymodbus/utilities.py

@@ -5,8 +5,8 @@
 A collection of utilities for packing data, unpacking
 data computing checksums, and decode checksums.
 '''
-from pymodbus.compat import int2byte, byte2int
-
+from pymodbus.compat import int2byte, byte2int, IS_PYTHON3
+from six import string_types
 
 #---------------------------------------------------------------------------#
 # Helpers
@@ -93,6 +93,15 @@ def unpack_bitstring(string):
     return bits
 
 
+def make_byte_string(s):
+    """
+    Returns byte string from a given string, python3 specific fix
+    :param s: 
+    :return: 
+    """
+    if IS_PYTHON3 and isinstance(s, string_types):
+        s = s.encode()
+    return s
 #---------------------------------------------------------------------------#
 # Error Detection Functions
 #---------------------------------------------------------------------------#

test/test_payload.py

@@ -79,7 +79,7 @@ def testBigEndianPayloadBuilder(self):
         builder.add_64bit_int(-4)
         builder.add_32bit_float(1.25)
         builder.add_64bit_float(6.25)
-        builder.add_string(b'test')
+        builder.add_string('test')
         builder.add_bits(self.bitstring)
         self.assertEqual(self.big_endian_payload, builder.to_string())
 
@@ -96,6 +96,18 @@ def testPayloadBuilderReset(self):
         self.assertEqual(b'', builder.to_string())
         self.assertEqual([], builder.build())
 
+    def testPayloadBuilderWithRawPayload(self):
+        ''' Test basic bit message encoding/decoding '''
+        builder = BinaryPayloadBuilder([b'\x12', b'\x34', b'\x56', b'\x78'])
+        self.assertEqual(b'\x12\x34\x56\x78', builder.to_string())
+        self.assertEqual([13330, 30806], builder.to_registers())
+
+        builder = BinaryPayloadBuilder([b'\x12', b'\x34', b'\x56', b'\x78'],
+                                       endian=Endian.Big)
+        self.assertEqual(b'\x12\x34\x56\x78', builder.to_string())
+        self.assertEqual([4660, 22136], builder.to_registers())
+        self.assertEqual('\x12\x34\x56\x78', str(builder))
+
     #-----------------------------------------------------------------------#
     # Payload Decoder Tests
     #-----------------------------------------------------------------------#
@@ -113,7 +125,7 @@ def testLittleEndianPayloadDecoder(self):
         self.assertEqual(-4,     decoder.decode_64bit_int())
         self.assertEqual(1.25,   decoder.decode_32bit_float())
         self.assertEqual(6.25,   decoder.decode_64bit_float())
-        self.assertEqual(b'test', decoder.decode_string(4))
+        self.assertEqual('test', decoder.decode_string(4).decode())
         self.assertEqual(self.bitstring, decoder.decode_bits())
 
     def testBigEndianPayloadDecoder(self):