[core] Add LineReader to collect lines in a fixed buffer (#20952)

esphome/core/config.py

@@ -1019,7 +1019,10 @@ _filter_platform_sources = filter_source_files_from_platform(
 
 # util.cpp holds only the out-of-line MQTT connectivity helpers; without
 # USE_MQTT the header provides inline stubs and the file must not compile.
-_filter_define_sources = filter_source_files_from_defines({"util.cpp": "USE_MQTT"})
+# line_reader.cpp compiles only for components that opt in with USE_LINE_READER.
+_filter_define_sources = filter_source_files_from_defines(
+    {"util.cpp": "USE_MQTT", "line_reader.cpp": "USE_LINE_READER"}
+)
 
 
 def FILTER_SOURCE_FILES() -> list[str]:

esphome/core/defines.h

@@ -114,6 +114,7 @@
 #define USE_LIGHT_GAMMA_LUT
 #define USE_LIGHT_RESUME_EFFECT
 #define USE_LIGHT_TRANSITION_PUBLISH_INTERVAL
+#define USE_LINE_READER
 #define USE_LOCK
 #define USE_LOGGER
 #define USE_LOGGER_LEVEL_LISTENERS

esphome/core/line_reader.cpp

@@ -0,0 +1,10 @@
+#include "esphome/core/line_reader.h"
+#include "esphome/core/log.h"
+
+namespace esphome {
+
+void log_line_dropped(const char *tag, size_t max_len) {
+  ESP_LOGW(tag, "Line over %u chars dropped", static_cast<unsigned>(max_len));
+}
+
+}  // namespace esphome

esphome/core/line_reader.h

@@ -0,0 +1,99 @@
+#pragma once
+
+#include <cstddef>
+#include <cstdint>
+#include <type_traits>
+
+namespace esphome {
+
+/// Logs the shared "line dropped" warning; out of line to keep each caller small. Components that call it
+/// (directly or via log_if_overflowed) add cg.add_define("USE_LINE_READER") so line_reader.cpp is compiled.
+void log_line_dropped(const char *tag, size_t max_len);
+
+/// Collects bytes into NUL terminated lines of up to N - 1 characters; IGNORE_CHAR bytes are skipped and an
+/// overlong line is dropped up to its terminator.
+template<size_t N, char TERMINATOR_CHAR = '\n', char IGNORE_CHAR = '\r'> class LineReader {
+  // pos_ is the length so far; N means "overflowed on this byte", N + 1 "dropping up to the terminator"
+  using pos_t = std::conditional_t<(N < 0xFE), uint8_t, uint16_t>;
+  static_assert(N >= 2 && N < 0xFFFE, "LineReader size out of range");
+  static_assert(TERMINATOR_CHAR != IGNORE_CHAR, "LineReader terminator and ignore byte must differ");
+
+ public:
+  /// Feed one byte; returns the completed line (empty lines included), valid until the next call, or nullptr.
+  char *feed(uint8_t c) {
+    size_t len = 0;
+    return this->feed(c, len);
+  }
+
+  /// As feed(c), and sets len to the line length when a line is returned.
+  char *feed(uint8_t c, size_t &len) {
+    pos_t pos = this->pos_;
+    if (pos == N) {
+      this->pos_ = pos = N + 1;
+    }
+    if (c == static_cast<uint8_t>(IGNORE_CHAR)) {
+      return nullptr;
+    }
+    if (c == static_cast<uint8_t>(TERMINATOR_CHAR)) {
+      this->pos_ = 0;
+      if (pos > N) {
+        return nullptr;
+      }
+      this->buf_[pos] = '\0';
+      len = pos;
+      return this->buf_;
+    }
+    if (pos < N - 1) {
+      this->buf_[pos] = static_cast<char>(c);
+      this->pos_ = pos + 1;
+    } else if (pos == N - 1) {
+      this->pos_ = N;
+    }
+    return nullptr;
+  }
+
+  /// Length of the partial line; 0 after a line is returned and while an overlong line is dropped.
+  size_t length() const { return this->pos_ < N ? this->pos_ : 0; }
+
+  /// The partial line, NUL terminated, for prompts with no line end; the first N - 1 characters on the byte
+  /// that overflowed, empty while the rest of that line is dropped.
+  const char *partial() {
+    this->buf_[this->end_()] = '\0';
+    return this->buf_;
+  }
+
+  /// True only right after the feed() that found the line too long; the rest of that line is then dropped.
+  bool overflowed() const { return this->pos_ == N; }
+
+  /// Log the shared "line dropped" warning when the last feed() overflowed.
+  void log_if_overflowed(const char *tag) const {
+    if (this->overflowed()) {
+      log_line_dropped(tag, N - 1);
+    }
+  }
+
+  /// Forget the partial line and start a new one with the next byte.
+  void reset() { this->pos_ = 0; }
+
+  /// Start a new line with c, e.g. the byte that overflowed; c must not be TERMINATOR_CHAR or IGNORE_CHAR.
+  void restart_with(uint8_t c) {
+    this->buf_[0] = static_cast<char>(c);
+    this->pos_ = 1;
+  }
+
+  /// Drop everything up to and including the next terminator.
+  void discard_line() { this->pos_ = N + 1; }
+
+ protected:
+  pos_t end_() const {
+    if (this->pos_ < N) {
+      return this->pos_;
+    }
+    return this->pos_ == N ? N - 1 : 0;
+  }
+
+  char buf_[N];
+  pos_t pos_{0};
+};
+
+}  // namespace esphome

script/build_helpers.py

@@ -285,10 +285,15 @@ def load_test_manifest_overrides(
     defines ``override_manifest(manifest)``, it is called to customise
     the override (e.g. ``manifest.enable_codegen()``).
     """
+    wrapped: dict[str, ComponentManifestOverride] = {}
     for comp_name in components:
-        override = _wrap_manifest(comp_name)
+        # tests/components/core and tests/components/esphome both override the
+        # "esphome" core manifest, so they share one wrapper
+        manifest_name = ESPHOME_KEY if comp_name == "core" else comp_name
+        override = wrapped.get(manifest_name) or _wrap_manifest(manifest_name)
         if override is None:
             continue
+        wrapped[manifest_name] = override
 
         if "." in comp_name:
             domain, component = comp_name.split(".", maxsplit=1)

tests/components/README.md

@@ -69,8 +69,9 @@ Key methods:
 
 | Method | Effect |
 |---|---|
-| `manifest.enable_codegen()` | Remove the `to_code` suppression, re-enabling code generation |
-| `manifest.restore()` | Clear **all** overrides, reverting every attribute to the original |
+| `manifest.enable_codegen()` | Remove the `to_code` suppression, re-enabling code generation; a custom `to_code` set by another override is kept |
+| `manifest.add_resources(*files)` | Copy extra `FileResource` files into the build; the component's own file list is still read at build time, after `FILTER_SOURCE_FILES` sees the defines |
+| `manifest.restore()` | Clear **all** overrides and added resources, reverting every attribute to the original |
 
 The function is called after `to_code` has already been set to `None`, so calling
 `enable_codegen()` is a deliberate opt-in.

tests/components/core/__init__.py

@@ -1,8 +1,9 @@
-from tests.testing_helpers import ComponentManifestOverride
+from tests.testing_helpers import ComponentManifestOverride, add_defines_to_code
 
 
 def override_manifest(manifest: ComponentManifestOverride) -> None:
     # core (esphome/core/config.py) must run its to_code during C++ test builds
     # because it bootstraps the fundamental application infrastructure that all
     # components depend on (component registration, event loop, etc.).
-    manifest.enable_codegen()
+    # USE_LINE_READER keeps line_reader.cpp in the build for its tests.
+    add_defines_to_code(manifest, "USE_LINE_READER")

tests/components/core/line_reader_test.cpp

@@ -0,0 +1,180 @@
+#include <gtest/gtest.h>
+#include <string>
+#include <vector>
+#include "esphome/core/line_reader.h"
+
+namespace esphome::core::testing {
+
+using Lines = std::vector<std::string>;
+
+// Feeds a string and collects every completed line.
+template<typename R> static Lines feed_all(R &reader, const std::string &input) {
+  Lines lines;
+  for (char c : input) {
+    const char *line = reader.feed(static_cast<uint8_t>(c));
+    if (line != nullptr) {
+      EXPECT_EQ(reader.length(), 0u);
+      lines.emplace_back(line);
+    }
+  }
+  return lines;
+}
+
+static_assert(sizeof(LineReader<80>) == 81);
+static_assert(sizeof(LineReader<1025>) == 1028);
+// 0xFD is the largest size with a one byte index; 0xFE switches to two bytes
+static_assert(sizeof(LineReader<0xFD>) == 0xFE);
+static_assert(sizeof(LineReader<0xFE>) == 0x100);
+
+TEST(LineReaderTest, CrLfLines) {
+  LineReader<16> reader;
+  EXPECT_EQ(feed_all(reader, "abc\r\ndef\r\n"), (Lines{"abc", "def"}));
+}
+
+TEST(LineReaderTest, LfOnlyLines) {
+  LineReader<16> reader;
+  EXPECT_EQ(feed_all(reader, "abc\ndef\n"), (Lines{"abc", "def"}));
+}
+
+TEST(LineReaderTest, CrOnlyLinesWithCrTerminator) {
+  LineReader<16, '\r', '\n'> reader;
+  EXPECT_EQ(feed_all(reader, "R1234\rR0567\r"), (Lines{"R1234", "R0567"}));
+  // The LF of a CRLF pair is ignored with this setup
+  EXPECT_EQ(feed_all(reader, "x\r\ny\r\n"), (Lines{"x", "y"}));
+}
+
+TEST(LineReaderTest, CrInsideLineIsIgnored) {
+  LineReader<16> reader;
+  EXPECT_EQ(feed_all(reader, "a\rb\n"), (Lines{"ab"}));
+}
+
+TEST(LineReaderTest, EmptyLinesAreReturned) {
+  LineReader<16> reader;
+  EXPECT_EQ(feed_all(reader, "\r\n\nx\n"), (Lines{"", "", "x"}));
+}
+
+TEST(LineReaderTest, LineOfExactlyMaxLengthFits) {
+  LineReader<6> reader;
+  EXPECT_EQ(feed_all(reader, "12345\n"), (Lines{"12345"}));
+  EXPECT_FALSE(reader.overflowed());
+}
+
+TEST(LineReaderTest, OverlongLineIsDroppedUpToTerminator) {
+  LineReader<6> reader;
+  int overflow_count = 0;
+  Lines lines;
+  for (char c : std::string("123456789\r\nok\n")) {
+    const char *line = reader.feed(c);
+    if (reader.overflowed()) {
+      overflow_count++;
+      EXPECT_STREQ(reader.partial(), "12345");
+      EXPECT_EQ(reader.length(), 0u);
+      reader.log_if_overflowed("line_reader_test");
+    }
+    if (line != nullptr)
+      lines.emplace_back(line);
+  }
+  EXPECT_EQ(overflow_count, 1);
+  EXPECT_EQ(lines, (Lines{"ok"}));
+}
+
+TEST(LineReaderTest, SixteenBitIndexOverflowAndRecovery) {
+  LineReader<300> reader;
+  const std::string full(299, 'a');
+  EXPECT_EQ(feed_all(reader, full + "\n"), (Lines{full}));
+  int overflow_count = 0;
+  Lines lines;
+  for (char c : full + "bcd\r\nok\n") {
+    const char *line = reader.feed(c);
+    if (reader.overflowed()) {
+      overflow_count++;
+      EXPECT_EQ(std::string(reader.partial()), full);
+    }
+    if (line != nullptr)
+      lines.emplace_back(line);
+  }
+  EXPECT_EQ(overflow_count, 1);
+  EXPECT_EQ(lines, (Lines{"ok"}));
+}
+
+TEST(LineReaderTest, ResetAfterOverflowRestartsAtNextByte) {
+  LineReader<4> reader;
+  Lines lines;
+  for (char c : std::string("abcdefg\n")) {
+    const char *line = reader.feed(c);
+    if (reader.overflowed())
+      reader.reset();
+    if (line != nullptr)
+      lines.emplace_back(line);
+  }
+  // "abc" fills the buffer, 'd' overflows and is dropped, "efg" is the next line
+  EXPECT_EQ(lines, (Lines{"efg"}));
+}
+
+TEST(LineReaderTest, RestartWithKeepsOverflowByte) {
+  LineReader<4> reader;
+  Lines lines;
+  for (char c : std::string("abcdef\n")) {
+    const char *line = reader.feed(c);
+    if (reader.overflowed()) {
+      reader.restart_with(c);
+    }
+    if (line != nullptr) {
+      lines.emplace_back(line);
+    }
+  }
+  // 'd' overflows and starts the next line
+  EXPECT_EQ(lines, (Lines{"def"}));
+}
+
+TEST(LineReaderTest, FeedReportsLength) {
+  LineReader<16> reader;
+  size_t len = 99;
+  char *line = nullptr;
+  for (char c : std::string("ab\rc\n")) {
+    line = reader.feed(c, len);
+  }
+  ASSERT_NE(line, nullptr);
+  EXPECT_STREQ(line, "abc");
+  EXPECT_EQ(len, 3u);
+  EXPECT_NE(reader.feed('\n', len), nullptr);
+  EXPECT_EQ(len, 0u);
+}
+
+TEST(LineReaderTest, PartialForPromptWithoutLineEnd) {
+  LineReader<32> reader;
+  EXPECT_EQ(feed_all(reader, "DFRobot:/>"), Lines{});
+  EXPECT_STREQ(reader.partial(), "DFRobot:/>");
+  EXPECT_EQ(reader.length(), 10u);
+  reader.reset();
+  EXPECT_STREQ(reader.partial(), "");
+  EXPECT_EQ(feed_all(reader, "sensorStart\r\n"), (Lines{"sensorStart"}));
+}
+
+TEST(LineReaderTest, DiscardLineDropsUntilTerminator) {
+  LineReader<16> reader;
+  feed_all(reader, "half");
+  reader.discard_line();
+  EXPECT_EQ(reader.length(), 0u);
+  EXPECT_STREQ(reader.partial(), "");
+  EXPECT_EQ(feed_all(reader, "rest\nnext\n"), (Lines{"next"}));
+}
+
+TEST(LineReaderTest, CustomIgnoreByte) {
+  LineReader<8, '\r', static_cast<char>(0xFF)> reader;
+  EXPECT_EQ(feed_all(reader, "R12\xFF"
+                             "34\r"),
+            (Lines{"R1234"}));
+}
+
+TEST(LineReaderTest, LineIsWritable) {
+  LineReader<16> reader;
+  char *line = nullptr;
+  for (char c : std::string("a,b\n"))
+    line = reader.feed(c);
+  ASSERT_NE(line, nullptr);
+  line[1] = '\0';
+  EXPECT_STREQ(line, "a");
+}
+
+}  // namespace esphome::core::testing

tests/components/esphome/__init__.py

@@ -6,7 +6,7 @@ def override_manifest(manifest: ComponentManifestOverride) -> None:
     # to_code emits the component count the application needs
     manifest.enable_codegen()
     # Only the decoder is under test; its ota platform is not in this build
-    manifest.resources = manifest.resources + [
+    manifest.add_resources(
         FileResource("esphome.components.esphome", "ota/ota_esphome_inflate.c"),
         FileResource("esphome.components.esphome", "ota/ota_esphome_inflate.h"),
-    ]
+    )

tests/script/test_test_helpers.py

@@ -12,7 +12,7 @@ sys.path.insert(0, str((Path(__file__).parent / ".." / ".." / "script").resolve(
 
 import build_helpers  # noqa: E402
 
-from esphome.loader import ComponentManifest  # noqa: E402
+from esphome.loader import ComponentManifest, FileResource  # noqa: E402
 from tests.testing_helpers import ComponentManifestOverride  # noqa: E402
 
 # ---------------------------------------------------------------------------
@@ -241,6 +241,77 @@ def test_load_wraps_top_level_component(tmp_path: Path) -> None:
     assert installed.to_code is None
 
 
+def test_load_core_overrides_esphome_manifest(tmp_path: Path) -> None:
+    """tests/components/core/__init__.py overrides the "esphome" core manifest."""
+    core_dir = tmp_path / "core"
+    core_dir.mkdir()
+    (core_dir / "__init__.py").write_text(
+        textwrap.dedent("""\
+        def override_manifest(manifest):
+            manifest.dependencies = ["injected"]
+    """)
+    )
+    inner = _make_component_manifest()
+
+    with (
+        patch("build_helpers.get_component", return_value=inner) as mock_get,
+        patch("build_helpers.set_testing_manifest") as mock_set,
+    ):
+        build_helpers.load_test_manifest_overrides(["core"], tmp_path)
+
+    mock_get.assert_called_once_with("esphome")
+    key, installed = mock_set.call_args[0]
+    assert key == "esphome"
+    assert installed.dependencies == ["injected"]
+
+
+def test_load_core_and_esphome_share_one_wrapper(tmp_path: Path) -> None:
+    """Both test dirs override the "esphome" manifest; both overrides apply to one wrapper."""
+    for name, dep in (("core", "from_core"), ("esphome", "from_esphome")):
+        (tmp_path / name).mkdir()
+        (tmp_path / name / "__init__.py").write_text(
+            textwrap.dedent(f"""\
+            def override_manifest(manifest):
+                manifest.dependencies = manifest.dependencies + ["{dep}"]
+        """)
+        )
+    inner = _make_component_manifest()
+
+    with (
+        patch("build_helpers.get_component", return_value=inner),
+        patch("build_helpers.set_testing_manifest") as mock_set,
+    ):
+        build_helpers.load_test_manifest_overrides(["core", "esphome"], tmp_path)
+
+    mock_set.assert_called_once()
+    installed: ComponentManifestOverride = mock_set.call_args[0][1]
+    assert installed.dependencies == ["from_core", "from_esphome"]
+
+
+def test_enable_codegen_keeps_custom_to_code() -> None:
+    """enable_codegen() lifts the suppression but keeps a to_code another override set."""
+
+    async def custom_to_code(config):
+        pass
+
+    override = ComponentManifestOverride(_make_component_manifest())
+    override.to_code = custom_to_code
+    override.enable_codegen()
+    assert override.to_code is custom_to_code
+
+
+def test_add_resources_reads_component_resources_lazily() -> None:
+    """add_resources() appends to the resources computed at read time."""
+    inner = MagicMock()
+    inner.resources = [FileResource("pkg", "a.cpp")]
+    override = ComponentManifestOverride(inner)
+    override.add_resources(FileResource("pkg", "extra.c"))
+    inner.resources = [FileResource("pkg", "a.cpp"), FileResource("pkg", "b.cpp")]
+    assert [r.resource for r in override.resources] == ["a.cpp", "b.cpp", "extra.c"]
+    override.restore()
+    assert [r.resource for r in override.resources] == ["a.cpp", "b.cpp"]
+
+
 def test_load_wraps_platform_component(tmp_path: Path) -> None:
     inner = _make_component_manifest()
 

tests/testing_helpers.py

@@ -1,6 +1,6 @@
 from typing import Any
 
-from esphome.loader import ComponentManifest, _replace_component_manifest
+from esphome.loader import ComponentManifest, FileResource, _replace_component_manifest
 from esphome.types import ConfigType
 
 
@@ -29,12 +29,16 @@ class ComponentManifestOverride:
     def __init__(self, wrapped: "ComponentManifest") -> None:
         object.__setattr__(self, "_wrapped", wrapped)
         object.__setattr__(self, "_overrides", {})
+        object.__setattr__(self, "_extra_resources", [])
 
     def __getattr__(self, name: str) -> Any:
         overrides: dict[str, Any] = object.__getattribute__(self, "_overrides")
         if name in overrides:
             return overrides[name]
         wrapped: ComponentManifest = object.__getattribute__(self, "_wrapped")
+        if name == "resources":
+            # Read lazily so FILTER_SOURCE_FILES sees the defines to_code added
+            return wrapped.resources + object.__getattribute__(self, "_extra_resources")
         return getattr(wrapped, name)
 
     def __setattr__(self, name: str, value: Any) -> None:
@@ -48,11 +52,18 @@ class ComponentManifestOverride:
         custom stub) ``to_code`` to run during C++ unit test builds.
         """
         overrides: dict[str, Any] = object.__getattribute__(self, "_overrides")
-        overrides.pop("to_code", None)
+        # Keep a custom to_code another override of the same manifest installed
+        if overrides.get("to_code") is None:
+            overrides.pop("to_code", None)
+
+    def add_resources(self, *resources: FileResource) -> None:
+        """Copy extra source files into the build next to the component's own."""
+        object.__getattribute__(self, "_extra_resources").extend(resources)
 
     def restore(self) -> None:
-        """Clear all overrides, reverting to the wrapped manifest's values."""
+        """Clear all overrides and added resources, reverting to the wrapped manifest's values."""
         object.__getattribute__(self, "_overrides").clear()
+        object.__getattribute__(self, "_extra_resources").clear()
 
 
 def set_testing_manifest(domain: str, manifest: ComponentManifestOverride) -> None: