From d13e87b5b5497c98060ab9c336346b8282ac0548 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 7 Mar 2026 17:37:57 -1000 Subject: [PATCH] [modbus] Fix timeout for non-hardware UARTs (e.g., USB UART) The rx_full_threshold is only meaningful for ESP32 native UARTs where it controls the hardware FIFO interrupt threshold. On other platforms (USB UART, Arduino, etc.) it was left at the default of 1, causing the long_rx_buffer_delay_ms calculation to produce a tiny value (~1-2ms). This caused false timeouts on partial responses when data arrives in USB packets with inherent USB-level latency, leading to cascading CRC failures as the remaining bytes were parsed as garbage. Change the default rx_full_threshold to 0 (unset sentinel) and use 50ms when unset, matching the previous hardcoded timeout behavior. --- esphome/components/modbus/modbus.cpp | 10 ++++++++-- esphome/components/uart/uart_component.h | 5 ++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/esphome/components/modbus/modbus.cpp b/esphome/components/modbus/modbus.cpp index 28e26e307e3..da5bfa5ef85 100644 --- a/esphome/components/modbus/modbus.cpp +++ b/esphome/components/modbus/modbus.cpp @@ -21,8 +21,14 @@ void Modbus::setup() { // 3.5 characters * 11 bits per character * 1000ms/sec / (bits/sec) (Standard modbus frame delay) (uint16_t) (3.5 * 11 * 1000 / this->parent_->get_baud_rate()) + 1); - this->long_rx_buffer_delay_ms_ = - (this->parent_->get_rx_full_threshold() * 11 * 1000 / this->parent_->get_baud_rate()) + 1; + // When rx_full_threshold is configured (non-zero), the UART has a hardware FIFO with a + // meaningful threshold (e.g., ESP32 native UART), so we can calculate a precise delay. + // Otherwise (e.g., USB UART), use 50ms to handle data arriving in chunks. + static constexpr uint16_t DEFAULT_LONG_RX_BUFFER_DELAY_MS = 50; + size_t rx_threshold = this->parent_->get_rx_full_threshold(); + this->long_rx_buffer_delay_ms_ = rx_threshold != uart::UARTComponent::RX_FULL_THRESHOLD_UNSET + ? (rx_threshold * 11 * 1000 / this->parent_->get_baud_rate()) + 1 + : DEFAULT_LONG_RX_BUFFER_DELAY_MS; } void Modbus::loop() { diff --git a/esphome/components/uart/uart_component.h b/esphome/components/uart/uart_component.h index 078ce64b30f..3d2a848462e 100644 --- a/esphome/components/uart/uart_component.h +++ b/esphome/components/uart/uart_component.h @@ -187,7 +187,10 @@ class UARTComponent { InternalGPIOPin *rx_pin_{}; InternalGPIOPin *flow_control_pin_{}; size_t rx_buffer_size_{}; - size_t rx_full_threshold_{1}; + static constexpr size_t RX_FULL_THRESHOLD_UNSET = 0; + // ESP-IDF always sets this at codegen time via set_rx_full_threshold(). + // Other platforms (USB UART, Arduino, etc.) leave it unset. + size_t rx_full_threshold_{RX_FULL_THRESHOLD_UNSET}; size_t rx_timeout_{0}; uint32_t baud_rate_{0}; uint8_t stop_bits_{0};