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

Simulator with defalut config gives back different byte order between float32 and uint32 #1661

Closed
fhydralisk opened this issue Jul 17, 2023 · 9 comments

Comments

@fhydralisk
Copy link
Contributor

Versions

  • Python: 3.9.x
  • OS: macos 13.4
  • Pymodbus: 3.2.2
  • Modbus Hardware (if used): None

Pymodbus Specific

  • Server: tcp, simulator
  • Client: telegraf

Description

Hi, I run my pymodbus simulator by starting main.py in pymodbus/datastore/simulator with default config file and started a telegraf program which fetches data from this pymodbus server.

We can see in config that register of address [04, 05] holds value of uint32: 617001 and [06, 07] holds float32: 404.17.
The telegraf(client or master) side is configured to fetch uint32 value from 04, 05 address and float32 value from 06, 07. It is also configured to use BIG ENDIAN byte order to decode fetched data.

What I expected is that both of uint32 value and float32 value are fetched correctly, or at least, all of them are fetched incorrectly. However, the result shows that uint32 is correctly fetched with decoded value of 617001 and the float32 value is decoded with wrong value of -299.5801696777344.

I did some study and found that bytes [195 149 202 67] represent 404.17 in little endian byte order. Is this a feature or a bug?

Sorry for my bad Englsih, hope someone can understand what I'm talking about.

Telegraf Logs

2023/07/17 17:12:10 D! [inputs.modubs] Reading slave 1 for tcp://localhost:5020...
2023/07/17 17:12:10 D! [inputs.modubs] trying to read input@4[4]...
2023/07/17 17:12:10 D! [inputs.modubs] got input@4[4]: [0 9 106 41 195 149 202 67]
2023/07/17 17:12:10 D! [inputs.modubs]   field int_test with offset 0 with len 4: [0 9 106 41] --> 617001
2023/07/17 17:12:10 D! [inputs.modubs]   field float_test with offset 4 with len 4: [195 149 202 67] --> -299.5801696777344
@janiversen
Copy link
Collaborator

Modbus always transmit in big endian, the simulator convert the value locally if the CPU uses little endian.

Please make a debug log in the simulator, so we can see what is actually transmitted. all types of 32bit should be converted (if needed).

@fhydralisk
Copy link
Contributor Author

FYI

(pymodbus) localhost:pymodbus hydra$ python pymodbus/server/simulator/main.py --log debug
2023-07-17 18:45:19,380 INFO  logging:96 Start simulator
2023-07-17 18:45:19,507 INFO  logging:96 Modbus server started
Executing <Task pending name='Task-1' coro=<run_main() running at /Users/hydra/Documents/coding/pymodbus/pymodbus/server/simulator/main.py:116> wait_for=<_GatheringFuture pending cb=[<TaskWakeupMethWrapper object at 0x1068588b0>()] created at /Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/tasks.py:706> cb=[_run_until_complete_cb() at /Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/base_events.py:184] created at /Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/base_events.py:621> took 0.128 seconds
2023-07-17 18:45:19,508 INFO  logging:96 HTTP server started
2023-07-17 18:45:19,508 INFO  logging:96 Server(TCP) listening.
2023-07-17 18:45:26,442 DEBUG logging:102 Socket [('127.0.0.1', 5020)] opened
2023-07-17 18:45:26,443 DEBUG logging:102 TCP client connection established [('127.0.0.1', 61749)]
2023-07-17 18:45:26,445 DEBUG logging:102 Handling data: 0x0 0x1 0x0 0x0 0x0 0x6 0x1 0x4 0x0 0x4 0x0 0x4
2023-07-17 18:45:26,445 DEBUG logging:102 Processing: 0x0 0x1 0x0 0x0 0x0 0x6 0x1 0x4 0x0 0x4 0x0 0x4
2023-07-17 18:45:26,446 DEBUG logging:102 Factory Request[ReadInputRegistersRequest': 4]
2023-07-17 18:45:26,446 DEBUG logging:102 send: [ReadInputRegistersResponse (4)]- b'00010000000b01040800096a29c395ca43'
2023-07-17 18:45:27,453 DEBUG logging:102 Handler for stream [('127.0.0.1', 61749)] has been canceled
2023-07-17 18:45:27,453 DEBUG logging:102 TCP client disconnected [('127.0.0.1', 61749)]

@janiversen
Copy link
Collaborator

Just controlled the code (pymodbus/datastore/simulator.py#764) we handle int32 and float32 identical.

@janiversen
Copy link
Collaborator

janiversen commented Jul 17, 2023

The log only shows one read not a float and a int read.

Furthermore ReadInputRegisters are bit registers and thus not converted extra. The values were converted during setup.

@fhydralisk
Copy link
Contributor Author

fhydralisk commented Jul 17, 2023

Thanks for your information.
So I think the issue is because pymodbus simulator and modbus plugin of Telegraf, which is a popular data collector used by influxDB, were incompatible.

Telegraf modbus plugin's configuration file is like below:

  [[inputs.modbus.request]]
    ## Holding example
    ## All of those examples will result in FLOAT64 field outputs
    slave_id = 1
    byte_order = "ABCD"  # Which means BIG ENDIAN
    register = "holding"
    fields = [
      { address=0, name="voltage",      type="INT16",   scale=0.1   },
      { address=1, name="current",      type="INT32",   scale=0.001 },
      { address=3, name="power",        type="UINT32",  omit=true   },
      { address=5, name="energy",       type="FLOAT32", scale=0.001, measurement="W" },
      { address=7, name="frequency",    type="UINT32",  scale=0.1   },
      { address=8, name="power_factor", type="INT64",   scale=0.01  },
    ]

From your information, I guess that modbus-telegraf reads slave's registry byte by byte and assumes that the slave's reg stores data, no matter uint32 or float32, in BIG ENDIAN(byte_order='ABCD') or in LITTLE ENDIAN(byte_order='DCBA'). However, in pymodbus, that may not be true and depends on the host of pymodbus's CPU or memory layout or some kind of stuffs.

@janiversen
Copy link
Collaborator

Modbus do not work with bytes !! it works with registers each 16bit.

Modbus defines that on the wire data are transferred as BIG ENDIAN. Normal clients/servers check the CPU and if little endian converts. Pymodbus does this automatically for server (and simulator) as well as client.

@fhydralisk
Copy link
Contributor Author

Modbus do not work with bytes !! it works with registers each 16bit.

Sorry my mistake. I just wonder why uint32 and float32 gives different results in this case. Maybe I should test it on other platform/os and should dig deeper in Telegraf and pymodbus.

@janiversen
Copy link
Collaborator

Closing for now, as it does not seem to be a pymodbus problem. If you investigate further and see that pymodbus transmit data wrongly, then please write us a message here with a debug log.

@fhydralisk fhydralisk reopened this Jul 18, 2023
fhydralisk added a commit to fhydralisk/pymodbus that referenced this issue Jul 18, 2023
@fhydralisk
Copy link
Contributor Author

fhydralisk commented Jul 18, 2023

After digging into source code of pymodbus, I figured out a way to fix my issue and opened a PR. Please review, thanks.

Code causing my issue is located at pymodbus/datastore/simulator.py#770 & pymodbus/datastore/simulator.py#784, where use struct.pack/unpack with flag f. According to https://docs.python.org/3/library/struct.html, f without byte order flag results in packing float32 using the machine’s native format and byte order. In my case, my M1 macbook happens to store data in little endian.

In pymodbus's source, I can see int32 are stored into registers explicitly using big endian with code value_bytes = int.to_bytes(value, 4, "big"). However, float32 are stored using machine's native byte order, which causes this issue on host using little endian like mine.

So I create a PR(#4468), which forces float32 to pack/unpack using big endian by adding > before f.

This PR cannot pass unittest written before because TestSimulator.test_registers assumes host uses little endian(commonly x86, amd64, M1). If you run this unittest on a rare host using big endian, it will not pass. I recommend changing all Cells related to FLOAT32 and verifying them with big-endian byte order in unittest.

Suggested unittest modification:
Change test_simulator.py#L133-142 to

        Cell(type=CellType.FLOAT32, access=True, value=17731),
        Cell(type=CellType.NEXT, access=True, value=18432),
        Cell(type=CellType.FLOAT32, access=True, value=17841),
        Cell(type=CellType.NEXT, access=True, value=29061),
        Cell(type=CellType.FLOAT32, access=True, value=17841),
        Cell(type=CellType.NEXT, access=True, value=29061),
        Cell(type=CellType.FLOAT32, value=18600, action=1),
        Cell(type=CellType.NEXT, value=29958),  # 40
        Cell(type=CellType.FLOAT32, value=18600, action=1),
        Cell(type=CellType.NEXT, value=29958),

will let this PR pass unittest.

The values above are evaluated by this code:

import struct

def evaluate(value):
    bytes = stryct.pack('>f', value)
    return struct.unpack('>h', bytes[:2]), struct.unpack('>h', bytes[2:])  # same as int.from_bytes(bytes[:2], 'big'), ...

>>> evaluate(3124.5)
((17731,), (18432,))

janiversen pushed a commit that referenced this issue Jul 18, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants