Merge commit '5e30279ab64986656a5f869d1173434c951eac33' into const-vector-append-copy

esphome/components/am43/am43_base.cpp

@@ -1,127 +1,129 @@
 #include "am43_base.h"
 #include "esphome/core/helpers.h"
-#include "esphome/core/log.h"
 #include <cstring>
 
 namespace esphome::am43 {
 
-ESPHOME_LOG_TAG(TAG, "am43");
+const uint8_t START_PACKET[5] = {0x00, 0xff, 0x00, 0x00, 0x9a};
 
-static constexpr uint8_t START_PACKET[5] = {0x00, 0xff, 0x00, 0x00, 0x9a};
-
-Am43Packet Am43Encoder::get_battery_level_request() {
+Am43Packet *Am43Encoder::get_battery_level_request() {
   uint8_t data = 0x1;
-  return encode(CMD_GET_BATTERY_LEVEL, &data, 1);
+  return this->encode_(0xA2, &data, 1);
 }
 
-Am43Packet Am43Encoder::get_light_level_request() {
+Am43Packet *Am43Encoder::get_light_level_request() {
   uint8_t data = 0x1;
-  return encode(CMD_GET_LIGHT_LEVEL, &data, 1);
+  return this->encode_(0xAA, &data, 1);
 }
 
-Am43Packet Am43Encoder::get_position_request() {
+Am43Packet *Am43Encoder::get_position_request() {
   uint8_t data = 0x1;
-  return encode(CMD_GET_POSITION, &data, 1);
+  return this->encode_(CMD_GET_POSITION, &data, 1);
 }
 
-Am43Packet Am43Encoder::get_send_pin_request(uint16_t pin) {
+Am43Packet *Am43Encoder::get_send_pin_request(uint16_t pin) {
   uint8_t data[2];
   data[0] = (pin & 0xFF00) >> 8;
   data[1] = pin & 0xFF;
-  return encode(CMD_SEND_PIN, data, 2);
+  return this->encode_(CMD_SEND_PIN, data, 2);
 }
 
-Am43Packet Am43Encoder::get_open_request() {
+Am43Packet *Am43Encoder::get_open_request() {
   uint8_t data = 0xDD;
-  return encode(CMD_SET_STATE, &data, 1);
+  return this->encode_(CMD_SET_STATE, &data, 1);
 }
 
-Am43Packet Am43Encoder::get_close_request() {
+Am43Packet *Am43Encoder::get_close_request() {
   uint8_t data = 0xEE;
-  return encode(CMD_SET_STATE, &data, 1);
+  return this->encode_(CMD_SET_STATE, &data, 1);
 }
 
-Am43Packet Am43Encoder::get_stop_request() {
+Am43Packet *Am43Encoder::get_stop_request() {
   uint8_t data = 0xCC;
-  return encode(CMD_SET_STATE, &data, 1);
+  return this->encode_(CMD_SET_STATE, &data, 1);
 }
 
-Am43Packet Am43Encoder::get_set_position_request(uint8_t position) { return encode(CMD_SET_POSITION, &position, 1); }
-
-Am43Packet Am43Encoder::encode(uint8_t command, const uint8_t *data, uint8_t length) {
-  Am43Packet packet;
-  memcpy(packet.data, START_PACKET, sizeof(START_PACKET));
-  packet.data[5] = command;
-  packet.data[6] = length;
-  memcpy(&packet.data[7], data, length);
-  packet.length = length + 7;
-  // The device checks the inverted XOR of everything before it
-  packet.data[packet.length] = xor_bytes(packet.data, packet.length, 0xff);
-  packet.length++;
-  char hex_buf[format_hex_size(sizeof(packet.data))];
-  ESP_LOGV(TAG, "ENC(%d): 0x%s", packet.length, format_hex_to(hex_buf, packet.data, packet.length));
-  return packet;
+Am43Packet *Am43Encoder::get_set_position_request(uint8_t position) {
+  return this->encode_(CMD_SET_POSITION, &position, 1);
 }
 
+void Am43Encoder::checksum_() {
+  uint8_t checksum = 0;
+  int i = 0;
+  for (i = 0; i < this->packet_.length; i++)
+    checksum = checksum ^ this->packet_.data[i];
+  this->packet_.data[i] = checksum ^ 0xff;
+  this->packet_.length++;
+}
+
+Am43Packet *Am43Encoder::encode_(uint8_t command, uint8_t *data, uint8_t length) {
+  memcpy(this->packet_.data, START_PACKET, 5);
+  this->packet_.data[5] = command;
+  this->packet_.data[6] = length;
+  memcpy(&this->packet_.data[7], data, length);
+  this->packet_.length = length + 7;
+  this->checksum_();
+  char hex_buf[format_hex_size(sizeof(this->packet_.data))];
+  ESP_LOGV("am43", "ENC(%d): 0x%s", this->packet_.length,
+           format_hex_to(hex_buf, this->packet_.data, this->packet_.length));
+  return &this->packet_;
+}
+
+#define VERIFY_MIN_LENGTH(x) \
+  if (length < (x)) \
+    return;
+
 void Am43Decoder::decode(const uint8_t *data, uint16_t length) {
-  // Reset so a decoder reused across frames reports only the latest one
   this->has_battery_level_ = false;
   this->has_light_level_ = false;
   this->has_set_position_response_ = false;
   this->has_set_state_response_ = false;
   this->has_position_ = false;
   this->has_pin_response_ = false;
-  char hex_buf[format_hex_size(sizeof(Am43Packet::data))];  // Max expected packet size
-  ESP_LOGV(TAG, "DEC(%d): 0x%s", length, format_hex_to(hex_buf, data, length));
+  char hex_buf[format_hex_size(24)];  // Max expected packet size
+  ESP_LOGV("am43", "DEC(%d): 0x%s", length, format_hex_to(hex_buf, data, length));
 
   if (length < 2 || data[0] != 0x9a)
     return;
   switch (data[1]) {
     case CMD_GET_BATTERY_LEVEL: {
-      if (length < 8)
-        return;
+      VERIFY_MIN_LENGTH(8);
       this->battery_level_ = data[7];
       this->has_battery_level_ = true;
       break;
     }
     case CMD_GET_LIGHT_LEVEL: {
-      if (length < 5)
-        return;
+      VERIFY_MIN_LENGTH(5);
       this->light_level_ = 100 * ((float) data[4] / 9);
       this->has_light_level_ = true;
       break;
     }
     case CMD_GET_POSITION: {
-      if (length < 6)
-        return;
+      VERIFY_MIN_LENGTH(6);
       this->position_ = data[5];
       this->has_position_ = true;
       break;
     }
     case CMD_NOTIFY_POSITION: {
-      if (length < 5)
-        return;
+      VERIFY_MIN_LENGTH(5);
       this->position_ = data[4];
       this->has_position_ = true;
       break;
     }
     case CMD_SEND_PIN: {
-      if (length < 4)
-        return;
+      VERIFY_MIN_LENGTH(4);
       this->pin_ok_ = data[3] == RESPONSE_ACK;
       this->has_pin_response_ = true;
       break;
     }
     case CMD_SET_POSITION: {
-      if (length < 4)
-        return;
+      VERIFY_MIN_LENGTH(4);
       this->set_position_ok_ = data[3] == RESPONSE_ACK;
       this->has_set_position_response_ = true;
       break;
     }
     case CMD_SET_STATE: {
-      if (length < 4)
-        return;
+      VERIFY_MIN_LENGTH(4);
       this->set_state_ok_ = data[3] == RESPONSE_ACK;
       this->has_set_state_response_ = true;
       break;
@@ -129,6 +131,6 @@ void Am43Decoder::decode(const uint8_t *data, uint16_t length) {
     default:
       break;
   }
-}
+};
 
 }  // namespace esphome::am43

esphome/components/am43/am43_base.h

@@ -1,57 +1,59 @@
 #pragma once
 
-#include <cstdint>
+#include "esphome/core/helpers.h"
+#include "esphome/core/log.h"
 
 namespace esphome::am43 {
 
-static constexpr uint16_t AM43_SERVICE_UUID = 0xFE50;
-static constexpr uint16_t AM43_CHARACTERISTIC_UUID = 0xFE51;
+static const uint16_t AM43_SERVICE_UUID = 0xFE50;
+static const uint16_t AM43_CHARACTERISTIC_UUID = 0xFE51;
+//
 // Tuya identifiers, only to detect and warn users as they are incompatible.
-static constexpr uint16_t AM43_TUYA_SERVICE_UUID = 0x1910;
-static constexpr uint16_t AM43_TUYA_CHARACTERISTIC_UUID = 0x2b11;
+static const uint16_t AM43_TUYA_SERVICE_UUID = 0x1910;
+static const uint16_t AM43_TUYA_CHARACTERISTIC_UUID = 0x2b11;
 
 struct Am43Packet {
   uint8_t length;
   uint8_t data[24];
 };
 
-static constexpr uint8_t CMD_GET_BATTERY_LEVEL = 0xA2;
-static constexpr uint8_t CMD_GET_LIGHT_LEVEL = 0xAA;
-static constexpr uint8_t CMD_GET_POSITION = 0xA7;
-static constexpr uint8_t CMD_SEND_PIN = 0x17;
-static constexpr uint8_t CMD_SET_STATE = 0x0A;
-static constexpr uint8_t CMD_SET_POSITION = 0x0D;
-static constexpr uint8_t CMD_NOTIFY_POSITION = 0xA1;
+static const uint8_t CMD_GET_BATTERY_LEVEL = 0xA2;
+static const uint8_t CMD_GET_LIGHT_LEVEL = 0xAA;
+static const uint8_t CMD_GET_POSITION = 0xA7;
+static const uint8_t CMD_SEND_PIN = 0x17;
+static const uint8_t CMD_SET_STATE = 0x0A;
+static const uint8_t CMD_SET_POSITION = 0x0D;
+static const uint8_t CMD_NOTIFY_POSITION = 0xA1;
 
-static constexpr uint8_t RESPONSE_ACK = 0x5A;
-static constexpr uint8_t RESPONSE_NACK = 0xA5;
+static const uint8_t RESPONSE_ACK = 0x5A;
+static const uint8_t RESPONSE_NACK = 0xA5;
 
-/// Builds request packets; each returns a packet the caller sends from its own stack.
 class Am43Encoder {
  public:
-  static Am43Packet get_battery_level_request();
-  static Am43Packet get_light_level_request();
-  static Am43Packet get_position_request();
-  static Am43Packet get_send_pin_request(uint16_t pin);
-  static Am43Packet get_open_request();
-  static Am43Packet get_close_request();
-  static Am43Packet get_stop_request();
-  static Am43Packet get_set_position_request(uint8_t position);
+  Am43Packet *get_battery_level_request();
+  Am43Packet *get_light_level_request();
+  Am43Packet *get_position_request();
+  Am43Packet *get_send_pin_request(uint16_t pin);
+  Am43Packet *get_open_request();
+  Am43Packet *get_close_request();
+  Am43Packet *get_stop_request();
+  Am43Packet *get_set_position_request(uint8_t position);
 
  protected:
-  static Am43Packet encode(uint8_t command, const uint8_t *data, uint8_t length);
+  void checksum_();
+  Am43Packet *encode_(uint8_t command, uint8_t *data, uint8_t length);
+  Am43Packet packet_;
 };
 
-/// Decodes one notification; construct it on the stack for each frame.
 class Am43Decoder {
  public:
   void decode(const uint8_t *data, uint16_t length);
-  bool has_battery_level() const { return this->has_battery_level_; }
-  bool has_light_level() const { return this->has_light_level_; }
-  bool has_set_position_response() const { return this->has_set_position_response_; }
-  bool has_set_state_response() const { return this->has_set_state_response_; }
-  bool has_position() const { return this->has_position_; }
-  bool has_pin_response() const { return this->has_pin_response_; }
+  bool has_battery_level() { return this->has_battery_level_; }
+  bool has_light_level() { return this->has_light_level_; }
+  bool has_set_position_response() { return this->has_set_position_response_; }
+  bool has_set_state_response() { return this->has_set_state_response_; }
+  bool has_position() { return this->has_position_; }
+  bool has_pin_response() { return this->has_pin_response_; }
 
   union {
     uint8_t position_;
@@ -63,12 +65,12 @@ class Am43Decoder {
   };
 
  protected:
-  bool has_battery_level_{false};
-  bool has_light_level_{false};
-  bool has_set_position_response_{false};
-  bool has_set_state_response_{false};
-  bool has_position_{false};
-  bool has_pin_response_{false};
+  bool has_battery_level_;
+  bool has_light_level_;
+  bool has_set_position_response_;
+  bool has_set_state_response_;
+  bool has_position_;
+  bool has_pin_response_;
 };
 
 }  // namespace esphome::am43

esphome/components/am43/cover/am43_cover.cpp

@@ -17,12 +17,25 @@ void Am43Component::dump_config() {
                 this->pin_, (int) this->invert_position_);
 }
 
-void Am43Component::setup() { this->position = COVER_OPEN; }
+void Am43Component::setup() {
+  this->position = COVER_OPEN;
+  this->encoder_ = make_unique<Am43Encoder>();
+  this->decoder_ = make_unique<Am43Decoder>();
+  this->logged_in_ = false;
+}
 
 void Am43Component::loop() {
   if (this->node_state == espbt::ClientState::ESTABLISHED && !this->logged_in_) {
+    auto *packet = this->encoder_->get_send_pin_request(this->pin_);
+    auto status =
+        esp_ble_gattc_write_char(this->parent_->get_gattc_if(), this->parent_->get_conn_id(), this->char_handle_,
+                                 packet->length, packet->data, ESP_GATT_WRITE_TYPE_NO_RSP, ESP_GATT_AUTH_REQ_NONE);
     ESP_LOGI(TAG, "[%s] Logging into AM43", LOG_STR_ARG(this->get_log_name()));
-    this->logged_in_ = this->write_packet_(Am43Encoder::get_send_pin_request(this->pin_), LOG_STR("set_pin"));
+    if (status) {
+      ESP_LOGW(TAG, "[%s] Error writing set_pin to device, error = %d", LOG_STR_ARG(this->get_log_name()), status);
+    } else {
+      this->logged_in_ = true;
+    }
   }
 }
 
@@ -40,15 +53,29 @@ void Am43Component::control(const CoverCall &call) {
     ESP_LOGW(TAG, "[%s] Cannot send cover control, not connected", LOG_STR_ARG(this->get_log_name()));
     return;
   }
-  if (call.get_stop())
-    this->write_packet_(Am43Encoder::get_stop_request(), LOG_STR("stop"));
+  if (call.get_stop()) {
+    auto *packet = this->encoder_->get_stop_request();
+    auto status =
+        esp_ble_gattc_write_char(this->parent_->get_gattc_if(), this->parent_->get_conn_id(), this->char_handle_,
+                                 packet->length, packet->data, ESP_GATT_WRITE_TYPE_NO_RSP, ESP_GATT_AUTH_REQ_NONE);
+    if (status) {
+      ESP_LOGW(TAG, "[%s] Error writing stop command to device, error = %d", LOG_STR_ARG(this->get_log_name()), status);
+    }
+  }
   auto opt_pos = call.get_position();
   if (opt_pos.has_value()) {
     auto pos = *opt_pos;
 
     if (this->invert_position_)
       pos = 1 - pos;
-    this->write_packet_(Am43Encoder::get_set_position_request(100 - (uint8_t) (pos * 100)), LOG_STR("set_position"));
+    auto *packet = this->encoder_->get_set_position_request(100 - (uint8_t) (pos * 100));
+    auto status =
+        esp_ble_gattc_write_char(this->parent_->get_gattc_if(), this->parent_->get_conn_id(), this->char_handle_,
+                                 packet->length, packet->data, ESP_GATT_WRITE_TYPE_NO_RSP, ESP_GATT_AUTH_REQ_NONE);
+    if (status) {
+      ESP_LOGW(TAG, "[%s] Error writing set_position command to device, error = %d", LOG_STR_ARG(this->get_log_name()),
+               status);
+    }
   }
 }
 
@@ -86,11 +113,10 @@ void Am43Component::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_
     case ESP_GATTC_NOTIFY_EVT: {
       if (param->notify.handle != this->char_handle_)
         break;
-      Am43Decoder decoder;
-      decoder.decode(param->notify.value, param->notify.value_len);
+      this->decoder_->decode(param->notify.value, param->notify.value_len);
 
-      if (decoder.has_position()) {
-        this->position = ((float) decoder.position_ / 100.0f);
+      if (this->decoder_->has_position()) {
+        this->position = ((float) this->decoder_->position_ / 100.0f);
         if (!this->invert_position_)
           this->position = 1 - this->position;
         if (this->position > 0.97f)
@@ -100,20 +126,27 @@ void Am43Component::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_
         this->publish_state();
       }
 
-      if (decoder.has_pin_response()) {
-        if (decoder.pin_ok_) {
+      if (this->decoder_->has_pin_response()) {
+        if (this->decoder_->pin_ok_) {
           ESP_LOGI(TAG, "[%s] AM43 pin accepted.", LOG_STR_ARG(this->get_log_name()));
-          this->write_packet_(Am43Encoder::get_position_request(), LOG_STR("get_position"));
+          auto *packet = this->encoder_->get_position_request();
+          auto status = esp_ble_gattc_write_char(this->parent_->get_gattc_if(), this->parent_->get_conn_id(),
+                                                 this->char_handle_, packet->length, packet->data,
+                                                 ESP_GATT_WRITE_TYPE_NO_RSP, ESP_GATT_AUTH_REQ_NONE);
+          if (status) {
+            ESP_LOGW(TAG, "[%s] Error writing set_position to device, error = %d", LOG_STR_ARG(this->get_log_name()),
+                     status);
+          }
         } else {
           ESP_LOGW(TAG, "[%s] AM43 pin rejected!", LOG_STR_ARG(this->get_log_name()));
         }
       }
 
-      if (decoder.has_set_position_response() && !decoder.set_position_ok_) {
+      if (this->decoder_->has_set_position_response() && !this->decoder_->set_position_ok_) {
         ESP_LOGW(TAG, "[%s] Got nack after sending set_position. Bad pin?", LOG_STR_ARG(this->get_log_name()));
       }
 
-      if (decoder.has_set_state_response() && !decoder.set_state_ok_) {
+      if (this->decoder_->has_set_state_response() && !this->decoder_->set_state_ok_) {
         ESP_LOGW(TAG, "[%s] Got nack after sending set_state. Bad pin?", LOG_STR_ARG(this->get_log_name()));
       }
       break;
@@ -123,19 +156,6 @@ void Am43Component::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_
   }
 }
 
-bool Am43Component::write_packet_(const Am43Packet &packet, const LogString *what) {
-  // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) the IDF API takes a mutable buffer it only reads
-  auto status = esp_ble_gattc_write_char(this->parent_->get_gattc_if(), this->parent_->get_conn_id(),
-                                         this->char_handle_, packet.length, const_cast<uint8_t *>(packet.data),
-                                         ESP_GATT_WRITE_TYPE_NO_RSP, ESP_GATT_AUTH_REQ_NONE);
-  if (status) {
-    ESP_LOGW(TAG, "[%s] Error writing %s to device, error = %d", LOG_STR_ARG(this->get_log_name()), LOG_STR_ARG(what),
-             status);
-    return false;
-  }
-  return true;
-}
-
 }  // namespace esphome::am43
 
 #endif

esphome/components/am43/cover/am43_cover.h

@@ -27,12 +27,14 @@ class Am43Component final : public cover::Cover, public esphome::ble_client::BLE
 
  protected:
   void control(const cover::CoverCall &call) override;
-  /// Writes a request to the control characteristic; logs and returns false on failure.
-  bool write_packet_(const Am43Packet &packet, const LogString *what);
-  uint16_t char_handle_{0};
-  uint16_t pin_{0};
-  bool invert_position_{false};
-  bool logged_in_{false};
+  uint16_t char_handle_;
+  uint16_t pin_;
+  bool invert_position_;
+  std::unique_ptr<Am43Encoder> encoder_;
+  std::unique_ptr<Am43Decoder> decoder_;
+  bool logged_in_;
+
+  float position_;
 };
 
 }  // namespace esphome::am43

esphome/components/am43/sensor/am43_sensor.cpp

@@ -14,9 +14,24 @@ void Am43::dump_config() {
   LOG_SENSOR(" ", "Illuminance", this->illuminance_);
 }
 
+void Am43::setup() {
+  this->encoder_ = make_unique<Am43Encoder>();
+  this->decoder_ = make_unique<Am43Decoder>();
+  this->logged_in_ = false;
+  this->last_battery_update_ = 0;
+  this->current_sensor_ = 0;
+}
+
 void Am43::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param) {
   switch (event) {
+    case ESP_GATTC_OPEN_EVT: {
+      if (param->open.status == ESP_GATT_OK) {
+        this->logged_in_ = false;
+      }
+      break;
+    }
     case ESP_GATTC_DISCONNECT_EVT: {
+      this->logged_in_ = false;
       this->node_state = espbt::ClientState::IDLE;
       if (this->battery_ != nullptr)
         this->battery_->publish_state(NAN);
@@ -45,21 +60,28 @@ void Am43::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_i
     case ESP_GATTC_NOTIFY_EVT: {
       if (param->notify.handle != this->char_handle_)
         break;
-      Am43Decoder decoder;
-      decoder.decode(param->notify.value, param->notify.value_len);
+      this->decoder_->decode(param->notify.value, param->notify.value_len);
 
-      if (this->battery_ != nullptr && decoder.has_battery_level() && millis() - this->last_battery_update_ > 10000) {
-        this->battery_->publish_state(decoder.battery_level_);
+      if (this->battery_ != nullptr && this->decoder_->has_battery_level() &&
+          millis() - this->last_battery_update_ > 10000) {
+        this->battery_->publish_state(this->decoder_->battery_level_);
         this->last_battery_update_ = millis();
       }
 
-      if (this->illuminance_ != nullptr && decoder.has_light_level()) {
-        this->illuminance_->publish_state(decoder.light_level_);
+      if (this->illuminance_ != nullptr && this->decoder_->has_light_level()) {
+        this->illuminance_->publish_state(this->decoder_->light_level_);
       }
 
       if (this->current_sensor_ > 0) {
-        if (this->illuminance_ != nullptr)
-          this->write_packet_(Am43Encoder::get_light_level_request());
+        if (this->illuminance_ != nullptr) {
+          auto *packet = this->encoder_->get_light_level_request();
+          auto status = esp_ble_gattc_write_char(this->parent_->get_gattc_if(), this->parent_->get_conn_id(),
+                                                 this->char_handle_, packet->length, packet->data,
+                                                 ESP_GATT_WRITE_TYPE_NO_RSP, ESP_GATT_AUTH_REQ_NONE);
+          if (status) {
+            ESP_LOGW(TAG, "[%s] esp_ble_gattc_write_char failed, status=%d", this->parent_->address_str(), status);
+          }
+        }
         this->current_sensor_ = 0;
       }
       break;
@@ -75,22 +97,19 @@ void Am43::update() {
     return;
   }
   if (this->current_sensor_ == 0) {
-    if (this->battery_ != nullptr)
-      this->write_packet_(Am43Encoder::get_battery_level_request());
+    if (this->battery_ != nullptr) {
+      auto *packet = this->encoder_->get_battery_level_request();
+      auto status =
+          esp_ble_gattc_write_char(this->parent_->get_gattc_if(), this->parent_->get_conn_id(), this->char_handle_,
+                                   packet->length, packet->data, ESP_GATT_WRITE_TYPE_NO_RSP, ESP_GATT_AUTH_REQ_NONE);
+      if (status) {
+        ESP_LOGW(TAG, "[%s] esp_ble_gattc_write_char failed, status=%d", this->parent_->address_str(), status);
+      }
+    }
     this->current_sensor_++;
   }
 }
 
-void Am43::write_packet_(const Am43Packet &packet) {
-  // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) the IDF API takes a mutable buffer it only reads
-  auto status = esp_ble_gattc_write_char(this->parent_->get_gattc_if(), this->parent_->get_conn_id(),
-                                         this->char_handle_, packet.length, const_cast<uint8_t *>(packet.data),
-                                         ESP_GATT_WRITE_TYPE_NO_RSP, ESP_GATT_AUTH_REQ_NONE);
-  if (status) {
-    ESP_LOGW(TAG, "[%s] esp_ble_gattc_write_char failed, status=%d", this->parent_->address_str(), status);
-  }
-}
-
 }  // namespace esphome::am43
 
 #endif

esphome/components/am43/sensor/am43_sensor.h

@@ -16,6 +16,7 @@ namespace espbt = esphome::esp32_ble_tracker;
 
 class Am43 final : public esphome::ble_client::BLEClientNode, public PollingComponent {
  public:
+  void setup() override;
   void update() override;
   void gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if,
                            esp_ble_gattc_cb_param_t *param) override;
@@ -24,15 +25,16 @@ class Am43 final : public esphome::ble_client::BLEClientNode, public PollingComp
   void set_illuminance(sensor::Sensor *illuminance) { illuminance_ = illuminance; }
 
  protected:
-  /// Writes a request to the control characteristic; logs on failure.
-  void write_packet_(const Am43Packet &packet);
+  uint16_t char_handle_;
+  std::unique_ptr<Am43Encoder> encoder_;
+  std::unique_ptr<Am43Decoder> decoder_;
+  bool logged_in_;
   sensor::Sensor *battery_{nullptr};
   sensor::Sensor *illuminance_{nullptr};
+  uint8_t current_sensor_;
   // The AM43 often gets into a state where it spams loads of battery update
   // notifications. Here we will limit to no more than every 10s.
-  uint32_t last_battery_update_{0};
-  uint16_t char_handle_{0};
-  uint8_t current_sensor_{0};
+  uint32_t last_battery_update_;
 };
 
 }  // namespace esphome::am43

esphome/components/as7341/as7341.cpp

@@ -110,12 +110,12 @@ bool AS7341Component::read_channels(uint16_t *data) {
   this->set_smux_low_channels(true);
   this->enable_spectral_measurement(true);
   this->wait_for_data();
-  bool low_success = this->read_bytes_16_le(AS7341_CH0_DATA_L, data, 6);
+  bool low_success = this->read_bytes_16(AS7341_CH0_DATA_L, data, 6);
 
   this->set_smux_low_channels(false);
   this->enable_spectral_measurement(true);
   this->wait_for_data();
-  bool high_sucess = this->read_bytes_16_le(AS7341_CH0_DATA_L, &data[6], 6);
+  bool high_sucess = this->read_bytes_16(AS7341_CH0_DATA_L, &data[6], 6);
 
   return low_success && high_sucess;
 }

esphome/core/helpers.h

@@ -149,10 +149,10 @@ template<typename T, bool Owning = false> class ConstVector {
 
 /// Owning variant: a codegen table that outlives it, or a heap copy of a runtime list it owns.
 /// Ownership is the top bit of the size; it is not copyable, so a copy can never outlive the owner.
-/// Elements must be whole words so ESP8266 can read a codegen table from flash.
+/// Elements must be whole words: on ESP8266, iterating or indexing a codegen table reads them straight from flash.
 template<typename T> class ConstVector<T, true> {
   static_assert(std::is_trivially_copyable_v<T> && sizeof(T) % sizeof(uint32_t) == 0,
-                "ConstVector elements must be whole words so ESP8266 can read them from flash");
+                "ConstVector elements must be whole words so ESP8266 can index them in flash");
 
  public:
   using value_type = T;
@@ -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,24 @@ 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_items(first, first_count, table);
+    copy_items(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;
+  }
+  // ESP8266: the source may be a table in flash
+  static void copy_items(const T *src, size_t count, T *dst) {
+    if (count == 0)
+      return;
+#ifdef USE_ESP8266
+    memcpy_P(dst, src, count * sizeof(T));
+#else
+    std::copy(src, src + count, dst);
+#endif
+  }
 
   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/am43/am43_codec_test.cpp

@@ -1,127 +0,0 @@
-#include <gtest/gtest.h>
-
-#include <vector>
-
-#include "esphome/components/am43/am43_base.h"
-
-namespace esphome::am43::testing {
-
-static std::vector<uint8_t> bytes(const Am43Packet &packet) {
-  return std::vector<uint8_t>(packet.data, packet.data + packet.length);
-}
-
-TEST(Am43Encoder, RequestsMatchTheDeviceFraming) {
-  // 00 ff 00 00 9a, command, length, data, then the inverted XOR of everything before it
-  EXPECT_EQ(bytes(Am43Encoder::get_battery_level_request()),
-            (std::vector<uint8_t>{0x00, 0xff, 0x00, 0x00, 0x9a, 0xa2, 0x01, 0x01, 0x38}));
-  EXPECT_EQ(bytes(Am43Encoder::get_light_level_request()),
-            (std::vector<uint8_t>{0x00, 0xff, 0x00, 0x00, 0x9a, 0xaa, 0x01, 0x01, 0x30}));
-  EXPECT_EQ(bytes(Am43Encoder::get_position_request()),
-            (std::vector<uint8_t>{0x00, 0xff, 0x00, 0x00, 0x9a, 0xa7, 0x01, 0x01, 0x3d}));
-  EXPECT_EQ(bytes(Am43Encoder::get_send_pin_request(0x1234)),
-            (std::vector<uint8_t>{0x00, 0xff, 0x00, 0x00, 0x9a, 0x17, 0x02, 0x12, 0x34, 0xa9}));
-  EXPECT_EQ(bytes(Am43Encoder::get_open_request()),
-            (std::vector<uint8_t>{0x00, 0xff, 0x00, 0x00, 0x9a, 0x0a, 0x01, 0xdd, 0x4c}));
-  EXPECT_EQ(bytes(Am43Encoder::get_close_request()),
-            (std::vector<uint8_t>{0x00, 0xff, 0x00, 0x00, 0x9a, 0x0a, 0x01, 0xee, 0x7f}));
-  EXPECT_EQ(bytes(Am43Encoder::get_stop_request()),
-            (std::vector<uint8_t>{0x00, 0xff, 0x00, 0x00, 0x9a, 0x0a, 0x01, 0xcc, 0x5d}));
-  EXPECT_EQ(bytes(Am43Encoder::get_set_position_request(42)),
-            (std::vector<uint8_t>{0x00, 0xff, 0x00, 0x00, 0x9a, 0x0d, 0x01, 0x2a, 0xbc}));
-}
-
-TEST(Am43Decoder, ReadsEachResponse) {
-  Am43Decoder decoder;
-  const uint8_t battery[] = {0x9a, CMD_GET_BATTERY_LEVEL, 0, 0, 0, 0, 0, 77};
-  decoder.decode(battery, sizeof(battery));
-  ASSERT_TRUE(decoder.has_battery_level());
-  EXPECT_EQ(decoder.battery_level_, 77);
-  EXPECT_FALSE(decoder.has_position());
-
-  const uint8_t light[] = {0x9a, CMD_GET_LIGHT_LEVEL, 0, 0, 9};
-  decoder.decode(light, sizeof(light));
-  ASSERT_TRUE(decoder.has_light_level());
-  EXPECT_FLOAT_EQ(decoder.light_level_, 100.0f);
-  EXPECT_FALSE(decoder.has_battery_level());  // flags reset on every frame
-
-  const uint8_t position[] = {0x9a, CMD_GET_POSITION, 0, 0, 0, 30};
-  decoder.decode(position, sizeof(position));
-  ASSERT_TRUE(decoder.has_position());
-  EXPECT_EQ(decoder.position_, 30);
-
-  const uint8_t notify[] = {0x9a, CMD_NOTIFY_POSITION, 0, 0, 55};
-  decoder.decode(notify, sizeof(notify));
-  ASSERT_TRUE(decoder.has_position());
-  EXPECT_EQ(decoder.position_, 55);
-
-  const uint8_t pin_ack[] = {0x9a, CMD_SEND_PIN, 0, RESPONSE_ACK};
-  decoder.decode(pin_ack, sizeof(pin_ack));
-  ASSERT_TRUE(decoder.has_pin_response());
-  EXPECT_TRUE(decoder.pin_ok_);
-
-  const uint8_t position_nack[] = {0x9a, CMD_SET_POSITION, 0, RESPONSE_NACK};
-  decoder.decode(position_nack, sizeof(position_nack));
-  ASSERT_TRUE(decoder.has_set_position_response());
-  EXPECT_FALSE(decoder.set_position_ok_);
-
-  const uint8_t state_ack[] = {0x9a, CMD_SET_STATE, 0, RESPONSE_ACK};
-  decoder.decode(state_ack, sizeof(state_ack));
-  ASSERT_TRUE(decoder.has_set_state_response());
-  EXPECT_TRUE(decoder.set_state_ok_);
-}
-
-TEST(Am43Decoder, IgnoresShortAndForeignFrames) {
-  Am43Decoder decoder;
-  const uint8_t short_battery[] = {0x9a, CMD_GET_BATTERY_LEVEL, 0, 0};
-  decoder.decode(short_battery, sizeof(short_battery));
-  EXPECT_FALSE(decoder.has_battery_level());
-
-  const uint8_t foreign[] = {0x55, CMD_GET_POSITION, 0, 0, 0, 30};
-  decoder.decode(foreign, sizeof(foreign));
-  EXPECT_FALSE(decoder.has_position());
-}
-
-TEST(Am43Decoder, EachResponseNeedsItsMinimumLength) {
-  struct Case {
-    uint8_t command;
-    uint8_t min_length;
-    bool (*has)(Am43Decoder &);
-  };
-  const Case cases[] = {
-      {CMD_GET_BATTERY_LEVEL, 8, [](Am43Decoder &d) { return d.has_battery_level(); }},
-      {CMD_GET_LIGHT_LEVEL, 5, [](Am43Decoder &d) { return d.has_light_level(); }},
-      {CMD_GET_POSITION, 6, [](Am43Decoder &d) { return d.has_position(); }},
-      {CMD_NOTIFY_POSITION, 5, [](Am43Decoder &d) { return d.has_position(); }},
-      {CMD_SEND_PIN, 4, [](Am43Decoder &d) { return d.has_pin_response(); }},
-      {CMD_SET_POSITION, 4, [](Am43Decoder &d) { return d.has_set_position_response(); }},
-      {CMD_SET_STATE, 4, [](Am43Decoder &d) { return d.has_set_state_response(); }},
-  };
-  for (const auto &c : cases) {
-    uint8_t frame[8] = {0x9a, c.command, 0, RESPONSE_ACK, 0, 0, 0, 0};
-    Am43Decoder decoder;
-    decoder.decode(frame, c.min_length - 1);
-    EXPECT_FALSE(c.has(decoder)) << "command 0x" << std::hex << int(c.command);
-    decoder.decode(frame, c.min_length);
-    EXPECT_TRUE(c.has(decoder)) << "command 0x" << std::hex << int(c.command);
-  }
-}
-
-TEST(Am43Decoder, IgnoresTinyAndUnknownFrames) {
-  auto expect_nothing = [](Am43Decoder &decoder) {
-    EXPECT_FALSE(decoder.has_battery_level());
-    EXPECT_FALSE(decoder.has_light_level());
-    EXPECT_FALSE(decoder.has_position());
-    EXPECT_FALSE(decoder.has_pin_response());
-    EXPECT_FALSE(decoder.has_set_position_response());
-    EXPECT_FALSE(decoder.has_set_state_response());
-  };
-  Am43Decoder decoder;
-  const uint8_t one_byte[] = {0x9a};
-  decoder.decode(one_byte, sizeof(one_byte));
-  expect_nothing(decoder);
-  const uint8_t unknown[] = {0x9a, 0x42, 0, RESPONSE_ACK, 0, 0, 0, 0};
-  decoder.decode(unknown, sizeof(unknown));
-  expect_nothing(decoder);
-}
-
-}  // namespace esphome::am43::testing

tests/components/as7341/__init__.py

@@ -1,6 +0,0 @@
-from tests.testing_helpers import ComponentManifestOverride
-
-
-def override_manifest(manifest: ComponentManifestOverride) -> None:
-    # The driver lives in the sensor platform, so pull in what it builds against.
-    manifest.dependencies = manifest.dependencies + ["i2c", "sensor"]

tests/components/as7341/test_as7341.cpp

@@ -1,57 +0,0 @@
-#include <gtest/gtest.h>
-
-#include <array>
-
-#include "esphome/components/as7341/as7341.h"
-
-namespace esphome::as7341::testing {
-
-static constexpr uint8_t ADDRESS = 0x39;
-static constexpr uint8_t SMUXEN = 1 << 4;
-static constexpr uint8_t AVALID = 1 << 6;
-static constexpr uint8_t NUM_ADC_CHANNELS = 6;
-
-// The chip's registers behind an address pointer that moves on after every byte.
-// SMUX commands finish at once and spectral data is always valid.
-class RegisterBus final : public i2c::I2CBus {
- public:
-  i2c::ErrorCode write_readv(uint8_t address, const uint8_t *data, size_t size, uint8_t *out,
-                             size_t out_size) override {
-    EXPECT_EQ(address, ADDRESS);
-    if (size != 0) {
-      this->pointer_ = data[0];
-      for (size_t i = 1; i < size; i++)
-        this->registers[this->pointer_++] = data[i];
-    }
-    this->registers[AS7341_ENABLE] &= ~SMUXEN;
-    for (size_t i = 0; i < out_size; i++)
-      out[i] = this->registers[this->pointer_++];
-    return i2c::ERROR_OK;
-  }
-
-  std::array<uint8_t, 256> registers{};
-
- protected:
-  uint8_t pointer_{0};
-};
-
-TEST(AS7341Test, ReadChannelsDecodesLowByteFirst) {
-  RegisterBus bus;
-  AS7341Component sensor;
-  sensor.set_i2c_bus(&bus);
-  sensor.set_i2c_address(ADDRESS);
-  bus.registers[AS7341_STATUS2] = AVALID;
-
-  // CHn_DATA_L holds the low byte and CHn_DATA_H, one address up, the high byte.
-  for (uint8_t channel = 0; channel < NUM_ADC_CHANNELS; channel++) {
-    bus.registers[AS7341_CH0_DATA_L + 2 * channel] = 0x34 + channel;
-    bus.registers[AS7341_CH0_DATA_H + 2 * channel] = 0x12;
-  }
-
-  std::array<uint16_t, 2 * NUM_ADC_CHANNELS> data{};
-  ASSERT_TRUE(sensor.read_channels(data.data()));
-  for (size_t i = 0; i < data.size(); i++)
-    EXPECT_EQ(data[i], 0x1234 + i % NUM_ADC_CHANNELS) << "value " << unsigned(i);
-}
-
-}  // namespace esphome::as7341::testing

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