diff --git a/esphome/components/gpio_expander/cached_gpio.h b/esphome/components/gpio_expander/cached_gpio.h index 78c675cdb2..d7230eb0b3 100644 --- a/esphome/components/gpio_expander/cached_gpio.h +++ b/esphome/components/gpio_expander/cached_gpio.h @@ -2,10 +2,11 @@ #include #include +#include +#include #include "esphome/core/hal.h" -namespace esphome { -namespace gpio_expander { +namespace esphome::gpio_expander { /// @brief A class to cache the read state of a GPIO expander. /// This class caches reads between GPIO Pins which are on the same bank. @@ -17,12 +18,22 @@ namespace gpio_expander { /// N - Number of pins template class CachedGpioExpander { public: + /// @brief Read the state of the given pin. This will invalidate the cache for the given pin number. + /// @param pin Pin number to read + /// @return Pin state bool digital_read(T pin) { - uint8_t bank = pin / (sizeof(T) * BITS_PER_BYTE); - if (this->read_cache_invalidated_[bank]) { - this->read_cache_invalidated_[bank] = false; + const uint8_t bank = pin / BANK_SIZE; + const T pin_mask = (1 << (pin % BANK_SIZE)); + // Check if specific pin cache is valid + if (this->read_cache_valid_[bank] & pin_mask) { + // Invalidate pin + this->read_cache_valid_[bank] &= ~pin_mask; + } else { + // Read whole bank from hardware if (!this->digital_read_hw(pin)) return false; + // Mark bank cache as valid except the pin that is being returned now + this->read_cache_valid_[bank] = std::numeric_limits::max() & ~pin_mask; } return this->digital_read_cache(pin); } @@ -36,18 +47,16 @@ template class CachedGpioExpander { virtual bool digital_read_cache(T pin) = 0; /// @brief Call component low level function to write GPIO state to device virtual void digital_write_hw(T pin, bool value) = 0; - const uint8_t cache_byte_size_ = N / (sizeof(T) * BITS_PER_BYTE); /// @brief Invalidate cache. This function should be called in component loop(). - void reset_pin_cache_() { - for (T i = 0; i < this->cache_byte_size_; i++) { - this->read_cache_invalidated_[i] = true; - } - } + void reset_pin_cache_() { memset(this->read_cache_valid_, 0x00, CACHE_SIZE_BYTES); } - static const uint8_t BITS_PER_BYTE = 8; - std::array read_cache_invalidated_{}; + static constexpr uint8_t BITS_PER_BYTE = 8; + static constexpr uint8_t BANK_SIZE = sizeof(T) * BITS_PER_BYTE; + static constexpr size_t BANKS = N / BANK_SIZE; + static constexpr size_t CACHE_SIZE_BYTES = BANKS * sizeof(T); + + T read_cache_valid_[BANKS]{0}; }; -} // namespace gpio_expander -} // namespace esphome +} // namespace esphome::gpio_expander diff --git a/tests/integration/fixtures/external_components/gpio_expander_test_component/__init__.py b/tests/integration/fixtures/external_components/gpio_expander_test_component/__init__.py new file mode 100644 index 0000000000..5672f80004 --- /dev/null +++ b/tests/integration/fixtures/external_components/gpio_expander_test_component/__init__.py @@ -0,0 +1,25 @@ +import esphome.codegen as cg +import esphome.config_validation as cv +from esphome.const import CONF_ID + +AUTO_LOAD = ["gpio_expander"] + +gpio_expander_test_component_ns = cg.esphome_ns.namespace( + "gpio_expander_test_component" +) + +GPIOExpanderTestComponent = gpio_expander_test_component_ns.class_( + "GPIOExpanderTestComponent", cg.Component +) + + +CONFIG_SCHEMA = cv.Schema( + { + cv.GenerateID(): cv.declare_id(GPIOExpanderTestComponent), + } +).extend(cv.COMPONENT_SCHEMA) + + +async def to_code(config): + var = cg.new_Pvariable(config[CONF_ID]) + await cg.register_component(var, config) diff --git a/tests/integration/fixtures/external_components/gpio_expander_test_component/gpio_expander_test_component.cpp b/tests/integration/fixtures/external_components/gpio_expander_test_component/gpio_expander_test_component.cpp new file mode 100644 index 0000000000..7e88950592 --- /dev/null +++ b/tests/integration/fixtures/external_components/gpio_expander_test_component/gpio_expander_test_component.cpp @@ -0,0 +1,38 @@ +#include "gpio_expander_test_component.h" + +#include "esphome/core/application.h" +#include "esphome/core/log.h" + +namespace esphome::gpio_expander_test_component { + +static const char *const TAG = "gpio_expander_test"; + +void GPIOExpanderTestComponent::setup() { + for (uint8_t pin = 0; pin < 32; pin++) { + this->digital_read(pin); + } + + this->digital_read(3); + this->digital_read(3); + this->digital_read(4); + this->digital_read(3); + this->digital_read(10); + this->reset_pin_cache_(); // Reset cache to ensure next read is from hardware + this->digital_read(15); + this->digital_read(14); + this->digital_read(14); + + ESP_LOGD(TAG, "DONE"); +} + +bool GPIOExpanderTestComponent::digital_read_hw(uint8_t pin) { + ESP_LOGD(TAG, "digital_read_hw pin=%d", pin); + return true; +} + +bool GPIOExpanderTestComponent::digital_read_cache(uint8_t pin) { + ESP_LOGD(TAG, "digital_read_cache pin=%d", pin); + return true; +} + +} // namespace esphome::gpio_expander_test_component diff --git a/tests/integration/fixtures/external_components/gpio_expander_test_component/gpio_expander_test_component.h b/tests/integration/fixtures/external_components/gpio_expander_test_component/gpio_expander_test_component.h new file mode 100644 index 0000000000..ffaee2cd65 --- /dev/null +++ b/tests/integration/fixtures/external_components/gpio_expander_test_component/gpio_expander_test_component.h @@ -0,0 +1,18 @@ +#pragma once + +#include "esphome/components/gpio_expander/cached_gpio.h" +#include "esphome/core/component.h" + +namespace esphome::gpio_expander_test_component { + +class GPIOExpanderTestComponent : public Component, public esphome::gpio_expander::CachedGpioExpander { + public: + void setup() override; + + protected: + bool digital_read_hw(uint8_t pin) override; + bool digital_read_cache(uint8_t pin) override; + void digital_write_hw(uint8_t pin, bool value) override{}; +}; + +} // namespace esphome::gpio_expander_test_component diff --git a/tests/integration/fixtures/gpio_expander_cache.yaml b/tests/integration/fixtures/gpio_expander_cache.yaml new file mode 100644 index 0000000000..7d7ca1a876 --- /dev/null +++ b/tests/integration/fixtures/gpio_expander_cache.yaml @@ -0,0 +1,17 @@ +esphome: + name: gpio-expander-cache +host: + +logger: + level: DEBUG + +api: + +# External component that uses gpio_expander::CachedGpioExpander +external_components: + - source: + type: local + path: EXTERNAL_COMPONENT_PATH + components: [gpio_expander_test_component] + +gpio_expander_test_component: diff --git a/tests/integration/test_gpio_expander_cache.py b/tests/integration/test_gpio_expander_cache.py new file mode 100644 index 0000000000..9353bb1dd6 --- /dev/null +++ b/tests/integration/test_gpio_expander_cache.py @@ -0,0 +1,123 @@ +"""Integration test for CachedGPIOExpander to ensure correct behavior.""" + +from __future__ import annotations + +import asyncio +from pathlib import Path +import re + +import pytest + +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_gpio_expander_cache( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test gpio_expander::CachedGpioExpander correctly calls hardware functions.""" + # Get the path to the external components directory + external_components_path = str( + Path(__file__).parent / "fixtures" / "external_components" + ) + + # Replace the placeholder in the YAML config with the actual path + yaml_config = yaml_config.replace( + "EXTERNAL_COMPONENT_PATH", external_components_path + ) + + logs_done = asyncio.Event() + + # Patterns to match in logs + digital_read_hw_pattern = re.compile(r"digital_read_hw pin=(\d+)") + digital_read_cache_pattern = re.compile(r"digital_read_cache pin=(\d+)") + + # ensure logs are in the expected order + log_order = [ + (digital_read_hw_pattern, 0), + [(digital_read_cache_pattern, i) for i in range(0, 8)], + (digital_read_hw_pattern, 8), + [(digital_read_cache_pattern, i) for i in range(8, 16)], + (digital_read_hw_pattern, 16), + [(digital_read_cache_pattern, i) for i in range(16, 24)], + (digital_read_hw_pattern, 24), + [(digital_read_cache_pattern, i) for i in range(24, 32)], + (digital_read_hw_pattern, 3), + (digital_read_cache_pattern, 3), + (digital_read_hw_pattern, 3), + (digital_read_cache_pattern, 3), + (digital_read_cache_pattern, 4), + (digital_read_hw_pattern, 3), + (digital_read_cache_pattern, 3), + (digital_read_hw_pattern, 10), + (digital_read_cache_pattern, 10), + # full cache reset here for testing + (digital_read_hw_pattern, 15), + (digital_read_cache_pattern, 15), + (digital_read_cache_pattern, 14), + (digital_read_hw_pattern, 14), + (digital_read_cache_pattern, 14), + ] + # Flatten the log order for easier processing + log_order: list[tuple[re.Pattern, int]] = [ + item + for sublist in log_order + for item in (sublist if isinstance(sublist, list) else [sublist]) + ] + + index = 0 + + def check_output(line: str) -> None: + """Check log output for expected messages.""" + nonlocal index + if logs_done.is_set(): + return + + clean_line = re.sub(r"\x1b\[[0-9;]*m", "", line) + + if "digital_read" in clean_line: + if index >= len(log_order): + print(f"Received unexpected log line: {clean_line}") + logs_done.set() + return + + pattern, expected_pin = log_order[index] + match = pattern.search(clean_line) + + if not match: + print(f"Log line did not match next expected pattern: {clean_line}") + logs_done.set() + return + + pin = int(match.group(1)) + if pin != expected_pin: + print(f"Unexpected pin number. Expected {expected_pin}, got {pin}") + logs_done.set() + return + + index += 1 + + elif "DONE" in clean_line: + # Check if we reached the end of the expected log entries + logs_done.set() + + # Run with log monitoring + async with ( + run_compiled(yaml_config, line_callback=check_output), + api_client_connected() as client, + ): + # Verify device info + device_info = await client.device_info() + assert device_info is not None + assert device_info.name == "gpio-expander-cache" + + try: + await asyncio.wait_for(logs_done.wait(), timeout=5.0) + except TimeoutError: + pytest.fail("Timeout waiting for logs to complete") + + assert index == len(log_order), ( + f"Expected {len(log_order)} log entries, but got {index}" + )