BenKissBox
Hi everybody,

I think I have found a bug in the Arduino modbus library from André Sarmento Barbosa used in OpenPLC Uno project. The bug is not in Thiago code, but depper in the modbus processing section. It appears only when you have 8 or more outputs ("coils"), so the code written by Thiago for the Uno I/O is not impacted

When setting or clearing the 8th "coil" using "Write Single Coil" (function 5), the coil is controlled as expected. Reading the same coil using "Read Coils" (function 1) returns the last status written by function 5.

But if you read coils 0 to 7 using function 1 (setting number of coils to 8 in the request), the modbus library always sends back 0, whatever the reals state of 8th coil.
A similar issue occurs with function 15 (Write Multiple Coils) : setting or clearing 8th coil has no effect. Even worse, setting or clearing coil 0 also changes coil 7 😓
I think I have identified the faulty function in modbus.cpp, I am currently checking if it is better to correct the bug or provide a new Modbus library (I wrote one for myself a long time ago), as I have probably found two other flaws in the current code.

Benoit
Quote 0 0
thiagoralves
Great finding Benoit! Thanks for helping. I got a few users with issues on the Arduino Mega. This is probably the culprit
Quote 0 0
BenKissBox
Great finding Benoit! Thanks for helping. I got a few users with issues on the Arduino Mega. This is probably the culprit


I always think that when other users get similar issues, there is a bigger chance for a "real" bug... When you are the only one finding a bug, most of the time *you* are the bug 😜

So it seems this one is real...😈

Benoit
Quote 0 0
BenKissBox
Hello everybody,

I confirm that I have found two bugs in the Modbus library used by the OpenPLC :
- the library crashes when it receives messages with more than 64 bytes, while Modbus specification requires a RTU device to support 256 bytes (the application must reply in that case with an exception, but it shall not crash)
- the library does not handle properly 8th bit on digital outputs (coils).  Coils 8, 16, 24 etc... are impacted (so especially on ATMega256)

Moreover, I found that the library uses dynamic allocation (malloc / free) for buffer handling, which is really a bad idea on Arduino platform (dealing with heap on a device with 2KB of RAM is a non-sense)

I have almost finished to write a new Modbus library for the OpenPLC Arduino projects, I am now dealing with tests to be sure that I don't release something crappy (first version will not work on Due or Leonardo, only Uno/Mega and similar chips)

Thiago, what would you recommend me to do in order to provide the code "publicly" on the OpenPLC repository ?
I can send the code to you so you can publish it, but since I will probably extend it, maybe there is a way for me to upload and update it directly ?

Benoit
Quote 0 0
thiagoralves
Hi Benoit, thanks for your help! The best way to contribute is pushing code changes directly to GitHub. As of now the repository has only a .zip file with the code (which is a bad idea to keep track of changes, but a good idea for users that are downloading the code). I’ll make changes to the repository and send you a link tomorrow
Quote 0 0
thiagoralves
Hi @BenKissBox. Sorry for taking so long to put it together, but here is the repository with all firmwares for OpenPLC: https://github.com/thiagoralves/OpenPLC_Files/tree/master/Firmware/src_v3

You can push your changes to there and then it will be official! =)


Thanks for your help
Quote 0 0