BinaryPayloadDecoder.decode_string(n) does not take care of byteorder and wordorder

[MrWaloo]

On a Modbus Ethernet device, I have 'stri' coded on 2 registers.

from pymodbus.client import ModbusTcpClient
from pymodbus.framer.socket_framer import ModbusSocketFramer

framer = ModbusSocketFramer
client = ModbusTcpClient(host='192.168.1.20', port=502, framer=framer, reconnect_delay=0)
client.connect()
response = client.read_holding_registers(address=12468, count=2)
client.close()

regs = response.registers

regs contains [29811, 26994]

I wrote a small test case:

from pymodbus.payload import (BinaryPayloadDecoder, BinaryPayloadBuilder)

def show_as_hex(registers):
  ret = ''
  for value in registers:
    ret += hex(value)[2:]
  return ret

regs = [29811, 26994]

print(f"show registers in hex: {show_as_hex(regs)}\n")

decoder = BinaryPayloadDecoder.fromRegisters(regs, '>', '>')
iBB = decoder.decode_32bit_int()
decoder = BinaryPayloadDecoder.fromRegisters(regs, '<', '>')
iLB = decoder.decode_32bit_int()
decoder = BinaryPayloadDecoder.fromRegisters(regs, '>', '<')
iBL = decoder.decode_32bit_int()
decoder = BinaryPayloadDecoder.fromRegisters(regs, '<', '<')
iLL = decoder.decode_32bit_int()

decoder = BinaryPayloadDecoder.fromRegisters(regs, '>', '>')
sBB = decoder.decode_string(4)
decoder = BinaryPayloadDecoder.fromRegisters(regs, '<', '>')
sLB = decoder.decode_string(4)
decoder = BinaryPayloadDecoder.fromRegisters(regs, '>', '<')
sBL = decoder.decode_string(4)
decoder = BinaryPayloadDecoder.fromRegisters(regs, '<', '<')
sLL = decoder.decode_string(4)

print(f"** BB **\nint32:  {hex(iBB)[2:]}\nstring: {sBB.hex()}\n")
print(f"** LB **\nint32:  {hex(iLB)[2:]}\nstring: {sLB.hex()}\n")
print(f"** BL **\nint32:  {hex(iBL)[2:]}\nstring: {sBL.hex()}\n")
print(f"** LL **\nint32:  {hex(iLL)[2:]}\nstring: {sLL.hex()}\n")

It returns:

show registers in hex: 74736972

** BB **
int32:  74736972
string: 74736972

** LB **
int32:  73747269
string: 74736972

** BL **
int32:  69727473
string: 74736972

** LL **
int32:  72697374
string: 74736972

The strings are all the same, the decode_string() function does not take care of the byte and word endianess.
Is this a desired behavior?

My problem is that decoder.decode_string(4) always returns tsir instead of stri and there is no way to decode it correctly.

[janiversen]

Did you read the documentation? you can specify the order you want.

Please have a look at our examples too, they can show you how to use the methods we provide.

[MrWaloo]

So every data types are decoded according to the parameterized endianess but the strings? OK then.

My problem is that the strings are sent with a byte swap, so I guess I have to manage strings differently than all other data types?

[janiversen]

Feel free to submit a pull request.

[MrWaloo]

Seems you did not read the documentation, the parameter for decode_32 is "registers", which pr standard are 16 bit integers and not a string.

decode_string combines to registers to a string, there are no conversion as pr endian definition.

I did a search for decode_string on the documentation: https://pymodbus.readthedocs.io/en/latest/index.html (the only documentation I know about pymodbus)
It returns only 2 hits and the fact that the endianess is ignored for the strings did not appear. Can you tell me from which documentation you are talking about please? It looks like I don't know all the documentations...

---

I will consider the option of making a PR. The swapping should be optional and not done by default for the backward compatibility, I assume. Should this point be discussed in a different discussion?

[janiversen]

Look here, that is the definition of the call, decode_string do not ignore parameters it is simply not defined.

What do you mean ! parameters that are not positional are optional.

[MrWaloo]

For decode_32bit_int, for example, it is not defined either. It is defined in the class call: decoder = BinaryPayloadDecoder.fromRegisters(regs, '>', '>') and then the payload can be decoded with the specified endianess. I am just wondering and asking why the strings are not decoded according to the endianess, like every other variable types.
It is just a question, not an aggression. My goal is to understand and eventually to help changing decode_string according to my needs. I probably am not the only one needing this behavior (?).
If this is the desired behavior, I propose to add an optional parameter to change the behavior of decode_string() and decode according to the class endianess. The optional parameter could be something like with_endianess, force_endianess or use_endianess. What do you think about it?

[janiversen]

I think your suggestion is overly complicated, and in will cause conflict with the from_register call....all you need is the parameter convert: bool.

[jelbin313]

I am also having this issue. It seems to be happening with the BinaryPayloadBuilder as well as the BinaryPayloadDecoder.

Here is some code you can run to test how the BinaryPayloadBuilder and BinaryPayloadDecoder handle endianness of strings:

from pymodbus.payload import BinaryPayloadDecoder, BinaryPayloadBuilder
from pymodbus.constants import Endian

"""All of these tests are done using the string 'yellow'
The bytes for the chars in the string "yellow" are as follows:
y: 01111001 e: 01100101 l: 01101100 o: 01101111 w: 01110111
Each register holds 16 bits, or two bytes, or two chars"""

#############################################################################################################

print("BinaryPayloadDecoder Tests:")
"""The decoder tests are done as if the string is stored on a modbus device in the endianness specified.
As long as the corrrect endianness is specified, we should be able to decode the string back to "yellow", 
regardless of what order it is stored in in teh registers."""

