Issues with high baudrates

[romainreignier]

Hi,

Thank you for sharing your library that seems very interesting as it is the only one I have found that use callback functions. I cannot imagine implementing a Modbus slave that copy the values of the registers at each iteration of the loop.
Nevertheless, I did not manage to use your simple.ino example with a baudrate higher than 9600, using qmodbus as the Master, while it works well with other libraries like this one at 19200 or 115200 baud.
I have tried to change the timeout defined in Modbus::setT35 without any success.

On qmodbus, I have the following error:

Slave threw exception "Connection timed out" or function not implemented.

I have tested it on Arduino UNO and Arduino MEGA.

Let me know if you have already encountered that issue or have any ideas.

Romain

[yaacov]

I hope to look into it in the weekend, maybe copy the algorithm from the library you linked to ;-)

[romainreignier]

Oh, thanks for your quick reply!
Note that I have also found this library and in one of the examples, some callbacks are created but the whole library seems quite complicated! I did not tried it yet.

[yaacov]

Thanks for the issue report.

[yaacov]

Played around with different ways to implement inter-frame timeout.
The best boudrate with my nano was 19200.

Changed the timeout mechanism: 3589a31?w=1

Will play with it more next weekend :-)

[romainreignier]

Hi,

Thank you for your work on that issue.

Also, some notes about the modifications:

1. Why do you use a so big t3.5?
   The Modbus Specifiation advises to use a 1.750 ms timer for communication above 19200 Bps (p.13).

2. I am not sure about the use of Serial.flush() in the constructor. This method only:

   Waits for the transmission of outgoing serial data to complete.

   Its name is a bit confusing but it does not clear the serial buffers.

3. Instead of your serialRead method, I would suggest to use Serial.readBytes(), maybe with a serial Timeout set to 0 with Serial.setTimeout(0). In fact, the implementation of readBytes() is quite close to your method if the Timeout is set to 0:

   size_t Stream::readBytes(char *buffer, size_t length)
   {
       size_t count = 0;
       while (count < length) {
         int c = timedRead();
         if (c < 0) break;
         *buffer++ = (char)c;
         count++;
       }
       return count;
   }

4. About the control pin, maybe the use of Serial.flush() would be more portable than the direct register access:

   while (!(UCSR0A & (1 << TXC0)));

   Because the Arduino flush() seems more complete.

   Actually, I would like a non blocking method to trigger the control pin so I am waiting for that isFlushed() method to be part of the Arduino core.

5. Last but not least, I like the new way you have implemented the reading part in the poll method, I found it clear and easy to understand.

I hope these feedbacks would help you to improve your library.

Thanks again for your work.

[yaacov]

can you make a pull request with that:

1. make the timout less then 5.
2. remove Serial.flush from constactor.
3. use Serial.readBytes() with timeout 0.

If you can make this changes and test them to make sure they work in the wild :-) I will merge them.

[yaacov]

#2 merged 👍