From da43b6292dbc7a51a1ac8b5056e190371e424e6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Metrich?= <45318189+FredM67@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:41:58 +0200 Subject: [PATCH] [mk2pvrouter] Store the frame as bytes and read with read_byte() (#20530) Co-authored-by: Claude Opus 5.5 --- .../components/mk2pvrouter/mk2pvrouter.cpp | 24 +++++++++---------- esphome/components/mk2pvrouter/mk2pvrouter.h | 8 +++---- .../mk2pvrouter/mk2pvrouter_test.cpp | 15 ++++++++---- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/esphome/components/mk2pvrouter/mk2pvrouter.cpp b/esphome/components/mk2pvrouter/mk2pvrouter.cpp index b759758019..98dd0b74ec 100644 --- a/esphome/components/mk2pvrouter/mk2pvrouter.cpp +++ b/esphome/components/mk2pvrouter/mk2pvrouter.cpp @@ -19,8 +19,8 @@ constexpr uint8_t CRC_OFFSET = 0x20; // Extracts a TAB-delimited field from [buf_start, buf_end) into dest. // Returns the field length, or 0 if no TAB was found, or the (uncopied) field // length if it's >= max_len. -static size_t get_field(char *dest, const char *buf_start, const char *buf_end, size_t max_len) { - const auto *const field_end = static_cast(memchr(buf_start, TAB, buf_end - buf_start)); +static size_t get_field(char *dest, const uint8_t *buf_start, const uint8_t *buf_end, size_t max_len) { + const auto *const field_end = static_cast(memchr(buf_start, TAB, buf_end - buf_start)); if (!field_end) return 0; const size_t len = field_end - buf_start; @@ -34,13 +34,13 @@ static size_t get_field(char *dest, const char *buf_start, const char *buf_end, return len; } -// Calculates the CRC (checksum) for a given group of characters. -uint8_t Mk2PVRouter::calculate_crc_(const char *grp, size_t grp_len) { - return (sum_bytes(reinterpret_cast(grp), grp_len - CRC_SUFFIX_LEN) & CRC_MASK) + CRC_OFFSET; +// Calculates the CRC (checksum) for a given group of bytes. +uint8_t Mk2PVRouter::calculate_crc_(const uint8_t *grp, size_t grp_len) { + return (sum_bytes(grp, grp_len - CRC_SUFFIX_LEN) & CRC_MASK) + CRC_OFFSET; } // Verifies the CRC of a group against its trailing CRC byte. -bool Mk2PVRouter::check_crc_(const char *grp, const char *grp_end) { +bool Mk2PVRouter::check_crc_(const uint8_t *grp, const uint8_t *grp_end) { const auto grp_len = grp_end - grp; if (grp_len < static_cast(CRC_SUFFIX_LEN)) { ESP_LOGE(TAG, "Empty or too short group"); @@ -58,7 +58,7 @@ bool Mk2PVRouter::check_crc_(const char *grp, const char *grp_end) { } // Validates, parses, and publishes a single tag/value group. -void Mk2PVRouter::process_group_(const char *grp, const char *grp_end) { +void Mk2PVRouter::process_group_(const uint8_t *grp, const uint8_t *grp_end) { if (!this->check_crc_(grp, grp_end)) return; @@ -83,9 +83,9 @@ bool Mk2PVRouter::read_chars_until_(bool drop, uint8_t c) { size_t j{0}; while (this->available() > 0 && j++ < MAX_ITERATIONS) { - const auto received = this->read(); - if (received < 0) - continue; + uint8_t received; + if (!this->read_byte(&received)) + break; if (received == c) return true; if (drop) @@ -127,10 +127,10 @@ void Mk2PVRouter::loop() { // Each group: 0xa(LF) | Tag | 0x9(TAB) | Data | 0x9(TAB) | CRC | 0xd(CR) // CRC is computed over "Tag | TAB | Data | TAB". - while ((buf_finger = static_cast(memchr(buf_finger, LINE_FEED, buf_end - buf_finger))) != nullptr) { + while ((buf_finger = static_cast(memchr(buf_finger, LINE_FEED, buf_end - buf_finger))) != nullptr) { ++buf_finger; // Skip LF to the start of the group. - auto *const grp_end = static_cast(memchr(buf_finger, CARRIAGE_RETURN, buf_end - buf_finger)); + auto *const grp_end = static_cast(memchr(buf_finger, CARRIAGE_RETURN, buf_end - buf_finger)); if (!grp_end) { ESP_LOGE(TAG, "No group found"); break; diff --git a/esphome/components/mk2pvrouter/mk2pvrouter.h b/esphome/components/mk2pvrouter/mk2pvrouter.h index 1a6af4b1b3..061a9a22e9 100644 --- a/esphome/components/mk2pvrouter/mk2pvrouter.h +++ b/esphome/components/mk2pvrouter/mk2pvrouter.h @@ -57,12 +57,12 @@ class Mk2PVRouter : public Component, public uart::UARTDevice { State state_{State::WAITING_FOR_START}; char tag_[MAX_TAG_SIZE]; char val_[MAX_VAL_SIZE]; - char buf_[MAX_BUF_SIZE]; // Large buffer last to reduce padding + uint8_t buf_[MAX_BUF_SIZE]; // Large buffer last to reduce padding bool read_chars_until_(bool drop, uint8_t c); - uint8_t calculate_crc_(const char *grp, size_t grp_len); - bool check_crc_(const char *grp, const char *grp_end); - void process_group_(const char *grp, const char *grp_end); + uint8_t calculate_crc_(const uint8_t *grp, size_t grp_len); + bool check_crc_(const uint8_t *grp, const uint8_t *grp_end); + void process_group_(const uint8_t *grp, const uint8_t *grp_end); void publish_value_(const char *tag, const char *val); }; } // namespace esphome::mk2pvrouter diff --git a/tests/components/mk2pvrouter/mk2pvrouter_test.cpp b/tests/components/mk2pvrouter/mk2pvrouter_test.cpp index 84106eb0b7..a2fb33a2a3 100644 --- a/tests/components/mk2pvrouter/mk2pvrouter_test.cpp +++ b/tests/components/mk2pvrouter/mk2pvrouter_test.cpp @@ -8,6 +8,9 @@ namespace esphome::mk2pvrouter::testing { namespace { +// The parser works on raw frame bytes; the groups below are written as text for readability. +const uint8_t *as_bytes(const char *str) { return reinterpret_cast(str); } + class TestListener final : public Mk2PVRouterListener { public: explicit TestListener(const char *tag) : Mk2PVRouterListener(tag) {} @@ -25,7 +28,9 @@ class Mk2PVRouterTest : public ::testing::Test { void SetUp() override { this->sut_.register_mk2pvrouter_listener(&this->listener_); } // Feeds one "tagvaluecrc" literal, as the frame parser does after END_FRAME. - template void process_(const char (&group)[N]) { this->sut_.process_group_(group, group + N - 1); } + template void process_(const char (&group)[N]) { + this->sut_.process_group_(as_bytes(group), as_bytes(group) + N - 1); + } TestableMk2PVRouter sut_; TestListener listener_{"P1"}; @@ -40,22 +45,22 @@ TEST(Mk2PVRouterListenerTest, GetTagReturnsConstructorTag) { TEST_F(Mk2PVRouterTest, CalculateCrcMatchesKnownGroup) { // "P1\t1234\t" sums to 0x3D ('=') per the mk2pvrouter CRC algorithm. const char grp[] = "P1\t1234\t="; - EXPECT_EQ(this->sut_.calculate_crc_(grp, sizeof(grp) - 1), '='); + EXPECT_EQ(this->sut_.calculate_crc_(as_bytes(grp), sizeof(grp) - 1), '='); } TEST_F(Mk2PVRouterTest, CheckCrcAcceptsMatchingCrc) { const char grp[] = "P1\t1234\t="; - EXPECT_TRUE(this->sut_.check_crc_(grp, grp + sizeof(grp) - 1)); + EXPECT_TRUE(this->sut_.check_crc_(as_bytes(grp), as_bytes(grp) + sizeof(grp) - 1)); } TEST_F(Mk2PVRouterTest, CheckCrcRejectsMismatchedCrc) { const char grp[] = "P1\t1234\t!"; - EXPECT_FALSE(this->sut_.check_crc_(grp, grp + sizeof(grp) - 1)); + EXPECT_FALSE(this->sut_.check_crc_(as_bytes(grp), as_bytes(grp) + sizeof(grp) - 1)); } TEST_F(Mk2PVRouterTest, CheckCrcRejectsEmptyGroup) { const char grp[] = ""; - EXPECT_FALSE(this->sut_.check_crc_(grp, grp)); + EXPECT_FALSE(this->sut_.check_crc_(as_bytes(grp), as_bytes(grp))); } TEST_F(Mk2PVRouterTest, ProcessGroupPublishesValidGroupToMatchingListener) {