[core] Assert a frame came back before each resync() in the FrameReader tests

AGENTS.md

@@ -216,9 +216,10 @@ file does, and it is the authority when they disagree. The most useful starting
         the helper's arguments instead of writing a loop. 7-byte BCD RTC calendars use `time::decode_bcd_calendar<LAYOUT>`
         and `encode_bcd_calendar<LAYOUT>` with `EPSON_BCD_CALENDAR`, `DS_BCD_CALENDAR` or `PCF_BCD_CALENDAR`.
     *   **Bus helpers:** UART drains with `read_available(on_byte)` and `discard_available()`, text lines with
-        `LineReader<N>`; I2C has `update_register`, `write_registers` (PROGMEM `I2CRegValue` table), `check_chip_id`
-        and `read_bytes_16_le`. `if (!this->status_comm_result(ok)) return;` sets or clears the warning.
-        A child sensor that skips repeated values is `SUB_SENSOR_WITH_DEDUP(name, type)`.
+        `LineReader<N>`, fixed-length binary frames with `FrameReader<N, HEADER...>`; I2C has `update_register`,
+        `write_registers` (PROGMEM `I2CRegValue` table), `check_chip_id` and `read_bytes_16_le`.
+        `if (!this->status_comm_result(ok)) return;` sets or clears the warning. A child sensor that skips repeated
+        values is `SUB_SENSOR_WITH_DEDUP(name, type)`.
     *   **An unset float is `NAN`**, not `optional<float>`; the optional costs 4 to 8 bytes of padding per member.
     *   **`PollingComponent` first runs `update()`** after a random share of the interval; call `this->update()` at the
         end of `setup()` when the entities need a state at boot.

esphome/core/frame_reader.h

@@ -0,0 +1,67 @@
+#pragma once
+
+#include <cstddef>
+#include <cstdint>
+#include <type_traits>
+
+namespace esphome {
+
+/// Collects fixed length binary frames of N bytes that start with the header bytes HEADER...; the binary twin of
+/// LineReader. Bytes before a header are skipped; checksums are left to the caller.
+template<size_t N, uint8_t... HEADER> class FrameReader {
+  using pos_t = std::conditional_t<(N <= 0xFF), uint8_t, uint16_t>;
+  static constexpr size_t HEADER_LEN = sizeof...(HEADER);
+  static_assert(HEADER_LEN >= 1 && HEADER_LEN <= 8, "FrameReader takes 1 to 8 header bytes");  // bounds replay_() depth
+  static_assert(HEADER_LEN < N && N <= 0xFFFF, "FrameReader size out of range");
+
+ public:
+  /// Feed one byte; returns the completed frame, header included, valid until the next call, or nullptr.
+  const uint8_t *feed(uint8_t c) {
+    pos_t pos = this->pos_;
+    if (pos < HEADER_LEN && c != header_byte(pos)) {
+      // A later byte of the broken header, or c, may start the real one
+      this->buf_[pos] = c;
+      this->replay_(pos);
+      return nullptr;
+    }
+    this->buf_[pos++] = c;
+    if (pos < N) {
+      this->pos_ = pos;
+      return nullptr;
+    }
+    this->pos_ = 0;
+    return this->buf_;
+  }
+
+  /// Number of bytes of the partial frame; 0 after a frame is returned and while looking for a header.
+  size_t length() const { return this->pos_; }
+
+  /// Forget the partial frame and look for a header from the next byte.
+  void reset() { this->pos_ = 0; }
+
+  /// For a frame the caller rejected (bad checksum): drop its first byte and realign on a header in the rest.
+  /// Call only right after feed() returned that frame; use reset() otherwise.
+  void resync() { this->replay_(N - 1); }
+
+ protected:
+  // A fold, not an array, so no header table lands in RAM on ESP8266
+  static constexpr uint8_t header_byte(size_t i) {
+    size_t j = 0;
+    uint8_t r = 0;
+    ((r = (j++ == i) ? HEADER : r), ...);
+    return r;
+  }
+
+  // Start over and feed buf_[1..last] again; never completes a frame as last < N
+  void replay_(size_t last) {
+    this->pos_ = 0;
+    for (size_t i = 1; i <= last; i++) {
+      this->feed(this->buf_[i]);
+    }
+  }
+
+  uint8_t buf_[N];
+  pos_t pos_{0};
+};
+
+}  // namespace esphome

