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

# Conflicts:
#	esphome/components/light/light_call.h

AGENTS.md

@@ -212,9 +212,9 @@ 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. 7-byte BCD RTC calendars use `time::decode_bcd_calendar<LAYOUT>` and
-        `encode_bcd_calendar<LAYOUT>` with `EPSON_BCD_CALENDAR`, `DS_BCD_CALENDAR` or `PCF_BCD_CALENDAR`.
+        `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. 7-byte BCD RTC calendars use `time::decode_bcd_calendar<LAYOUT>`
+        and `encode_bcd_calendar<LAYOUT>` with `EPSON_BCD_CALENDAR`, `DS_BCD_CALENDAR` or `PCF_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`
         and `read_bytes_16_le`. `if (!this->status_comm_result(ok)) return;` sets or clears the warning.

esphome/components/ds248x/ds248x_one_wire_bus.cpp

@@ -1,5 +1,6 @@
 #include "ds248x_one_wire_bus.h"
 #include "ds248x.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
 namespace esphome::ds248x {
@@ -151,11 +152,7 @@ uint64_t DS248xOneWireBus::search_int() {
     }
 
     // Update address based on direction taken
-    if (dir_taken) {
-      address |= bit_mask;
-    } else {
-      address &= ~bit_mask;
-    }
+    assign_bits(address, bit_mask, dir_taken);
   }
 
   // Search successful

esphome/components/htu21d/htu21d.cpp

@@ -1,6 +1,7 @@
 #include "htu21d.h"
-#include "esphome/core/log.h"
 #include "esphome/core/hal.h"
+#include "esphome/core/helpers.h"
+#include "esphome/core/log.h"
 
 namespace esphome::htu21d {
 
@@ -112,11 +113,7 @@ void HTU21DComponent::set_heater(bool status) {
     this->status_set_warning();
     return;
   }
-  if (status) {
-    raw_heater |= (1 << HTU21D_REG_HTRE_BIT);
-  } else {
-    raw_heater &= ~(1 << HTU21D_REG_HTRE_BIT);
-  }
+  assign_bits(raw_heater, 1 << HTU21D_REG_HTRE_BIT, status);
   if (this->write_register(HTU21D_WRITERHT_REG_CMD, &raw_heater, 1) != i2c::ERROR_OK) {
     this->status_set_warning();
     return;

esphome/components/light/light_call.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "light_color_values.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/string_ref.h"
 
 namespace esphome {
@@ -229,13 +230,7 @@ class LightCall {
   inline bool get_save_() { return (this->flags_ & FLAG_SAVE) != 0; }
 
   // Helper to set flag - defaults to true for common case
-  void set_flag_(FieldFlags flag, bool value = true) ESPHOME_ALWAYS_INLINE {
-    if (value) {
-      this->flags_ |= flag;
-    } else {
-      this->flags_ &= ~flag;
-    }
-  }
+  void set_flag_(FieldFlags flag, bool value = true) ESPHOME_ALWAYS_INLINE { assign_bits(this->flags_, flag, value); }
 
   // Helper to clear flag - reduces code size for common case
   void clear_flag_(FieldFlags flag) ESPHOME_ALWAYS_INLINE { this->flags_ &= ~flag; }

esphome/components/media_player/media_player.cpp

@@ -1,6 +1,7 @@
 #include "media_player.h"
-#include "esphome/core/defines.h"
 #include "esphome/core/controller_registry.h"
+#include "esphome/core/defines.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 #include "esphome/core/progmem.h"
 
@@ -77,19 +78,12 @@ const char *media_player_command_to_string(MediaPlayerCommand command) {
 }
 
 void MediaPlayerTraits::set_supports_pause(bool supports_pause) {
-  if (supports_pause) {
-    this->feature_flags_ |= MediaPlayerEntityFeature::PAUSE | MediaPlayerEntityFeature::PLAY;
-  } else {
-    this->feature_flags_ &= ~(MediaPlayerEntityFeature::PAUSE | MediaPlayerEntityFeature::PLAY);
-  }
+  assign_bits(this->feature_flags_, MediaPlayerEntityFeature::PAUSE | MediaPlayerEntityFeature::PLAY, supports_pause);
 }
 
 void MediaPlayerTraits::set_supports_turn_off_on(bool supports_turn_off_on) {
-  if (supports_turn_off_on) {
-    this->feature_flags_ |= MediaPlayerEntityFeature::TURN_OFF | MediaPlayerEntityFeature::TURN_ON;
-  } else {
-    this->feature_flags_ &= ~(MediaPlayerEntityFeature::TURN_OFF | MediaPlayerEntityFeature::TURN_ON);
-  }
+  assign_bits(this->feature_flags_, MediaPlayerEntityFeature::TURN_OFF | MediaPlayerEntityFeature::TURN_ON,
+              supports_turn_off_on);
 }
 
 void MediaPlayerCall::validate_() {

esphome/components/mpr121/mpr121.cpp

@@ -3,6 +3,7 @@
 #include <cstdint>
 
 #include "esphome/core/hal.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
 namespace esphome::mpr121 {
@@ -84,11 +85,7 @@ bool MPR121Component::digital_read(uint8_t pin) { return (this->gpio_input_ & (1
 
 void MPR121Component::digital_write(uint8_t pin, bool value) {
   const uint8_t ionum = pin - 4;
-  if (value) {
-    this->gpio_output_ |= (1 << ionum);
-  } else {
-    this->gpio_output_ &= ~(1 << ionum);
-  }
+  assign_bits(this->gpio_output_, 1 << ionum, value);
   this->flush_gpio_();
 }
 

esphome/components/provisioning/provisioning.h

@@ -46,11 +46,7 @@ class ProvisioningManager : public Component {
   void set_source_provisioned(uint8_t source, bool provisioned) {
     if (source >= MAX_SOURCES)
       return;
-    if (provisioned) {
-      this->provisioned_mask_ |= (1UL << source);
-    } else {
-      this->provisioned_mask_ &= ~(1UL << source);
-    }
+    assign_bits(this->provisioned_mask_, 1UL << source, provisioned);
   }
 
   // True once every registered source is provisioned. Config validation guarantees

esphome/components/rx8111/rx8111.cpp

@@ -1,4 +1,5 @@
 #include "rx8111.h"
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
 namespace esphome::rx8111 {
@@ -106,11 +107,7 @@ bool RX8111Component::set_stop_bit_(bool stop) {
   uint8_t control;
   if (!this->read_byte(RX8111_REG_CONTROL, &control))
     return false;
-  if (stop) {
-    control |= RX8111_CTRL_STOP;
-  } else {
-    control &= ~RX8111_CTRL_STOP;
-  }
+  assign_bits(control, RX8111_CTRL_STOP, stop);
   return this->write_byte(RX8111_REG_CONTROL, control);
 }
 

esphome/components/socket/bsd_sockets_impl.cpp

@@ -62,11 +62,7 @@ int BSDSocketImpl::setblocking(bool blocking) {
   if (fl < 0) {
     return fl;
   }
-  if (blocking) {
-    fl &= ~O_NONBLOCK;
-  } else {
-    fl |= O_NONBLOCK;
-  }
+  assign_bits(fl, O_NONBLOCK, !blocking);
   return ::fcntl(this->fd_, F_SETFL, fl);
 }
 

esphome/components/socket/lwip_sockets_impl.cpp

@@ -52,11 +52,7 @@ int LwIPSocketImpl::setblocking(bool blocking) {
   if (fl < 0) {
     return fl;
   }
-  if (blocking) {
-    fl &= ~O_NONBLOCK;
-  } else {
-    fl |= O_NONBLOCK;
-  }
+  assign_bits(fl, O_NONBLOCK, !blocking);
   return lwip_fcntl(this->fd_, F_SETFL, fl);
 }
 

esphome/components/uart/uart_component_host.cpp

@@ -162,11 +162,7 @@ void HostUartComponent::setup() {
       break;
   };
   // Set stop bits
-  if (this->stop_bits_ == 2) {
-    options.c_cflag |= CSTOPB;
-  } else {
-    options.c_cflag &= ~CSTOPB;
-  }
+  assign_bits(options.c_cflag, CSTOPB, this->stop_bits_ == 2);
   cfsetispeed(&options, baud);
   cfsetospeed(&options, baud);
   tcsetattr(this->file_descriptor_, TCSANOW, &options);

esphome/components/ufire_ec/ufire_ec.cpp

@@ -1,3 +1,4 @@
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 #include "ufire_ec.h"
 #include <cmath>
@@ -19,11 +20,8 @@ void UFireECComponent::setup() {
   // Write option for temperature adjustments
   uint8_t config;
   this->read_byte(REGISTER_CONFIG, &config);
-  if (this->temperature_sensor_ == nullptr && this->temperature_sensor_external_ == nullptr) {
-    config &= ~CONFIG_TEMP_COMPENSATION;
-  } else {
-    config |= CONFIG_TEMP_COMPENSATION;
-  }
+  assign_bits(config, CONFIG_TEMP_COMPENSATION,
+              this->temperature_sensor_ != nullptr || this->temperature_sensor_external_ != nullptr);
   this->write_byte(REGISTER_CONFIG, config);
 
   // Update temperature compensation

esphome/components/ufire_ise/ufire_ise.cpp

@@ -1,3 +1,4 @@
+#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 #include "ufire_ise.h"
 
@@ -20,11 +21,8 @@ void UFireISEComponent::setup() {
   // Write option for temperature adjustments
   uint8_t config;
   this->read_byte(REGISTER_CONFIG, &config);
-  if (this->temperature_sensor_ == nullptr && this->temperature_sensor_external_ == nullptr) {
-    config &= ~CONFIG_TEMP_COMPENSATION;
-  } else {
-    config |= CONFIG_TEMP_COMPENSATION;
-  }
+  assign_bits(config, CONFIG_TEMP_COMPENSATION,
+              this->temperature_sensor_ != nullptr || this->temperature_sensor_external_ != nullptr);
   this->write_byte(REGISTER_CONFIG, config);
 }
 

esphome/components/water_heater/water_heater.cpp

@@ -5,6 +5,7 @@
 #include "esphome/core/application.h"
 #include "esphome/core/controller_registry.h"
 #include "esphome/core/progmem.h"
+#include "esphome/core/helpers.h"
 
 #include <cmath>
 #include <cstring>
@@ -59,21 +60,13 @@ WaterHeaterCall &WaterHeaterCall::set_target_temperature_high(float temperature)
 }
 
 WaterHeaterCall &WaterHeaterCall::set_away(bool away) {
-  if (away) {
-    this->state_ |= WATER_HEATER_STATE_AWAY;
-  } else {
-    this->state_ &= ~WATER_HEATER_STATE_AWAY;
-  }
+  assign_bits(this->state_, WATER_HEATER_STATE_AWAY, away);
   this->state_mask_ |= WATER_HEATER_STATE_AWAY;
   return *this;
 }
 
 WaterHeaterCall &WaterHeaterCall::set_on(bool on) {
-  if (on) {
-    this->state_ |= WATER_HEATER_STATE_ON;
-  } else {
-    this->state_ &= ~WATER_HEATER_STATE_ON;
-  }
+  assign_bits(this->state_, WATER_HEATER_STATE_ON, on);
   this->state_mask_ |= WATER_HEATER_STATE_ON;
   return *this;
 }

esphome/components/water_heater/water_heater.h

@@ -252,13 +252,7 @@ class WaterHeater : public EntityBase {
   /// Set the state flags. Should only be called from control().
   void set_state_(uint32_t state) { this->state_ = state; }
   /// Set or clear a state flag. Should only be called from control().
-  void set_state_flag_(uint32_t flag, bool value) {
-    if (value) {
-      this->state_ |= flag;
-    } else {
-      this->state_ &= ~flag;
-    }
-  }
+  void set_state_flag_(uint32_t flag, bool value) { assign_bits(this->state_, flag, value); }
 
   WaterHeaterMode mode_{WATER_HEATER_MODE_OFF};
   float current_temperature_{NAN};

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