diff --git a/esphome/components/cdc_acm_uart/bridge/__init__.py b/esphome/components/cdc_acm_uart/bridge/__init__.py index cee048df5d..133ea92a48 100644 --- a/esphome/components/cdc_acm_uart/bridge/__init__.py +++ b/esphome/components/cdc_acm_uart/bridge/__init__.py @@ -36,19 +36,6 @@ CONFIG_SCHEMA = cv.All( ) -def _subtree_references_uart(node: object, uart_id: str) -> bool: - """Return True if any dict in the subtree has a uart_id entry naming this bus.""" - if isinstance(node, dict): - return any( - (key == CONF_UART_ID and str(value) == uart_id) - or _subtree_references_uart(value, uart_id) - for key, value in node.items() - ) - if isinstance(node, list): - return any(_subtree_references_uart(item, uart_id) for item in node) - return False - - def _reject_debug(uart_conf: ConfigType) -> ConfigType: # The worker tasks use the IDF driver directly, so the uart debugger never sees # bridge traffic and its dummy_receiver would drain RX bytes on the main loop. @@ -86,7 +73,7 @@ def _final_validate(config: ConfigType) -> ConfigType: for domain, domain_conf in full_config.items(): if domain == BRIDGE_DOMAIN: continue - if _subtree_references_uart(domain_conf, owned_id): + if uart.subtree_references_uart(domain_conf, owned_id): raise cv.Invalid( f"The {label} '{owned_id}' is also used by '{domain}'; a bridge " f"requires exclusive use of its {label}.", diff --git a/esphome/components/uart/__init__.py b/esphome/components/uart/__init__.py index 25fbfb7599..2250b9d508 100644 --- a/esphome/components/uart/__init__.py +++ b/esphome/components/uart/__init__.py @@ -533,6 +533,25 @@ def final_validate_device_schema( ) +def subtree_references_uart( + node: object, uart_id: str, conf_key: str = CONF_UART_ID +) -> bool: + """Return True if any dict in the subtree has a conf_key entry naming this bus. + + For the final validation of a component that needs a UART for itself. Bare + `id:` references (a uart.write action) and lambdas are not found. + """ + if isinstance(node, dict): + return any( + (key == conf_key and str(value) == uart_id) + or subtree_references_uart(value, uart_id, conf_key) + for key, value in node.items() + ) + if isinstance(node, list): + return any(subtree_references_uart(item, uart_id, conf_key) for item in node) + return False + + async def register_uart_device(var, config): """Register a UART device, setting up all the internal values. diff --git a/esphome/components/uart/uart_component.cpp b/esphome/components/uart/uart_component.cpp index 6516174635..82bc592702 100644 --- a/esphome/components/uart/uart_component.cpp +++ b/esphome/components/uart/uart_component.cpp @@ -1,9 +1,15 @@ #include "uart_component.h" +#include "esphome/core/application.h" + +#include namespace esphome::uart { ESPHOME_LOG_TAG(TAG, "uart"); +// Keeps the pacing product in 32 bits up to about 10 Mbaud. +static constexpr uint32_t MAX_PACE_SPAN_MS = 4000; + bool UARTComponent::check_read_timeout_(size_t len) { if (this->available() >= len) return true; @@ -19,6 +25,19 @@ bool UARTComponent::check_read_timeout_(size_t len) { return true; } +size_t UARTComponent::paced_write_room(uint32_t last_write_ms) { + size_t room = this->available_for_write(); + if (room != SIZE_MAX) { + return room; + } + // A pass woken early writes little. + uint32_t span = + std::min({App.get_loop_component_start_time() - last_write_ms, App.get_loop_interval(), MAX_PACE_SPAN_MS}); + // 10 bits per byte on the line. + uint32_t paced = this->baud_rate_ / 10 * span / 1000; + return std::max(1, paced); +} + void UARTComponent::set_rx_full_threshold_ms(uint8_t time) { uint8_t bytelength = this->data_bits_ + this->stop_bits_ + 1; if (this->parity_ != UARTParityOptions::UART_CONFIG_PARITY_NONE) diff --git a/esphome/components/uart/uart_component.h b/esphome/components/uart/uart_component.h index 8f143b7977..2543cb5476 100644 --- a/esphome/components/uart/uart_component.h +++ b/esphome/components/uart/uart_component.h @@ -86,6 +86,10 @@ class UARTComponent { // Platforms that cannot tell return SIZE_MAX: write_array() takes everything and may block. virtual size_t available_for_write() { return SIZE_MAX; } + // Bytes a write can take now. Where available_for_write() cannot tell (SIZE_MAX), paces to the line time since + // last_write_ms, a loop start time: at most one loop interval and 4 s, at least one byte. + size_t paced_write_room(uint32_t last_write_ms); + // Pure virtual method to block until all bytes have been written to the UART bus. // @return UARTFlushResult indicating whether the flush was confirmed, timed out, failed, or assumed successful. virtual UARTFlushResult flush() = 0; diff --git a/esphome/components/uart_tcp/__init__.py b/esphome/components/uart_tcp/__init__.py index 601fdeb327..9d62d48d7c 100644 --- a/esphome/components/uart_tcp/__init__.py +++ b/esphome/components/uart_tcp/__init__.py @@ -71,18 +71,6 @@ CONFIG_SCHEMA = cv.All( ) -def _subtree_references_uart(node: object, uart_id: str) -> bool: - if isinstance(node, dict): - return any( - (key == CONF_UART_ID and str(value) == uart_id) - or _subtree_references_uart(value, uart_id) - for key, value in node.items() - ) - if isinstance(node, list): - return any(_subtree_references_uart(item, uart_id) for item in node) - return False - - def _reject_dummy_receiver(uart_conf: ConfigType) -> ConfigType: debug = uart_conf.get(CONF_DEBUG) if isinstance(debug, dict) and debug.get(CONF_DUMMY_RECEIVER): @@ -111,7 +99,7 @@ def _final_validate(config: ConfigType) -> ConfigType: # Bare `id:` references (a uart.write action) and lambdas are not caught. if not CORE.testing_mode: for domain, domain_conf in full_config.items(): - if domain != DOMAIN and _subtree_references_uart(domain_conf, uart_id): + if domain != DOMAIN and uart.subtree_references_uart(domain_conf, uart_id): raise cv.Invalid( f"The UART '{uart_id}' is also used by '{domain}'. " "uart_tcp requires exclusive use of that UART.", diff --git a/esphome/components/uart_tcp/uart_tcp.cpp b/esphome/components/uart_tcp/uart_tcp.cpp index bc75eb04c7..d634ac579f 100644 --- a/esphome/components/uart_tcp/uart_tcp.cpp +++ b/esphome/components/uart_tcp/uart_tcp.cpp @@ -11,9 +11,6 @@ namespace esphome::uart_tcp { ESPHOME_LOG_TAG(TAG, "uart_tcp"); -// Keeps the pacing product in 32 bits up to about 10 Mbaud. -static constexpr uint32_t MAX_PACE_SPAN_MS = 4000; - void UartTcp::setup() { this->link_.begin(TAG); #ifdef USE_SOCKET_TCP_LISTENER @@ -75,16 +72,7 @@ void UartTcp::sync_link_() { void UartTcp::read_socket_() { // A hardware write blocks until the driver takes every byte. Leave what does // not fit in the socket, so TCP flow control throttles the peer. - size_t room = this->parent_->available_for_write(); - if (room == SIZE_MAX) { - // Capacity unknown on this platform; pace to the UART time since the last write, - // at most one loop interval and 4 s, so a pass woken early by the socket writes little. - uint32_t span = std::min( - {App.get_loop_component_start_time() - this->last_write_ms_, App.get_loop_interval(), MAX_PACE_SPAN_MS}); - // 10 bits per byte on the line. - uint32_t paced = this->parent_->get_baud_rate() / 10 * span / 1000; - room = std::max(1, paced); - } + size_t room = this->parent_->paced_write_room(this->last_write_ms_); if (room == 0) { this->rx_pending_ = true; return; diff --git a/tests/components/uart/uart_paced_write_room.cpp b/tests/components/uart/uart_paced_write_room.cpp new file mode 100644 index 0000000000..ed64695dd0 --- /dev/null +++ b/tests/components/uart/uart_paced_write_room.cpp @@ -0,0 +1,62 @@ +#include + +#include "common.h" +#include "esphome/core/application.h" + +namespace esphome::uart::testing { + +class RoomUART : public MockUARTComponent { + public: + size_t available_for_write() override { return this->room; } + size_t room{SIZE_MAX}; +}; + +class PacedWriteRoom : public ::testing::Test { + protected: + void SetUp() override { + App.set_loop_interval(16); + this->uart_.set_baud_rate(9600); + } + void TearDown() override { + App.set_loop_interval(16); + this->at_(0); + } + // Publishes now_ms as the loop start time, as Application::loop() does. + void at_(uint32_t now_ms) { LoopBlockingGuard dispatch{nullptr, nullptr, now_ms}; } + + RoomUART uart_; +}; + +TEST_F(PacedWriteRoom, KnownRoomIsReturned) { + this->uart_.room = 7; + this->at_(1000); + EXPECT_EQ(this->uart_.paced_write_room(0), 7u); + this->uart_.room = 0; + EXPECT_EQ(this->uart_.paced_write_room(0), 0u); +} + +TEST_F(PacedWriteRoom, UnknownRoomTakesOneLoopInterval) { + this->at_(1000); + // 9600 baud at 10 bits per byte for 16 ms. + EXPECT_EQ(this->uart_.paced_write_room(0), 15u); +} + +TEST_F(PacedWriteRoom, EarlyPassTakesTheTimeSinceTheLastWrite) { + this->at_(1005); + EXPECT_EQ(this->uart_.paced_write_room(1000), 4u); +} + +TEST_F(PacedWriteRoom, AtLeastOneByte) { + this->at_(1000); + EXPECT_EQ(this->uart_.paced_write_room(1000), 1u); +} + +TEST_F(PacedWriteRoom, SpanCapsAtFourSeconds) { + App.set_loop_interval(10000); + this->uart_.set_baud_rate(300); + this->at_(6000); + // 30 bytes/s for 4 s. + EXPECT_EQ(this->uart_.paced_write_room(0), 120u); +} + +} // namespace esphome::uart::testing diff --git a/tests/unit_tests/components/test_uart.py b/tests/unit_tests/components/test_uart.py new file mode 100644 index 0000000000..4b062b4729 --- /dev/null +++ b/tests/unit_tests/components/test_uart.py @@ -0,0 +1,24 @@ +"""Tests for the uart helpers shared by components that own a UART.""" + +from esphome.components.uart import subtree_references_uart +from esphome.core import ID + + +def test_finds_a_nested_uart_id() -> None: + config = {"modbus": [{"id": ID("hub"), "uart_id": ID("bus")}]} + assert subtree_references_uart(config, "bus") + + +def test_ignores_other_uarts_and_other_keys() -> None: + config = {"modbus": [{"uart_id": ID("other")}], "sensor": [{"id": ID("bus")}]} + assert not subtree_references_uart(config, "bus") + + +def test_finds_another_key() -> None: + config = {"modbus_tcp_uart": [{"tcp_uart_id": ID("bus")}]} + assert subtree_references_uart(config, "bus", "tcp_uart_id") + assert not subtree_references_uart(config, "bus") + + +def test_handles_scalars() -> None: + assert not subtree_references_uart("bus", "bus")