From 4e27b2fc125933d94cb9f0362550a55a3fb99816 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 16 Oct 2026 19:54:11 -1000 Subject: [PATCH] [components] Collect UART lines with LineReader (#20956) --- .../apc_proteous/apc_proteous_cover.cpp | 40 +++--- .../apc_proteous/apc_proteous_cover.h | 6 +- .../components/dfrobot_sen0609/__init__.py | 1 + .../dfrobot_sen0609/dfrobot_sen0609.cpp | 27 +--- .../dfrobot_sen0609/dfrobot_sen0609.h | 5 +- esphome/components/emontx/__init__.py | 1 + esphome/components/emontx/emontx.cpp | 67 ++++------ esphome/components/emontx/emontx.h | 8 +- .../hrxl_maxsonar_wr/hrxl_maxsonar_wr.cpp | 55 ++++---- .../hrxl_maxsonar_wr/hrxl_maxsonar_wr.h | 6 +- .../components/hydreon_rgxx/hydreon_rgxx.cpp | 122 +++++++++--------- .../components/hydreon_rgxx/hydreon_rgxx.h | 9 +- esphome/components/hydreon_rgxx/sensor.py | 1 + esphome/components/sim800l/__init__.py | 1 + esphome/components/sim800l/sim800l.cpp | 26 ++-- esphome/components/sim800l/sim800l.h | 4 +- esphome/components/smt100/sensor.py | 1 + esphome/components/smt100/smt100.cpp | 31 +---- esphome/components/smt100/smt100.h | 9 +- tests/components/apc_proteous/common.h | 2 +- .../cover/apc_proteous_cover_test.cpp | 8 +- tests/components/dfrobot_sen0609/__init__.py | 14 ++ tests/components/emontx/__init__.py | 11 ++ 23 files changed, 203 insertions(+), 252 deletions(-) create mode 100644 tests/components/dfrobot_sen0609/__init__.py create mode 100644 tests/components/emontx/__init__.py diff --git a/esphome/components/apc_proteous/apc_proteous_cover.cpp b/esphome/components/apc_proteous/apc_proteous_cover.cpp index f6a8432faa0..c17773da29c 100644 --- a/esphome/components/apc_proteous/apc_proteous_cover.cpp +++ b/esphome/components/apc_proteous/apc_proteous_cover.cpp @@ -32,40 +32,35 @@ void APCProteousCover::loop() { if (!this->read_byte(&data)) { continue; } - if (data == '\r') { - if (this->rx_len_ != 0) { - this->rx_buffer_[this->rx_len_] = '\0'; - ESP_LOGVV(TAG, "rx: '%s'", this->rx_buffer_); - this->parse_response_(); - this->rx_len_ = 0; - } - } else if (data != '\n') { - if (this->rx_len_ >= MAX_RESPONSE_LEN) { - ESP_LOGVV(TAG, "rx overflow, discarding"); - this->rx_len_ = 0; - } - this->rx_buffer_[this->rx_len_++] = static_cast(data); + const char *line = this->line_reader_.feed(data); + if (this->line_reader_.overflowed()) { + // Start over from this byte, so a frame right after unterminated noise is still read + ESP_LOGVV(TAG, "rx overflow, discarding"); + this->line_reader_.restart_with(data); + } else if (line != nullptr && line[0] != '\0') { + ESP_LOGVV(TAG, "rx: '%s'", line); + this->parse_response_(line); } } } -void APCProteousCover::parse_response_() { +void APCProteousCover::parse_response_(const char *line) { // Expected format: "?s=XX" or "?x=XX" where XX is a hex value - if (this->rx_len_ < 4 || this->rx_buffer_[0] != '?' || this->rx_buffer_[2] != '=') { - ESP_LOGV(TAG, "Invalid response: %s", this->rx_buffer_); + if (line[0] != '?' || line[1] == '\0' || line[2] != '=' || line[3] == '\0') { + ESP_LOGV(TAG, "Invalid response: %s", line); return; } - const char *hex_str = this->rx_buffer_ + 3; + const char *hex_str = line + 3; char *end_ptr; uint8_t value = static_cast(strtol(hex_str, &end_ptr, 16)); if (end_ptr == hex_str) { - ESP_LOGW(TAG, "Failed to parse hex value: %s", this->rx_buffer_); + ESP_LOGW(TAG, "Failed to parse hex value: %s", line); return; } bool state_changed = false; - char type = this->rx_buffer_[1]; + char type = line[1]; if (type == 's') { // bit 0 = operating, bit 1 = direction (0 = opening, 1 = closing) CoverOperation new_operation = COVER_OPERATION_IDLE; @@ -154,11 +149,10 @@ void APCProteousCover::update() { return; } // The line is idle, so anything left in the buffer is an unterminated echo - if (this->rx_len_ != 0) { - this->rx_buffer_[this->rx_len_] = '\0'; - ESP_LOGVV(TAG, "discarding stale rx '%s' before query", this->rx_buffer_); - this->rx_len_ = 0; + if (this->line_reader_.length() != 0) { + ESP_LOGVV(TAG, "discarding stale rx '%s' before query", this->line_reader_.partial()); } + this->line_reader_.reset(); const char *query = this->query_s_next_ ? QUERY_S : QUERY_X; ESP_LOGVV(TAG, "tx: %s", query); diff --git a/esphome/components/apc_proteous/apc_proteous_cover.h b/esphome/components/apc_proteous/apc_proteous_cover.h index 03c8eb5c4d1..dd15a1a15c7 100644 --- a/esphome/components/apc_proteous/apc_proteous_cover.h +++ b/esphome/components/apc_proteous/apc_proteous_cover.h @@ -1,6 +1,7 @@ #pragma once #include "esphome/core/component.h" +#include "esphome/core/line_reader.h" #include "esphome/components/uart/uart.h" #include "esphome/components/cover/cover.h" @@ -17,7 +18,7 @@ class APCProteousCover : public cover::Cover, public PollingComponent, public ua protected: void control(const cover::CoverCall &call) override; - void parse_response_(); + void parse_response_(const char *line); void send_command_(const char *cmd); void write_command_(const char *cmd, uint32_t now); void stop_cmd_(uint32_t now); @@ -43,8 +44,7 @@ class APCProteousCover : public cover::Cover, public PollingComponent, public ua const char *pending_command_{nullptr}; uint32_t pending_command_time_{0}; uint32_t last_command_tx_{0}; - char rx_buffer_[MAX_RESPONSE_LEN + 1]; - uint8_t rx_len_{0}; + LineReader line_reader_; uint8_t command_retries_{0}; bool query_s_next_{true}; }; diff --git a/esphome/components/dfrobot_sen0609/__init__.py b/esphome/components/dfrobot_sen0609/__init__.py index c1d2f806377..76ca362949a 100644 --- a/esphome/components/dfrobot_sen0609/__init__.py +++ b/esphome/components/dfrobot_sen0609/__init__.py @@ -80,5 +80,6 @@ def final_validate_mode(keys: dict[str, str]) -> Callable[[ConfigType], None]: async def to_code(config: ConfigType) -> None: var = cg.new_Pvariable(config[CONF_ID], RUN_MODES[config[CONF_MODE]]) + cg.add_define("USE_LINE_READER") await cg.register_component(var, config) await uart.register_uart_device(var, config) diff --git a/esphome/components/dfrobot_sen0609/dfrobot_sen0609.cpp b/esphome/components/dfrobot_sen0609/dfrobot_sen0609.cpp index 5f42d05a827..e1a1dbe89d0 100644 --- a/esphome/components/dfrobot_sen0609/dfrobot_sen0609.cpp +++ b/esphome/components/dfrobot_sen0609/dfrobot_sen0609.cpp @@ -210,30 +210,15 @@ void DFRobotSEN0609Component::loop() { } void DFRobotSEN0609Component::readline_(uint8_t c) { - if (c == '\r') - return; - if (c == '\n') { - if (!this->line_overflow_) { - this->line_[this->line_pos_] = '\0'; - this->handle_line_(this->line_); - } - this->line_overflow_ = false; - this->line_pos_ = 0; + char *line = this->line_reader_.feed(c); + if (line != nullptr) { + this->handle_line_(line); return; } - if (this->line_overflow_) - return; - if (this->line_pos_ >= MAX_LINE_LENGTH - 1) { - ESP_LOGW(TAG, "Line too long, dropped"); - this->line_overflow_ = true; - return; - } - this->line_[this->line_pos_++] = static_cast(c); + this->line_reader_.log_if_overflowed(TAG); // The prompt has no line end, and the echo of the next command follows it on the same line - if (c == '>') { - this->line_[this->line_pos_] = '\0'; - if (ESPHOME_strcmp_P(this->line_, ESPHOME_PSTR("DFRobot:/>")) == 0) - this->line_pos_ = 0; + if (c == '>' && ESPHOME_strcmp_P(this->line_reader_.partial(), ESPHOME_PSTR("DFRobot:/>")) == 0) { + this->line_reader_.reset(); } } diff --git a/esphome/components/dfrobot_sen0609/dfrobot_sen0609.h b/esphome/components/dfrobot_sen0609/dfrobot_sen0609.h index ca70fa14943..2a017492691 100644 --- a/esphome/components/dfrobot_sen0609/dfrobot_sen0609.h +++ b/esphome/components/dfrobot_sen0609/dfrobot_sen0609.h @@ -24,6 +24,7 @@ #ifdef USE_BUTTON #include "esphome/components/button/button.h" #endif +#include "esphome/core/line_reader.h" #include "esphome/components/uart/uart.h" #include @@ -188,7 +189,7 @@ class DFRobotSEN0609Component final : public Component, public uart::UARTDevice std::array switch_states_{}; #endif - char line_[MAX_LINE_LENGTH]; + LineReader line_reader_; char command_[MAX_COMMAND_LENGTH]{}; float response_[MAX_RESPONSE_VALUES]{}; uint32_t sent_ms_{0}; @@ -210,7 +211,6 @@ class DFRobotSEN0609Component final : public Component, public uart::UARTDevice RunMode mode_; Operation operation_{Operation::OPERATION_NONE}; Param param_{PARAM_COUNT}; - uint8_t line_pos_{0}; uint8_t response_count_{0}; uint8_t out_pin_level_{0}; uint8_t attempts_{0}; @@ -221,7 +221,6 @@ class DFRobotSEN0609Component final : public Component, public uart::UARTDevice uint8_t led_on_{0}; bool sent_led_on_{false}; #endif - bool line_overflow_{false}; bool read_failed_{false}; bool bad_frame_logged_{false}; bool echo_seen_{false}; diff --git a/esphome/components/emontx/__init__.py b/esphome/components/emontx/__init__.py index 8d2351f1fad..bdb07ed4990 100644 --- a/esphome/components/emontx/__init__.py +++ b/esphome/components/emontx/__init__.py @@ -132,6 +132,7 @@ _CALLBACK_AUTOMATIONS = ( async def to_code(config: ConfigType) -> None: var = cg.new_Pvariable(config[CONF_ID]) + cg.add_define("USE_LINE_READER") # Initialize sensor storage with count from final_validate before any # await, so platform to_code() calls always see it initialized diff --git a/esphome/components/emontx/emontx.cpp b/esphome/components/emontx/emontx.cpp index dca5ae2131d..888b12f4bf4 100644 --- a/esphome/components/emontx/emontx.cpp +++ b/esphome/components/emontx/emontx.cpp @@ -55,7 +55,6 @@ static bool supports_lock(const char *version, const char *end) { #endif void EmonTx::setup() { - this->buffer_pos_ = 0; if (this->lock_on_reboot_ == LockOnReboot::LOCK_ON_REBOOT_ALWAYS) { // The line feed ends any boot noise this->write_byte('\n'); @@ -111,11 +110,14 @@ void EmonTx::loop() { if (this->was_paused_) { // Drop what arrived during the pause and resync at the next line start this->was_paused_ = false; - this->buffer_pos_ = 0; - this->discarding_line_ = true; - uint8_t dropped; - while (this->available() > 0 && this->read_byte(&dropped)) - this->discarding_line_ = dropped != '\n'; + uint8_t dropped = 0; + while (this->available() > 0 && this->read_byte(&dropped)) { + } + if (dropped == '\n') { + this->line_reader_.reset(); + } else { + this->line_reader_.discard_line(); + } #ifdef USE_EMONTX_VERSION // The emonTx may have been flashed, or a request dropped, during the pause if (this->wants_version_()) @@ -129,43 +131,24 @@ void EmonTx::loop() { if (!this->read_byte(&received)) break; - if (received == '\n') { - // End of line - process the buffer - if (this->discarding_line_) { - this->discarding_line_ = false; - } else if (this->buffer_pos_ > 0) { - // Null-terminate for safe logging and c_str() use - size_t len = this->buffer_pos_; - this->buffer_[len] = '\0'; - this->buffer_pos_ = 0; - - StringRef line(this->buffer_.data(), len); - ESP_LOGD(TAG, "Received line: %s", line.c_str()); - - // Fire data callbacks for all received lines - this->data_callbacks_.call(line); - - // Check if this line is JSON (starts with '{') - if (this->buffer_[0] == '{') { - ESP_LOGV(TAG, "Line is JSON, parsing..."); - this->parse_json_(this->buffer_.data(), len); - } -#ifdef USE_EMONTX_VERSION - if (this->wants_version_()) { - this->parse_msg_number_(this->buffer_.data(), len); - this->parse_version_line_(this->buffer_.data(), len); - } -#endif - } - } else if (received == '\r' || this->discarding_line_) { - // Ignore CR, and the rest of a line that overflowed or was interrupted by a pause - } else if (this->buffer_pos_ >= MAX_LINE_LENGTH) { - ESP_LOGW(TAG, "Buffer overflow (>%zu bytes), discarding line", MAX_LINE_LENGTH); - this->buffer_pos_ = 0; - this->discarding_line_ = true; - } else { - this->buffer_[this->buffer_pos_++] = static_cast(received); + size_t len; + char *line = this->line_reader_.feed(received, len); + this->line_reader_.log_if_overflowed(TAG); + if (line == nullptr || len == 0) { + continue; } + ESP_LOGD(TAG, "Received line: %s", line); + this->data_callbacks_.call(StringRef(line, len)); + if (line[0] == '{') { + ESP_LOGV(TAG, "Line is JSON, parsing..."); + this->parse_json_(line, len); + } +#ifdef USE_EMONTX_VERSION + if (this->wants_version_()) { + this->parse_msg_number_(line, len); + this->parse_version_line_(line, len); + } +#endif } } diff --git a/esphome/components/emontx/emontx.h b/esphome/components/emontx/emontx.h index a8d91d36eb6..520e7772a06 100644 --- a/esphome/components/emontx/emontx.h +++ b/esphome/components/emontx/emontx.h @@ -4,10 +4,10 @@ #include "esphome/core/defines.h" #include "esphome/core/helpers.h" #include "esphome/core/string_ref.h" +#include "esphome/core/line_reader.h" #include "esphome/components/uart/uart.h" #include "esphome/components/json/json_util.h" -#include #include #ifdef USE_SENSOR @@ -19,7 +19,7 @@ namespace esphome::emontx { -/// Maximum line length in bytes (plus one byte reserved for null terminator) +/// Maximum line length in characters static constexpr size_t MAX_LINE_LENGTH = 1024; enum class LockOnReboot : uint8_t { @@ -92,18 +92,16 @@ class EmonTx final : public Component, public uart::UARTDevice { #endif LazyCallbackManager json_callbacks_; LazyCallbackManager data_callbacks_; - uint16_t buffer_pos_{0}; std::atomic paused_{false}; bool was_paused_{false}; LockOnReboot lock_on_reboot_{LockOnReboot::LOCK_ON_REBOOT_NEVER}; - bool discarding_line_{false}; // Set after an overflow or a pause until the end of that line #ifdef USE_EMONTX_VERSION bool lock_supported_{false}; // The last version reported is v1.2.0 or later bool unlock_sent_{false}; // AUTO has sent 'emonunlock' since boot uint8_t version_frames_left_{0}; // Data lines still to wait for a version reply; 0 when none is owed uint32_t last_msg_number_{0}; #endif - std::array buffer_{}; + LineReader line_reader_; }; } // namespace esphome::emontx diff --git a/esphome/components/hrxl_maxsonar_wr/hrxl_maxsonar_wr.cpp b/esphome/components/hrxl_maxsonar_wr/hrxl_maxsonar_wr.cpp index 1d58a4317a6..5415b7af74f 100644 --- a/esphome/components/hrxl_maxsonar_wr/hrxl_maxsonar_wr.cpp +++ b/esphome/components/hrxl_maxsonar_wr/hrxl_maxsonar_wr.cpp @@ -12,9 +12,6 @@ namespace esphome::hrxl_maxsonar_wr { ESPHOME_LOG_TAG(TAG, "hrxl.maxsonar.wr.sensor"); -static const uint8_t ASCII_CR = 0x0D; -static const uint8_t ASCII_NBSP = 0xFF; -static const int MAX_DATA_LENGTH_BYTES = 6; /** * HRXL sensors output the format "R1234\r" at 6Hz @@ -25,44 +22,36 @@ static const int MAX_DATA_LENGTH_BYTES = 6; void HrxlMaxsonarWrComponent::loop() { uint8_t data; while (this->available() > 0) { - if (this->read_byte(&data)) { - buffer_ += (char) data; - this->check_buffer_(); + if (!this->read_byte(&data)) + continue; + size_t len; + const char *line = this->line_reader_.feed(data, len); + if (line != nullptr) { + ESP_LOGV(TAG, "Read from serial: %s", line); + this->handle_line_(line, len); + } else if (this->line_reader_.overflowed()) { + // No CR in time: report it and start over with the next byte + ESP_LOGW(TAG, "Invalid data read from sensor: %s", this->line_reader_.partial()); + this->line_reader_.reset(); } } } -void HrxlMaxsonarWrComponent::check_buffer_() { - // The sensor seems to inject a rogue ASCII 255 byte from time to time. Get rid of that. - if (this->buffer_.back() == static_cast(ASCII_NBSP)) { - this->buffer_.pop_back(); +void HrxlMaxsonarWrComponent::handle_line_(const char *line, size_t len) { + if (line[0] != 'R') { + ESP_LOGW(TAG, "Invalid data read from sensor: %s", line); return; } + int millimeters = parse_number(line + 1).value_or(0); - // Stop reading at ASCII_CR. Also prevent the buffer from growing - // indefinitely if no ASCII_CR is received after MAX_DATA_LENGTH_BYTES. - if (this->buffer_.back() == static_cast(ASCII_CR) || this->buffer_.length() >= MAX_DATA_LENGTH_BYTES) { - ESP_LOGV(TAG, "Read from serial: %s", this->buffer_.c_str()); - - size_t rpos = this->buffer_.find(static_cast(ASCII_CR)); - - if (this->buffer_.length() <= MAX_DATA_LENGTH_BYTES && this->buffer_[0] == 'R' && rpos != std::string::npos) { - std::string distance = this->buffer_.substr(1, rpos - 1); - int millimeters = parse_number(distance).value_or(0); - - // XL reports in cm instead of mm and reports 3 digits instead of 4 - if (distance.length() == 3) { - millimeters = millimeters * 10; - } - - float meters = float(millimeters) / 1000.0f; - ESP_LOGV(TAG, "Distance from sensor: %d mm, %f m", millimeters, meters); - this->publish_state(meters); - } else { - ESP_LOGW(TAG, "Invalid data read from sensor: %s", this->buffer_.c_str()); - } - this->buffer_.clear(); + // XL reports in cm instead of mm and reports 3 digits instead of 4 + if (len == 4) { + millimeters *= 10; } + + float meters = float(millimeters) / 1000.0f; + ESP_LOGV(TAG, "Distance from sensor: %d mm, %f m", millimeters, meters); + this->publish_state(meters); } void HrxlMaxsonarWrComponent::dump_config() { diff --git a/esphome/components/hrxl_maxsonar_wr/hrxl_maxsonar_wr.h b/esphome/components/hrxl_maxsonar_wr/hrxl_maxsonar_wr.h index 36346c8293e..ada26d21e29 100644 --- a/esphome/components/hrxl_maxsonar_wr/hrxl_maxsonar_wr.h +++ b/esphome/components/hrxl_maxsonar_wr/hrxl_maxsonar_wr.h @@ -2,6 +2,7 @@ #include "esphome/core/component.h" #include "esphome/components/sensor/sensor.h" +#include "esphome/core/line_reader.h" #include "esphome/components/uart/uart.h" namespace esphome::hrxl_maxsonar_wr { @@ -15,9 +16,10 @@ class HrxlMaxsonarWrComponent final : public sensor::Sensor, public Component, p void dump_config() override; protected: - void check_buffer_(); + void handle_line_(const char *line, size_t len); - std::string buffer_; + // "R1234" plus the NUL; the sensor ends each reading with CR and sometimes injects a rogue 0xFF byte + LineReader<6, '\r', static_cast(0xFF)> line_reader_; }; } // namespace esphome::hrxl_maxsonar_wr diff --git a/esphome/components/hydreon_rgxx/hydreon_rgxx.cpp b/esphome/components/hydreon_rgxx/hydreon_rgxx.cpp index a2f8fc50862..a1d6553073a 100644 --- a/esphome/components/hydreon_rgxx/hydreon_rgxx.cpp +++ b/esphome/components/hydreon_rgxx/hydreon_rgxx.cpp @@ -1,5 +1,6 @@ #include "hydreon_rgxx.h" #include "esphome/core/log.h" +#include namespace esphome::hydreon_rgxx { @@ -7,8 +8,6 @@ ESPHOME_LOG_TAG(TAG, "hydreon_rgxx.sensor"); static constexpr uint32_t REBOOT_INTERVAL_ID = 0; -static const int MAX_DATA_LENGTH_BYTES = 80; -static const uint8_t ASCII_LF = 0x0A; #define HYDREON_RGXX_COMMA , static const char *const PROTOCOL_NAMES[] = {HYDREON_RGXX_PROTOCOL_LIST(, HYDREON_RGXX_COMMA)}; static const char *const IGNORE_STRINGS[] = {HYDREON_RGXX_IGNORE_LIST(, HYDREON_RGXX_COMMA)}; @@ -111,13 +110,14 @@ void HydreonRGxxComponent::update() { void HydreonRGxxComponent::loop() { uint8_t data; while (this->available() > 0) { - if (this->read_byte(&data)) { - buffer_ += (char) data; - if (this->buffer_.back() == static_cast(ASCII_LF) || this->buffer_.length() >= MAX_DATA_LENGTH_BYTES) { - // complete line received - this->process_line_(); - this->buffer_.clear(); - } + if (!this->read_byte(&data)) { + continue; + } + size_t len; + const char *line = this->line_reader_.feed(data, len); + this->line_reader_.log_if_overflowed(TAG); + if (line != nullptr) { + this->process_line_(StringRef(line, len)); } } } @@ -156,33 +156,31 @@ void HydreonRGxxComponent::schedule_reboot_() { }); } -void HydreonRGxxComponent::process_line_() { - ESP_LOGV(TAG, "Read from serial: %s", this->buffer_.substr(0, this->buffer_.size() - 2).c_str()); +void HydreonRGxxComponent::process_line_(StringRef line) { + ESP_LOGV(TAG, "Read from serial: %s", line.c_str()); - if (buffer_[0] == ';') { - ESP_LOGI(TAG, "Comment: %s", this->buffer_.substr(0, this->buffer_.size() - 2).c_str()); + if (line[0] == ';') { + ESP_LOGI(TAG, "Comment: %s", line.c_str()); return; } - std::string::size_type newlineposn = this->buffer_.find('\n'); - if (newlineposn <= 1) { - // allow both \r\n and \n + if (line.empty()) { ESP_LOGD(TAG, "Received empty line"); return; } - if (newlineposn <= 2) { + if (line.size() == 1) { // single character lines, such as acknowledgements - ESP_LOGD(TAG, "Received ack: %s", this->buffer_.substr(0, this->buffer_.size() - 2).c_str()); + ESP_LOGD(TAG, "Received ack: %s", line.c_str()); return; } - if (this->buffer_.find("LensBad") != std::string::npos) { + if (line.find("LensBad") != std::string::npos) { ESP_LOGW(TAG, "Received LensBad!"); this->lens_bad_ = true; } - if (this->buffer_.find("EmSat") != std::string::npos) { + if (line.find("EmSat") != std::string::npos) { ESP_LOGW(TAG, "Received EmSat!"); this->em_sat_ = true; } - if (buffer_.starts_with("PwrDays")) { + if (line.starts_with("PwrDays")) { if (this->boot_count_ <= 0) { this->boot_count_ = 1; } else { @@ -190,7 +188,7 @@ void HydreonRGxxComponent::process_line_() { } this->cancel_interval(REBOOT_INTERVAL_ID); this->no_response_count_ = 0; - ESP_LOGI(TAG, "Boot detected: %s", this->buffer_.substr(0, this->buffer_.size() - 2).c_str()); + ESP_LOGI(TAG, "Boot detected: %s", line.c_str()); if (this->model_ == RG15) { if (this->resolution_ == FORCE_HIGH) { @@ -211,17 +209,18 @@ void HydreonRGxxComponent::process_line_() { } return; } - if (buffer_.starts_with("SW")) { - std::string::size_type majend = this->buffer_.find('.'); - std::string::size_type endversion = this->buffer_.find(' ', 3); + if (line.starts_with("SW")) { + StringRef::size_type majend = line.find('.'); + StringRef::size_type endversion = line.find(' ', 3); if (majend == std::string::npos || endversion == std::string::npos || majend > endversion) { - ESP_LOGW(TAG, "invalid version string: %s", this->buffer_.substr(0, this->buffer_.size() - 2).c_str()); + ESP_LOGW(TAG, "invalid version string: %s", line.c_str()); } - int major = strtol(this->buffer_.substr(3, majend - 3).c_str(), nullptr, 10); - int minor = strtol(this->buffer_.substr(majend + 1, endversion - (majend + 1)).c_str(), nullptr, 10); + // strtol stops at the '.' and the ' ' + int major = line.size() > 3 ? strtol(line.c_str() + 3, nullptr, 10) : 0; + int minor = strtol(line.c_str() + (majend == std::string::npos ? 0 : majend + 1), nullptr, 10); if (major > 10 || minor >= 1000 || minor < 0 || major < 0) { - ESP_LOGW(TAG, "invalid version: %s", this->buffer_.substr(0, this->buffer_.size() - 2).c_str()); + ESP_LOGW(TAG, "invalid version: %s", line.c_str()); } this->sw_version_ = major * 1000 + minor; ESP_LOGI(TAG, "detected sw version %i", this->sw_version_); @@ -229,56 +228,51 @@ void HydreonRGxxComponent::process_line_() { } bool is_data_line = false; for (int i = 0; i < NUM_SENSORS; i++) { - if (this->sensors_[i] != nullptr && this->buffer_.find(PROTOCOL_NAMES[i]) != std::string::npos) { - is_data_line = true; - break; + if (this->sensors_[i] == nullptr) { + continue; } - } - if (is_data_line) { - std::string::size_type tc = this->buffer_.find("TooCold"); - this->too_cold_ |= tc != std::string::npos; - if (this->too_cold_) { - ESP_LOGD(TAG, "Received TooCold"); + StringRef::size_type n = line.find(PROTOCOL_NAMES[i]); + if (n == std::string::npos) { + continue; } - for (int i = 0; i < NUM_SENSORS; i++) { - if (this->sensors_[i] == nullptr) { - continue; - } - std::string::size_type n = this->buffer_.find(PROTOCOL_NAMES[i]); + is_data_line = true; + + if (n == line.find('t', n)) { + // The device temperature ('t') response contains both °C and °F values: + // "t 72F 22C". + // ESPHome uses only °C, only parse °C value (move past 'F'). + n = line.find('F', n); if (n == std::string::npos) { continue; } + n += 1; // move past 'F' + } else { + n += strlen(PROTOCOL_NAMES[i]); // move past protocol name + } - if (n == this->buffer_.find('t', n)) { - // The device temperature ('t') response contains both °C and °F values: - // "t 72F 22C". - // ESPHome uses only °C, only parse °C value (move past 'F'). - n = this->buffer_.find('F', n); - if (n == std::string::npos) { - continue; - } - n += 1; // move past 'F' - } else { - n += strlen(PROTOCOL_NAMES[i]); // move past protocol name - } - - // parse value, starting at str position n - float data = strtof(this->buffer_.substr(n).c_str(), nullptr); - this->sensors_[i]->publish_state(data); - ESP_LOGD(TAG, "Received %s: %f", PROTOCOL_NAMES[i], this->sensors_[i]->get_raw_state()); - this->sensors_received_ |= (1 << i); + // parse value, starting at str position n + float data = strtof(line.c_str() + n, nullptr); + this->sensors_[i]->publish_state(data); + ESP_LOGD(TAG, "Received %s: %f", PROTOCOL_NAMES[i], this->sensors_[i]->get_raw_state()); + this->sensors_received_ |= (1 << i); + } + if (is_data_line) { + StringRef::size_type tc = line.find("TooCold"); + this->too_cold_ |= tc != std::string::npos; + if (this->too_cold_) { + ESP_LOGD(TAG, "Received TooCold"); } if (this->request_temperature_ && this->num_sensors_missing_() == 1) { this->write_str("T\n"); } } else { for (const auto *ignore : IGNORE_STRINGS) { - if (buffer_.starts_with(ignore)) { - ESP_LOGI(TAG, "Ignoring %s", this->buffer_.substr(0, this->buffer_.size() - 2).c_str()); + if (line.starts_with(ignore)) { + ESP_LOGI(TAG, "Ignoring %s", line.c_str()); return; } } - ESP_LOGI(TAG, "Got unknown line: %s", this->buffer_.c_str()); + ESP_LOGI(TAG, "Got unknown line: %s", line.c_str()); } } diff --git a/esphome/components/hydreon_rgxx/hydreon_rgxx.h b/esphome/components/hydreon_rgxx/hydreon_rgxx.h index a7bde6105c6..e0fa8fc2b4a 100644 --- a/esphome/components/hydreon_rgxx/hydreon_rgxx.h +++ b/esphome/components/hydreon_rgxx/hydreon_rgxx.h @@ -2,10 +2,12 @@ #include "esphome/core/component.h" #include "esphome/core/defines.h" +#include "esphome/core/string_ref.h" #include "esphome/components/sensor/sensor.h" #ifdef USE_BINARY_SENSOR #include "esphome/components/binary_sensor/binary_sensor.h" #endif +#include "esphome/core/line_reader.h" #include "esphome/components/uart/uart.h" namespace esphome::hydreon_rgxx { @@ -55,10 +57,9 @@ class HydreonRGxxComponent final : public PollingComponent, public uart::UARTDev void set_disable_led(bool disable_led) { this->disable_led_ = disable_led; } protected: - void process_line_(); + /// line is NUL terminated + void process_line_(StringRef line); void schedule_reboot_(); - bool buffer_starts_with_(const std::string &prefix); - bool buffer_starts_with_(const char *prefix); int num_sensors_missing_(); sensor::Sensor *sensors_[NUM_SENSORS] = {nullptr}; @@ -70,7 +71,6 @@ class HydreonRGxxComponent final : public PollingComponent, public uart::UARTDev int16_t boot_count_ = 0; int16_t no_response_count_ = 0; - std::string buffer_; RGModel model_ = RG9; RG15Resolution resolution_ = FORCE_HIGH; @@ -81,6 +81,7 @@ class HydreonRGxxComponent final : public PollingComponent, public uart::UARTDev bool em_sat_ = false; bool request_temperature_ = false; bool disable_led_ = false; + LineReader<80> line_reader_; // bit field showing which sensors we have received data for int sensors_received_ = -1; diff --git a/esphome/components/hydreon_rgxx/sensor.py b/esphome/components/hydreon_rgxx/sensor.py index 8e269fef9a0..222a5308ae1 100644 --- a/esphome/components/hydreon_rgxx/sensor.py +++ b/esphome/components/hydreon_rgxx/sensor.py @@ -141,6 +141,7 @@ FINAL_VALIDATE_SCHEMA = uart.final_validate_device_schema( async def to_code(config: ConfigType) -> None: var = cg.new_Pvariable(config[CONF_ID]) + cg.add_define("USE_LINE_READER") await cg.register_component(var, config) await uart.register_uart_device(var, config) diff --git a/esphome/components/sim800l/__init__.py b/esphome/components/sim800l/__init__.py index e20c091b75c..efc742610d9 100644 --- a/esphome/components/sim800l/__init__.py +++ b/esphome/components/sim800l/__init__.py @@ -67,6 +67,7 @@ _CALLBACK_AUTOMATIONS = ( async def to_code(config): var = cg.new_Pvariable(config[CONF_ID]) + cg.add_define("USE_LINE_READER") await cg.register_component(var, config) await uart.register_uart_device(var, config) diff --git a/esphome/components/sim800l/sim800l.cpp b/esphome/components/sim800l/sim800l.cpp index e75e5e95dd3..ba07bc3d6d3 100644 --- a/esphome/components/sim800l/sim800l.cpp +++ b/esphome/components/sim800l/sim800l.cpp @@ -426,26 +426,20 @@ void Sim800LComponent::loop() { uint8_t byte; this->read_byte(&byte); - if (this->read_pos_ == SIM800L_READ_BUFFER_LENGTH) - this->read_pos_ = 0; + ESP_LOGVV(TAG, "Buffer pos: %u %d", this->line_reader_.length(), byte); // NOLINT - ESP_LOGVV(TAG, "Buffer pos: %u %d", this->read_pos_, byte); // NOLINT - - if (byte == ASCII_CR) - continue; if (byte >= 0x7F) byte = '?'; // need to be valid utf8 string for log functions. - this->read_buffer_[this->read_pos_] = byte; - - if (this->state_ == STATE_SENDING_SMS_2 && this->read_pos_ == 0 && byte == '>') - this->read_buffer_[++this->read_pos_] = ASCII_LF; - - if (this->read_buffer_[this->read_pos_] == ASCII_LF) { - this->read_buffer_[this->read_pos_] = 0; - this->read_pos_ = 0; - this->parse_cmd_(this->read_buffer_); + const char *line = this->line_reader_.feed(byte); + if (line == nullptr && this->state_ == STATE_SENDING_SMS_2 && byte == '>' && this->line_reader_.length() == 1) { + // The send prompt has no line end + line = this->line_reader_.partial(); + this->line_reader_.reset(); + } + if (line != nullptr) { + this->parse_cmd_(line); } else { - this->read_pos_++; + this->line_reader_.log_if_overflowed(TAG); } } if (state_ == STATE_INIT && this->registered_ && diff --git a/esphome/components/sim800l/sim800l.h b/esphome/components/sim800l/sim800l.h index bbe8e14cf7e..a6f9544781d 100644 --- a/esphome/components/sim800l/sim800l.h +++ b/esphome/components/sim800l/sim800l.h @@ -10,6 +10,7 @@ #ifdef USE_SENSOR #include "esphome/components/sensor/sensor.h" #endif +#include "esphome/core/line_reader.h" #include "esphome/components/uart/uart.h" #include "esphome/core/automation.h" @@ -95,8 +96,7 @@ class Sim800LComponent final : public uart::UARTDevice, public PollingComponent #endif std::string sender_; std::string message_; - char read_buffer_[SIM800L_READ_BUFFER_LENGTH]; - size_t read_pos_{0}; + LineReader line_reader_; uint8_t parse_index_{0}; uint8_t watch_dog_{0}; bool expect_ack_{false}; diff --git a/esphome/components/smt100/sensor.py b/esphome/components/smt100/sensor.py index dae98bc1897..fba10eab9ae 100644 --- a/esphome/components/smt100/sensor.py +++ b/esphome/components/smt100/sensor.py @@ -80,6 +80,7 @@ FINAL_VALIDATE_SCHEMA = uart.final_validate_device_schema( async def to_code(config: ConfigType) -> None: var = cg.new_Pvariable(config[CONF_ID]) + cg.add_define("USE_LINE_READER") await cg.register_component(var, config) await uart.register_uart_device(var, config) diff --git a/esphome/components/smt100/smt100.cpp b/esphome/components/smt100/smt100.cpp index 7785bbca2d7..8a8f82b8504 100644 --- a/esphome/components/smt100/smt100.cpp +++ b/esphome/components/smt100/smt100.cpp @@ -12,8 +12,13 @@ void SMT100Component::update() { void SMT100Component::loop() { while (this->available() != 0) { - if (this->readline_(this->read(), this->readline_buffer_, MAX_LINE_LENGTH) > 0) { - char *token = strtok(this->readline_buffer_, ","); + const int readch = this->read(); + if (readch <= 0) + continue; + char *line = this->line_reader_.feed(readch); + this->line_reader_.log_if_overflowed(TAG); + if (line != nullptr && line[0] != '\0') { + char *token = strtok(line, ","); if (!token) continue; int counts = (int) strtol(token, nullptr, 10); @@ -67,26 +72,4 @@ void SMT100Component::dump_config() { LOG_UPDATE_INTERVAL(this); } -int SMT100Component::readline_(int readch, char *buffer, int len) { - int rpos; - - if (readch > 0) { - switch (readch) { - case '\n': // Ignore new-lines - break; - case '\r': // Return on CR - rpos = this->readline_pos_; - this->readline_pos_ = 0; // Reset position index ready for next time - return rpos; - default: - if (this->readline_pos_ < len - 1) { - buffer[this->readline_pos_++] = readch; - buffer[this->readline_pos_] = 0; - } - } - } - // No end of line has been found, so return -1. - return -1; -} - } // namespace esphome::smt100 diff --git a/esphome/components/smt100/smt100.h b/esphome/components/smt100/smt100.h index 55977a5caf6..12641121fe6 100644 --- a/esphome/components/smt100/smt100.h +++ b/esphome/components/smt100/smt100.h @@ -2,12 +2,14 @@ #include "esphome/core/component.h" #include "esphome/components/sensor/sensor.h" +#include "esphome/core/line_reader.h" #include "esphome/components/uart/uart.h" namespace esphome::smt100 { class SMT100Component final : public PollingComponent, public uart::UARTDevice { - static const uint16_t MAX_LINE_LENGTH = 31; + // 33 characters plus the NUL; the longest line, "15000,80.00,100.00,-20.00,12.00", is 31 + static constexpr uint16_t LINE_BUFFER_SIZE = 34; public: SMT100Component() = default; @@ -25,10 +27,7 @@ class SMT100Component final : public PollingComponent, public uart::UARTDevice { void set_voltage_sensor(sensor::Sensor *voltage_sensor) { this->voltage_sensor_ = voltage_sensor; } protected: - int readline_(int readch, char *buffer, int len); - - char readline_buffer_[MAX_LINE_LENGTH]{}; - int readline_pos_{0}; + LineReader line_reader_; sensor::Sensor *counts_sensor_{nullptr}; sensor::Sensor *permittivity_sensor_{nullptr}; diff --git a/tests/components/apc_proteous/common.h b/tests/components/apc_proteous/common.h index a3baa5131de..1b595104f75 100644 --- a/tests/components/apc_proteous/common.h +++ b/tests/components/apc_proteous/common.h @@ -51,9 +51,9 @@ class TestableAPCProteousCover : public APCProteousCover { using APCProteousCover::MAX_RESPONSE_LEN; using APCProteousCover::command_retries_; using APCProteousCover::last_command_tx_; + using APCProteousCover::line_reader_; using APCProteousCover::pending_command_; using APCProteousCover::pending_command_time_; - using APCProteousCover::rx_len_; using APCProteousCover::target_position_; }; diff --git a/tests/components/apc_proteous/cover/apc_proteous_cover_test.cpp b/tests/components/apc_proteous/cover/apc_proteous_cover_test.cpp index 566701338b3..3e784f06bfc 100644 --- a/tests/components/apc_proteous/cover/apc_proteous_cover_test.cpp +++ b/tests/components/apc_proteous/cover/apc_proteous_cover_test.cpp @@ -111,7 +111,7 @@ TEST_F(APCProteousCoverTest, MalformedFramesAreIgnored) { EXPECT_FLOAT_EQ(this->cover_.position, 0.5f); EXPECT_EQ(this->cover_.current_operation, COVER_OPERATION_IDLE); EXPECT_EQ(this->publishes_, 0); - EXPECT_EQ(this->cover_.rx_len_, 0); + EXPECT_EQ(this->cover_.line_reader_.length(), 0); } TEST_F(APCProteousCoverTest, LineFeedsAreIgnored) { @@ -134,7 +134,7 @@ TEST_F(APCProteousCoverTest, UnterminatedNoiseIsDiscarded) { EXPECT_FLOAT_EQ(this->cover_.position, 0.25f); this->feed_("0123456789ABCDEF0123456789ABCDEF"); - EXPECT_LE(this->cover_.rx_len_, TestableAPCProteousCover::MAX_RESPONSE_LEN); + EXPECT_LE(this->cover_.line_reader_.length(), TestableAPCProteousCover::MAX_RESPONSE_LEN); } TEST_F(APCProteousCoverTest, OpenSendsOpenCommand) { @@ -405,9 +405,9 @@ TEST_F(APCProteousCoverTest, UpdateHoldsOffPollingAfterCommand) { // A command echo left without a terminator must not merge with the next response. TEST_F(APCProteousCoverTest, UpdateDiscardsStaleEcho) { this->feed_("*6"); - EXPECT_NE(this->cover_.rx_len_, 0); + EXPECT_NE(this->cover_.line_reader_.length(), 0); this->cover_.update(); - EXPECT_EQ(this->cover_.rx_len_, 0); + EXPECT_EQ(this->cover_.line_reader_.length(), 0); this->feed_("?s=01\r"); EXPECT_EQ(this->cover_.current_operation, COVER_OPERATION_OPENING); } diff --git a/tests/components/dfrobot_sen0609/__init__.py b/tests/components/dfrobot_sen0609/__init__.py new file mode 100644 index 00000000000..5cf3c18e999 --- /dev/null +++ b/tests/components/dfrobot_sen0609/__init__.py @@ -0,0 +1,14 @@ +import esphome.codegen as cg +from esphome.types import ConfigType +from tests.testing_helpers import ComponentManifestOverride + + +def override_manifest(manifest: ComponentManifestOverride) -> None: + # One empty config entry so to_code runs and sets the define that keeps + # core/line_reader.cpp in the test build. + manifest.multi_conf = False + + async def to_code(config: ConfigType) -> None: + cg.add_define("USE_LINE_READER") + + manifest.to_code = to_code diff --git a/tests/components/emontx/__init__.py b/tests/components/emontx/__init__.py new file mode 100644 index 00000000000..143c58c8699 --- /dev/null +++ b/tests/components/emontx/__init__.py @@ -0,0 +1,11 @@ +import esphome.codegen as cg +from esphome.types import ConfigType +from tests.testing_helpers import ComponentManifestOverride + + +def override_manifest(manifest: ComponentManifestOverride) -> None: + # Only the define that keeps core/line_reader.cpp in the test build + async def to_code(config: ConfigType) -> None: + cg.add_define("USE_LINE_READER") + + manifest.to_code = to_code