kiug / QModbus

Modbus library (http://libmodbus.org) wrapper for Qt framework.
GNU Lesser General Public License v3.0
19 stars 13 forks source link

Encoding bug on little endian systems -> consistently wrong byteswap when writing modbus float32 #3

Open ghorwin opened 8 months ago

ghorwin commented 8 months ago

Hi,

just wanted to let you know that you have a consistent bug when writing Float32 and Int32 encoded values into Modbus on little endian systems (linux etc.). This effectively causes Float32_abcd to be written as cdab encoding into modbus stream.

Here is the problem:

void QModbusRegisters::setFloat32 (unsigned int index, float value)
{
    if (size () <= (int)index + 1) {
        modbusError.set (OUT_OF_RANGE_ENO);
    }
    else {
        modbusError.clear ();
    }
    twoRegistersRepresentation valueRep;
    valueRep.float32 = value;
    operator [] (index) = valueRep.component[0];
    operator [] (index + 1) = valueRep.component[1];
}

The problem is the use of the union twoRegistersRepresentation.

Example: Float value 20.1 has binary expression in big endian as:

0x41a0147b

(see https://www.h-schmidt.net/FloatConverter/IEEE754.html)

where 0x41a0 encodes 20, and 0x137b encodes .1

This is abcd notation. The setFloat32() code, however, writes as cdab (i.e. results in 0x147b41a0 in the Modbus byte stream). The Modbus standard, however, requires abcd (big endian) encoding to be used in the byte stream.

This should be fixed, or clarified by adding a suffix setFloat32_cdab

kiug commented 7 months ago

Hi, thank you for your report. What about the Float64 type? There should be also swap: abcdefgh -> ghefcdab?

ghorwin commented 7 months ago

The MODBUS standard says "Big endian" in byte stream. Any other encoding should be noted by the device manufacturerer. As far as I know, there are different formats, see

https://en.wikipedia.org/wiki/Double-precision_floating-point_format

for examples. Best would be, if values written by QModbus could be checked and confirmed with a third-party tool (Mobdus Poll/Slave or similar) so that the encoding is labeled correctly. As long as users of QModbus know, which encoding is used by setFloat64(), all is ok (maybe adding that info to the documentation would be enough?).

ghorwin commented 7 months ago

Hi again, I've given this some thought: I guess, the reason for the different byte stream encodings used by Modbus devices lies in the naiive storage of native memory layout to modbus registers. As a modbus register is always 16 bit (2 byte) wide, the native memory is then passed to modbus byte stream in such 2 byte blocks. With 64bit values there are a number of possible combinations, like:

abcdefgh cdabghef ghefcdab

or even

hgfedsba

and further mutation of the 2-byte words.

Supporting all these in QModbus wouldn't be meaningful. I guess, Modbus devices with non-standard encodings need to document their encodings or users must use a modbus diagnose tool to identify the encoding. I'd suggest to extend QModbus to support the most relevant flavors, that is:

Question: should QModbus support also native big endian architectures (some ARM cores etc.) or just x86 platforms? If you want to be platform independent, the code needs to be changed (see my pull request of libmodbus https://github.com/stephane/libmodbus/pull/770)