Merge remote-tracking branch 'origin/assign-bits-helper' into assign-bits-migrate-gpio

# Conflicts:
#	esphome/components/pca9554/pca9554.cpp

AGENTS.md

@@ -211,8 +211,8 @@ file does, and it is the authority when they disagree. The most useful starting
         logged with `get_log_str` and `LOG_STR_ARG`, copied out with `ESPHOME_strncpy_P` before `publish_state`.
         A `dump_config` helper that picks a name returns `const LogString *` from `LOG_STR(...)`.
     *   **CRCs and bit tricks** come from `esphome/core/helpers.h` (`crc8`, `crc16`, `xor_bytes`, `sum16_bytes`,
-        `reverse_bits`, `encode_uint16`, `encode_value_le`, `sign_extend<BITS>`, `bcd_to_uint8`); pick the helper's
-        arguments instead of writing a loop. Epson style 7-byte RTC calendars (one-hot weekday) use
+        `reverse_bits`, `encode_uint16`, `encode_value_le`, `sign_extend<BITS>`, `bcd_to_uint8`, `assign_bits`); pick
+        the helper's arguments instead of writing a loop. Epson style 7-byte RTC calendars (one-hot weekday) use
         `time::decode_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`

esphome/components/ch422g/ch422g.cpp

@@ -1,4 +1,5 @@
 #include "ch422g.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
 namespace esphome::ch422g {
@@ -59,11 +60,7 @@ bool CH422GComponent::digital_read(uint8_t pin) {
 }
 
 void CH422GComponent::digital_write(uint8_t pin, bool value) {
-  if (value) {
-    this->output_bits_ |= (1 << pin);
-  } else {
-    this->output_bits_ &= ~(1 << pin);
-  }
+  assign_bits(this->output_bits_, 1 << pin, value);
   this->write_outputs_();
 }
 

esphome/components/ch423/ch423.cpp

@@ -1,4 +1,5 @@
 #include "ch423.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 #include "esphome/core/progmem.h"
 
@@ -62,11 +63,7 @@ bool CH423Component::digital_read(uint8_t pin) {
 }
 
 void CH423Component::digital_write(uint8_t pin, bool value) {
-  if (value) {
-    this->output_bits_ |= (1 << pin);
-  } else {
-    this->output_bits_ &= ~(1 << pin);
-  }
+  assign_bits(this->output_bits_, 1 << pin, value);
   this->write_outputs_();
 }
 

esphome/components/mcp23016/mcp23016.cpp

@@ -1,4 +1,5 @@
 #include "mcp23016.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 #include <cstdio>
 
@@ -79,11 +80,7 @@ void MCP23016::update_reg_(uint8_t pin, bool pin_value, uint8_t reg_addr) {
     this->read_reg_(reg_addr, &reg_value);
   }
 
-  if (pin_value) {
-    reg_value |= 1 << pin;
-  } else {
-    reg_value &= ~(1 << pin);
-  }
+  assign_bits(reg_value, 1 << pin, pin_value);
 
   this->write_reg_(reg_addr, reg_value);
 

esphome/components/mcp23x08_base/mcp23x08_base.cpp

@@ -1,4 +1,5 @@
 #include "mcp23x08_base.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
 namespace esphome::mcp23x08_base {
@@ -78,11 +79,7 @@ void MCP23X08Base::update_reg(uint8_t pin, bool pin_value, uint8_t reg_addr) {
     this->read_reg(reg_addr, &reg_value);
   }
 
-  if (pin_value) {
-    reg_value |= 1 << bit;
-  } else {
-    reg_value &= ~(1 << bit);
-  }
+  assign_bits(reg_value, 1 << bit, pin_value);
 
   this->write_reg(reg_addr, reg_value);
 

esphome/components/mcp23x17_base/mcp23x17_base.cpp

@@ -92,11 +92,7 @@ void MCP23X17Base::update_reg(uint8_t pin, bool pin_value, uint8_t reg_addr) {
     this->read_reg(reg_addr, &reg_value);
   }
 
-  if (pin_value) {
-    reg_value |= 1 << bit;
-  } else {
-    reg_value &= ~(1 << bit);
-  }
+  assign_bits(reg_value, 1 << bit, pin_value);
 
   this->write_reg(reg_addr, reg_value);
 

esphome/components/pca6416a/pca6416a.cpp

@@ -1,4 +1,5 @@
 #include "pca6416a.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
 namespace esphome::pca6416a {
@@ -170,11 +171,7 @@ void PCA6416AComponent::update_register_(uint8_t pin, bool pin_value, uint8_t re
     this->read_register_(reg_addr, &reg_value);
   }
 
-  if (pin_value) {
-    reg_value |= 1 << bit;
-  } else {
-    reg_value &= ~(1 << bit);
-  }
+  assign_bits(reg_value, 1 << bit, pin_value);
 
   this->write_register_(reg_addr, reg_value);
 

esphome/components/pca9505/pca9505.cpp

@@ -1,4 +1,5 @@
 #include "pca9505.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
 namespace esphome::pca9505 {
@@ -74,11 +75,7 @@ bool PCA9505Component::digital_read_cache(uint8_t pin) { return this->input_[pin
 void PCA9505Component::digital_write_hw(uint8_t pin, bool value) {
   const uint8_t bank = pin / 8;
   const uint8_t mask = 1 << (pin % 8);
-  if (value) {
-    this->output_[bank] |= mask;
-  } else {
-    this->output_[bank] &= ~mask;
-  }
+  assign_bits(this->output_[bank], mask, value);
   this->write_bank_(OUTPUT_REG, bank, this->output_[bank]);
 }
 

esphome/components/pca9554/pca9554.cpp

@@ -1,4 +1,5 @@
 #include "pca9554.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
 namespace esphome::pca9554 {
@@ -99,11 +100,7 @@ bool PCA9554Component::digital_read_cache(uint8_t pin) {
 }
 
 void PCA9554Component::digital_write_hw(uint8_t pin, bool value) {
-  if (value) {
-    this->output_mask_ |= (1 << pin);
-  } else {
-    this->output_mask_ &= ~(1 << pin);
-  }
+  assign_bits(this->output_mask_, 1 << pin, value);
 #ifdef USE_PCA9554_PCAL
   if (this->write_register_(OUTPUT_REG, this->output_mask_))
     this->retry_pcal_shadows_();  // an output-only hub never reads, so it retries here

esphome/components/pcf8574/pcf8574.cpp

@@ -60,11 +60,7 @@ bool PCF8574Component::digital_read_hw(uint8_t pin) {
 bool PCF8574Component::digital_read_cache(uint8_t pin) { return this->input_mask_ & (1 << pin); }
 
 void PCF8574Component::digital_write_hw(uint8_t pin, bool value) {
-  if (value) {
-    this->output_mask_ |= (1 << pin);
-  } else {
-    this->output_mask_ &= ~(1 << pin);
-  }
+  assign_bits(this->output_mask_, 1 << pin, value);
   this->write_gpio_();
 }
 void PCF8574Component::pin_mode(uint8_t pin, gpio::Flags flags) {

esphome/components/pi4ioe5v6408/pi4ioe5v6408.cpp

@@ -1,4 +1,5 @@
 #include "pi4ioe5v6408.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
 namespace esphome::pi4ioe5v6408 {
@@ -129,11 +130,7 @@ void PI4IOE5V6408Component::digital_write_hw(uint8_t pin, bool value) {
   if (this->is_failed())
     return;
 
-  if (value) {
-    this->output_mask_ |= (1 << pin);
-  } else {
-    this->output_mask_ &= ~(1 << pin);
-  }
+  assign_bits(this->output_mask_, 1 << pin, value);
   if (!this->status_comm_result(this->write_byte(PI4IOE5V6408_REGISTER_OUT_SET, this->output_mask_),
                                 LOG_STR("Failed to write output register")))
     return;

esphome/components/sx1509/sx1509.cpp

@@ -92,20 +92,9 @@ void SX1509Component::digital_write_hw(uint8_t pin, bool bit_value) {
     // If the pin is an output, write high/low
     uint16_t temp_reg_data = 0;
     this->read_byte_16(REG_DATA_B, &temp_reg_data);
-    if (bit_value) {
-      output_state_ |= (1 << pin);  // set bit in shadow register
-    } else {
-      output_state_ &= ~(1 << pin);  // reset bit shadow register
-    }
-    for (uint16_t b = 0x8000; b; b >>= 1) {
-      if ((~ddr_mask_) & b) {  // transfer bits of outputs, but don't mess with inputs
-        if (output_state_ & b) {
-          temp_reg_data |= b;
-        } else {
-          temp_reg_data &= ~b;
-        }
-      }
-    }
+    assign_bits(this->output_state_, 1 << pin, bit_value);
+    // Outputs take the shadow state, inputs keep what was read
+    temp_reg_data = (temp_reg_data & this->ddr_mask_) | (this->output_state_ & ~this->ddr_mask_);
     this->write_byte_16(REG_DATA_B, temp_reg_data);
   }
 }
@@ -148,20 +137,12 @@ void SX1509Component::pin_mode(uint8_t pin, gpio::Flags flags) {
 
     // Pullup
     this->read_byte_16(REG_PULL_UP_B, &temp_word);
-    if (flags & gpio::FLAG_PULLUP) {
-      temp_word |= (1 << pin);
-    } else {
-      temp_word &= ~(1 << pin);
-    }
+    assign_bits(temp_word, 1 << pin, flags & gpio::FLAG_PULLUP);
     this->write_byte_16(REG_PULL_UP_B, temp_word);
 
     // Pulldown
     this->read_byte_16(REG_PULL_DOWN_B, &temp_word);
-    if (flags & gpio::FLAG_PULLDOWN) {
-      temp_word |= (1 << pin);
-    } else {
-      temp_word &= ~(1 << pin);
-    }
+    assign_bits(temp_word, 1 << pin, flags & gpio::FLAG_PULLDOWN);
     this->write_byte_16(REG_PULL_DOWN_B, temp_word);
 
     // Set direction to input

esphome/components/tca9555/tca9555.cpp

@@ -1,4 +1,5 @@
 #include "tca9555.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
 static const uint8_t TCA9555_INPUT_PORT_REGISTER_0 = 0x00;
@@ -108,11 +109,7 @@ void TCA9555Component::digital_write_hw(uint8_t pin, bool value) {
   if (this->is_failed())
     return;
 
-  if (value) {
-    this->output_mask_ |= (1 << pin);
-  } else {
-    this->output_mask_ &= ~(1 << pin);
-  }
+  assign_bits(this->output_mask_, 1 << pin, value);
 
   this->status_comm_result(this->write_byte_16_le(TCA9555_OUTPUT_PORT_REGISTER_0, this->output_mask_),
                            LOG_STR("Failed to write output register"));

esphome/components/waveshare_io_ch32v003/waveshare_io_ch32v003.cpp

@@ -1,4 +1,5 @@
 #include "waveshare_io_ch32v003.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
 namespace esphome::waveshare_io_ch32v003 {
@@ -110,11 +111,7 @@ void WaveshareIOCH32V003Component::digital_write_hw(uint8_t pin, bool value) {
   if (this->is_failed())
     return;
 
-  if (value) {
-    this->output_mask_ |= (1 << pin);
-  } else {
-    this->output_mask_ &= ~(1 << pin);
-  }
+  assign_bits(this->output_mask_, 1 << pin, value);
 
   uint8_t data = this->output_mask_;
   this->status_comm_result(this->write_byte(IO_EXTENSION_IO_OUTPUT_ADDR, data),

esphome/components/weikai/weikai.cpp

@@ -210,11 +210,7 @@ bool WeikaiComponent::read_pin_val_(uint8_t pin) {
 }
 
 void WeikaiComponent::write_pin_val_(uint8_t pin, bool value) {
-  if (value) {
-    this->output_state_ |= (1 << pin);
-  } else {
-    this->output_state_ &= ~(1 << pin);
-  }
+  assign_bits(this->output_state_, 1 << pin, value);
   char bin_buf[9];
   ESP_LOGVV(TAG, "writing output pin %d with %d out_state %s", pin, uint8_t(value),
             format_bin_to(bin_buf, this->output_state_));

esphome/core/helpers.h

@@ -1067,6 +1067,16 @@ constexpr uint8_t bcd_to_uint8(uint8_t bcd) { return (bcd >> 4) * 10 + (bcd & 0x
 /// Encode a value from 0 to 99 as a two-digit packed BCD byte, e.g. 59 to 0x59.
 constexpr uint8_t uint8_to_bcd(uint8_t value) { return ((value / 10) << 4) | (value % 10); }
 
+/// Set the bits of \p mask in \p value when \p on is true, clear them otherwise.
+/// Use an unsigned mask for a register wider than int; a negative int mask is sign-extended.
+template<typename T> ESPHOME_ALWAYS_INLINE constexpr void assign_bits(T &value, std::type_identity_t<T> mask, bool on) {
+  if (on) {
+    value |= mask;
+  } else {
+    value &= ~mask;
+  }
+}
+
 /// Reverse the order of 8 bits.
 inline uint8_t reverse_bits(uint8_t x) {
   x = ((x & 0xAA) >> 1) | ((x & 0x55) << 1);

tests/components/core/test_assign_bits.cpp

@@ -0,0 +1,30 @@
+#include <gtest/gtest.h>
+
+#include "esphome/core/helpers.h"
+
+namespace esphome::core::testing {
+
+static_assert([] {
+  uint8_t v = 0;
+  assign_bits(v, 1 << 3, true);
+  return v;
+}() == 0x08);
+
+TEST(AssignBits, SetsAndClearsOnlyTheMask) {
+  uint8_t reg = 0b1010'0001;
+  assign_bits(reg, 0b0000'0110, true);
+  EXPECT_EQ(reg, 0b1010'0111);
+  assign_bits(reg, 0b1000'0010, false);
+  EXPECT_EQ(reg, 0b0010'0101);
+}
+
+TEST(AssignBits, ClearsAtTheTargetWidth) {
+  uint16_t reg16 = 0xFFFF;
+  assign_bits(reg16, 1 << 15, false);
+  EXPECT_EQ(reg16, 0x7FFF);
+  uint64_t reg64 = UINT64_MAX;
+  assign_bits(reg64, uint32_t{1} << 31, false);
+  EXPECT_EQ(reg64, UINT64_MAX & ~(uint64_t{1} << 31));
+}
+
+}  // namespace esphome::core::testing