zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.06k stars 6.18k forks source link

Modbus Holding/Input Register Read Float - Incorrect response byte size #68788

Open KyleKotowick opened 5 months ago

KyleKotowick commented 5 months ago

Describe the bug Using the Modbus server role, the input/holding register floating point read functions return an incorrect number of bytes. They return 64 bits (4 registers) instead of the 32 bits (2 registers) required for a 32-bit float.

A modbus client, when requesting a 32-bit float value, requests two 16-bit registers. Therefore, &ctx->rx_adu.data[2] is 2.

The Zephyr modbus library then multiplies the number of registers (2) by the size of a float (32) and thinks it should return 64 bits. However, since the client only requested two 16-bit registers (32 bits), it gets an unexpected response size.

To Reproduce Steps to reproduce the behavior:

  1. Run a Zephyr modbus server, and enable CONFIG_MODBUS_FP_EXTENSIONS.
  2. From a connected modbus client (in our case, an Automation Direct CM5-T7W), request a float32 value from an address with value >= 5000.
  3. See that the server tries to return 64 bits (4 registers) instead of the requested 2 registers (32 bits).

Expected behavior When a 32-bit float is requested by a modbus client (two 16-bit registers), the modbus server should return 32 bits of data, not 64.

Impact Showstopper. We cannot use the Zephyr modbus server with our Automation Direct CM5-T7W HMI, as we require the use of 32-bit floats.

Environment (please complete the following information): OS: Windows 11 Toolchain: Zephyr 3.5.0

jfischer-no commented 4 months ago

Indeed, it looks suspicious, I am on it.

KyleKotowick commented 2 months ago

Not stale, problem still exists.

github-actions[bot] commented 2 weeks ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

KyleKotowick commented 2 weeks ago

Not stale, problem still exists. @jfischer-no any luck?

maksjapan commented 2 weeks ago

Hi, I have the same problem (albeit with a different HMI). From what I understand, the register quantity requested in the query header (reg_qty) is described in 2-byte length registers. However, floats are in 4-byte registers, so the quantity of the floating point register should be counted as half of the register quantity requested in the query header.

I've added the below snipped in line 457 of the modbus_server.c: reg_qty = (reg_qty + (sizeof(uint16_t) - 1))/sizeof(uint16_t);

and it seems to fix the problem for Reading Input Registers (FC04). I used the idea from this discussion thread in StackOverflow for the math.

I haven't tried it yet, but I presume for Reading Holding Registers (FC03), you'd add the same snipped in line 347 to fix it (assuming it has the same issue).