Merge remote-tracking branch 'upstream/fix-loop-ambiguous-multiple-inheritance' into integration

esphome/components/mcp23016/mcp23016.cpp

@@ -8,90 +8,71 @@ namespace mcp23016 {
 static const char *const TAG = "mcp23016";
 
 void MCP23016::setup() {
-  uint8_t iocon;
-  if (!this->read_reg_(MCP23016_IOCON0, &iocon)) {
+  uint16_t iocon;
+  // MCP23016 registers operate as paired 16-bit registers. Addressing the
+  // odd register (e.g. IOCON1) reads/writes that register first, then wraps
+  // to the even register (IOCON0) in the same pair. Starting from the odd
+  // address gives the correct byte order for 1 << pin mapping:
+  // high byte = port 1 (pins 8-15), low byte = port 0 (pins 0-7).
+  if (!this->read_reg_(MCP23016_IOCON1, &iocon)) {
     this->mark_failed();
     return;
   }
 
   // Read current output register state
-  this->read_reg_(MCP23016_OLAT0, &this->olat_0_);
-  this->read_reg_(MCP23016_OLAT1, &this->olat_1_);
+  this->read_reg_(MCP23016_OLAT1, &this->olat_);
 
   // all pins input
-  this->write_reg_(MCP23016_IODIR0, 0xFF);
-  this->write_reg_(MCP23016_IODIR1, 0xFF);
+  this->write_reg_(MCP23016_IODIR1, 0xFFFF);
 }
 
 void MCP23016::loop() {
   // Invalidate cache at the start of each loop
   this->reset_pin_cache_();
 }
-bool MCP23016::digital_read_hw(uint8_t pin) {
-  uint8_t reg_addr = pin < 8 ? MCP23016_GP0 : MCP23016_GP1;
-  uint8_t value = 0;
-  if (!this->read_reg_(reg_addr, &value)) {
-    return false;
-  }
-
-  // Update the appropriate part of input_mask_
-  if (pin < 8) {
-    this->input_mask_ = (this->input_mask_ & 0xFF00) | value;
-  } else {
-    this->input_mask_ = (this->input_mask_ & 0x00FF) | (uint16_t(value) << 8);
-  }
-  return true;
-}
+bool MCP23016::digital_read_hw(uint8_t pin) { return this->read_reg_(MCP23016_GP1, &this->input_mask_); }
 
 bool MCP23016::digital_read_cache(uint8_t pin) { return this->input_mask_ & (1 << pin); }
-void MCP23016::digital_write_hw(uint8_t pin, bool value) {
-  uint8_t reg_addr = pin < 8 ? MCP23016_OLAT0 : MCP23016_OLAT1;
-  this->update_reg_(pin, value, reg_addr);
-}
+void MCP23016::digital_write_hw(uint8_t pin, bool value) { this->update_reg_(pin, value, MCP23016_OLAT1); }
 void MCP23016::pin_mode(uint8_t pin, gpio::Flags flags) {
-  uint8_t iodir = pin < 8 ? MCP23016_IODIR0 : MCP23016_IODIR1;
   if (flags == gpio::FLAG_INPUT) {
-    this->update_reg_(pin, true, iodir);
+    this->update_reg_(pin, true, MCP23016_IODIR1);
   } else if (flags == gpio::FLAG_OUTPUT) {
-    this->update_reg_(pin, false, iodir);
+    this->update_reg_(pin, false, MCP23016_IODIR1);
   }
 }
 float MCP23016::get_setup_priority() const { return setup_priority::IO; }
-bool MCP23016::read_reg_(uint8_t reg, uint8_t *value) {
+bool MCP23016::read_reg_(uint8_t reg, uint16_t *value) {
   if (this->is_failed())
     return false;
 
-  return this->read_byte(reg, value);
+  return this->read_byte_16(reg, value);
 }
-bool MCP23016::write_reg_(uint8_t reg, uint8_t value) {
+bool MCP23016::write_reg_(uint8_t reg, uint16_t value) {
   if (this->is_failed())
     return false;
 
-  return this->write_byte(reg, value);
+  return this->write_byte_16(reg, value);
 }
 void MCP23016::update_reg_(uint8_t pin, bool pin_value, uint8_t reg_addr) {
-  uint8_t bit = pin % 8;
-  uint8_t reg_value = 0;
-  if (reg_addr == MCP23016_OLAT0) {
-    reg_value = this->olat_0_;
-  } else if (reg_addr == MCP23016_OLAT1) {
-    reg_value = this->olat_1_;
+  uint16_t reg_value = 0;
+
+  if (reg_addr == MCP23016_OLAT1) {
+    reg_value = this->olat_;
   } else {
     this->read_reg_(reg_addr, &reg_value);
   }
 
   if (pin_value) {
-    reg_value |= 1 << bit;
+    reg_value |= 1 << pin;
   } else {
-    reg_value &= ~(1 << bit);
+    reg_value &= ~(1 << pin);
   }
 
   this->write_reg_(reg_addr, reg_value);
 
-  if (reg_addr == MCP23016_OLAT0) {
-    this->olat_0_ = reg_value;
-  } else if (reg_addr == MCP23016_OLAT1) {
-    this->olat_1_ = reg_value;
+  if (reg_addr == MCP23016_OLAT1) {
+    this->olat_ = reg_value;
   }
 }
 

esphome/components/mcp23016/mcp23016.h

@@ -19,13 +19,13 @@ enum MCP23016GPIORegisters {
   // 1 side
   MCP23016_GP1 = 0x01,
   MCP23016_OLAT1 = 0x03,
-  MCP23016_IPOL1 = 0x04,
+  MCP23016_IPOL1 = 0x05,
   MCP23016_IODIR1 = 0x07,
-  MCP23016_INTCAP1 = 0x08,
+  MCP23016_INTCAP1 = 0x09,
   MCP23016_IOCON1 = 0x0B,
 };
 
-class MCP23016 : public Component, public i2c::I2CDevice, public gpio_expander::CachedGpioExpander<uint8_t, 16> {
+class MCP23016 : public Component, public i2c::I2CDevice, public gpio_expander::CachedGpioExpander<uint16_t, 16> {
  public:
   MCP23016() = default;
 
@@ -42,16 +42,15 @@ class MCP23016 : public Component, public i2c::I2CDevice, public gpio_expander::
   void digital_write_hw(uint8_t pin, bool value) override;
 
   // read a given register
-  bool read_reg_(uint8_t reg, uint8_t *value);
+  bool read_reg_(uint8_t reg, uint16_t *value);
   // write a value to a given register
-  bool write_reg_(uint8_t reg, uint8_t value);
+  bool write_reg_(uint8_t reg, uint16_t value);
   // update registers with given pin value.
   void update_reg_(uint8_t pin, bool pin_value, uint8_t reg_a);
 
-  uint8_t olat_0_{0x00};
-  uint8_t olat_1_{0x00};
+  uint16_t olat_{0x0000};
   // Cache for input values (16-bit combined for both banks)
-  uint16_t input_mask_{0x00};
+  uint16_t input_mask_{0x0000};
 };
 
 class MCP23016GPIOPin : public GPIOPin {

esphome/core/application.h

@@ -127,6 +127,13 @@ void original_setup();  // NOLINT(readability-redundant-declaration) - used by c
 
 namespace esphome {
 
+/// SFINAE helper: resolves to true when &T::loop compiles and differs from &Component::loop.
+/// Falls back to true when &T::loop is ambiguous (e.g. multiple inheritance with separate loop() methods).
+template<typename T, typename = void> struct has_loop_override : std::true_type {};
+template<typename T>
+struct has_loop_override<T, std::void_t<decltype(&T::loop)>>
+    : std::bool_constant<!std::is_same_v<decltype(&T::loop), decltype(&Component::loop)>> {};
+
 // Teardown timeout constant (in milliseconds)
 // For reboots, it's more important to shut down quickly than disconnect cleanly
 // since we're not entering deep sleep. The only consequence of not shutting down
@@ -669,10 +676,9 @@ class Application {
 #endif
 
   /// Register a component, detecting loop() override at compile time.
-  /// The template resolves &T::loop vs &Component::loop as a constexpr bool
-  /// and forwards it to register_component_impl_ which stores it in component_state_.
+  /// Uses has_loop_override<T> which handles ambiguous &T::loop from multiple inheritance.
   template<typename T> void register_component_(T *comp) {
-    this->register_component_impl_(comp, !std::is_same_v<decltype(&T::loop), decltype(&Component::loop)>);
+    this->register_component_impl_(comp, has_loop_override<T>::value);
   }
 
   void register_component_impl_(Component *comp, bool has_loop);

esphome/core/config.py

@@ -517,9 +517,10 @@ async def _add_looping_components() -> None:
         return
 
     # Build constexpr sum for the exact count, deduplicating by type
+    # Uses has_loop_override<T> which handles ambiguous &T::loop from multiple inheritance
     type_counts = Counter(entries)
     terms = [
-        f"({count} * !std::is_same_v<decltype(&{cpp_type}::loop), decltype(&Component::loop)>)"
+        f"({count} * has_loop_override<{cpp_type}>::value)"
         for cpp_type, count in type_counts.items()
     ]
     constexpr_expr = " + \\\n  ".join(terms)