[core] Copy without uint32_t aliasing, share the copy with assign_copy, and add prepend_copy

esphome/core/helpers.h

@@ -177,14 +177,13 @@ template<typename T> class ConstVector<T, true> {
     this->data_ = data;
     this->size_ = size;
   }
+  /// Appends count elements into one new heap array this owns; the current list and extra may be codegen tables in
+  /// flash.
+  void append_copy(const T *extra, size_t count) { this->merge_copy_(this->data_, this->size(), extra, count); }
+  /// Like append_copy(), with extra placed in front of the current list.
+  void prepend_copy(const T *extra, size_t count) { this->merge_copy_(extra, count, this->data_, this->size()); }
   /// Copies the list into a heap array this owns, freeing a previous owned copy.
-  void assign_copy(const T *data, size_t size) {
-    auto *table = new T[size];  // NOLINT(cppcoreguidelines-owning-memory)
-    std::copy(data, data + size, table);
-    this->release_();
-    this->data_ = table;
-    this->size_ = size | OWNED_BIT;
-  }
+  void assign_copy(const T *data, size_t size) { this->merge_copy_(data, size, nullptr, 0); }
 
  protected:
   static constexpr size_t OWNED_BIT = size_t{1} << (sizeof(size_t) * 8 - 1);
@@ -193,6 +192,23 @@ template<typename T> class ConstVector<T, true> {
     if (this->size_ & OWNED_BIT)
       delete[] this->data_;  // NOLINT(cppcoreguidelines-owning-memory)
   }
+  void merge_copy_(const T *first, size_t first_count, const T *second, size_t second_count) {
+    auto *table = new T[first_count + second_count];  // NOLINT(cppcoreguidelines-owning-memory)
+    copy_words_(first, first_count, table);
+    copy_words_(second, second_count, table + first_count);
+    this->release_();  // after the copies: first or second may be the current list
+    this->data_ = table;
+    this->size_ = (first_count + second_count) | OWNED_BIT;
+  }
+  // Word loads through volatile, so the loop never becomes a memcpy that reads ESP8266 flash byte by byte
+  static void copy_words_(const T *src, size_t count, T *dst) {
+    auto *from = reinterpret_cast<const volatile uint32_t *>(src);
+    auto *to = reinterpret_cast<uint8_t *>(dst);
+    for (size_t i = 0; i < count * sizeof(T) / sizeof(uint32_t); i++) {
+      const uint32_t word = from[i];
+      std::memcpy(to + i * sizeof(uint32_t), &word, sizeof(word));  // into RAM: no aliasing through uint32_t
+    }
+  }
 
   const T *data_{nullptr};
   size_t size_{0};  // top bit set when data_ is an owned heap copy

tests/component_tests/remote_receiver/config/receiver_two_dumper_lists.yaml

@@ -0,0 +1,19 @@
+esphome:
+  name: test
+
+esp32:
+  board: esp32dev
+
+logger:
+
+remote_receiver:
+  - id: ir_rcvr
+    pin: GPIO4
+    dump:
+      - nec
+      - rc_switch
+      - raw
+  - id: rf_rcvr
+    pin: GPIO5
+    dump:
+      - rc_switch

tests/component_tests/remote_receiver/test_slot_counts.py

@@ -2,6 +2,7 @@
 
 from collections.abc import Callable, Generator
 from pathlib import Path
+import re
 import sys
 
 import pytest
@@ -135,3 +136,24 @@ def test_dump_list_is_deduplicated_across_forms() -> None:
 def test_dump_list_rejects_invalid_entries_with_a_validation_error(bad: list) -> None:
     with pytest.raises(cv.Invalid):
         remote_base.validate_dumpers(bad)
+
+
+def test_dumper_count_is_the_busiest_receivers(
+    generate_main: Callable[[str | Path], str],
+    component_config_path: Callable[[str], Path],
+) -> None:
+    generate_main(component_config_path("receiver_two_dumper_lists.yaml"))
+    # nec, rc_switch and raw on one receiver, rc_switch on the other
+    assert get_define_value("REMOTE_BASE_DUMPER_COUNT") == "3"
+
+
+@pytest.mark.usefixtures("restore_protocol_registries")
+def test_external_dumpers_are_constructed_in_ram(
+    generate_main: Callable[[str | Path], str],
+    component_config_path: Callable[[str], Path],
+) -> None:
+    """A dumper registered outside remote_base may hold state."""
+    main_cpp = generate_main(
+        component_config_path("receiver_with_external_protocol.yaml")
+    )
+    assert re.search(r"new\(\w+\) fake_protocol::FakeDumper\(\);", main_cpp)

tests/components/core/const_vector_test.cpp

@@ -78,6 +78,40 @@ TEST(ConstVector, CopyFromItsOwnStorage) {
   EXPECT_STREQ(list[2], "c");
 }
 
+TEST(ConstVector, AppendCopiesTheTableAndTheExtrasIntoOneOwnedArray) {
+  static constexpr const char *const EXTRA[] = {"d", "e"};
+  ProbeVector empty;
+  empty.append_copy(EXTRA, 1);
+  EXPECT_TRUE(empty.owned());
+  ASSERT_EQ(empty.size(), 1U);
+  EXPECT_STREQ(empty[0], "d");
+
+  ProbeVector list;
+  list.assign_static(TABLE, 3);
+  list.append_copy(EXTRA, 2);
+  EXPECT_TRUE(list.owned());
+  EXPECT_NE(list.data(), TABLE);
+  ASSERT_EQ(list.size(), 5U);
+  EXPECT_TRUE(std::equal(TABLE, TABLE + 3, list.begin()));
+  EXPECT_STREQ(list[3], "d");
+  EXPECT_STREQ(list[4], "e");
+  list.append_copy(EXTRA, 1);  // frees the previous owned array
+  EXPECT_EQ(list.size(), 6U);
+  EXPECT_STREQ(list[5], "d");
+}
+
+TEST(ConstVector, PrependPutsTheExtrasInFront) {
+  static constexpr const char *const EXTRA[] = {"d", "e"};
+  ProbeVector list;
+  list.assign_static(TABLE, 3);
+  list.append_copy(EXTRA, 1);
+  list.prepend_copy(EXTRA + 1, 1);  // frees the previous owned array
+  ASSERT_EQ(list.size(), 5U);
+  EXPECT_STREQ(list[0], "e");
+  EXPECT_TRUE(std::equal(TABLE, TABLE + 3, list.begin() + 1));
+  EXPECT_STREQ(list[4], "d");
+}
+
 TEST(ConstVector, PlainViewHasNoOwnershipCost) {
   static_assert(std::is_trivially_copyable_v<ConstVector<const char *>>);
   static_assert(std::is_trivially_destructible_v<ConstVector<const char *>>);

tests/components/remote_base/__init__.py

@@ -1,3 +1,4 @@
+import esphome.codegen as cg
 from esphome.types import ConfigType
 from tests.testing_helpers import ComponentManifestOverride
 
@@ -24,6 +25,8 @@ def override_manifest(manifest: ComponentManifestOverride) -> None:
             "panasonic",
         ):
             request_protocol(protocol)
+        # receiver_dumpers_test.cpp registers dumpers by hand
+        cg.add_define("REMOTE_BASE_DUMPER_COUNT", 4)
 
     manifest.to_code = to_code_testing
     # AUTO_LOAD is not resolved by the unit test build.

tests/components/remote_base/receiver_dumpers_test.cpp

@@ -0,0 +1,76 @@
+#include <gtest/gtest.h>
+
+#include <vector>
+
+#include "esphome/components/remote_base/remote_base.h"
+
+namespace esphome::remote_base::testing {
+
+class RecordingDumper : public RemoteReceiverDumperBase {
+ public:
+  RecordingDumper(std::vector<int> *calls, int id, bool matches, bool secondary = false)
+      : calls_(calls), id_(id), matches_(matches), secondary_(secondary) {}
+  bool dump(RemoteReceiveData /*src*/) override {
+    this->calls_->push_back(this->id_);
+    return this->matches_;
+  }
+  bool is_secondary() override { return this->secondary_; }
+
+ protected:
+  std::vector<int> *calls_;
+  int id_;
+  bool matches_;
+  bool secondary_;
+};
+
+class TestReceiver : public RemoteReceiverBase {
+ public:
+  TestReceiver() : RemoteReceiverBase(nullptr) {}
+  void receive() { this->call_listeners_dumpers_(); }
+};
+
+TEST(RemoteReceiverDumpers, PrimariesRunInOrderAndAMatchSkipsTheSecondary) {
+  std::vector<int> calls;
+  RecordingDumper raw(&calls, 0, true, true), first(&calls, 1, false), second(&calls, 2, true), third(&calls, 3, false);
+  TestReceiver receiver;
+  receiver.register_dumper(&raw);
+  receiver.register_dumper(&first);
+  receiver.register_dumper(&second);
+  receiver.register_dumper(&third);
+  receiver.receive();
+  EXPECT_EQ(calls, (std::vector<int>{1, 2, 3}));
+}
+
+TEST(RemoteReceiverDumpers, SecondaryRunsWhenNoPrimaryMatched) {
+  std::vector<int> calls;
+  RecordingDumper raw(&calls, 0, true, true), miss(&calls, 1, false);
+  TestReceiver receiver;
+  receiver.register_dumper(&miss);
+  receiver.register_dumper(&raw);
+  receiver.receive();
+  EXPECT_EQ(calls, (std::vector<int>{1, 0}));
+}
+
+TEST(RemoteReceiverDumpers, OnlyTheFirstSecondaryRuns) {
+  std::vector<int> calls;
+  RecordingDumper raw(&calls, 0, true, true), extra_raw(&calls, 9, true, true);
+  TestReceiver receiver;
+  receiver.register_dumper(&raw);
+  receiver.register_dumper(&extra_raw);
+  receiver.receive();
+  EXPECT_EQ(calls, (std::vector<int>{0}));
+}
+
+// External components (vevor decoders) register themselves from their own setup()
+TEST(RemoteReceiverDumpers, RegisteringAfterConstructionAddsTheDumper) {
+  std::vector<int> calls;
+  RecordingDumper early(&calls, 1, false), late(&calls, 2, false);
+  TestReceiver receiver;
+  receiver.register_dumper(&early);
+  receiver.receive();
+  receiver.register_dumper(&late);
+  receiver.receive();
+  EXPECT_EQ(calls, (std::vector<int>{1, 1, 2}));
+}
+
+}  // namespace esphome::remote_base::testing