zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.9k stars 6.64k forks source link

esp32: driver, config: ESP32 UART RX FIFO Threshold Configuration #74311

Closed OmerGozderesi closed 4 months ago

OmerGozderesi commented 5 months ago

Describe the bug

The esp32 UART rx FIFO threshold is fixed at 0x16 bytes, leading to premature end of frame detection and incorrect decoding of Modbus packets larger than 0x16 bytes.

When Modbus message size is greater than 0x16, callback in Modbus layer is called with 0x16 bytes of data and end of frame detection timer is started. But before the callback is called (due to rx event) again, the end of frame timer expires, leading to premature decoding attempt at the, now, incomplete modbus message. This is because the rest of the message does not trigger rx threshold interrupt to fire. Since the premature decoding attempt throws away the first 0x16 bytes of data, the attempt to decode the data remaining in the FIFO also fails.

This is demonstrated in the following example. A successful response for the following message is 25 bytes. esp32 UART stack reports first 22 (0x16) bytes of data via callback. Decoding it fails. And then UART timeout happens, and Modbus stack tries to decode the remaining 3 (25-22) bytes and it results in a frame length error as per https://github.com/zephyrproject-rtos/zephyr/blob/9ae5352372dd9965805980e0c712612f21ed2229/subsys/modbus/modbus_serial.c#L251-L255

To Reproduce

While developing on a custom board, encountering issues due to the fixed fifo threshold. Below is a snippet of the code:

const uint8_t qty = 10;
int16_t modbus_rx_buffer[127] = {0};
const uint8_t slave_id = 1;
const uint8_t offset = 0;

err = modbus_read_holding_regs(client_iface, slave_id, offset, modbus_rx_buffer, qty); 
if (err)
{
  LOG_ERR("Modbus read holding registers failed (err %d)\n", err);
  return err;
}

Running the build and flash commands:

west build -b esp32c3_devkitm applatications/app && west flash && west espressif monitor

Expected behavior

Expected to be able to read holding registers up to the maximum quantity (127) as defined in the Modbus specifications.

Impact

Unable to handle Modbus buffers larger than 0x16 bytes, leading to errors during communication.

Logs and console output

[00:00:13.127,000] <wrn> modbus_serial: Calculated CRC does not match received CRC

[00:00:13.127,000] <inf> main: Modbus read holding registers failed (err -5)

[00:00:13.632,000] <wrn> modbus_serial: Frame length error

[00:00:13.632,000] <inf> main: Modbus read holding registers failed (err -122)

Describe the solution you'd like

Introduce Kconfig options to define both the RX and TX FIFO thresholds for the ESP32 UART, allowing them to be configurable while keeping the default value as 1.

CONFIG_UART_ESP32_TX_FIFO_THRESH CONFIG_UART_ESP32_RX_FIFO_THRESH

Environment

OS: (MacOS Sonoma 14.5) Toolchain: (Zephyr SDK 0.16.6) Zephyr : latest release

github-actions[bot] commented 5 months ago

Hi @OmerGozderesi! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