Fix serial timing and baud rate validation

src/main/java/com/ghgande/j2mod/modbus/Modbus.java

@@ -319,7 +319,7 @@ public interface Modbus {
      * The number of characters delay that must be maintained between adjacent requests on
      * the same serial port (within the same transaction)
      */
-    double INTER_MESSAGE_GAP = 4;
+    double INTER_MESSAGE_GAP = 3.5;
 
     /**
      * The number of characters delay that is the allowed maximum between characters on

src/main/java/com/ghgande/j2mod/modbus/io/ModbusRTUTransport.java

@@ -365,15 +365,16 @@ protected ModbusRequest readRequestIn(AbstractModbusListener listener) throws Mo
                     else {
                         // This message is not for us, read and wait for the 3.5t delay
 
+                        final long charTimeout = getMaxCharTimeout();
                         // Wait for max 1.5t for data to be available
                         while (true) {
                             boolean bytesAvailable = availableBytes() > 0;
                             if (!bytesAvailable) {
                                 // Sleep the 1.5t to see if there will be more data
                                 if (logger.isDebugEnabled()) {
-                                    logger.debug("Waiting for {} microsec", getMaxCharDelay());
+                                    logger.debug("Waiting for {} microsec", charTimeout);
                                 }
-                                bytesAvailable = spinUntilBytesAvailable(getMaxCharDelay());
+                                bytesAvailable = spinUntilBytesAvailable(charTimeout);
                             }
 
                             if (bytesAvailable) {
@@ -388,12 +389,13 @@ protected ModbusRequest readRequestIn(AbstractModbusListener listener) throws Mo
                             }
                         }
 
-                        // Wait for 2t to complete the 3.5t wait
+                        // Wait for the remaining time to complete the inter-frame interval
+                        final long remainingWait = getInterFrameDelay() - charTimeout;
                         // Is there is data available the interval was not respected, we should discard the message
                         if (logger.isDebugEnabled()) {
-                            logger.debug("Waiting for {} microsec", getCharIntervalMicro(2));
+                            logger.debug("Waiting for {} microsec", remainingWait);
                         }
-                        if (spinUntilBytesAvailable(getCharIntervalMicro(2))) {
+                        if (spinUntilBytesAvailable(remainingWait)) {
                             // Discard the message
                             if (logger.isDebugEnabled()) {
                                 logger.debug("Discarding message (More than 1.5t between characters!) - {}", ModbusUtil.toHex(byteInputOutputStream.getBuffer(), 0, byteInputOutputStream.size()));

src/main/java/com/ghgande/j2mod/modbus/io/ModbusSerialTransport.java

@@ -54,14 +54,19 @@ public abstract class ModbusSerialTransport extends AbstractModbusTransport {
     static final int FRAME_END = 2000;
 
     /**
-     * The number of nanoseconds there is in a millisecond
+     * The number of nanoseconds in a millisecond
      */
-    private static final int NS_IN_A_MS = 1_000_000;
+    private static final double NS_IN_A_MS = 1_000_000.0;
 
     /**
-     * The number of nanoseconds there is in a second
+     * The number of microseconds in a second.
      */
-    private static final int NS_IN_A_SEC = 1_000_000_000;
+    private static final double MICROS_IN_A_SEC = 1_000_000.0;
+
+    /**
+     * The number of nanoseconds in a second
+     */
+    private static final double NS_IN_A_SEC = 1_000_000_000.0;
 
     private static final String CANNOT_READ_FROM_SERIAL_PORT = "Cannot read from serial port";
     private static final String COMM_PORT_IS_NOT_VALID_OR_NOT_OPEN = "Comm port is not valid or not open";
@@ -121,7 +126,7 @@ private void waitForTransmission(long startTime, double transmissionTimeNanos) {
         if (transmissionTimeNanos >= NS_IN_A_MS) {
             try {
                 final long adjustedDelay = (long) (transmissionTimeNanos * LONG_DELAY_FUDGE_FACTOR);
-                final long sleepMillis = adjustedDelay / NS_IN_A_MS;
+                final long sleepMillis = (long) (adjustedDelay / NS_IN_A_MS);
                 final int sleepNanos = (int) (adjustedDelay % NS_IN_A_MS);
 
                 Thread.sleep(sleepMillis, sleepNanos);
@@ -631,13 +636,13 @@ void waitBetweenFrames(int transDelayMS, long lastTransactionTimestamp) {
             ModbusUtil.sleep(transDelayMS);
         }
         else {
-            // Make use we have a gap of 3.5 characters between adjacent requests
+            // Make sure we have a gap of 3.5 characters between adjacent requests
             // We have to do the calculations here because it is possible that the caller may have changed
             // the connection characteristics if they provided the connection instance
-            int delay = getInterFrameDelay() / 1000;
+            int delay = (int) Math.ceil(getInterFrameDelay() / 1000.0);
 
             // How long since the last message we received
-            long gapSinceLastMessage = (System.nanoTime() - lastTransactionTimestamp) / NS_IN_A_MS;
+            final long gapSinceLastMessage = (long) ((System.nanoTime() - lastTransactionTimestamp) / NS_IN_A_MS);
             if (delay > gapSinceLastMessage) {
                 long sleepTime = delay - gapSinceLastMessage;
 
@@ -651,28 +656,38 @@ void waitBetweenFrames(int transDelayMS, long lastTransactionTimestamp) {
     }
 
     /**
-     * In microseconds
+     * Calculates the inter-frame delay according to the
+     * MODBUS over Serial Line Specification V1.02.
+     * <ul>
+     *  <li> baud rates &le; 19200: 3.5 Character time </li>
+     *  <li> baud rates &gt; 19200: 1750 microseconds </li>
+     * </ul>
      *
-     * @return Delay between frames
+     * @return the inter-frame delay in microseconds
      */
     int getInterFrameDelay() {
         if (commPort.getBaudRate() > 19200) {
             return 1750;
         }
         else {
             long delay = Math.max(getCharIntervalMicro(Modbus.INTER_MESSAGE_GAP), Modbus.MINIMUM_TRANSMIT_DELAY * 1000L);
-            return delay > Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) delay;
+            return (int) Math.min(Integer.MAX_VALUE, delay);
         }
     }
 
     /**
-     * The maximum delay between characters in microseconds
+     * Calculates the inter-character time-out according to the
+     * MODBUS over Serial Line Specification V1.02.
+     * <ul>
+     *  <li> baud rates &le; 19200: 1.5 Character time </li>
+     *  <li> baud rates &gt; 19200: 750 microseconds </li>
+     * </ul>
      *
-     * @return microseconds
+     * @return the inter-character time-out in microseconds
      */
-    long getMaxCharDelay() {
+    long getMaxCharTimeout() {
         if (commPort.getBaudRate() > 19200) {
-            return 1750;
+            return 750;
         }
         else {
             return getCharIntervalMicro(Modbus.INTER_CHARACTER_GAP);
@@ -690,15 +705,16 @@ long getCharIntervalMicro(double chars) {
         // Make use we have a gap of 3.5 characters between adjacent requests
         // We have to do the calculations here because it is possible that the caller may have changed
         // the connection characteristics if they provided the connection instance
-        return (long) (chars * NS_IN_A_MS * commPort.getBitsPerCharacter() / commPort.getBaudRate());
+        final double microsPerChar = (commPort.getBitsPerCharacter() / (double) commPort.getBaudRate()) * MICROS_IN_A_SEC;
+        return (long) Math.ceil(microsPerChar * chars);
     }
 
     /**
      * Spins until the timeout or the condition is met.
      * This method will repeatedly poll the available bytes, so it should not have any side effects.
      *
      * @param waitTimeMicroSec The time to wait for the condition to be true in microseconds
-     * @return true if the condition ended the spin, false if the tim
+     * @return true if the condition ended the spin, false if the timeout was reached
      */
     boolean spinUntilBytesAvailable(long waitTimeMicroSec) {
         long start = System.nanoTime();

src/main/java/com/ghgande/j2mod/modbus/net/AbstractSerialConnection.java

@@ -101,9 +101,11 @@ public abstract class AbstractSerialConnection {
     public abstract void close();
 
     /**
-     * Returns current baud rate
+     * Returns current baud rate.
+     * <p>
+     * For UART interfaces (RS-232 / RS-485), this is equal to the line bit rate in bits/s.
      *
-     * @return Baud rate
+     * @return Baud rate (bits/s)
      */
     public abstract int getBaudRate();
 
@@ -212,7 +214,7 @@ public double getBitsPerCharacter() {
         final int numDataBits = getNumDataBits();
         final int dataBits = numDataBits == 0 ? 8 : numDataBits;
         final double stopBits = getStopBits();
-        final double parityBits = getParity() == SerialPort.NO_PARITY ? 0 : 1;
+        final double parityBits = getParity() == NO_PARITY ? 0 : 1;
 
         return startBit + dataBits + stopBits + parityBits;
     }

src/main/java/com/ghgande/j2mod/modbus/slave/ModbusSlaveFactory.java

@@ -165,6 +165,7 @@ public static synchronized ModbusSlave createSerialSlave(SerialParameters serial
      * Creates a serial modbus slave or returns the one already allocated to this port
      *
      * @param serialParams Serial parameters for serial type slaves
+     * @param listenerFactory Factory to create the listener for this slave
      * @return new or existing Serial modbus slave associated with the port
      * @throws ModbusException If a problem occurs e.g. port already in use
      */

src/main/java/com/ghgande/j2mod/modbus/util/SerialParameters.java

@@ -37,7 +37,7 @@ public class SerialParameters {
     private static final boolean DEFAULT_RS485_MODE = false;
     private static final boolean DEFAULT_RS485_TX_ENABLE_ACTIVE_HIGH = true;
     private static final boolean DEFAULT_RS485_ENABLE_TERMINATION = false;
-    private static final boolean DEFAULT_RS485_TX_DURING_RX = false;
+    private static final boolean DEFAULT_RS485_RX_DURING_TX = false;
     private static final int DEFAULT_RS485_DELAY_BEFORE_TX_MICROSECONDS = 1000;
     private static final int DEFAULT_RS485_DELAY_AFTER_TX_MICROSECONDS = 1000;
 
@@ -65,24 +65,13 @@ public class SerialParameters {
      * default values.
      */
     public SerialParameters() {
-        portName = "";
-        baudRate = 9600;
-        flowControlIn = AbstractSerialConnection.FLOW_CONTROL_DISABLED;
-        flowControlOut = AbstractSerialConnection.FLOW_CONTROL_DISABLED;
-        databits = 8;
-        stopbits = AbstractSerialConnection.ONE_STOP_BIT;
-        parity = AbstractSerialConnection.NO_PARITY;
-        // Historically, the encoding has been null which got converted to RTU
-        // by SerialConnection.open(). Let's make it more explicit which serial
-        // protocol will be used by default.
-        encoding = Modbus.SERIAL_ENCODING_RTU;
-        echo = false;
-        openDelay = AbstractSerialConnection.OPEN_DELAY;
-        rs485Mode = DEFAULT_RS485_MODE;
-        rs485TxEnableActiveHigh = DEFAULT_RS485_TX_ENABLE_ACTIVE_HIGH;
-        rs485DelayBeforeTxMicroseconds = DEFAULT_RS485_DELAY_BEFORE_TX_MICROSECONDS;
-        rs485DelayAfterTxMicroseconds = DEFAULT_RS485_DELAY_AFTER_TX_MICROSECONDS;
-        rs485DisableControl = DEFAULT_RS485_DISABLE_CONTROL;
+        this("", 9600,
+                AbstractSerialConnection.FLOW_CONTROL_DISABLED,
+                AbstractSerialConnection.FLOW_CONTROL_DISABLED,
+                8,
+                AbstractSerialConnection.ONE_STOP_BIT,
+                AbstractSerialConnection.NO_PARITY,
+                false);
     }
 
     /**
@@ -105,17 +94,17 @@ public SerialParameters(String portName, int baudRate,
                             int stopbits,
                             int parity,
                             boolean echo) {
-        // Perform default initialization and update fields of interest
-        // afterwards.
-        this();
-        this.portName = portName;
-        this.baudRate = baudRate;
-        this.flowControlIn = flowControlIn;
-        this.flowControlOut = flowControlOut;
-        this.databits = databits;
-        this.stopbits = stopbits;
-        this.parity = parity;
-        this.echo = echo;
+        this(portName, baudRate,
+                flowControlIn,
+                flowControlOut,
+                databits,
+                stopbits,
+                parity,
+                echo,
+                DEFAULT_RS485_MODE,
+                DEFAULT_RS485_TX_ENABLE_ACTIVE_HIGH,
+                DEFAULT_RS485_DELAY_BEFORE_TX_MICROSECONDS,
+                DEFAULT_RS485_DELAY_AFTER_TX_MICROSECONDS);
     }
 
     /**
@@ -160,13 +149,26 @@ public SerialParameters(String portName, int baudRate,
                             int rs485DelayBeforeTxMicroseconds,
                             int rs485DelayAfterTxMicroseconds
                             ) {
-        // Perform default non-RS-485 initialization and update fields of
-        // interest afterwards.
-        this(portName, baudRate, flowControlIn, flowControlOut, databits, stopbits, parity, echo);
+        this.portName = portName;
+        this.setBaudRate(baudRate);
+        this.flowControlIn = flowControlIn;
+        this.flowControlOut = flowControlOut;
+        this.databits = databits;
+        this.stopbits = stopbits;
+        this.parity = parity;
+        this.echo = echo;
         this.rs485Mode = rs485Mode;
         this.rs485TxEnableActiveHigh = rs485TxEnableActiveHigh;
         this.rs485DelayBeforeTxMicroseconds = rs485DelayBeforeTxMicroseconds;
         this.rs485DelayAfterTxMicroseconds = rs485DelayAfterTxMicroseconds;
+        // Historically, the encoding has been null which got converted to RTU
+        // by SerialConnection.open(). Let's make it more explicit which serial
+        // protocol will be used by default.
+        this.encoding = Modbus.SERIAL_ENCODING_RTU;
+        this.openDelay = AbstractSerialConnection.OPEN_DELAY;
+        this.rs485DisableControl = DEFAULT_RS485_DISABLE_CONTROL;
+        this.rs485EnableTermination = DEFAULT_RS485_ENABLE_TERMINATION;
+        this.rs485RxDuringTx = DEFAULT_RS485_RX_DURING_TX;
     }
 
     /**
@@ -217,30 +219,35 @@ public void setPortName(String name) {
     }
 
     /**
-     * Sets the baud rate.
+     * Sets the baud rate. Has to be ≥1.
      *
      * @param rate the new baud rate.
      */
     public void setBaudRate(int rate) {
+        if (rate < 1) {
+            throw new IllegalArgumentException("Baud rate must be greater than 0, but was: " + rate);
+        }
         baudRate = rate;
     }
 
     /**
-     * Return the baud rate as <tt>int</tt>.
+     * Sets the baud rate.
      *
-     * @return the baud rate as <tt>int</tt>.
+     * @param rate the new baud rate.
      */
-    public int getBaudRate() {
-        return baudRate;
+    public void setBaudRate(String rate) {
+        setBaudRate(Integer.parseInt(rate));
     }
 
     /**
-     * Sets the baud rate.
+     * Return the baud rate as <tt>int</tt>.
+     * <p>
+     * Is guaranteed to return a value &ge;1
      *
-     * @param rate the new baud rate.
+     * @return the baud rate as <tt>int</tt>.
      */
-    public void setBaudRate(String rate) {
-        baudRate = Integer.parseInt(rate);
+    public int getBaudRate() {
+        return baudRate;
     }
 
     /**
@@ -938,9 +945,9 @@ public String toString() {
                 ", echo=" + echo +
                 ", openDelay=" + openDelay +
                 ", rs485Mode=" + rs485Mode +
-                ", rs485TxEnableActiveHight=" + rs485TxEnableActiveHigh +
+                ", rs485TxEnableActiveHigh=" + rs485TxEnableActiveHigh +
                 ", rs485EnableTermination=" + rs485EnableTermination +
-                ", rs485RxDuringTx" + rs485RxDuringTx +
+                ", rs485RxDuringTx=" + rs485RxDuringTx +
                 ", rs485DelayBeforeTxMicroseconds=" + rs485DelayBeforeTxMicroseconds +
                 ", rs485DelayAfterTxMicroseconds=" + rs485DelayAfterTxMicroseconds +
                 '}';