print("word endianness big, byte endianness big")
print("we expect the string to be 'yellow'")
# the registers contain [ye, ll, ow] or [0111100101100101, 0110110001101100, 0110111101110111]
# in decimal that is [31077, 27756, 28535]
registers = [31077, 27756, 28535]
decoder = BinaryPayloadDecoder.fromRegisters(
    registers, byteorder=Endian.BIG, wordorder=Endian.BIG
)
decoded_string = decoder.decode_string(size=6)
decoded_string = decoded_string.decode("utf-8")
print(decoded_string)

print("\nword endianness big, byte endianness little")
print("we expect the string to be 'yellow'")
# word endianness big, byte endianness little
# the registers contain [ey, ll, wo] or [0110010101111001, 0110110001101100, 0111011101101111]
registers = [25977, 27756, 30575]
decoder = BinaryPayloadDecoder.fromRegisters(
    registers, byteorder=Endian.LITTLE, wordorder=Endian.BIG
)
decoded_string = decoder.decode_string(size=6)
decoded_string = decoded_string.decode("utf-8")
print(decoded_string)

print("\nword endianness little, byte endianness big")
print("we expect the string to be 'yellow'")
# word endianness little, byte endianness big
# the registers contain [ow, ll, ye] or [0110111101110111, 0110110001101100, 0111100101100101]
registers = [28535, 27756, 31077]
decoder = BinaryPayloadDecoder.fromRegisters(
    registers, byteorder=Endian.BIG, wordorder=Endian.LITTLE
)
decoded_string = decoder.decode_string(size=6)
decoded_string = decoded_string.decode("utf-8")
print(decoded_string)

print("\nword endianness little, byte endianness little")
print("we expect the string to be 'yellow'")
# word endianness little, byte endianness little
# the registers contain [wo, ll, ey] or [0111011101101111, 0110110001101100, 0110010101111001]
registers = [30575, 27756, 25977]
decoder = BinaryPayloadDecoder.fromRegisters(
    registers, byteorder=Endian.LITTLE, wordorder=Endian.LITTLE
)
decoded_string = decoder.decode_string(size=6)
decoded_string = decoded_string.decode("utf-8")
print(decoded_string)

#############################################################################################################

print("\nBinaryPayloadBuilder Tests:")
"""The builder tests are done as if the string must be written to a modbus device in the endianness specified.
We want to make sure that the sring gets encoded in the right order."""

yellow = "yellow"

print("word endianness big, byte endianness big")
print("we expect the registers to be [31077, 27756, 28535]")
encoder = BinaryPayloadBuilder(byteorder=Endian.BIG, wordorder=Endian.BIG)
encoder.add_string(yellow)
print(encoder.to_registers())

print("\nword endianness big, byte endianness little")
print("we expect the registers to be [25977, 27756, 30575]")
encoder = BinaryPayloadBuilder(byteorder=Endian.LITTLE, wordorder=Endian.BIG)
encoder.add_string(yellow)
print(encoder.to_registers())

print("\nword endianness little, byte endianness big")
print("we expect the registers to be [28535, 27756, 31077]")
encoder = BinaryPayloadBuilder(byteorder=Endian.BIG, wordorder=Endian.LITTLE)
encoder.add_string(yellow)
print(encoder.to_registers())

print("\nword endianness little, byte endianness little")
print("we expect the registers to be [30575, 27756, 25977]")
encoder = BinaryPayloadBuilder(
    byteorder=Endian.LITTLE, wordorder=Endian.LITTLE
)
encoder.add_string(yellow)
print(encoder.to_registers())

When I run this, it returns:

BinaryPayloadDecoder Tests:
word endianness big, byte endianness big
we expect the string to be 'yellow'
yellow

word endianness big, byte endianness little
we expect the string to be 'yellow'
eyllwo

word endianness little, byte endianness big
we expect the string to be 'yellow'
owllye

word endianness little, byte endianness little
we expect the string to be 'yellow'
wolley

BinaryPayloadBuilder Tests:
word endianness big, byte endianness big
we expect the registers to be [31077, 27756, 28535]
[31077, 27756, 28535]

word endianness big, byte endianness little
we expect the registers to be [25977, 27756, 30575]
[31077, 27756, 28535]

word endianness little, byte endianness big
we expect the registers to be [28535, 27756, 31077]
[31077, 27756, 28535]

word endianness little, byte endianness little
we expect the registers to be [30575, 27756, 25977]
[31077, 27756, 28535]

Is this the correct behavior?

[jelbin313]

I have read the client and server payload examples. As far as I can tell I am doing things the same way here as they are done there.
I don't think I understand what you mean when you say that the registers will be returned in the word order of the CPU. Isn't the intention to encode/decode the registers in the endianness specified when the user specifies an endianness? What is the purpose of specifying endianness if it always gets determined automatically? I thought Enum.AUTO was meant to be used when you want to allow the computer to determine the endianness automatically.
Also, specifying the endianness seems to work for ints, uints, and floats, so I don't understand why it doesn't work for strings?

[janiversen]

Maybe because strings do not have a notation of endianess.

[janiversen]

Follow the documentation and the examples, I do not think I can give you more advice....if you want to dig deeper read the modbus standard (there is a copy referenced in the README).

[jelbin313]

Oh, ok, thanks for letting me know. The documentation doesn't say that strings are handled differently than numerical data, so maybe you should specify that, as the current documentation seems to imply that strings can be decoded/encoded with different endianness.
For anyone who is dealing with a modbus device that does store the bytes of string data in a weird order, a workaround might be to interpret the bytes that make up the string as 8-bit ints, then put them in the right order, then interpret them as a string. I think I'm going to try to do something like that as a solution.

[janiversen]

strings can be encoded like all other data ! it depends on your device whether it is needed or not.