modbus: serial: Ignore received data when reception is disabled

A byte received when reception has been disabled
corrupts internal state of the server
(e.g. during transmission of a reply in server mode).
The reponse packet is corrupted and its transmission is aborted and the
data in the buffer is treated by the server as a new incoming packet.
Since the buffer is corrupted CRC doesn't match and the following log
message is printed:

  <wrn> modbus_serial: Calculated CRC does not match received CRC

This condition happens when uart_irq_rx_ready() returns true if there is
a new byte in the receive FIFO even with disabled RX interrupt.

The issue has been discovered on a nucleo_u083rc board with a RS485
transceiver with the RI signal floating (a pull-down gives more stable
reproduction). The pull-down ensures that RI is low during transmission
which is seen as byte 0 with a framing error by the receiver.
The byte is received by the MCU and corrupts the response.
Similar effect can be achieved by not disabling the receiver during
transmission (i.e. nRE is driven by the MCU and is fixed low).

The fix discards any data received when reception has been disabled.

Signed-off-by: Maksim Salau <msalau@iotecha.com>

Author: Maksim Salau

subsys/modbus/modbus_internal.h

@@ -88,6 +88,7 @@ struct modbus_serial_config {
 };
 
 #define MODBUS_STATE_CONFIGURED		0
+#define MODBUS_STATE_RX_ENABLED		1
 
 struct modbus_context {
 	/* Interface name */

subsys/modbus/modbus_serial.c

@@ -50,6 +50,17 @@ static void modbus_serial_tx_off(struct modbus_context *ctx)
 	}
 }
 
+static void modbus_serial_rx_fifo_drain(struct modbus_context *ctx)
+{
+	struct modbus_serial_config *cfg = ctx->cfg;
+	uint8_t buf[8];
+	int n;
+
+	do {
+		n = uart_fifo_read(cfg->dev, buf, sizeof(buf));
+	} while (n == sizeof(buf));
+}
+
 static void modbus_serial_rx_on(struct modbus_context *ctx)
 {
 	struct modbus_serial_config *cfg = ctx->cfg;
@@ -58,6 +69,7 @@ static void modbus_serial_rx_on(struct modbus_context *ctx)
 		gpio_pin_set(cfg->re->port, cfg->re->pin, 1);
 	}
 
+	atomic_set_bit(&ctx->state, MODBUS_STATE_RX_ENABLED);
 	uart_irq_rx_enable(cfg->dev);
 }
 
@@ -66,6 +78,8 @@ static void modbus_serial_rx_off(struct modbus_context *ctx)
 	struct modbus_serial_config *cfg = ctx->cfg;
 
 	uart_irq_rx_disable(cfg->dev);
+	atomic_clear_bit(&ctx->state, MODBUS_STATE_RX_ENABLED);
+
 	if (cfg->re != NULL) {
 		gpio_pin_set(cfg->re->port, cfg->re->pin, 0);
 	}
@@ -312,6 +326,11 @@ static void cb_handler_rx(struct modbus_context *ctx)
 {
 	struct modbus_serial_config *cfg = ctx->cfg;
 
+	if (!atomic_test_bit(&ctx->state, MODBUS_STATE_RX_ENABLED)) {
+		modbus_serial_rx_fifo_drain(ctx);
+		return;
+	}
+
 	if ((ctx->mode == MODBUS_MODE_ASCII) &&
 	    IS_ENABLED(CONFIG_MODBUS_ASCII_MODE)) {
 		uint8_t c;
@@ -379,6 +398,7 @@ static void cb_handler_tx(struct modbus_context *ctx)
 		/* Disable transmission */
 		cfg->uart_buf_ptr = &cfg->uart_buf[0];
 		modbus_serial_tx_off(ctx);
+		modbus_serial_rx_fifo_drain(ctx);
 		modbus_serial_rx_on(ctx);
 	}
 }