diff --git a/esphome/core/config.py b/esphome/core/config.py index 531a0276471..667d780f71f 100644 --- a/esphome/core/config.py +++ b/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]: diff --git a/esphome/core/defines.h b/esphome/core/defines.h index a5fb354199b..756485e0157 100644 --- a/esphome/core/defines.h +++ b/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 diff --git a/esphome/core/line_reader.cpp b/esphome/core/line_reader.cpp new file mode 100644 index 00000000000..e821dd0bc4e --- /dev/null +++ b/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(max_len)); +} + +} // namespace esphome diff --git a/esphome/core/line_reader.h b/esphome/core/line_reader.h new file mode 100644 index 00000000000..201267ec3d8 --- /dev/null +++ b/esphome/core/line_reader.h @@ -0,0 +1,99 @@ +#pragma once + +#include +#include +#include + +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 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(IGNORE_CHAR)) { + return nullptr; + } + if (c == static_cast(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(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(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 diff --git a/script/build_helpers.py b/script/build_helpers.py index 885a5a14758..c15c7118b23 100644 --- a/script/build_helpers.py +++ b/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) diff --git a/tests/components/README.md b/tests/components/README.md index be5e887767e..a28e7f4f89c 100644 --- a/tests/components/README.md +++ b/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. diff --git a/tests/components/core/__init__.py b/tests/components/core/__init__.py index 34ca4fbe4f0..6d2829511a7 100644 --- a/tests/components/core/__init__.py +++ b/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") diff --git a/tests/components/core/line_reader_test.cpp b/tests/components/core/line_reader_test.cpp new file mode 100644 index 00000000000..3b2fad1ce61 --- /dev/null +++ b/tests/components/core/line_reader_test.cpp @@ -0,0 +1,180 @@ +#include +#include +#include +#include "esphome/core/line_reader.h" + +namespace esphome::core::testing { + +using Lines = std::vector; + +// Feeds a string and collects every completed line. +template static Lines feed_all(R &reader, const std::string &input) { + Lines lines; + for (char c : input) { + const char *line = reader.feed(static_cast(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(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 diff --git a/tests/components/esphome/__init__.py b/tests/components/esphome/__init__.py index 713c0d5a63a..c1d86f29db4 100644 --- a/tests/components/esphome/__init__.py +++ b/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"), - ] + ) diff --git a/tests/script/test_test_helpers.py b/tests/script/test_test_helpers.py index 4b05cab3767..d67d7071aba 100644 --- a/tests/script/test_test_helpers.py +++ b/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() diff --git a/tests/testing_helpers.py b/tests/testing_helpers.py index 23bef718694..8459a552bad 100644 --- a/tests/testing_helpers.py +++ b/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: