espressif / esp-modbus

ESP-Modbus - the officially suppported library for Modbus protocol (serial RS485 + TCP over WiFi or Ethernet).
Apache License 2.0
110 stars 52 forks source link

Has "mbfuncinput.c" file a bug? (IDFGH-13284) #66

Closed NOM-99 closed 1 month ago

NOM-99 commented 3 months ago

Tools Used:

Good afternoon,

Right now, I am using the "esp-modbus" component to work as a slave with a Modbus-RTU communication, 9600 baud rate, without parity and with two stop bits. I am triying to read 125 Holding registers and 125 Input registers.

When the Holding registers are read the communication is correct between the master and the slave; however, when the Input registers are read the communication returns "Modbus illegal data value" messages. Therefore, investigating the files that manage these communication functions I have found that in the Holding registers reading function the maximum number of registers that can be read are 125 and in the Input registers reading function the maximum number of registers that can be read are 124.

Screenshots of "mbfuncholding.c" file of the "eMBFuncReadHoldingRegister()" function:

Captura de pantalla 2024-07-18 141821 image

Screenshots of "mbfuncinput.c" file of the "eMBFuncReadInputRegister()" function:

image image

I have read the documentation "Modbus application protocol specification" and the reading limit of the Holding and Input registers should be the same, 125.

image image

I would like to know if this is really a bug of the file "mbfuncinput.c" or if this limit has become different for some particular reason.

Thank you very much.

alisitsyn commented 2 months ago

@NOM-99,

Thank you for report.

I would like to know if this is really a bug of the file "mbfuncinput.c" or if this limit has become different for some particular reason.

No any particular reason for this. I think it is legacy code issue. It will be fixed as low severity issue.

NOM-99 commented 2 months ago

Thank you very much.

alisitsyn commented 2 months ago

@NOM-99 ,

Thank you for your attention to this. This will be fixed with low severity.

alisitsyn commented 1 month ago

The update merged with the commit d0be4c9478a7c2e19acf4e03da314936a390bc72.