diff --git a/esphome/components/esp32/gpio.cpp b/esphome/components/esp32/gpio.cpp index 27572063ca..a98245b889 100644 --- a/esphome/components/esp32/gpio.cpp +++ b/esphome/components/esp32/gpio.cpp @@ -54,13 +54,13 @@ struct ISRPinArg { ISRInternalGPIOPin ESP32InternalGPIOPin::to_isr() const { auto *arg = new ISRPinArg{}; // NOLINT(cppcoreguidelines-owning-memory) - arg->pin = this->pin_; + arg->pin = this->get_pin_num(); arg->flags = gpio::FLAG_NONE; - arg->inverted = inverted_; + arg->inverted = this->pin_flags_.inverted; #if defined(USE_ESP32_VARIANT_ESP32) - arg->use_rtc = rtc_gpio_is_valid_gpio(this->pin_); + arg->use_rtc = rtc_gpio_is_valid_gpio(this->get_pin_num()); if (arg->use_rtc) - arg->rtc_pin = rtc_io_number_get(this->pin_); + arg->rtc_pin = rtc_io_number_get(this->get_pin_num()); #endif return ISRInternalGPIOPin((void *) arg); } @@ -69,23 +69,23 @@ void ESP32InternalGPIOPin::attach_interrupt(void (*func)(void *), void *arg, gpi gpio_int_type_t idf_type = GPIO_INTR_ANYEDGE; switch (type) { case gpio::INTERRUPT_RISING_EDGE: - idf_type = inverted_ ? GPIO_INTR_NEGEDGE : GPIO_INTR_POSEDGE; + idf_type = this->pin_flags_.inverted ? GPIO_INTR_NEGEDGE : GPIO_INTR_POSEDGE; break; case gpio::INTERRUPT_FALLING_EDGE: - idf_type = inverted_ ? GPIO_INTR_POSEDGE : GPIO_INTR_NEGEDGE; + idf_type = this->pin_flags_.inverted ? GPIO_INTR_POSEDGE : GPIO_INTR_NEGEDGE; break; case gpio::INTERRUPT_ANY_EDGE: idf_type = GPIO_INTR_ANYEDGE; break; case gpio::INTERRUPT_LOW_LEVEL: - idf_type = inverted_ ? GPIO_INTR_HIGH_LEVEL : GPIO_INTR_LOW_LEVEL; + idf_type = this->pin_flags_.inverted ? GPIO_INTR_HIGH_LEVEL : GPIO_INTR_LOW_LEVEL; break; case gpio::INTERRUPT_HIGH_LEVEL: - idf_type = inverted_ ? GPIO_INTR_LOW_LEVEL : GPIO_INTR_HIGH_LEVEL; + idf_type = this->pin_flags_.inverted ? GPIO_INTR_LOW_LEVEL : GPIO_INTR_HIGH_LEVEL; break; } - gpio_set_intr_type(pin_, idf_type); - gpio_intr_enable(pin_); + gpio_set_intr_type(this->get_pin_num(), idf_type); + gpio_intr_enable(this->get_pin_num()); if (!isr_service_installed) { auto res = gpio_install_isr_service(ESP_INTR_FLAG_LEVEL3); if (res != ESP_OK) { @@ -94,31 +94,31 @@ void ESP32InternalGPIOPin::attach_interrupt(void (*func)(void *), void *arg, gpi } isr_service_installed = true; } - gpio_isr_handler_add(pin_, func, arg); + gpio_isr_handler_add(this->get_pin_num(), func, arg); } std::string ESP32InternalGPIOPin::dump_summary() const { char buffer[32]; - snprintf(buffer, sizeof(buffer), "GPIO%" PRIu32, static_cast(pin_)); + snprintf(buffer, sizeof(buffer), "GPIO%" PRIu32, static_cast(this->pin_)); return buffer; } void ESP32InternalGPIOPin::setup() { gpio_config_t conf{}; - conf.pin_bit_mask = 1ULL << static_cast(pin_); - conf.mode = flags_to_mode(flags_); - conf.pull_up_en = flags_ & gpio::FLAG_PULLUP ? GPIO_PULLUP_ENABLE : GPIO_PULLUP_DISABLE; - conf.pull_down_en = flags_ & gpio::FLAG_PULLDOWN ? GPIO_PULLDOWN_ENABLE : GPIO_PULLDOWN_DISABLE; + conf.pin_bit_mask = 1ULL << static_cast(this->pin_); + conf.mode = flags_to_mode(this->flags_); + conf.pull_up_en = this->flags_ & gpio::FLAG_PULLUP ? GPIO_PULLUP_ENABLE : GPIO_PULLUP_DISABLE; + conf.pull_down_en = this->flags_ & gpio::FLAG_PULLDOWN ? GPIO_PULLDOWN_ENABLE : GPIO_PULLDOWN_DISABLE; conf.intr_type = GPIO_INTR_DISABLE; gpio_config(&conf); - if (flags_ & gpio::FLAG_OUTPUT) { - gpio_set_drive_capability(pin_, drive_strength_); + if (this->flags_ & gpio::FLAG_OUTPUT) { + gpio_set_drive_capability(this->get_pin_num(), this->get_drive_strength()); } } void ESP32InternalGPIOPin::pin_mode(gpio::Flags flags) { // can't call gpio_config here because that logs in esp-idf which may cause issues - gpio_set_direction(pin_, flags_to_mode(flags)); + gpio_set_direction(this->get_pin_num(), flags_to_mode(flags)); gpio_pull_mode_t pull_mode = GPIO_FLOATING; if ((flags & gpio::FLAG_PULLUP) && (flags & gpio::FLAG_PULLDOWN)) { pull_mode = GPIO_PULLUP_PULLDOWN; @@ -127,12 +127,16 @@ void ESP32InternalGPIOPin::pin_mode(gpio::Flags flags) { } else if (flags & gpio::FLAG_PULLDOWN) { pull_mode = GPIO_PULLDOWN_ONLY; } - gpio_set_pull_mode(pin_, pull_mode); + gpio_set_pull_mode(this->get_pin_num(), pull_mode); } -bool ESP32InternalGPIOPin::digital_read() { return bool(gpio_get_level(pin_)) != inverted_; } -void ESP32InternalGPIOPin::digital_write(bool value) { gpio_set_level(pin_, value != inverted_ ? 1 : 0); } -void ESP32InternalGPIOPin::detach_interrupt() const { gpio_intr_disable(pin_); } +bool ESP32InternalGPIOPin::digital_read() { + return bool(gpio_get_level(this->get_pin_num())) != this->pin_flags_.inverted; +} +void ESP32InternalGPIOPin::digital_write(bool value) { + gpio_set_level(this->get_pin_num(), value != this->pin_flags_.inverted ? 1 : 0); +} +void ESP32InternalGPIOPin::detach_interrupt() const { gpio_intr_disable(this->get_pin_num()); } } // namespace esp32 diff --git a/esphome/components/esp32/gpio.h b/esphome/components/esp32/gpio.h index 0fefc1c058..565e276ea8 100644 --- a/esphome/components/esp32/gpio.h +++ b/esphome/components/esp32/gpio.h @@ -7,12 +7,18 @@ namespace esphome { namespace esp32 { +// Static assertions to ensure our bit-packed fields can hold the enum values +static_assert(GPIO_NUM_MAX <= 256, "gpio_num_t has too many values for uint8_t"); +static_assert(GPIO_DRIVE_CAP_MAX <= 4, "gpio_drive_cap_t has too many values for 2-bit field"); + class ESP32InternalGPIOPin : public InternalGPIOPin { public: - void set_pin(gpio_num_t pin) { pin_ = pin; } - void set_inverted(bool inverted) { inverted_ = inverted; } - void set_drive_strength(gpio_drive_cap_t drive_strength) { drive_strength_ = drive_strength; } - void set_flags(gpio::Flags flags) { flags_ = flags; } + void set_pin(gpio_num_t pin) { this->pin_ = static_cast(pin); } + void set_inverted(bool inverted) { this->pin_flags_.inverted = inverted; } + void set_drive_strength(gpio_drive_cap_t drive_strength) { + this->pin_flags_.drive_strength = static_cast(drive_strength); + } + void set_flags(gpio::Flags flags) { this->flags_ = flags; } void setup() override; void pin_mode(gpio::Flags flags) override; @@ -21,17 +27,26 @@ class ESP32InternalGPIOPin : public InternalGPIOPin { std::string dump_summary() const override; void detach_interrupt() const override; ISRInternalGPIOPin to_isr() const override; - uint8_t get_pin() const override { return (uint8_t) pin_; } - gpio::Flags get_flags() const override { return flags_; } - bool is_inverted() const override { return inverted_; } + uint8_t get_pin() const override { return this->pin_; } + gpio::Flags get_flags() const override { return this->flags_; } + bool is_inverted() const override { return this->pin_flags_.inverted; } + gpio_num_t get_pin_num() const { return static_cast(this->pin_); } + gpio_drive_cap_t get_drive_strength() const { return static_cast(this->pin_flags_.drive_strength); } protected: void attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const override; - gpio_num_t pin_; - gpio_drive_cap_t drive_strength_; - gpio::Flags flags_; - bool inverted_; + // Memory layout: 8 bytes total on 32-bit systems + // - 3 bytes for members below + // - 1 byte padding for alignment + // - 4 bytes for vtable pointer + uint8_t pin_; // GPIO pin number (0-255, actual max ~54 on ESP32) + gpio::Flags flags_; // GPIO flags (1 byte) + struct PinFlags { + uint8_t inverted : 1; // Invert pin logic (1 bit) + uint8_t drive_strength : 2; // Drive strength 0-3 (2 bits) + uint8_t reserved : 5; // Reserved for future use (5 bits) + } pin_flags_; // Total: 1 byte // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) static bool isr_service_installed; };