From 3edd746c6c6a089daf3b1fe04d9fde1acf44c698 Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Wed, 6 Aug 2025 11:01:57 +1200 Subject: [PATCH] [mcp23xxx] Use CachedGpioExpander (#10078) --- esphome/components/mcp23008/__init__.py | 2 +- esphome/components/mcp23017/__init__.py | 2 +- esphome/components/mcp23s08/__init__.py | 2 +- esphome/components/mcp23s17/__init__.py | 2 +- esphome/components/mcp23x08_base/__init__.py | 2 ++ .../mcp23x08_base/mcp23x08_base.cpp | 16 ++++++----- .../components/mcp23x08_base/mcp23x08_base.h | 13 ++++++--- esphome/components/mcp23x17_base/__init__.py | 2 ++ .../mcp23x17_base/mcp23x17_base.cpp | 27 ++++++++++++++----- .../components/mcp23x17_base/mcp23x17_base.h | 13 ++++++--- esphome/components/mcp23xxx_base/__init__.py | 16 +++++++---- .../mcp23xxx_base/mcp23xxx_base.cpp | 27 ++++++++++--------- .../components/mcp23xxx_base/mcp23xxx_base.h | 15 ++++++----- 13 files changed, 89 insertions(+), 50 deletions(-) diff --git a/esphome/components/mcp23008/__init__.py b/esphome/components/mcp23008/__init__.py index ed48eb06a6..8ff938114a 100644 --- a/esphome/components/mcp23008/__init__.py +++ b/esphome/components/mcp23008/__init__.py @@ -24,5 +24,5 @@ CONFIG_SCHEMA = ( async def to_code(config): - var = await mcp23xxx_base.register_mcp23xxx(config) + var = await mcp23xxx_base.register_mcp23xxx(config, mcp23x08_base.NUM_PINS) await i2c.register_i2c_device(var, config) diff --git a/esphome/components/mcp23017/__init__.py b/esphome/components/mcp23017/__init__.py index 33b8a680cf..e5cc1856eb 100644 --- a/esphome/components/mcp23017/__init__.py +++ b/esphome/components/mcp23017/__init__.py @@ -24,5 +24,5 @@ CONFIG_SCHEMA = ( async def to_code(config): - var = await mcp23xxx_base.register_mcp23xxx(config) + var = await mcp23xxx_base.register_mcp23xxx(config, mcp23x17_base.NUM_PINS) await i2c.register_i2c_device(var, config) diff --git a/esphome/components/mcp23s08/__init__.py b/esphome/components/mcp23s08/__init__.py index c6152d58c0..3d4e304f9b 100644 --- a/esphome/components/mcp23s08/__init__.py +++ b/esphome/components/mcp23s08/__init__.py @@ -27,6 +27,6 @@ CONFIG_SCHEMA = ( async def to_code(config): - var = await mcp23xxx_base.register_mcp23xxx(config) + var = await mcp23xxx_base.register_mcp23xxx(config, mcp23x08_base.NUM_PINS) cg.add(var.set_device_address(config[CONF_DEVICEADDRESS])) await spi.register_spi_device(var, config) diff --git a/esphome/components/mcp23s17/__init__.py b/esphome/components/mcp23s17/__init__.py index 9a763d09b0..ea8433af2e 100644 --- a/esphome/components/mcp23s17/__init__.py +++ b/esphome/components/mcp23s17/__init__.py @@ -27,6 +27,6 @@ CONFIG_SCHEMA = ( async def to_code(config): - var = await mcp23xxx_base.register_mcp23xxx(config) + var = await mcp23xxx_base.register_mcp23xxx(config, mcp23x17_base.NUM_PINS) cg.add(var.set_device_address(config[CONF_DEVICEADDRESS])) await spi.register_spi_device(var, config) diff --git a/esphome/components/mcp23x08_base/__init__.py b/esphome/components/mcp23x08_base/__init__.py index ba44917202..a3c12165f0 100644 --- a/esphome/components/mcp23x08_base/__init__.py +++ b/esphome/components/mcp23x08_base/__init__.py @@ -4,5 +4,7 @@ from esphome.components import mcp23xxx_base AUTO_LOAD = ["mcp23xxx_base"] CODEOWNERS = ["@jesserockz"] +NUM_PINS = 8 + mcp23x08_base_ns = cg.esphome_ns.namespace("mcp23x08_base") MCP23X08Base = mcp23x08_base_ns.class_("MCP23X08Base", mcp23xxx_base.MCP23XXXBase) diff --git a/esphome/components/mcp23x08_base/mcp23x08_base.cpp b/esphome/components/mcp23x08_base/mcp23x08_base.cpp index 0c20e902c4..e4fb51174b 100644 --- a/esphome/components/mcp23x08_base/mcp23x08_base.cpp +++ b/esphome/components/mcp23x08_base/mcp23x08_base.cpp @@ -6,19 +6,21 @@ namespace mcp23x08_base { static const char *const TAG = "mcp23x08_base"; -bool MCP23X08Base::digital_read(uint8_t pin) { - uint8_t bit = pin % 8; - uint8_t reg_addr = mcp23x08_base::MCP23X08_GPIO; - uint8_t value = 0; - this->read_reg(reg_addr, &value); - return value & (1 << bit); +bool MCP23X08Base::digital_read_hw(uint8_t pin) { + if (!this->read_reg(mcp23x08_base::MCP23X08_GPIO, &this->input_mask_)) { + this->status_set_warning(ESP_LOG_MSG_COMM_FAIL); + return false; + } + return true; } -void MCP23X08Base::digital_write(uint8_t pin, bool value) { +void MCP23X08Base::digital_write_hw(uint8_t pin, bool value) { uint8_t reg_addr = mcp23x08_base::MCP23X08_OLAT; this->update_reg(pin, value, reg_addr); } +bool MCP23X08Base::digital_read_cache(uint8_t pin) { return this->input_mask_ & (1 << pin); } + void MCP23X08Base::pin_mode(uint8_t pin, gpio::Flags flags) { uint8_t iodir = mcp23x08_base::MCP23X08_IODIR; uint8_t gppu = mcp23x08_base::MCP23X08_GPPU; diff --git a/esphome/components/mcp23x08_base/mcp23x08_base.h b/esphome/components/mcp23x08_base/mcp23x08_base.h index 910519119b..6eee8274b1 100644 --- a/esphome/components/mcp23x08_base/mcp23x08_base.h +++ b/esphome/components/mcp23x08_base/mcp23x08_base.h @@ -1,7 +1,7 @@ #pragma once -#include "esphome/core/component.h" #include "esphome/components/mcp23xxx_base/mcp23xxx_base.h" +#include "esphome/core/component.h" #include "esphome/core/hal.h" namespace esphome { @@ -22,10 +22,12 @@ enum MCP23S08GPIORegisters { MCP23X08_OLAT = 0x0A, }; -class MCP23X08Base : public mcp23xxx_base::MCP23XXXBase { +class MCP23X08Base : public mcp23xxx_base::MCP23XXXBase<8> { public: - bool digital_read(uint8_t pin) override; - void digital_write(uint8_t pin, bool value) override; + bool digital_read_hw(uint8_t pin) override; + void digital_write_hw(uint8_t pin, bool value) override; + bool digital_read_cache(uint8_t pin) override; + void pin_mode(uint8_t pin, gpio::Flags flags) override; void pin_interrupt_mode(uint8_t pin, mcp23xxx_base::MCP23XXXInterruptMode interrupt_mode) override; @@ -33,6 +35,9 @@ class MCP23X08Base : public mcp23xxx_base::MCP23XXXBase { void update_reg(uint8_t pin, bool pin_value, uint8_t reg_a) override; uint8_t olat_{0x00}; + + /// State read in digital_read_hw + uint8_t input_mask_{0x00}; }; } // namespace mcp23x08_base diff --git a/esphome/components/mcp23x17_base/__init__.py b/esphome/components/mcp23x17_base/__init__.py index 97e0b3823d..1b93d16ff3 100644 --- a/esphome/components/mcp23x17_base/__init__.py +++ b/esphome/components/mcp23x17_base/__init__.py @@ -4,5 +4,7 @@ from esphome.components import mcp23xxx_base AUTO_LOAD = ["mcp23xxx_base"] CODEOWNERS = ["@jesserockz"] +NUM_PINS = 16 + mcp23x17_base_ns = cg.esphome_ns.namespace("mcp23x17_base") MCP23X17Base = mcp23x17_base_ns.class_("MCP23X17Base", mcp23xxx_base.MCP23XXXBase) diff --git a/esphome/components/mcp23x17_base/mcp23x17_base.cpp b/esphome/components/mcp23x17_base/mcp23x17_base.cpp index 99064f8880..020b8a5ddf 100644 --- a/esphome/components/mcp23x17_base/mcp23x17_base.cpp +++ b/esphome/components/mcp23x17_base/mcp23x17_base.cpp @@ -1,4 +1,5 @@ #include "mcp23x17_base.h" +#include "esphome/core/helpers.h" #include "esphome/core/log.h" namespace esphome { @@ -6,19 +7,31 @@ namespace mcp23x17_base { static const char *const TAG = "mcp23x17_base"; -bool MCP23X17Base::digital_read(uint8_t pin) { - uint8_t bit = pin % 8; - uint8_t reg_addr = pin < 8 ? mcp23x17_base::MCP23X17_GPIOA : mcp23x17_base::MCP23X17_GPIOB; - uint8_t value = 0; - this->read_reg(reg_addr, &value); - return value & (1 << bit); +bool MCP23X17Base::digital_read_hw(uint8_t pin) { + uint8_t data; + if (pin < 8) { + if (!this->read_reg(mcp23x17_base::MCP23X17_GPIOA, &data)) { + this->status_set_warning(ESP_LOG_MSG_COMM_FAIL); + return false; + } + this->input_mask_ = encode_uint16(this->input_mask_ >> 8, data); + } else { + if (!this->read_reg(mcp23x17_base::MCP23X17_GPIOB, &data)) { + this->status_set_warning(ESP_LOG_MSG_COMM_FAIL); + return false; + } + this->input_mask_ = encode_uint16(data, this->input_mask_ & 0xFF); + } + return true; } -void MCP23X17Base::digital_write(uint8_t pin, bool value) { +void MCP23X17Base::digital_write_hw(uint8_t pin, bool value) { uint8_t reg_addr = pin < 8 ? mcp23x17_base::MCP23X17_OLATA : mcp23x17_base::MCP23X17_OLATB; this->update_reg(pin, value, reg_addr); } +bool MCP23X17Base::digital_read_cache(uint8_t pin) { return this->input_mask_ & (1 << pin); } + void MCP23X17Base::pin_mode(uint8_t pin, gpio::Flags flags) { uint8_t iodir = pin < 8 ? mcp23x17_base::MCP23X17_IODIRA : mcp23x17_base::MCP23X17_IODIRB; uint8_t gppu = pin < 8 ? mcp23x17_base::MCP23X17_GPPUA : mcp23x17_base::MCP23X17_GPPUB; diff --git a/esphome/components/mcp23x17_base/mcp23x17_base.h b/esphome/components/mcp23x17_base/mcp23x17_base.h index 3d50ee8c03..bdd66503e2 100644 --- a/esphome/components/mcp23x17_base/mcp23x17_base.h +++ b/esphome/components/mcp23x17_base/mcp23x17_base.h @@ -1,7 +1,7 @@ #pragma once -#include "esphome/core/component.h" #include "esphome/components/mcp23xxx_base/mcp23xxx_base.h" +#include "esphome/core/component.h" #include "esphome/core/hal.h" namespace esphome { @@ -34,10 +34,12 @@ enum MCP23X17GPIORegisters { MCP23X17_OLATB = 0x15, }; -class MCP23X17Base : public mcp23xxx_base::MCP23XXXBase { +class MCP23X17Base : public mcp23xxx_base::MCP23XXXBase<16> { public: - bool digital_read(uint8_t pin) override; - void digital_write(uint8_t pin, bool value) override; + bool digital_read_hw(uint8_t pin) override; + void digital_write_hw(uint8_t pin, bool value) override; + bool digital_read_cache(uint8_t pin) override; + void pin_mode(uint8_t pin, gpio::Flags flags) override; void pin_interrupt_mode(uint8_t pin, mcp23xxx_base::MCP23XXXInterruptMode interrupt_mode) override; @@ -46,6 +48,9 @@ class MCP23X17Base : public mcp23xxx_base::MCP23XXXBase { uint8_t olat_a_{0x00}; uint8_t olat_b_{0x00}; + + /// State read in digital_read_hw + uint16_t input_mask_{0x00}; }; } // namespace mcp23x17_base diff --git a/esphome/components/mcp23xxx_base/__init__.py b/esphome/components/mcp23xxx_base/__init__.py index 8cf0ebcd44..d6e82101ad 100644 --- a/esphome/components/mcp23xxx_base/__init__.py +++ b/esphome/components/mcp23xxx_base/__init__.py @@ -12,8 +12,9 @@ from esphome.const import ( CONF_OUTPUT, CONF_PULLUP, ) -from esphome.core import coroutine +from esphome.core import CORE, ID, coroutine +AUTO_LOAD = ["gpio_expander"] CODEOWNERS = ["@jesserockz"] mcp23xxx_base_ns = cg.esphome_ns.namespace("mcp23xxx_base") @@ -36,9 +37,11 @@ MCP23XXX_CONFIG_SCHEMA = cv.Schema( @coroutine -async def register_mcp23xxx(config): - var = cg.new_Pvariable(config[CONF_ID]) +async def register_mcp23xxx(config, num_pins): + id: ID = config[CONF_ID] + var = cg.new_Pvariable(id) await cg.register_component(var, config) + CORE.data.setdefault(CONF_MCP23XXX, {})[id.id] = num_pins cg.add(var.set_open_drain_ints(config[CONF_OPEN_DRAIN_INTERRUPT])) return var @@ -73,9 +76,12 @@ MCP23XXX_PIN_SCHEMA = pins.gpio_base_schema( @pins.PIN_SCHEMA_REGISTRY.register(CONF_MCP23XXX, MCP23XXX_PIN_SCHEMA) async def mcp23xxx_pin_to_code(config): - var = cg.new_Pvariable(config[CONF_ID]) - parent = await cg.get_variable(config[CONF_MCP23XXX]) + parent_id: ID = config[CONF_MCP23XXX] + parent = await cg.get_variable(parent_id) + num_pins = cg.TemplateArguments(CORE.data[CONF_MCP23XXX][parent_id.id]) + + var = cg.new_Pvariable(config[CONF_ID], num_pins) cg.add(var.set_parent(parent)) num = config[CONF_NUMBER] diff --git a/esphome/components/mcp23xxx_base/mcp23xxx_base.cpp b/esphome/components/mcp23xxx_base/mcp23xxx_base.cpp index fc49f216ee..81324e794f 100644 --- a/esphome/components/mcp23xxx_base/mcp23xxx_base.cpp +++ b/esphome/components/mcp23xxx_base/mcp23xxx_base.cpp @@ -1,24 +1,27 @@ #include "mcp23xxx_base.h" +#include "esphome/core/helpers.h" #include "esphome/core/log.h" namespace esphome { namespace mcp23xxx_base { -float MCP23XXXBase::get_setup_priority() const { return setup_priority::IO; } - -void MCP23XXXGPIOPin::setup() { - pin_mode(flags_); +template void MCP23XXXGPIOPin::setup() { + this->pin_mode(flags_); this->parent_->pin_interrupt_mode(this->pin_, this->interrupt_mode_); } - -void MCP23XXXGPIOPin::pin_mode(gpio::Flags flags) { this->parent_->pin_mode(this->pin_, flags); } -bool MCP23XXXGPIOPin::digital_read() { return this->parent_->digital_read(this->pin_) != this->inverted_; } -void MCP23XXXGPIOPin::digital_write(bool value) { this->parent_->digital_write(this->pin_, value != this->inverted_); } -std::string MCP23XXXGPIOPin::dump_summary() const { - char buffer[32]; - snprintf(buffer, sizeof(buffer), "%u via MCP23XXX", pin_); - return buffer; +template void MCP23XXXGPIOPin::pin_mode(gpio::Flags flags) { this->parent_->pin_mode(this->pin_, flags); } +template bool MCP23XXXGPIOPin::digital_read() { + return this->parent_->digital_read(this->pin_) != this->inverted_; } +template void MCP23XXXGPIOPin::digital_write(bool value) { + this->parent_->digital_write(this->pin_, value != this->inverted_); +} +template std::string MCP23XXXGPIOPin::dump_summary() const { + return str_snprintf("%u via MCP23XXX", 15, pin_); +} + +template class MCP23XXXGPIOPin<8>; +template class MCP23XXXGPIOPin<16>; } // namespace mcp23xxx_base } // namespace esphome diff --git a/esphome/components/mcp23xxx_base/mcp23xxx_base.h b/esphome/components/mcp23xxx_base/mcp23xxx_base.h index 9686c9fd33..ab7f8ec398 100644 --- a/esphome/components/mcp23xxx_base/mcp23xxx_base.h +++ b/esphome/components/mcp23xxx_base/mcp23xxx_base.h @@ -1,5 +1,6 @@ #pragma once +#include "esphome/components/gpio_expander/cached_gpio.h" #include "esphome/core/component.h" #include "esphome/core/hal.h" @@ -8,15 +9,15 @@ namespace mcp23xxx_base { enum MCP23XXXInterruptMode : uint8_t { MCP23XXX_NO_INTERRUPT = 0, MCP23XXX_CHANGE, MCP23XXX_RISING, MCP23XXX_FALLING }; -class MCP23XXXBase : public Component { +template class MCP23XXXBase : public Component, public gpio_expander::CachedGpioExpander { public: - virtual bool digital_read(uint8_t pin); - virtual void digital_write(uint8_t pin, bool value); virtual void pin_mode(uint8_t pin, gpio::Flags flags); virtual void pin_interrupt_mode(uint8_t pin, MCP23XXXInterruptMode interrupt_mode); void set_open_drain_ints(const bool value) { this->open_drain_ints_ = value; } - float get_setup_priority() const override; + float get_setup_priority() const override { return setup_priority::IO; } + + void loop() override { this->reset_pin_cache_(); } protected: // read a given register @@ -29,7 +30,7 @@ class MCP23XXXBase : public Component { bool open_drain_ints_; }; -class MCP23XXXGPIOPin : public GPIOPin { +template class MCP23XXXGPIOPin : public GPIOPin { public: void setup() override; void pin_mode(gpio::Flags flags) override; @@ -37,7 +38,7 @@ class MCP23XXXGPIOPin : public GPIOPin { void digital_write(bool value) override; std::string dump_summary() const override; - void set_parent(MCP23XXXBase *parent) { parent_ = parent; } + void set_parent(MCP23XXXBase *parent) { parent_ = parent; } void set_pin(uint8_t pin) { pin_ = pin; } void set_inverted(bool inverted) { inverted_ = inverted; } void set_flags(gpio::Flags flags) { flags_ = flags; } @@ -46,7 +47,7 @@ class MCP23XXXGPIOPin : public GPIOPin { gpio::Flags get_flags() const override { return this->flags_; } protected: - MCP23XXXBase *parent_; + MCP23XXXBase *parent_; uint8_t pin_; bool inverted_; gpio::Flags flags_;