From b9b1af1c3dd11e2d41c0b88542637a6ceaec5ecd Mon Sep 17 00:00:00 2001 From: netixx Date: Mon, 2 Mar 2026 18:38:28 +0100 Subject: [PATCH 1/3] [mcp23016] Fix register access to use 16-bit paired transactions (#13676) Co-authored-by: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> --- esphome/components/mcp23016/mcp23016.cpp | 71 +++++++++--------------- esphome/components/mcp23016/mcp23016.h | 15 +++-- 2 files changed, 33 insertions(+), 53 deletions(-) diff --git a/esphome/components/mcp23016/mcp23016.cpp b/esphome/components/mcp23016/mcp23016.cpp index 56b2ecf9f4e..fbdb6903b84 100644 --- a/esphome/components/mcp23016/mcp23016.cpp +++ b/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::HARDWARE; } -bool MCP23016::read_reg_(uint8_t reg, uint8_t *value) { +float MCP23016::get_setup_priority() const { return setup_priority::IO; } +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, ®_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; } } diff --git a/esphome/components/mcp23016/mcp23016.h b/esphome/components/mcp23016/mcp23016.h index c2bc885c958..494bc9c197a 100644 --- a/esphome/components/mcp23016/mcp23016.h +++ b/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 { +class MCP23016 : public Component, public i2c::I2CDevice, public gpio_expander::CachedGpioExpander { 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 { From 2fa244715d21e30338702c0ea017884f338efd03 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Mar 2026 08:54:54 -1000 Subject: [PATCH 2/3] [socket] Fix pre-existing bugs found during socket devirtualization review (#14404) --- esphome/components/socket/bsd_sockets_impl.h | 2 +- .../components/socket/lwip_raw_tcp_impl.cpp | 21 +++++++++++++++++-- esphome/components/socket/lwip_sockets_impl.h | 2 +- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/esphome/components/socket/bsd_sockets_impl.h b/esphome/components/socket/bsd_sockets_impl.h index edee39f3154..d9ed9dc567c 100644 --- a/esphome/components/socket/bsd_sockets_impl.h +++ b/esphome/components/socket/bsd_sockets_impl.h @@ -83,7 +83,7 @@ class BSDSocketImpl { return ::write(this->fd_, buf, len); #endif } - ssize_t send(void *buf, size_t len, int flags) { return ::send(this->fd_, buf, len, flags); } + ssize_t send(const void *buf, size_t len, int flags) { return ::send(this->fd_, buf, len, flags); } ssize_t writev(const struct iovec *iov, int iovcnt) { #if defined(USE_ESP32) return ::lwip_writev(this->fd_, iov, iovcnt); diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index 6556979fe01..430356592fa 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -68,7 +68,7 @@ int LWIPRawCommon::bind(const struct sockaddr *name, socklen_t addrlen) { } if (name == nullptr) { errno = EINVAL; - return 0; + return -1; } ip_addr_t ip; in_port_t port; @@ -319,6 +319,13 @@ int LWIPRawCommon::ip2sockaddr_(ip_addr_t *ip, uint16_t port, struct sockaddr *n // ---- LWIPRawImpl methods ---- LWIPRawImpl::~LWIPRawImpl() { + // Free any received pbufs that LWIP transferred ownership of via recv_fn. + // tcp_abort() in the base destructor won't free these since LWIP considers + // ownership transferred once the recv callback accepts them. + if (this->rx_buf_ != nullptr) { + pbuf_free(this->rx_buf_); + this->rx_buf_ = nullptr; + } // Base class destructor handles pcb_ cleanup via tcp_abort } @@ -545,7 +552,14 @@ ssize_t LWIPRawImpl::writev(const struct iovec *iov, int iovcnt) { // ---- LWIPRawListenImpl methods ---- LWIPRawListenImpl::~LWIPRawListenImpl() { - // Base class destructor handles pcb_ cleanup via tcp_abort + // Listen PCBs must use tcp_close(), not tcp_abort(). + // tcp_abandon() asserts pcb->state != LISTEN and would access + // fields that don't exist in the smaller tcp_pcb_listen struct. + // Close here and null pcb_ so the base destructor skips tcp_abort. + if (this->pcb_ != nullptr) { + tcp_close(this->pcb_); + this->pcb_ = nullptr; + } } void LWIPRawListenImpl::init() { @@ -609,6 +623,9 @@ int LWIPRawListenImpl::listen(int backlog) { LWIP_LOG("tcp_arg(%p)", this->pcb_); tcp_arg(this->pcb_, this); tcp_accept(this->pcb_, LWIPRawListenImpl::s_accept_fn); + // Note: tcp_err() is NOT re-registered here. tcp_listen_with_backlog() converts the + // full tcp_pcb to a smaller tcp_pcb_listen struct that lacks the errf field. + // Calling tcp_err() on a listen PCB writes past the struct boundary (undefined behavior). return 0; } diff --git a/esphome/components/socket/lwip_sockets_impl.h b/esphome/components/socket/lwip_sockets_impl.h index 2e319fcc4df..d6699aded26 100644 --- a/esphome/components/socket/lwip_sockets_impl.h +++ b/esphome/components/socket/lwip_sockets_impl.h @@ -57,7 +57,7 @@ class LwIPSocketImpl { } ssize_t readv(const struct iovec *iov, int iovcnt) { return lwip_readv(this->fd_, iov, iovcnt); } ssize_t write(const void *buf, size_t len) { return lwip_write(this->fd_, buf, len); } - ssize_t send(void *buf, size_t len, int flags) { return lwip_send(this->fd_, buf, len, flags); } + ssize_t send(const void *buf, size_t len, int flags) { return lwip_send(this->fd_, buf, len, flags); } ssize_t writev(const struct iovec *iov, int iovcnt) { return lwip_writev(this->fd_, iov, iovcnt); } ssize_t sendto(const void *buf, size_t len, int flags, const struct sockaddr *to, socklen_t tolen) { return lwip_sendto(this->fd_, buf, len, flags, to, tolen); From cb232d828879b365bfa116ff3b4a7546c1e6ce62 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Mar 2026 09:11:47 -1000 Subject: [PATCH 3/3] [core] Fix compile-time loop() detection for multiple inheritance (#14411) --- esphome/core/application.h | 13 ++++++++++--- esphome/core/config.py | 3 ++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/esphome/core/application.h b/esphome/core/application.h index 44e8de7ee9f..13fd0180ab2 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -118,6 +118,14 @@ void original_setup(); // NOLINT(readability-redundant-declaration) - used by c namespace esphome { +/// SFINAE helper: detects whether T overrides Component::loop(). +/// When &T::loop is ambiguous (multiple inheritance with separate loop() methods), +/// the ambiguity itself proves an override exists, so the true_type default is correct. +template struct HasLoopOverride : std::true_type {}; +template +struct HasLoopOverride> + : std::bool_constant> {}; + // 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 @@ -544,10 +552,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 HasLoopOverride which handles ambiguous &T::loop from multiple inheritance. template void register_component_(T *comp) { - this->register_component_impl_(comp, !std::is_same_v); + this->register_component_impl_(comp, HasLoopOverride::value); } void register_component_impl_(Component *comp, bool has_loop); diff --git a/esphome/core/config.py b/esphome/core/config.py index 3835fd3875f..9411949bb92 100644 --- a/esphome/core/config.py +++ b/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 HasLoopOverride which handles ambiguous &T::loop from multiple inheritance type_counts = Counter(entries) terms = [ - f"({count} * !std::is_same_v)" + f"({count} * HasLoopOverride<{cpp_type}>::value)" for cpp_type, count in type_counts.items() ] constexpr_expr = " + \\\n ".join(terms)