CR characters lost on ModbusSerialTransport

[btbouwens]

I noticed that the communications with our equipment is very slow. When I enable debug logging, I get messages like:

17:09:27.399 DEBUG [main] c.g.j.m.i.ModbusSerialTransport - Read From buffer: 68 (44)
17:09:27.400 DEBUG [main] c.g.j.m.i.ModbusSerialTransport - Read From buffer: 70 (46)
17:09:27.400 DEBUG [main] c.g.j.m.i.ModbusSerialTransport - Returning combined value of: DF
17:09:27.401 DEBUG [main] c.g.j.m.i.ModbusSerialTransport - Read From buffer: 49 (31)
17:09:27.402 DEBUG [main] c.g.j.m.i.ModbusSerialTransport - Read From buffer: 51 (33)
17:09:27.402 DEBUG [main] c.g.j.m.i.ModbusSerialTransport - Returning combined value of: 13
17:09:27.404 DEBUG [main] c.g.j.m.i.ModbusSerialTransport - Read From buffer: 10 (0A)
17:09:33.403 DEBUG [main] c.g.j.m.i.ModbusASCIITransport - Cannot read from serial port
17:09:33.403 DEBUG [main] c.g.j.m.i.ModbusSerialTransaction - Execute try 1 error: I/O exception - failed to read
17:09:33.403 DEBUG [main] c.g.j.m.i.ModbusSerialTransport - Wrote FRAME_START
17:09:33.403 DEBUG [main] c.g.j.m.i.ModbusSerialTransport - Wrote byte 1=[48, 49]

which suggests to me that the expected CR before the NL isn't there, and the program is waiting for another byte while the line has already ended. AFAICS the stty settings are not set to drop them. This happens both on Linux and on Mac OSX, using the 2.1.1 version.

[btbouwens]

Effectively I don't care who eats the CR's, as long as the EOL is recognised, so for me this fixes the issue:

diff --git a/src/main/java/com/ghgande/j2mod/modbus/io/ModbusSerialTransport.java b/src/main/java/com/ghgande/j2mod/modbus/io/ModbusSerialTransport.java
index 7273d37..bc4e687 100644
--- a/src/main/java/com/ghgande/j2mod/modbus/io/ModbusSerialTransport.java
+++ b/src/main/java/com/ghgande/j2mod/modbus/io/ModbusSerialTransport.java
@@ -419,7 +419,7 @@ public abstract class ModbusSerialTransport extends AbstractModbusTransport {
             else if (buffer[0] == ':') {
                 return ModbusASCIITransport.FRAME_START;
             }
-            else if (buffer[0] == '\r') {
+            else if (buffer[0] == '\r' || buffer[0] == '\n') {
                 return ModbusASCIITransport.FRAME_END;
             }
             else {

[steveohara]

Strictly speaking, Modbus ASCII requires a CR/LF combination to properly terminate a message. Looking at the implementation in j2mod I see that it isn't actually following the standard. I don't have any ASCII devices to test it with so I can't vouch for the applicability of the current implementation other than it should actually be looking for 2 characters, rather than a single CR.

Working on the basis that j2mod is not following the standard now, adding your change doesn't make it any worse so it's fine by me.

[steveohara]

You can grab a snapshot with the change here - https://oss.sonatype.org/content/repositories/snapshots/com/ghgande/j2mod/2.1-SNAPSHOT/j2mod-2.1-20160530.112246-1.jar

[btbouwens]

Totally agreed. Thanks & regards. Bram.