Merge branch 'switch-initial-write-state' into switch-initial-write-state-users

AGENTS.md

@@ -449,6 +449,8 @@ file does, and it is the authority when they disagree. The most useful starting
           ```
           Restore an owned or standalone switch in `setup()` with `sw->restore()`, or `restore(default)`
           to still drive a state when the restore mode is disabled.
+          A hub that applies its switches' restored states itself reads `sw->get_initial_write_state()`, which
+          already applies `inverted`.
 
         - **Other codegen helpers:** `uart.new_uart_component(config, *ctor_args)` (and `i2c.new_i2c_component`,
           `spi.new_spi_component`, `modbus.new_modbus_client_component`, `ble_client.new_ble_node_component`)

esphome/components/dfrobot_sen0609/dfrobot_sen0609.cpp

@@ -173,8 +173,8 @@ void DFRobotSEN0609Component::setup() {
     switch_::Switch *sw = this->switches_[i];
     if (sw == nullptr)
       continue;
-    const bool on = sw->get_initial_state_with_restore_mode().value_or(true);
-    this->publish_switch_(static_cast<SwitchSetting>(i), on != sw->is_inverted());
+    // A disabled restore mode leaves the switch on, which an inverted switch writes as off
+    this->publish_switch_(static_cast<SwitchSetting>(i), sw->get_initial_write_state().value_or(!sw->is_inverted()));
   }
 #endif
   this->queue_configuration_();

esphome/components/hitachi_ac344/hitachi_ac344.cpp

@@ -155,12 +155,9 @@ void HitachiClimate::setup() {
     set_swing_v_(swing_vertical(restore->swing_mode));
   }
 #ifdef USE_SWITCH
-  // The restored value is the switch state, which an inverted switch flips relative to the frame bit
   if (this->mildew_proof_switch_ != nullptr) {
-    if (auto initial = this->mildew_proof_switch_->get_initial_state_with_restore_mode()) {
-      assign_bits(this->remote_state_[HITACHI_AC344_MILDEWPROOF_BYTE], 1 << HITACHI_AC344_MILDEWPROOF_OFFSET,
-                  *initial != this->mildew_proof_switch_->is_inverted());
-    }
+    if (auto initial = this->mildew_proof_switch_->get_initial_write_state())
+      assign_bits(this->remote_state_[HITACHI_AC344_MILDEWPROOF_BYTE], 1 << HITACHI_AC344_MILDEWPROOF_OFFSET, *initial);
   }
 #endif
   this->setup_(restore);

esphome/components/remote_base/dooya_protocol.cpp

@@ -7,35 +7,108 @@ ESPHOME_LOG_TAG(TAG, "remote.dooya");
 
 static constexpr uint32_t HEADER_HIGH_US = 5000;
 static constexpr uint32_t HEADER_LOW_US = 1500;
-static constexpr uint32_t SHORT_US = 350;
-static constexpr uint32_t LONG_US = 750;
-static constexpr BitTimings BIT_TIMINGS{.one_space = SHORT_US,
-                                        .zero_space = LONG_US,
-                                        .bit_order = BitOrder::BIT_ORDER_MSB_FIRST,
-                                        .one_mark = LONG_US,
-                                        .zero_mark = SHORT_US};
+static constexpr uint32_t BIT_ZERO_HIGH_US = 350;
+static constexpr uint32_t BIT_ZERO_LOW_US = 750;
+static constexpr uint32_t BIT_ONE_HIGH_US = 750;
+static constexpr uint32_t BIT_ONE_LOW_US = 350;
 
 void DooyaProtocol::encode(RemoteTransmitData *dst, const DooyaData &data) {
   dst->set_carrier_frequency(0);
   dst->reserve(2 + 40 * 2u);
 
   dst->item(HEADER_HIGH_US, HEADER_LOW_US);
-  // 24 bit id, 8 bit channel, 4 bit button, 4 bit check
-  const uint8_t bytes[5] = {static_cast<uint8_t>(data.id >> 16), static_cast<uint8_t>(data.id >> 8),
-                            static_cast<uint8_t>(data.id), data.channel,
-                            static_cast<uint8_t>((data.button & 0x0F) << 4 | (data.check & 0x0F))};
-  dst->item_bytes<BIT_TIMINGS>(bytes, sizeof(bytes));
+
+  for (uint32_t mask = 1UL << (23); mask != 0; mask >>= 1) {
+    if (data.id & mask) {
+      dst->item(BIT_ONE_HIGH_US, BIT_ONE_LOW_US);
+    } else {
+      dst->item(BIT_ZERO_HIGH_US, BIT_ZERO_LOW_US);
+    }
+  }
+
+  for (uint32_t mask = 1UL << (7); mask != 0; mask >>= 1) {
+    if (data.channel & mask) {
+      dst->item(BIT_ONE_HIGH_US, BIT_ONE_LOW_US);
+    } else {
+      dst->item(BIT_ZERO_HIGH_US, BIT_ZERO_LOW_US);
+    }
+  }
+
+  for (uint32_t mask = 1UL << (3); mask != 0; mask >>= 1) {
+    if (data.button & mask) {
+      dst->item(BIT_ONE_HIGH_US, BIT_ONE_LOW_US);
+    } else {
+      dst->item(BIT_ZERO_HIGH_US, BIT_ZERO_LOW_US);
+    }
+  }
+
+  for (uint32_t mask = 1UL << (3); mask != 0; mask >>= 1) {
+    if (data.check & mask) {
+      dst->item(BIT_ONE_HIGH_US, BIT_ONE_LOW_US);
+    } else {
+      dst->item(BIT_ZERO_HIGH_US, BIT_ZERO_LOW_US);
+    }
+  }
 }
 optional<DooyaData> DooyaProtocol::decode(RemoteReceiveData src) {
-  uint8_t bytes[5];
-  if (!src.expect_item(HEADER_HIGH_US, HEADER_LOW_US) || !src.expect_bytes_last_mark<BIT_TIMINGS>(bytes, sizeof(bytes)))
-    return {};
-  return DooyaData{
-      .id = encode_uint24(bytes[0], bytes[1], bytes[2]),
-      .channel = bytes[3],
-      .button = static_cast<uint8_t>(bytes[4] >> 4),
-      .check = static_cast<uint8_t>(bytes[4] & 0x0F),
+  DooyaData out{
+      .id = 0,
+      .channel = 0,
+      .button = 0,
+      .check = 0,
   };
+  if (!src.expect_item(HEADER_HIGH_US, HEADER_LOW_US))
+    return {};
+
+  for (uint8_t i = 0; i < 24; i++) {
+    if (src.expect_item(BIT_ONE_HIGH_US, BIT_ONE_LOW_US)) {
+      out.id = (out.id << 1) | 1;
+    } else if (src.expect_item(BIT_ZERO_HIGH_US, BIT_ZERO_LOW_US)) {
+      out.id = (out.id << 1) | 0;
+    } else {
+      return {};
+    }
+  }
+
+  for (uint8_t i = 0; i < 8; i++) {
+    if (src.expect_item(BIT_ONE_HIGH_US, BIT_ONE_LOW_US)) {
+      out.channel = (out.channel << 1) | 1;
+    } else if (src.expect_item(BIT_ZERO_HIGH_US, BIT_ZERO_LOW_US)) {
+      out.channel = (out.channel << 1) | 0;
+    } else {
+      return {};
+    }
+  }
+
+  for (uint8_t i = 0; i < 4; i++) {
+    if (src.expect_item(BIT_ONE_HIGH_US, BIT_ONE_LOW_US)) {
+      out.button = (out.button << 1) | 1;
+    } else if (src.expect_item(BIT_ZERO_HIGH_US, BIT_ZERO_LOW_US)) {
+      out.button = (out.button << 1) | 0;
+    } else {
+      return {};
+    }
+  }
+
+  for (uint8_t i = 0; i < 3; i++) {
+    if (src.expect_item(BIT_ONE_HIGH_US, BIT_ONE_LOW_US)) {
+      out.check = (out.check << 1) | 1;
+    } else if (src.expect_item(BIT_ZERO_HIGH_US, BIT_ZERO_LOW_US)) {
+      out.check = (out.check << 1) | 0;
+    } else {
+      return {};
+    }
+  }
+  // Last bit is not received properly but can be decoded
+  if (src.expect_mark(BIT_ONE_HIGH_US)) {
+    out.check = (out.check << 1) | 1;
+  } else if (src.expect_mark(BIT_ZERO_HIGH_US)) {
+    out.check = (out.check << 1) | 0;
+  } else {
+    return {};
+  }
+
+  return out;
 }
 void DooyaProtocol::dump(const DooyaData &data) {
   ESP_LOGI(TAG, "Received Dooya: id=0x%08" PRIX32 ", channel=%d, button=%d, check=%d", data.id, data.channel,

esphome/components/remote_base/magiquest_protocol.cpp

@@ -16,11 +16,6 @@ static constexpr uint32_t MAGIQUEST_ONE_MARK = 2 * MAGIQUEST_UNIT;
 static constexpr uint32_t MAGIQUEST_ONE_SPACE = 2 * MAGIQUEST_UNIT;
 static constexpr uint32_t MAGIQUEST_ZERO_MARK = MAGIQUEST_UNIT;
 static constexpr uint32_t MAGIQUEST_ZERO_SPACE = 3 * MAGIQUEST_UNIT;
-static constexpr BitTimings MAGIQUEST_BIT_TIMINGS{.one_space = MAGIQUEST_ONE_SPACE,
-                                                  .zero_space = MAGIQUEST_ZERO_SPACE,
-                                                  .bit_order = BitOrder::BIT_ORDER_MSB_FIRST,
-                                                  .one_mark = MAGIQUEST_ONE_MARK,
-                                                  .zero_mark = MAGIQUEST_ZERO_MARK};
 
 static bool checksum_is_valid(uint32_t wand_id, uint32_t magnitude_and_checksum) {
   // The frame's six payload bytes, which start at the wand id's top bit, must sum to zero.
@@ -40,10 +35,21 @@ void MagiQuestProtocol::encode(RemoteTransmitData *dst, const MagiQuestData &dat
   // 2 start bits
   dst->item(MAGIQUEST_ZERO_MARK, MAGIQUEST_ZERO_SPACE);
   dst->item(MAGIQUEST_ZERO_MARK, MAGIQUEST_ZERO_SPACE);
-  const uint8_t bytes[6] = {static_cast<uint8_t>(data.wand_id >> 24),  static_cast<uint8_t>(data.wand_id >> 16),
-                            static_cast<uint8_t>(data.wand_id >> 8),   static_cast<uint8_t>(data.wand_id),
-                            static_cast<uint8_t>(data.magnitude >> 8), static_cast<uint8_t>(data.magnitude)};
-  dst->item_bytes<MAGIQUEST_BIT_TIMINGS>(bytes, sizeof(bytes));
+  for (uint32_t mask = 1 << 31; mask; mask >>= 1) {
+    if (data.wand_id & mask) {
+      dst->item(MAGIQUEST_ONE_MARK, MAGIQUEST_ONE_SPACE);
+    } else {
+      dst->item(MAGIQUEST_ZERO_MARK, MAGIQUEST_ZERO_SPACE);
+    }
+  }
+
+  for (uint16_t mask = 1 << 15; mask; mask >>= 1) {
+    if (data.magnitude & mask) {
+      dst->item(MAGIQUEST_ONE_MARK, MAGIQUEST_ONE_SPACE);
+    } else {
+      dst->item(MAGIQUEST_ZERO_MARK, MAGIQUEST_ZERO_SPACE);
+    }
+  }
 
   dst->mark(MAGIQUEST_UNIT);
 }

esphome/components/remote_base/roomba_protocol.cpp

@@ -10,16 +10,18 @@ static constexpr uint32_t BIT_ONE_HIGH_US = 3000;
 static constexpr uint32_t BIT_ONE_LOW_US = 1000;
 static constexpr uint32_t BIT_ZERO_HIGH_US = BIT_ONE_LOW_US;
 static constexpr uint32_t BIT_ZERO_LOW_US = BIT_ONE_HIGH_US;
-static constexpr BitTimings BIT_TIMINGS{.one_space = BIT_ONE_LOW_US,
-                                        .zero_space = BIT_ZERO_LOW_US,
-                                        .bit_order = BitOrder::BIT_ORDER_MSB_FIRST,
-                                        .one_mark = BIT_ONE_HIGH_US,
-                                        .zero_mark = BIT_ZERO_HIGH_US};
 
 void RoombaProtocol::encode(RemoteTransmitData *dst, const RoombaData &data) {
   dst->set_carrier_frequency(38000);
   dst->reserve(NBITS * 2u);
-  dst->item_bytes<BIT_TIMINGS>(&data.data, 1);
+
+  for (uint32_t mask = 1UL << (NBITS - 1); mask != 0; mask >>= 1) {
+    if (data.data & mask) {
+      dst->item(BIT_ONE_HIGH_US, BIT_ONE_LOW_US);
+    } else {
+      dst->item(BIT_ZERO_HIGH_US, BIT_ZERO_LOW_US);
+    }
+  }
 }
 optional<RoombaData> RoombaProtocol::decode(RemoteReceiveData src) {
   RoombaData out{.data = 0};

esphome/components/seeed_multi_channel_relay/seeed_multi_channel_relay.cpp

@@ -25,7 +25,7 @@ void SeeedMultiChannelRelay::setup() {
     switch_::Switch *sw = this->switches_[channel];
     if (sw == nullptr)
       continue;
-    const bool on = sw->get_initial_state_with_restore_mode().value_or(sw->is_inverted()) != sw->is_inverted();
+    const bool on = sw->get_initial_write_state().value_or(false);
     assign_bits(value, 1 << channel, on);
   }
   if (!this->status_comm_result(this->write_byte(CMD_CHANNEL_CTRL, value)))

esphome/components/switch/switch.cpp

@@ -60,6 +60,11 @@ optional<bool> Switch::get_initial_state_with_restore_mode() {
   }
   return initial_state;
 }
+optional<bool> Switch::get_initial_write_state() {
+  if (auto initial = this->get_initial_state_with_restore_mode())
+    return *initial != this->inverted_;
+  return {};
+}
 void Switch::restore() {
   if (auto initial = this->get_initial_state_with_restore_mode())
     this->control(*initial);

esphome/components/switch/switch.h

@@ -114,6 +114,8 @@ class Switch : public EntityBase {
    * state.
    */
   optional<bool> get_initial_state_with_restore_mode();
+  /// get_initial_state_with_restore_mode() with inverted applied: the level write_state() takes.
+  optional<bool> get_initial_write_state();
 
   /// Drive the switch to the state its restore mode yields, through control(), so inversion and
   /// write_state() apply; a disabled restore mode drives \p when_disabled, or nothing without one.

tests/components/remote_base/__init__.py

@@ -18,8 +18,6 @@ def override_manifest(manifest: ComponentManifestOverride) -> None:
             "ira211",
             "nec",
             "magiquest",
-            "roomba",
-            "dooya",
             "rc5x",
             "pioneer_wyt",
             "drayton",

tests/components/remote_base/dooya_test.cpp

@@ -1,67 +0,0 @@
-#include <gtest/gtest.h>
-#include "esphome/components/remote_base/dooya_protocol.h"
-
-namespace esphome::remote_base::testing {
-
-namespace {
-
-constexpr DooyaData DATA{.id = 0x12A5C3, .channel = 0x4E, .button = 0x9, .check = 0x6};
-
-void add_bits(RawTimings &out, uint32_t value, uint8_t count) {
-  for (uint32_t mask = 1UL << (count - 1); mask != 0; mask >>= 1) {
-    const bool one = value & mask;
-    out.push_back(one ? 750 : 350);
-    out.push_back(one ? -350 : -750);
-  }
-}
-
-// The frame as the encoder wrote it before it used item_bytes
-RawTimings reference_frame(const DooyaData &data) {
-  RawTimings out = {5000, -1500};
-  add_bits(out, data.id, 24);
-  add_bits(out, data.channel, 8);
-  add_bits(out, data.button, 4);
-  add_bits(out, data.check, 4);
-  return out;
-}
-
-optional<DooyaData> decode(const RawTimings &timings) {
-  return DooyaProtocol().decode(RemoteReceiveData(timings, 25, TOLERANCE_MODE_PERCENTAGE));
-}
-
-}  // namespace
-
-TEST(DooyaProtocolTest, EncodesTheSameFrame) {
-  RemoteTransmitData tx;
-  DooyaProtocol().encode(&tx, DATA);
-  EXPECT_EQ(tx.get_data(), reference_frame(DATA));
-}
-
-TEST(DooyaProtocolTest, DecodesAFrameWithoutItsLastSpace) {
-  for (const uint8_t check : {0x0, 0x1, 0x6, 0xF}) {
-    DooyaData data = DATA;
-    data.check = check;
-    RawTimings timings = reference_frame(data);
-    timings.pop_back();
-    auto decoded = decode(timings);
-    ASSERT_TRUE(decoded.has_value()) << int(check);
-    if (!decoded.has_value()) {
-      return;
-    }
-    EXPECT_EQ(decoded->id, data.id);
-    EXPECT_EQ(decoded->channel, data.channel);
-    EXPECT_EQ(decoded->button, data.button);
-    EXPECT_EQ(decoded->check, data.check);
-  }
-}
-
-TEST(DooyaProtocolTest, RejectsABadMark) {
-  for (const size_t index : {size_t{10}, size_t{80}}) {  // a payload bit, then the last mark
-    RawTimings timings = reference_frame(DATA);
-    timings.pop_back();
-    timings[index] = 550;
-    EXPECT_FALSE(decode(timings).has_value()) << index;
-  }
-}
-
-}  // namespace esphome::remote_base::testing

tests/components/remote_base/magiquest_test.cpp

@@ -64,23 +64,6 @@ void expect_decodes_to(const RawTimings &timings, uint32_t wand_id, uint16_t mag
 
 }  // namespace
 
-TEST(MagiQuestProtocolTest, EncodesTheSameFrame) {
-  // The frame as the encoder wrote it before it used item_bytes: two zero start bits, the 32 bit wand id and
-  // the 16 bit magnitude, then a one unit stop mark
-  RawTimings expected;
-  add_bit(expected, false);
-  add_bit(expected, false);
-  for (uint32_t mask = 1UL << 31; mask != 0; mask >>= 1)
-    add_bit(expected, WAND_ID & mask);
-  for (uint32_t mask = 1UL << 15; mask != 0; mask >>= 1)
-    add_bit(expected, MAGNITUDE & mask);
-  expected.push_back(UNIT);
-
-  RemoteTransmitData tx;
-  MagiQuestProtocol().encode(&tx, DECODED);
-  EXPECT_EQ(tx.get_data(), expected);
-}
-
 TEST(MagiQuestProtocolTest, DecodesAValidFrame) {
   expect_decodes_to(build_frame(WAND_ID, MAGNITUDE), WAND_ID, MAGNITUDE);
 }

tests/components/remote_base/roomba_test.cpp

@@ -1,51 +0,0 @@
-#include <gtest/gtest.h>
-#include "esphome/components/remote_base/roomba_protocol.h"
-
-namespace esphome::remote_base::testing {
-
-namespace {
-
-// The frame as the encoder wrote it before it used item_bytes: eight bits, most significant first
-RawTimings reference_frame(uint8_t value) {
-  RawTimings out;
-  for (uint8_t mask = 0x80; mask != 0; mask >>= 1) {
-    const bool one = value & mask;
-    out.push_back(one ? 3000 : 1000);
-    out.push_back(one ? -1000 : -3000);
-  }
-  return out;
-}
-
-optional<RoombaData> decode(const RawTimings &timings) {
-  return RoombaProtocol().decode(RemoteReceiveData(timings, 25, TOLERANCE_MODE_PERCENTAGE));
-}
-
-}  // namespace
-
-TEST(RoombaProtocolTest, EncodesTheSameFrame) {
-  RemoteTransmitData tx;
-  RoombaProtocol().encode(&tx, RoombaData{.data = 0xA5});
-  EXPECT_EQ(tx.get_data(), reference_frame(0xA5));
-}
-
-TEST(RoombaProtocolTest, DecodesAFrameWithoutItsLastSpace) {
-  for (const uint8_t value : {0x00, 0x01, 0x88, 0xA5, 0xFF}) {
-    RawTimings timings = reference_frame(value);
-    timings.pop_back();
-    auto decoded = decode(timings);
-    ASSERT_TRUE(decoded.has_value()) << int(value);
-    if (!decoded.has_value()) {
-      return;
-    }
-    EXPECT_EQ(decoded->data, value);
-  }
-}
-
-TEST(RoombaProtocolTest, RejectsABadMark) {
-  RawTimings timings = reference_frame(0xA5);
-  timings.pop_back();
-  timings[4] = 2000;
-  EXPECT_FALSE(decode(timings).has_value());
-}
-
-}  // namespace esphome::remote_base::testing

tests/components/switch/restore_test.cpp

@@ -46,4 +46,16 @@ TEST(SwitchRestore, DefaultAppliesOnlyWhenTheModeIsDisabled) {
   EXPECT_EQ(on.writes, std::vector<bool>{true});
 }
 
+TEST(SwitchRestore, InitialWriteStateAppliesInversion) {
+  RecordingSwitch on, inverted, disabled;
+  on.set_restore_mode(SWITCH_ALWAYS_ON);
+  inverted.set_restore_mode(SWITCH_ALWAYS_ON);
+  inverted.set_inverted(true);
+  disabled.set_restore_mode(SWITCH_RESTORE_DISABLED);
+  disabled.set_inverted(true);
+  EXPECT_EQ(on.get_initial_write_state(), optional<bool>(true));
+  EXPECT_EQ(inverted.get_initial_write_state(), optional<bool>(false));
+  EXPECT_FALSE(disabled.get_initial_write_state().has_value());
+}
+
 }  // namespace esphome::switch_::testing