tests/components/core/frame_reader_test.cpp

@@ -0,0 +1,143 @@
+#include <gtest/gtest.h>
+#include <vector>
+#include "esphome/core/frame_reader.h"
+
+namespace esphome::core::testing {
+
+using Bytes = std::vector<uint8_t>;
+using Frames = std::vector<Bytes>;
+
+// Feeds bytes and collects every completed frame.
+template<size_t N, uint8_t... H> static Frames feed_all(FrameReader<N, H...> &reader, const Bytes &input) {
+  Frames frames;
+  for (uint8_t c : input) {
+    const uint8_t *frame = reader.feed(c);
+    if (frame != nullptr) {
+      EXPECT_EQ(reader.length(), 0u);
+      frames.emplace_back(frame, frame + N);
+    }
+  }
+  return frames;
+}
+
+static_assert(sizeof(FrameReader<4, 0xFF>) == 5);
+static_assert(sizeof(FrameReader<9, 0x55, 0x01, 0x01, 0x04>) == 10);
+static_assert(sizeof(FrameReader<0xFF, 0xAA>) == 0x100);
+static_assert(sizeof(FrameReader<0x100, 0xAA>) == 0x102);
+
+TEST(FrameReaderTest, SingleByteHeader) {
+  FrameReader<4, 0xFF> reader;
+  EXPECT_EQ(feed_all(reader, {0xFF, 0x01, 0x02, 0x02, 0xFF, 0x03, 0x04, 0x06}),
+            (Frames{{0xFF, 0x01, 0x02, 0x02}, {0xFF, 0x03, 0x04, 0x06}}));
+}
+
+TEST(FrameReaderTest, SkipsBytesBeforeHeader) {
+  FrameReader<3, 0xFF> reader;
+  EXPECT_EQ(feed_all(reader, {0x01, 0x02, 0xFF, 0x10, 0x20}), (Frames{{0xFF, 0x10, 0x20}}));
+}
+
+TEST(FrameReaderTest, HeaderByteInsidePayloadIsData) {
+  FrameReader<4, 0xFF> reader;
+  EXPECT_EQ(feed_all(reader, {0xFF, 0xFF, 0xFF, 0x00}), (Frames{{0xFF, 0xFF, 0xFF, 0x00}}));
+}
+
+TEST(FrameReaderTest, LengthTracksPartialFrame) {
+  FrameReader<4, 0xAA, 0xBB> reader;
+  EXPECT_EQ(reader.length(), 0u);
+  reader.feed(0x00);
+  EXPECT_EQ(reader.length(), 0u);
+  reader.feed(0xAA);
+  EXPECT_EQ(reader.length(), 1u);
+  reader.feed(0xBB);
+  reader.feed(0x01);
+  EXPECT_EQ(reader.length(), 3u);
+  reader.reset();
+  EXPECT_EQ(reader.length(), 0u);
+  EXPECT_EQ(feed_all(reader, {0x02, 0xAA, 0xBB, 0x03, 0x04}), (Frames{{0xAA, 0xBB, 0x03, 0x04}}));
+}
+
+TEST(FrameReaderTest, MultiByteHeaderMismatchRestarts) {
+  FrameReader<6, 0x55, 0x01, 0x01, 0x04> reader;
+  EXPECT_EQ(feed_all(reader, {0x55, 0x01, 0x02, 0x55, 0x01, 0x01, 0x04, 0x11, 0x22}),
+            (Frames{{0x55, 0x01, 0x01, 0x04, 0x11, 0x22}}));
+}
+
+TEST(FrameReaderTest, MismatchedByteCanStartHeader) {
+  FrameReader<6, 0x55, 0x01, 0x01, 0x04> reader;
+  // The second 0x55 breaks the first header and starts the real one
+  EXPECT_EQ(feed_all(reader, {0x55, 0x01, 0x55, 0x01, 0x01, 0x04, 0x11, 0x22}),
+            (Frames{{0x55, 0x01, 0x01, 0x04, 0x11, 0x22}}));
+}
+
+TEST(FrameReaderTest, RepeatedHeaderPrefixIsKept) {
+  // AA AA AA BB: the mismatch at the third byte still leaves AA AA matched
+  FrameReader<4, 0xAA, 0xAA, 0xBB> reader;
+  EXPECT_EQ(feed_all(reader, {0xAA, 0xAA, 0xAA, 0xBB, 0x01}), (Frames{{0xAA, 0xAA, 0xBB, 0x01}}));
+}
+
+TEST(FrameReaderTest, OverlappingHeaderPrefix) {
+  // A1 B2 A1 B2: the mismatch on the last B2 keeps the trailing A1 B2 as the start of the header
+  FrameReader<5, 0xA1, 0xB2, 0xA1, 0xC3> reader;
+  EXPECT_EQ(feed_all(reader, {0xA1, 0xB2, 0xA1, 0xB2, 0xA1, 0xC3, 0x07}), (Frames{{0xA1, 0xB2, 0xA1, 0xC3, 0x07}}));
+}
+
+TEST(FrameReaderTest, ResyncFindsHeaderInRejectedFrame) {
+  FrameReader<4, 0xFF> reader;
+  // Started mid-frame: the 0xFF taken as a header was payload; the real frame starts at the second 0xFF
+  const uint8_t *frame = nullptr;
+  for (uint8_t c : {0xFF, 0x12, 0xFF, 0x01}) {
+    frame = reader.feed(c);
+  }
+  ASSERT_NE(frame, nullptr);
+  reader.resync();
+  EXPECT_EQ(reader.length(), 2u);
+  EXPECT_EQ(feed_all(reader, {0x02, 0x02}), (Frames{{0xFF, 0x01, 0x02, 0x02}}));
+}
+
+TEST(FrameReaderTest, ResyncWithoutHeaderStartsOver) {
+  FrameReader<4, 0xFF> reader;
+  const uint8_t *frame = nullptr;
+  for (uint8_t c : {0xFF, 0x01, 0x02, 0x03}) {
+    frame = reader.feed(c);
+  }
+  ASSERT_NE(frame, nullptr);
+  reader.resync();
+  EXPECT_EQ(reader.length(), 0u);
+}
+
+TEST(FrameReaderTest, ResyncKeepsPartialHeaderAtEnd) {
+  FrameReader<5, 0x3C, 0x32> reader;
+  const uint8_t *frame = nullptr;
+  for (uint8_t c : {0x3C, 0x32, 0x00, 0x00, 0x3C}) {
+    frame = reader.feed(c);
+  }
+  ASSERT_NE(frame, nullptr);
+  reader.resync();
+  EXPECT_EQ(reader.length(), 1u);
+  EXPECT_EQ(feed_all(reader, {0x32, 0x01, 0x02, 0x03}), (Frames{{0x3C, 0x32, 0x01, 0x02, 0x03}}));
+}
+
+TEST(FrameReaderTest, ResyncMultiByteHeader) {
+  FrameReader<6, 0x3C, 0x32> reader;
+  const uint8_t *frame = nullptr;
+  for (uint8_t c : {0x3C, 0x32, 0x3C, 0x3C, 0x32, 0x09}) {
+    frame = reader.feed(c);
+  }
+  ASSERT_NE(frame, nullptr);
+  reader.resync();
+  EXPECT_EQ(reader.length(), 3u);
+  EXPECT_EQ(feed_all(reader, {0x0A, 0x0B, 0x0C}), (Frames{{0x3C, 0x32, 0x09, 0x0A, 0x0B, 0x0C}}));
+}
+
+TEST(FrameReaderTest, SixteenBitIndex) {
+  FrameReader<300, 0xAA, 0x55> reader;
+  Bytes input{0x00, 0xAA, 0x55};
+  input.resize(input.size() + 298, 0x42);
+  Frames frames = feed_all(reader, input);
+  ASSERT_EQ(frames.size(), 1u);
+  EXPECT_EQ(frames[0][0], 0xAA);
+  EXPECT_EQ(frames[0][1], 0x55);
+  EXPECT_EQ(frames[0][299], 0x42);
+}
+
+}  // namespace esphome::core::testing