Bug when converting pymodbus.DATATYPE.FLOAT32 to float

[dnshshkr]

Versions

• Python: 3.11.9
• OS: Windows
• Pymodbus: 3.6.8
• Modbus Hardware (if used): Keyence KV-7500

Pymodbus Specific

• Client: tcp

Description

This is my code with the library

from pymodbus.client import ModbusTcpClient as mb

client = mb('10.0.0.222')
client.connect()
result = client.read_holding_registers(4500, 2, 1)
num = mb.convert_from_registers(result.registers, mb.DATATYPE.FLOAT32)
print(num)
client.close()

In /pymodbus/client/mixin.py under convert_from_registers()

Code

    @classmethod
    def convert_from_registers(
        cls, registers: list[int], data_type: DATATYPE
    ) -> int | float | str:
        """Convert registers to int/float/str.

        :param registers: list of registers received from e.g. read_holding_registers()
        :param data_type: data type to convert to
        :returns: int, float or str depending on "data_type"
        :raises ModbusException: when size of registers is not 1, 2 or 4
        """
        byte_list = bytearray()
        for x in registers:
            byte_list.extend(int.to_bytes(x, 2, "big"))
        if data_type == cls.DATATYPE.STRING:
            if byte_list[-1:] == b"\00":
                byte_list = byte_list[:-1]
            return byte_list.decode("utf-8")
        if len(registers) != data_type.value[1]:
            raise ModbusException(
                f"Illegal size ({len(registers)}) of register array, cannot convert!"
            )
        return struct.unpack(f">{data_type.value[0]}", byte_list)[0]

PLC floating point numbers use either double words (DWORD) or quad words (QWORD) and they are interpreted in reversed manner eg a case for DWORD: DM0 = 0x5678, DM1 = 0x1234. If we interpret these 2 DMs as a HEX 32 bit number, then it should be 0x12345678 and the same goes for a floating number be it 32bit or 64bit. So, it is necessary to rearrange the registers in the code above in a reversed manner. I did some change to your code and it seemed to return the correct floating number that I put in my PLC. Below is the corrected code:

    @classmethod
    def convert_from_registers(
        cls, registers: list[int], data_type: DATATYPE
    ) -> int | float | str:
        """Convert registers to int/float/str.

        :param registers: list of registers received from e.g. read_holding_registers()
        :param data_type: data type to convert to
        :returns: int, float or str depending on "data_type"
        :raises ModbusException: when size of registers is not 1, 2 or 4
        """
        byte_list = bytearray()
        for x in reversed(registers): #change i made
            byte_list.extend(int.to_bytes(x, 2, "big"))
        if data_type == cls.DATATYPE.STRING:
            if byte_list[-1:] == b"\00":
                byte_list = byte_list[:-1]
            return byte_list.decode("utf-8")
        if len(registers) != data_type.value[1]:
            raise ModbusException(
                f"Illegal size ({len(registers)}) of register array, cannot convert!"
            )
        return struct.unpack(f">{data_type.value[0]}", byte_list)[0]

[janiversen]

Pull requests are welcome.

please remember to add tests that proves the functionality.

A simpler solution could be to change the unpack instruction for float64.

A quick test indicates that your solution might be wrong for int32, and possibly also for strings.

[dnshshkr]

Yes you are correct. I tested with strings and the result is reversed. I might need to add some special instruction for float32.

[janiversen]

struct.unpack should do this correctly (if using the correct unpack instruction).

We test float32, but I am not sure if we test float64...I will look later.

[janiversen]

please have a look at test_client.py, line 557++.

We do test all datatypes, so please start to see if that test is wrong (which I do not believe).

[janiversen]

According to this https://docs.python.org/3/library/struct.html we do convert correctly ("d" for float64 and "f" for float32)

[janiversen]

I have just added a test, that shows that the conversion is correct.

Please do not forget that on the wire the modbus protocol uses "big endian", so if you are testing with a "little endian" CPU you see different hex sequence.

If you still have a problem, then please use our debug log call, and show the log, that will show exactly what we receive.