Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ghorwin opened this issue Feb 6, 2024 · 3 comments

Comments

@ghorwin
Copy link
Contributor

ghorwin commented Feb 6, 2024

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
Copy link
Owner

kiug commented Feb 8, 2024

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

@ghorwin
Copy link
Contributor Author

ghorwin commented Feb 8, 2024

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
Copy link
Contributor Author

ghorwin commented Feb 9, 2024

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:

  • correct Modbus big-endian
  • big-endian byte swap (your example)

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 stephane/libmodbus#770)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants