From 668007707da498e12fd9904a152761245821f258 Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Fri, 10 Apr 2026 12:13:22 +1200 Subject: [PATCH 1/7] [CI] Add org fork detection warning to auto-label PR workflow (#15588) --- .github/scripts/auto-label-pr/constants.js | 1 + .github/scripts/auto-label-pr/detectors.js | 19 +++++++ .github/scripts/auto-label-pr/index.js | 16 ++++-- .github/scripts/auto-label-pr/reviews.js | 62 +++++++++++++++++++++- 4 files changed, 91 insertions(+), 7 deletions(-) diff --git a/.github/scripts/auto-label-pr/constants.js b/.github/scripts/auto-label-pr/constants.js index 1c33772c4c3..e02b450bf0e 100644 --- a/.github/scripts/auto-label-pr/constants.js +++ b/.github/scripts/auto-label-pr/constants.js @@ -4,6 +4,7 @@ module.exports = { CODEOWNERS_MARKER: '', TOO_BIG_MARKER: '', DEPRECATED_COMPONENT_MARKER: '', + ORG_FORK_MARKER: '', MANAGED_LABELS: [ 'new-component', diff --git a/.github/scripts/auto-label-pr/detectors.js b/.github/scripts/auto-label-pr/detectors.js index fb9dadc6a09..25c0ba49afc 100644 --- a/.github/scripts/auto-label-pr/detectors.js +++ b/.github/scripts/auto-label-pr/detectors.js @@ -281,6 +281,24 @@ async function detectDeprecatedComponents(github, context, changedFiles) { return { labels, deprecatedInfo }; } +// Strategy: Detect when maintainers cannot modify the PR branch +function detectMaintainerAccess(context) { + const pr = context.payload.pull_request; + + // Only relevant for cross-repo PRs (forks) + if (!pr.head.repo || pr.head.repo.full_name === pr.base.repo.full_name) { + return null; + } + + if (pr.maintainer_can_modify) { + return null; + } + + const isOrgFork = pr.head.repo.owner.type === 'Organization'; + console.log(`Maintainer cannot modify PR branch (${isOrgFork ? 'org fork: ' + pr.head.repo.owner.login : 'user disabled'})`); + return { isOrgFork, orgName: pr.head.repo.owner.login }; +} + // Strategy: Requirements detection async function detectRequirements(allLabels, prFiles, context) { const labels = new Set(); @@ -329,5 +347,6 @@ module.exports = { detectTests, detectPRTemplateCheckboxes, detectDeprecatedComponents, + detectMaintainerAccess, detectRequirements }; diff --git a/.github/scripts/auto-label-pr/index.js b/.github/scripts/auto-label-pr/index.js index 42588c0bc87..021e91a9ee1 100644 --- a/.github/scripts/auto-label-pr/index.js +++ b/.github/scripts/auto-label-pr/index.js @@ -12,9 +12,10 @@ const { detectTests, detectPRTemplateCheckboxes, detectDeprecatedComponents, + detectMaintainerAccess, detectRequirements } = require('./detectors'); -const { handleReviews } = require('./reviews'); +const { handleReviews, handleMaintainerAccessComment } = require('./reviews'); const { applyLabels, removeOldLabels } = require('./labels'); // Fetch API data @@ -114,7 +115,8 @@ module.exports = async ({ github, context }) => { codeOwnerLabels, testLabels, checkboxLabels, - deprecatedResult + deprecatedResult, + maintainerAccess ] = await Promise.all([ detectMergeBranch(context), detectComponentPlatforms(changedFiles, apiData), @@ -127,7 +129,8 @@ module.exports = async ({ github, context }) => { detectCodeOwner(github, context, changedFiles), detectTests(changedFiles), detectPRTemplateCheckboxes(context), - detectDeprecatedComponents(github, context, changedFiles) + detectDeprecatedComponents(github, context, changedFiles), + detectMaintainerAccess(context) ]); // Extract deprecated component info @@ -177,8 +180,11 @@ module.exports = async ({ github, context }) => { console.log('Computed labels:', finalLabels.join(', ')); - // Handle reviews - await handleReviews(github, context, finalLabels, originalLabelCount, deprecatedInfo, prFiles, totalAdditions, totalDeletions, MAX_LABELS, TOO_BIG_THRESHOLD); + // Handle reviews and org fork comment + await Promise.all([ + handleReviews(github, context, finalLabels, originalLabelCount, deprecatedInfo, prFiles, totalAdditions, totalDeletions, MAX_LABELS, TOO_BIG_THRESHOLD), + handleMaintainerAccessComment(github, context, maintainerAccess) + ]); // Apply labels await applyLabels(github, context, finalLabels); diff --git a/.github/scripts/auto-label-pr/reviews.js b/.github/scripts/auto-label-pr/reviews.js index 906e2c456ab..7ac136515de 100644 --- a/.github/scripts/auto-label-pr/reviews.js +++ b/.github/scripts/auto-label-pr/reviews.js @@ -2,7 +2,8 @@ const { BOT_COMMENT_MARKER, CODEOWNERS_MARKER, TOO_BIG_MARKER, - DEPRECATED_COMPONENT_MARKER + DEPRECATED_COMPONENT_MARKER, + ORG_FORK_MARKER } = require('./constants'); // Generate review messages @@ -136,6 +137,63 @@ async function handleReviews(github, context, finalLabels, originalLabelCount, d } } +// Handle maintainer access warning comment +async function handleMaintainerAccessComment(github, context, maintainerAccess) { + if (!maintainerAccess) { + return; + } + + const { owner, repo } = context.repo; + const pr_number = context.issue.number; + const prAuthor = context.payload.pull_request.user.login; + + // Check if we already posted the warning (iterate pages to exit early) + let existingComment; + for await (const { data: comments } of github.paginate.iterator( + github.rest.issues.listComments, + { owner, repo, issue_number: pr_number } + )) { + existingComment = comments.find(comment => + comment.user.type === 'Bot' && + comment.body && comment.body.includes(ORG_FORK_MARKER) + ); + if (existingComment) { + break; + } + } + + if (existingComment) { + console.log('Maintainer access warning comment already exists, skipping'); + return; + } + + let body; + if (maintainerAccess.isOrgFork) { + body = `${ORG_FORK_MARKER}\n### ⚠️ Organization Fork Detected\n\n` + + `Hey there @${prAuthor},\n` + + `It looks like this PR was submitted from a fork owned by the **${maintainerAccess.orgName}** organization. ` + + `GitHub does not allow maintainers to push changes to pull request branches when the fork is owned by an organization. ` + + `This means we won't be able to make small adjustments or fixups to your PR directly.\n\n` + + `To allow maintainer collaboration, please re-submit this PR from a personal fork instead.\n\n` + + `See: [Setting up the local repository](https://developers.esphome.io/contributing/development-environment/?h=org#set-up-the-local-repository) for more details.`; + } else { + body = `${ORG_FORK_MARKER}\n### ⚠️ Maintainer Access Disabled\n\n` + + `Hey there @${prAuthor},\n` + + `It looks like this PR does not have the "Allow edits from maintainers" option enabled. ` + + `This means we won't be able to make small adjustments or fixups to your PR directly.\n\n` + + `Please enable this option in the PR sidebar to allow maintainer collaboration.`; + } + + await github.rest.issues.createComment({ + owner, + repo, + issue_number: pr_number, + body + }); + console.log('Created maintainer access warning comment'); +} + module.exports = { - handleReviews + handleReviews, + handleMaintainerAccessComment }; From c04dfa922ecb5b71782ef250e7ba59bf7b63caa7 Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Fri, 10 Apr 2026 14:02:49 +1200 Subject: [PATCH 2/7] [hbridge] Move light pin switching to loop (#15615) --- esphome/components/hbridge/light/__init__.py | 2 +- .../hbridge/light/hbridge_light_output.h | 52 ++++++++++++------- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/esphome/components/hbridge/light/__init__.py b/esphome/components/hbridge/light/__init__.py index 65dd3196dff..ccb47237b64 100644 --- a/esphome/components/hbridge/light/__init__.py +++ b/esphome/components/hbridge/light/__init__.py @@ -8,7 +8,7 @@ from .. import hbridge_ns CODEOWNERS = ["@DotNetDann"] HBridgeLightOutput = hbridge_ns.class_( - "HBridgeLightOutput", cg.PollingComponent, light.LightOutput + "HBridgeLightOutput", cg.Component, light.LightOutput ) CONFIG_SCHEMA = light.RGB_LIGHT_SCHEMA.extend( diff --git a/esphome/components/hbridge/light/hbridge_light_output.h b/esphome/components/hbridge/light/hbridge_light_output.h index c3091548523..4e064d53522 100644 --- a/esphome/components/hbridge/light/hbridge_light_output.h +++ b/esphome/components/hbridge/light/hbridge_light_output.h @@ -1,20 +1,17 @@ #pragma once -#include "esphome/core/component.h" -#include "esphome/components/output/float_output.h" #include "esphome/components/light/light_output.h" -#include "esphome/core/log.h" +#include "esphome/components/output/float_output.h" +#include "esphome/core/component.h" +#include "esphome/core/helpers.h" namespace esphome { namespace hbridge { -// Using PollingComponent as the updates are more consistent and reduces flickering -class HBridgeLightOutput : public PollingComponent, public light::LightOutput { +class HBridgeLightOutput : public Component, public light::LightOutput { public: - HBridgeLightOutput() : PollingComponent(1) {} - - void set_pina_pin(output::FloatOutput *pina_pin) { pina_pin_ = pina_pin; } - void set_pinb_pin(output::FloatOutput *pinb_pin) { pinb_pin_ = pinb_pin; } + void set_pina_pin(output::FloatOutput *pina_pin) { this->pina_pin_ = pina_pin; } + void set_pinb_pin(output::FloatOutput *pinb_pin) { this->pinb_pin_ = pinb_pin; } light::LightTraits get_traits() override { auto traits = light::LightTraits(); @@ -24,16 +21,16 @@ class HBridgeLightOutput : public PollingComponent, public light::LightOutput { return traits; } - void setup() override { this->forward_direction_ = false; } + void setup() override { this->disable_loop(); } - void update() override { - // This method runs around 60 times per second - // We cannot do the PWM ourselves so we are reliant on the hardware PWM - if (!this->forward_direction_) { // First LED Direction + void loop() override { + // Only called when both channels are active — alternate H-bridge direction + // each iteration to multiplex cold and warm white. + if (!this->forward_direction_) { this->pina_pin_->set_level(this->pina_duty_); this->pinb_pin_->set_level(0); this->forward_direction_ = true; - } else { // Second LED Direction + } else { this->pina_pin_->set_level(0); this->pinb_pin_->set_level(this->pinb_duty_); this->forward_direction_ = false; @@ -43,15 +40,32 @@ class HBridgeLightOutput : public PollingComponent, public light::LightOutput { float get_setup_priority() const override { return setup_priority::HARDWARE; } void write_state(light::LightState *state) override { - state->current_values_as_cwww(&this->pina_duty_, &this->pinb_duty_, false); + float new_pina, new_pinb; + state->current_values_as_cwww(&new_pina, &new_pinb, false); + + this->pina_duty_ = new_pina; + this->pinb_duty_ = new_pinb; + + if (new_pina != 0.0f && new_pinb != 0.0f) { + // Both channels active — need loop to alternate H-bridge direction + this->high_freq_.start(); + this->enable_loop(); + } else { + // Zero or one channel active — drive pins directly, no multiplexing needed + this->high_freq_.stop(); + this->disable_loop(); + this->pina_pin_->set_level(new_pina); + this->pinb_pin_->set_level(new_pinb); + } } protected: output::FloatOutput *pina_pin_; output::FloatOutput *pinb_pin_; - float pina_duty_ = 0; - float pinb_duty_ = 0; - bool forward_direction_ = false; + float pina_duty_{0}; + float pinb_duty_{0}; + bool forward_direction_{false}; + HighFrequencyLoopRequester high_freq_; }; } // namespace hbridge From c90fa2378ab1fa8c5b30c118ff1f6ca5d10c21ac Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 9 Apr 2026 17:29:00 -1000 Subject: [PATCH 3/7] [tca9555] Add interrupt pin support (#15613) --- esphome/components/tca9555/__init__.py | 4 ++++ esphome/components/tca9555/tca9555.cpp | 19 ++++++++++++++++++- esphome/components/tca9555/tca9555.h | 5 +++++ tests/components/tca9555/common.yaml | 4 ++++ tests/components/tca9555/test.esp32-idf.yaml | 3 +++ .../components/tca9555/test.esp8266-ard.yaml | 3 +++ tests/components/tca9555/test.rp2040-ard.yaml | 3 +++ 7 files changed, 40 insertions(+), 1 deletion(-) diff --git a/esphome/components/tca9555/__init__.py b/esphome/components/tca9555/__init__.py index f42e0fe398b..5f571fcea6e 100644 --- a/esphome/components/tca9555/__init__.py +++ b/esphome/components/tca9555/__init__.py @@ -5,6 +5,7 @@ import esphome.config_validation as cv from esphome.const import ( CONF_ID, CONF_INPUT, + CONF_INTERRUPT_PIN, CONF_INVERTED, CONF_MODE, CONF_NUMBER, @@ -27,6 +28,7 @@ CONFIG_SCHEMA = ( cv.Schema( { cv.Required(CONF_ID): cv.declare_id(TCA9555Component), + cv.Optional(CONF_INTERRUPT_PIN): pins.internal_gpio_input_pin_schema, } ) .extend(cv.COMPONENT_SCHEMA) @@ -38,6 +40,8 @@ async def to_code(config): var = cg.new_Pvariable(config[CONF_ID]) await cg.register_component(var, config) await i2c.register_i2c_device(var, config) + if interrupt_pin := config.get(CONF_INTERRUPT_PIN): + cg.add(var.set_interrupt_pin(await cg.gpio_pin_expression(interrupt_pin))) def validate_mode(value): diff --git a/esphome/components/tca9555/tca9555.cpp b/esphome/components/tca9555/tca9555.cpp index 79c52538983..3eb794df441 100644 --- a/esphome/components/tca9555/tca9555.cpp +++ b/esphome/components/tca9555/tca9555.cpp @@ -24,9 +24,18 @@ void TCA9555Component::setup() { this->mark_failed(); return; } + + if (this->interrupt_pin_ != nullptr) { + this->interrupt_pin_->setup(); + this->interrupt_pin_->attach_interrupt(&TCA9555Component::gpio_intr, this, gpio::INTERRUPT_FALLING_EDGE); + this->set_invalidate_on_read_(false); + } + this->disable_loop(); } +void IRAM_ATTR TCA9555Component::gpio_intr(TCA9555Component *arg) { arg->enable_loop_soon_any_context(); } void TCA9555Component::dump_config() { ESP_LOGCONFIG(TAG, "TCA9555:"); + LOG_PIN(" Interrupt Pin: ", this->interrupt_pin_); LOG_I2C_DEVICE(this) if (this->is_failed()) { ESP_LOGE(TAG, ESP_LOG_MSG_COMM_FAIL); @@ -36,6 +45,9 @@ void TCA9555Component::pin_mode(uint8_t pin, gpio::Flags flags) { if (flags == gpio::FLAG_INPUT) { // Set mode mask bit this->mode_mask_ |= 1 << pin; + if (this->interrupt_pin_ == nullptr) { + this->enable_loop(); + } } else if (flags == gpio::FLAG_OUTPUT) { // Clear mode mask bit this->mode_mask_ &= ~(1 << pin); @@ -43,7 +55,12 @@ void TCA9555Component::pin_mode(uint8_t pin, gpio::Flags flags) { // Write GPIO to enable input mode this->write_gpio_modes_(); } -void TCA9555Component::loop() { this->reset_pin_cache_(); } +void TCA9555Component::loop() { + this->reset_pin_cache_(); + if (this->interrupt_pin_ != nullptr) { + this->disable_loop(); + } +} bool TCA9555Component::read_gpio_outputs_() { if (this->is_failed()) diff --git a/esphome/components/tca9555/tca9555.h b/esphome/components/tca9555/tca9555.h index 9f7273b1e7e..d4d070013c7 100644 --- a/esphome/components/tca9555/tca9555.h +++ b/esphome/components/tca9555/tca9555.h @@ -24,7 +24,10 @@ class TCA9555Component : public Component, void loop() override; + void set_interrupt_pin(InternalGPIOPin *pin) { this->interrupt_pin_ = pin; } + protected: + static void IRAM_ATTR gpio_intr(TCA9555Component *arg); 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; @@ -39,6 +42,8 @@ class TCA9555Component : public Component, bool read_gpio_modes_(); bool write_gpio_modes_(); bool read_gpio_outputs_(); + + InternalGPIOPin *interrupt_pin_{nullptr}; }; /// Helper class to expose a TCA9555 pin as an internal input GPIO pin. diff --git a/tests/components/tca9555/common.yaml b/tests/components/tca9555/common.yaml index 82b4c959d81..d1a68c575a8 100644 --- a/tests/components/tca9555/common.yaml +++ b/tests/components/tca9555/common.yaml @@ -2,6 +2,10 @@ tca9555: - id: tca9555_hub i2c_id: i2c_bus address: 0x21 + - id: tca9555_hub_int + i2c_id: i2c_bus + address: 0x22 + interrupt_pin: ${interrupt_pin} binary_sensor: - platform: gpio diff --git a/tests/components/tca9555/test.esp32-idf.yaml b/tests/components/tca9555/test.esp32-idf.yaml index b47e39c3898..8c3b341dce0 100644 --- a/tests/components/tca9555/test.esp32-idf.yaml +++ b/tests/components/tca9555/test.esp32-idf.yaml @@ -1,3 +1,6 @@ +substitutions: + interrupt_pin: GPIO15 + packages: i2c: !include ../../test_build_components/common/i2c/esp32-idf.yaml diff --git a/tests/components/tca9555/test.esp8266-ard.yaml b/tests/components/tca9555/test.esp8266-ard.yaml index 4a98b9388ab..69b243bfd80 100644 --- a/tests/components/tca9555/test.esp8266-ard.yaml +++ b/tests/components/tca9555/test.esp8266-ard.yaml @@ -1,3 +1,6 @@ +substitutions: + interrupt_pin: GPIO15 + packages: i2c: !include ../../test_build_components/common/i2c/esp8266-ard.yaml diff --git a/tests/components/tca9555/test.rp2040-ard.yaml b/tests/components/tca9555/test.rp2040-ard.yaml index 319a7c71a65..b8ad1e47925 100644 --- a/tests/components/tca9555/test.rp2040-ard.yaml +++ b/tests/components/tca9555/test.rp2040-ard.yaml @@ -1,3 +1,6 @@ +substitutions: + interrupt_pin: GPIO2 + packages: i2c: !include ../../test_build_components/common/i2c/rp2040-ard.yaml From 9cf9b02ba2a3f1fe634859da93ee38c1ca25d30e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 9 Apr 2026 17:29:26 -1000 Subject: [PATCH 4/7] [pca6416a] Add interrupt pin support (#15614) --- esphome/components/pca6416a/__init__.py | 10 +++++++++- esphome/components/pca6416a/pca6416a.cpp | 18 ++++++++++++++++++ esphome/components/pca6416a/pca6416a.h | 4 ++++ tests/components/pca6416a/common.yaml | 4 ++++ tests/components/pca6416a/test.esp32-idf.yaml | 3 +++ .../components/pca6416a/test.esp8266-ard.yaml | 3 +++ tests/components/pca6416a/test.rp2040-ard.yaml | 3 +++ 7 files changed, 44 insertions(+), 1 deletion(-) diff --git a/esphome/components/pca6416a/__init__.py b/esphome/components/pca6416a/__init__.py index b6e156e7ff4..813bb35c482 100644 --- a/esphome/components/pca6416a/__init__.py +++ b/esphome/components/pca6416a/__init__.py @@ -5,6 +5,7 @@ import esphome.config_validation as cv from esphome.const import ( CONF_ID, CONF_INPUT, + CONF_INTERRUPT_PIN, CONF_INVERTED, CONF_MODE, CONF_NUMBER, @@ -25,7 +26,12 @@ PCA6416AGPIOPin = pca6416a_ns.class_( CONF_PCA6416A = "pca6416a" CONFIG_SCHEMA = ( - cv.Schema({cv.Required(CONF_ID): cv.declare_id(PCA6416AComponent)}) + cv.Schema( + { + cv.Required(CONF_ID): cv.declare_id(PCA6416AComponent), + cv.Optional(CONF_INTERRUPT_PIN): pins.internal_gpio_input_pin_schema, + } + ) .extend(cv.COMPONENT_SCHEMA) .extend(i2c.i2c_device_schema(0x21)) ) @@ -35,6 +41,8 @@ async def to_code(config): var = cg.new_Pvariable(config[CONF_ID]) await cg.register_component(var, config) await i2c.register_i2c_device(var, config) + if interrupt_pin := config.get(CONF_INTERRUPT_PIN): + cg.add(var.set_interrupt_pin(await cg.gpio_pin_expression(interrupt_pin))) def validate_mode(value): diff --git a/esphome/components/pca6416a/pca6416a.cpp b/esphome/components/pca6416a/pca6416a.cpp index f393af88cea..dc7463b01ba 100644 --- a/esphome/components/pca6416a/pca6416a.cpp +++ b/esphome/components/pca6416a/pca6416a.cpp @@ -49,11 +49,22 @@ void PCA6416AComponent::setup() { ESP_LOGD(TAG, "Initialization complete. Warning: %d, Error: %d", this->status_has_warning(), this->status_has_error()); + + if (this->interrupt_pin_ != nullptr) { + this->interrupt_pin_->setup(); + this->interrupt_pin_->attach_interrupt(&PCA6416AComponent::gpio_intr, this, gpio::INTERRUPT_FALLING_EDGE); + this->set_invalidate_on_read_(false); + } + this->disable_loop(); } +void IRAM_ATTR PCA6416AComponent::gpio_intr(PCA6416AComponent *arg) { arg->enable_loop_soon_any_context(); } void PCA6416AComponent::loop() { // Invalidate cache at the start of each loop this->reset_pin_cache_(); + if (this->interrupt_pin_ != nullptr) { + this->disable_loop(); + } } void PCA6416AComponent::dump_config() { @@ -62,6 +73,7 @@ void PCA6416AComponent::dump_config() { } else { ESP_LOGCONFIG(TAG, "PCA6416A:"); } + LOG_PIN(" Interrupt Pin: ", this->interrupt_pin_); LOG_I2C_DEVICE(this) if (this->is_failed()) { ESP_LOGE(TAG, ESP_LOG_MSG_COMM_FAIL); @@ -101,6 +113,9 @@ void PCA6416AComponent::pin_mode(uint8_t pin, gpio::Flags flags) { this->update_register_(pin, true, pull_dir); this->update_register_(pin, false, pull_en); } + if (this->interrupt_pin_ == nullptr) { + this->enable_loop(); + } } else if (flags == (gpio::FLAG_INPUT | gpio::FLAG_PULLUP)) { this->update_register_(pin, true, io_dir); if (has_pullup_) { @@ -109,6 +124,9 @@ void PCA6416AComponent::pin_mode(uint8_t pin, gpio::Flags flags) { } else { ESP_LOGW(TAG, "Your PCA6416A does not support pull-up resistors"); } + if (this->interrupt_pin_ == nullptr) { + this->enable_loop(); + } } else if (flags == gpio::FLAG_OUTPUT) { this->update_register_(pin, false, io_dir); } diff --git a/esphome/components/pca6416a/pca6416a.h b/esphome/components/pca6416a/pca6416a.h index 138a51cc208..4d2e6b219e4 100644 --- a/esphome/components/pca6416a/pca6416a.h +++ b/esphome/components/pca6416a/pca6416a.h @@ -24,7 +24,10 @@ class PCA6416AComponent : public Component, void dump_config() override; + void set_interrupt_pin(InternalGPIOPin *pin) { this->interrupt_pin_ = pin; } + protected: + static void IRAM_ATTR gpio_intr(PCA6416AComponent *arg); // Virtual methods from CachedGpioExpander bool digital_read_hw(uint8_t pin) override; bool digital_read_cache(uint8_t pin) override; @@ -43,6 +46,7 @@ class PCA6416AComponent : public Component, esphome::i2c::ErrorCode last_error_; /// Only the PCAL6416A has pull-up resistors bool has_pullup_{false}; + InternalGPIOPin *interrupt_pin_{nullptr}; }; /// Helper class to expose a PCA6416A pin as an internal input GPIO pin. diff --git a/tests/components/pca6416a/common.yaml b/tests/components/pca6416a/common.yaml index 9ad6e2fb158..09083c6c156 100644 --- a/tests/components/pca6416a/common.yaml +++ b/tests/components/pca6416a/common.yaml @@ -2,6 +2,10 @@ pca6416a: - id: pca6416a_hub i2c_id: i2c_bus address: 0x21 + - id: pca6416a_hub_int + i2c_id: i2c_bus + address: 0x22 + interrupt_pin: ${interrupt_pin} binary_sensor: - platform: gpio diff --git a/tests/components/pca6416a/test.esp32-idf.yaml b/tests/components/pca6416a/test.esp32-idf.yaml index b47e39c3898..8c3b341dce0 100644 --- a/tests/components/pca6416a/test.esp32-idf.yaml +++ b/tests/components/pca6416a/test.esp32-idf.yaml @@ -1,3 +1,6 @@ +substitutions: + interrupt_pin: GPIO15 + packages: i2c: !include ../../test_build_components/common/i2c/esp32-idf.yaml diff --git a/tests/components/pca6416a/test.esp8266-ard.yaml b/tests/components/pca6416a/test.esp8266-ard.yaml index 4a98b9388ab..69b243bfd80 100644 --- a/tests/components/pca6416a/test.esp8266-ard.yaml +++ b/tests/components/pca6416a/test.esp8266-ard.yaml @@ -1,3 +1,6 @@ +substitutions: + interrupt_pin: GPIO15 + packages: i2c: !include ../../test_build_components/common/i2c/esp8266-ard.yaml diff --git a/tests/components/pca6416a/test.rp2040-ard.yaml b/tests/components/pca6416a/test.rp2040-ard.yaml index 319a7c71a65..b8ad1e47925 100644 --- a/tests/components/pca6416a/test.rp2040-ard.yaml +++ b/tests/components/pca6416a/test.rp2040-ard.yaml @@ -1,3 +1,6 @@ +substitutions: + interrupt_pin: GPIO2 + packages: i2c: !include ../../test_build_components/common/i2c/rp2040-ard.yaml From 17209df7b5f97ad3dc6d4442366959661362bf32 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 9 Apr 2026 17:29:52 -1000 Subject: [PATCH 5/7] [mcp23016] Add interrupt pin support (#15616) --- esphome/components/mcp23016/__init__.py | 4 ++++ esphome/components/mcp23016/mcp23016.cpp | 14 ++++++++++++++ esphome/components/mcp23016/mcp23016.h | 4 ++++ tests/components/mcp23016/common.yaml | 8 ++++++-- tests/components/mcp23016/test.esp32-idf.yaml | 3 +++ tests/components/mcp23016/test.esp8266-ard.yaml | 3 +++ tests/components/mcp23016/test.rp2040-ard.yaml | 3 +++ 7 files changed, 37 insertions(+), 2 deletions(-) diff --git a/esphome/components/mcp23016/__init__.py b/esphome/components/mcp23016/__init__.py index 5a1f011617a..b71d57498a2 100644 --- a/esphome/components/mcp23016/__init__.py +++ b/esphome/components/mcp23016/__init__.py @@ -5,6 +5,7 @@ import esphome.config_validation as cv from esphome.const import ( CONF_ID, CONF_INPUT, + CONF_INTERRUPT_PIN, CONF_INVERTED, CONF_MODE, CONF_NUMBER, @@ -24,6 +25,7 @@ CONFIG_SCHEMA = ( cv.Schema( { cv.Required(CONF_ID): cv.declare_id(MCP23016), + cv.Optional(CONF_INTERRUPT_PIN): pins.internal_gpio_input_pin_schema, } ) .extend(cv.COMPONENT_SCHEMA) @@ -35,6 +37,8 @@ async def to_code(config): var = cg.new_Pvariable(config[CONF_ID]) await cg.register_component(var, config) await i2c.register_i2c_device(var, config) + if interrupt_pin := config.get(CONF_INTERRUPT_PIN): + cg.add(var.set_interrupt_pin(await cg.gpio_pin_expression(interrupt_pin))) def validate_mode(value): diff --git a/esphome/components/mcp23016/mcp23016.cpp b/esphome/components/mcp23016/mcp23016.cpp index fbdb6903b84..118a77ce378 100644 --- a/esphome/components/mcp23016/mcp23016.cpp +++ b/esphome/components/mcp23016/mcp23016.cpp @@ -24,11 +24,22 @@ void MCP23016::setup() { // all pins input this->write_reg_(MCP23016_IODIR1, 0xFFFF); + + if (this->interrupt_pin_ != nullptr) { + this->interrupt_pin_->setup(); + this->interrupt_pin_->attach_interrupt(&MCP23016::gpio_intr, this, gpio::INTERRUPT_FALLING_EDGE); + this->set_invalidate_on_read_(false); + } + this->disable_loop(); } +void IRAM_ATTR MCP23016::gpio_intr(MCP23016 *arg) { arg->enable_loop_soon_any_context(); } void MCP23016::loop() { // Invalidate cache at the start of each loop this->reset_pin_cache_(); + if (this->interrupt_pin_ != nullptr) { + this->disable_loop(); + } } bool MCP23016::digital_read_hw(uint8_t pin) { return this->read_reg_(MCP23016_GP1, &this->input_mask_); } @@ -37,6 +48,9 @@ void MCP23016::digital_write_hw(uint8_t pin, bool value) { this->update_reg_(pin void MCP23016::pin_mode(uint8_t pin, gpio::Flags flags) { if (flags == gpio::FLAG_INPUT) { this->update_reg_(pin, true, MCP23016_IODIR1); + if (this->interrupt_pin_ == nullptr) { + this->enable_loop(); + } } else if (flags == gpio::FLAG_OUTPUT) { this->update_reg_(pin, false, MCP23016_IODIR1); } diff --git a/esphome/components/mcp23016/mcp23016.h b/esphome/components/mcp23016/mcp23016.h index 494bc9c197a..32149ba3e24 100644 --- a/esphome/components/mcp23016/mcp23016.h +++ b/esphome/components/mcp23016/mcp23016.h @@ -35,7 +35,10 @@ class MCP23016 : public Component, public i2c::I2CDevice, public gpio_expander:: float get_setup_priority() const override; + void set_interrupt_pin(InternalGPIOPin *pin) { this->interrupt_pin_ = pin; } + protected: + static void IRAM_ATTR gpio_intr(MCP23016 *arg); // Virtual methods from CachedGpioExpander bool digital_read_hw(uint8_t pin) override; bool digital_read_cache(uint8_t pin) override; @@ -51,6 +54,7 @@ class MCP23016 : public Component, public i2c::I2CDevice, public gpio_expander:: uint16_t olat_{0x0000}; // Cache for input values (16-bit combined for both banks) uint16_t input_mask_{0x0000}; + InternalGPIOPin *interrupt_pin_{nullptr}; }; class MCP23016GPIOPin : public GPIOPin { diff --git a/tests/components/mcp23016/common.yaml b/tests/components/mcp23016/common.yaml index e8e3ad9d08a..81f38b3f52e 100644 --- a/tests/components/mcp23016/common.yaml +++ b/tests/components/mcp23016/common.yaml @@ -1,6 +1,10 @@ mcp23016: - i2c_id: i2c_bus - id: mcp23016_hub + - i2c_id: i2c_bus + id: mcp23016_hub + - i2c_id: i2c_bus + id: mcp23016_hub_int + address: 0x21 + interrupt_pin: ${interrupt_pin} binary_sensor: - platform: gpio diff --git a/tests/components/mcp23016/test.esp32-idf.yaml b/tests/components/mcp23016/test.esp32-idf.yaml index b47e39c3898..8c3b341dce0 100644 --- a/tests/components/mcp23016/test.esp32-idf.yaml +++ b/tests/components/mcp23016/test.esp32-idf.yaml @@ -1,3 +1,6 @@ +substitutions: + interrupt_pin: GPIO15 + packages: i2c: !include ../../test_build_components/common/i2c/esp32-idf.yaml diff --git a/tests/components/mcp23016/test.esp8266-ard.yaml b/tests/components/mcp23016/test.esp8266-ard.yaml index 4a98b9388ab..69b243bfd80 100644 --- a/tests/components/mcp23016/test.esp8266-ard.yaml +++ b/tests/components/mcp23016/test.esp8266-ard.yaml @@ -1,3 +1,6 @@ +substitutions: + interrupt_pin: GPIO15 + packages: i2c: !include ../../test_build_components/common/i2c/esp8266-ard.yaml diff --git a/tests/components/mcp23016/test.rp2040-ard.yaml b/tests/components/mcp23016/test.rp2040-ard.yaml index 319a7c71a65..b8ad1e47925 100644 --- a/tests/components/mcp23016/test.rp2040-ard.yaml +++ b/tests/components/mcp23016/test.rp2040-ard.yaml @@ -1,3 +1,6 @@ +substitutions: + interrupt_pin: GPIO2 + packages: i2c: !include ../../test_build_components/common/i2c/rp2040-ard.yaml From ec420d57923bb6c38bf780e1bfcf854e48b51896 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 9 Apr 2026 17:33:56 -1000 Subject: [PATCH 6/7] [api] Add (inline_encode) proto option for sub-message inlining (#15599) --- esphome/components/api/api.proto | 1 + esphome/components/api/api_options.proto | 1 + esphome/components/api/api_pb2.cpp | 47 ++-- esphome/components/api/api_pb2.h | 2 - esphome/components/api/proto.h | 6 + script/api_protobuf/api_protobuf.py | 293 +++++++++++++++++++++-- 6 files changed, 304 insertions(+), 46 deletions(-) diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index 33d16f0339b..e4d0c2d16df 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -1625,6 +1625,7 @@ message BluetoothLEAdvertisementResponse { } message BluetoothLERawAdvertisement { + option (inline_encode) = true; uint64 address = 1 [(force) = true]; sint32 rssi = 2 [(force) = true]; uint32 address_type = 3 [(max_value) = 4]; diff --git a/esphome/components/api/api_options.proto b/esphome/components/api/api_options.proto index 0f71268d700..dacc290e31e 100644 --- a/esphome/components/api/api_options.proto +++ b/esphome/components/api/api_options.proto @@ -22,6 +22,7 @@ extend google.protobuf.MessageOptions { optional bool log = 1039 [default=true]; optional bool no_delay = 1040 [default=false]; optional string base_class = 1041; + optional bool inline_encode = 1042 [default=false]; } extend google.protobuf.FieldOptions { diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index d27cfa57cf9..c2d513f0d33 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -2328,40 +2328,37 @@ bool SubscribeBluetoothLEAdvertisementsRequest::decode_varint(uint32_t field_id, } return true; } -uint8_t *BluetoothLERawAdvertisement::encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const { - uint8_t *__restrict__ pos = buffer.get_pos(); - ProtoEncode::write_raw_byte(pos PROTO_ENCODE_DEBUG_ARG, 8); - ProtoEncode::encode_varint_raw_64(pos PROTO_ENCODE_DEBUG_ARG, this->address); - ProtoEncode::write_raw_byte(pos PROTO_ENCODE_DEBUG_ARG, 16); - ProtoEncode::encode_varint_raw_short(pos PROTO_ENCODE_DEBUG_ARG, encode_zigzag32(this->rssi)); - if (this->address_type) { - ProtoEncode::write_raw_byte(pos PROTO_ENCODE_DEBUG_ARG, 24); - ProtoEncode::encode_varint_raw(pos PROTO_ENCODE_DEBUG_ARG, this->address_type); - } - ProtoEncode::write_raw_byte(pos PROTO_ENCODE_DEBUG_ARG, 34); - ProtoEncode::write_raw_byte(pos PROTO_ENCODE_DEBUG_ARG, static_cast(this->data_len)); - ProtoEncode::encode_raw(pos PROTO_ENCODE_DEBUG_ARG, this->data, this->data_len); - return pos; -} -uint32_t BluetoothLERawAdvertisement::calculate_size() const { - uint32_t size = 0; - size += ProtoSize::calc_uint64_force(1, this->address); - size += ProtoSize::calc_sint32_force(1, this->rssi); - size += this->address_type ? 2 : 0; - size += 2 + this->data_len; - return size; -} uint8_t *BluetoothLERawAdvertisementsResponse::encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const { uint8_t *__restrict__ pos = buffer.get_pos(); for (uint16_t i = 0; i < this->advertisements_len; i++) { - ProtoEncode::encode_sub_message(pos PROTO_ENCODE_DEBUG_ARG, buffer, 1, this->advertisements[i]); + auto &sub_msg = this->advertisements[i]; + ProtoEncode::write_raw_byte(pos PROTO_ENCODE_DEBUG_ARG, 10); + uint8_t *len_pos = pos; + ProtoEncode::reserve_byte(pos PROTO_ENCODE_DEBUG_ARG); + ProtoEncode::write_raw_byte(pos PROTO_ENCODE_DEBUG_ARG, 8); + ProtoEncode::encode_varint_raw_64(pos PROTO_ENCODE_DEBUG_ARG, sub_msg.address); + ProtoEncode::write_raw_byte(pos PROTO_ENCODE_DEBUG_ARG, 16); + ProtoEncode::encode_varint_raw_short(pos PROTO_ENCODE_DEBUG_ARG, encode_zigzag32(sub_msg.rssi)); + if (sub_msg.address_type) { + ProtoEncode::write_raw_byte(pos PROTO_ENCODE_DEBUG_ARG, 24); + ProtoEncode::encode_varint_raw(pos PROTO_ENCODE_DEBUG_ARG, sub_msg.address_type); + } + ProtoEncode::write_raw_byte(pos PROTO_ENCODE_DEBUG_ARG, 34); + ProtoEncode::write_raw_byte(pos PROTO_ENCODE_DEBUG_ARG, static_cast(sub_msg.data_len)); + ProtoEncode::encode_raw(pos PROTO_ENCODE_DEBUG_ARG, sub_msg.data, sub_msg.data_len); + *len_pos = static_cast(pos - len_pos - 1); } return pos; } uint32_t BluetoothLERawAdvertisementsResponse::calculate_size() const { uint32_t size = 0; for (uint16_t i = 0; i < this->advertisements_len; i++) { - size += ProtoSize::calc_message_force(1, this->advertisements[i].calculate_size()); + auto &sub_msg = this->advertisements[i]; + size += 2; + size += ProtoSize::calc_uint64_force(1, sub_msg.address); + size += ProtoSize::calc_sint32_force(1, sub_msg.rssi); + size += sub_msg.address_type ? 2 : 0; + size += 2 + sub_msg.data_len; } return size; } diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index 3b239db36c5..5827a8728e6 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -1888,8 +1888,6 @@ class BluetoothLERawAdvertisement final : public ProtoMessage { uint32_t address_type{0}; uint8_t data[62]{}; uint8_t data_len{0}; - uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const; - uint32_t calculate_size() const; #ifdef HAS_PROTO_MESSAGE_DUMP const char *dump_to(DumpBuffer &out) const override; #endif diff --git a/esphome/components/api/proto.h b/esphome/components/api/proto.h index e0a4e03189e..8cac7fff3bf 100644 --- a/esphome/components/api/proto.h +++ b/esphome/components/api/proto.h @@ -352,6 +352,12 @@ class ProtoEncode { PROTO_ENCODE_CHECK_BOUNDS(pos, 1); *pos++ = b; } + /// Reserve one byte for later backpatch (e.g., sub-message length). + /// Advances pos past the reserved byte without writing a value. + static inline void ESPHOME_ALWAYS_INLINE reserve_byte(uint8_t *__restrict__ &pos PROTO_ENCODE_DEBUG_PARAM) { + PROTO_ENCODE_CHECK_BOUNDS(pos, 1); + pos++; + } /// Write raw bytes to the buffer (no tag, no length prefix). static inline void ESPHOME_ALWAYS_INLINE encode_raw(uint8_t *__restrict__ &pos PROTO_ENCODE_DEBUG_PARAM, const void *data, size_t len) { diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 526644842da..39bfc865d02 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -60,6 +60,10 @@ FILE_HEADER = """// This file was automatically generated with a tool. # Maps enum type name (e.g. ".BluetoothDeviceRequestType") to max enum value. _enum_max_values: dict[str, int] = {} +# Populated by main() before message generation. +# Maps message name (e.g. "BluetoothLERawAdvertisement") to its descriptor. +_message_desc_map: dict[str, Any] = {} + def indent_list(text: str, padding: str = " ") -> list[str]: """Indent each line of the given text with the specified padding.""" @@ -427,6 +431,23 @@ class TypeInfo(ABC): Estimated size in bytes including field ID and typical data """ + def get_max_encoded_size(self) -> int | None: + """Get the maximum possible encoded size in bytes for this field. + + Returns the worst-case encoded size including field ID and maximum + possible value encoding. Returns None if the size is unbounded + (e.g., variable-length strings without max_data_length). + + Used by (inline_encode) validation to ensure sub-messages fit in a + single-byte length varint (< 128 bytes). + """ + return None # Unbounded by default + + +def _varint_max_size(bits: int) -> int: + """Return the maximum varint encoding size for a value with the given number of bits.""" + return (max(bits, 1) + 6) // 7 # ceil(bits / 7), min 1 byte for varint(0) + TYPE_INFO: dict[int, TypeInfo] = {} @@ -514,8 +535,30 @@ def register_type(name: int): return func +class FixedSizeTypeMixin: + """Mixin for types with a known fixed encoded size (float, double, fixed32, fixed64).""" + + def get_max_encoded_size(self) -> int: + return self.calculate_field_id_size() + self.get_fixed_size_bytes() + + +class VarintTypeMixin: + """Mixin for varint types. Subclasses set _varint_max_bits.""" + + _varint_max_bits: int = 64 # Default to worst case + + def get_max_encoded_size(self) -> int: + max_val = self.max_value + if max_val is not None: + return self.calculate_field_id_size() + _varint_max_size( + max_val.bit_length() if max_val > 0 else 1 + ) + return self.calculate_field_id_size() + _varint_max_size(self._varint_max_bits) + + @register_type(1) -class DoubleType(TypeInfo): +class DoubleType(FixedSizeTypeMixin, TypeInfo): + # Unsupported but defined for completeness cpp_type = "double" default_value = "0.0" decode_64bit = "value.as_double()" @@ -541,7 +584,7 @@ class DoubleType(TypeInfo): @register_type(2) -class FloatType(TypeInfo): +class FloatType(FixedSizeTypeMixin, TypeInfo): cpp_type = "float" default_value = "0.0f" decode_32bit = "value.as_float()" @@ -567,8 +610,9 @@ class FloatType(TypeInfo): @register_type(3) -class Int64Type(TypeInfo): +class Int64Type(VarintTypeMixin, TypeInfo): cpp_type = "int64_t" + _varint_max_bits = 64 default_value = "0" decode_varint = "static_cast(value)" encode_func = "encode_int64" @@ -587,8 +631,9 @@ class Int64Type(TypeInfo): @register_type(4) -class UInt64Type(TypeInfo): +class UInt64Type(VarintTypeMixin, TypeInfo): cpp_type = "uint64_t" + _varint_max_bits = 64 default_value = "0" decode_varint = "value" encode_func = "encode_uint64" @@ -607,8 +652,9 @@ class UInt64Type(TypeInfo): @register_type(5) -class Int32Type(TypeInfo): +class Int32Type(VarintTypeMixin, TypeInfo): cpp_type = "int32_t" + _varint_max_bits = 64 # int32 is sign-extended to 64 bits in protobuf default_value = "0" decode_varint = "static_cast(value)" encode_func = "encode_int32" @@ -627,7 +673,7 @@ class Int32Type(TypeInfo): @register_type(6) -class Fixed64Type(TypeInfo): +class Fixed64Type(FixedSizeTypeMixin, TypeInfo): cpp_type = "uint64_t" default_value = "0" decode_64bit = "value.as_fixed64()" @@ -653,7 +699,7 @@ class Fixed64Type(TypeInfo): @register_type(7) -class Fixed32Type(TypeInfo): +class Fixed32Type(FixedSizeTypeMixin, TypeInfo): cpp_type = "uint32_t" default_value = "0" decode_32bit = "value.as_fixed32()" @@ -689,7 +735,8 @@ class Fixed32Type(TypeInfo): @register_type(8) -class BoolType(TypeInfo): +class BoolType(VarintTypeMixin, TypeInfo): + _varint_max_bits = 1 cpp_type = "bool" default_value = "false" decode_varint = "value != 0" @@ -807,6 +854,16 @@ class StringType(TypeInfo): def get_estimated_size(self) -> int: return self.calculate_field_id_size() + 8 # field ID + 8 bytes typical string + def get_max_encoded_size(self) -> int | None: + max_len = self.max_data_length + if max_len is not None: + return ( + self.calculate_field_id_size() + + _varint_max_size(max_len.bit_length()) + + max_len + ) + return None # Unbounded + @register_type(11) class MessageType(TypeInfo): @@ -1122,6 +1179,16 @@ class PointerToStringBufferType(PointerToBufferTypeBase): def get_estimated_size(self) -> int: return self.calculate_field_id_size() + 8 # field ID + 8 bytes typical string + def get_max_encoded_size(self) -> int | None: + max_len = self.max_data_length + if max_len is not None: + return ( + self.calculate_field_id_size() + + _varint_max_size(max_len.bit_length()) + + max_len + ) + return None + class PackedBufferTypeInfo(TypeInfo): """Type for packed repeated fields that expose raw buffer instead of decoding. @@ -1299,14 +1366,23 @@ class FixedArrayBytesType(TypeInfo): self.calculate_field_id_size() + 1 + 31 ) # field ID + length byte + typical 31 bytes + def get_max_encoded_size(self) -> int: + # field_id + varint(array_size) + array_size + return ( + self.calculate_field_id_size() + + _varint_max_size(self.array_size.bit_length()) + + self.array_size + ) + @property def wire_type(self) -> WireType: return WireType.LENGTH_DELIMITED @register_type(13) -class UInt32Type(TypeInfo): +class UInt32Type(VarintTypeMixin, TypeInfo): cpp_type = "uint32_t" + _varint_max_bits = 32 default_value = "0" decode_varint = "value" encode_func = "encode_uint32" @@ -1328,7 +1404,9 @@ class UInt32Type(TypeInfo): @register_type(14) -class EnumType(TypeInfo): +class EnumType(VarintTypeMixin, TypeInfo): + _varint_max_bits = 32 + @property def cpp_type(self) -> str: return f"enums::{self._field.type_name[1:]}" @@ -1379,7 +1457,7 @@ class EnumType(TypeInfo): @register_type(15) -class SFixed32Type(TypeInfo): +class SFixed32Type(FixedSizeTypeMixin, TypeInfo): cpp_type = "int32_t" default_value = "0" decode_32bit = "value.as_sfixed32()" @@ -1405,7 +1483,7 @@ class SFixed32Type(TypeInfo): @register_type(16) -class SFixed64Type(TypeInfo): +class SFixed64Type(FixedSizeTypeMixin, TypeInfo): cpp_type = "int64_t" default_value = "0" decode_64bit = "value.as_sfixed64()" @@ -1431,8 +1509,9 @@ class SFixed64Type(TypeInfo): @register_type(17) -class SInt32Type(TypeInfo): +class SInt32Type(VarintTypeMixin, TypeInfo): cpp_type = "int32_t" + _varint_max_bits = 32 # zigzag encoding keeps it 32-bit default_value = "0" decode_varint = "decode_zigzag32(static_cast(value))" encode_func = "encode_sint32" @@ -1451,8 +1530,9 @@ class SInt32Type(TypeInfo): @register_type(18) -class SInt64Type(TypeInfo): +class SInt64Type(VarintTypeMixin, TypeInfo): cpp_type = "int64_t" + _varint_max_bits = 64 default_value = "0" decode_varint = "decode_zigzag64(value)" encode_func = "encode_sint64" @@ -1500,6 +1580,91 @@ def _generate_array_dump_content( return o +def _is_inline_encode(sub_msg_name: str) -> bool: + """Check if a sub-message type has the (inline_encode) option set.""" + sub_desc = _message_desc_map.get(sub_msg_name) + if not sub_desc: + return False + inline_opt = getattr(pb, "inline_encode", None) + if inline_opt is None: + return False + return get_opt(sub_desc, inline_opt, False) + + +def _generate_inline_encode_block( + field_number: int, sub_msg_name: str, element: str +) -> str: + """Generate inline encode code for a sub-message with (inline_encode) = true. + + Instead of calling encode_sub_message (function pointer indirection), + this inlines the sub-message's field encoding directly. Uses 1-byte + backpatch for the length (validated to be < 128 at generation time). + + Uses a local reference alias 'sub_msg' to avoid issues with this-> replacement + on complex element expressions. + + Args: + field_number: The parent field number for this sub-message + sub_msg_name: The sub-message type name + element: C++ expression for the element (e.g., "it" or "this->field[i]") + """ + sub_desc = _message_desc_map[sub_msg_name] + tag = (field_number << 3) | 2 # wire type 2 = LENGTH_DELIMITED + assert tag < 128, f"inline_encode requires single-byte tag, got {tag}" + + lines = [] + lines.append(f"auto &sub_msg = {element};") + lines.append(f"ProtoEncode::write_raw_byte(pos, {tag});") + lines.append("uint8_t *len_pos = pos;") + lines.append("ProtoEncode::reserve_byte(pos);") + + # Generate inline field encoding for each sub-message field + for field in sub_desc.field: + if field.options.deprecated: + continue + ti = create_field_type_info(field, needs_decode=False, needs_encode=True) + encode_line = ti.encode_content + # Replace this-> with sub_msg reference for the sub-message fields + encode_line = encode_line.replace("this->", "sub_msg.") + lines.extend(wrap_with_ifdef(encode_line, get_field_opt(field, pb.field_ifdef))) + + lines.append("*len_pos = static_cast(pos - len_pos - 1);") + return "\n".join(lines) + + +def _generate_inline_size_block( + field_number: int, sub_msg_name: str, element: str +) -> str: + """Generate inline size calculation for a sub-message with (inline_encode) = true. + + Uses a local reference alias 'sub_msg' to avoid issues with this-> replacement + on complex element expressions like 'this->advertisements[i]'. + + Args: + field_number: The parent field number for this sub-message + sub_msg_name: The sub-message type name + element: C++ expression for the element + """ + sub_desc = _message_desc_map[sub_msg_name] + + lines = [] + lines.append(f"auto &sub_msg = {element};") + # 1 byte tag + 1 byte length (guaranteed < 128 by validation) + lines.append("size += 2;") + + for field in sub_desc.field: + if field.options.deprecated: + continue + ti = create_field_type_info(field, needs_decode=False, needs_encode=True) + force = get_field_opt(field, pb.force, False) + size_line = ti.get_size_calculation(f"sub_msg.{ti.field_name}", force) + # Replace hardcoded this-> references (e.g., FixedArrayBytesType uses this->field_len) + size_line = size_line.replace("this->", "sub_msg.") + lines.extend(wrap_with_ifdef(size_line, get_field_opt(field, pb.field_ifdef))) + + return "\n".join(lines) + + class FixedArrayRepeatedType(TypeInfo): """Special type for fixed-size repeated fields using std::array. @@ -1526,6 +1691,10 @@ class FixedArrayRepeatedType(TypeInfo): return f"ProtoEncode::{self._ti.encode_func}(pos, {self.number}, static_cast({element}), true);" # Repeated message elements use encode_sub_message (force=true is default) if isinstance(self._ti, MessageType): + if _is_inline_encode(self._ti.cpp_type): + return _generate_inline_encode_block( + self.number, self._ti.cpp_type, element + ) return f"ProtoEncode::encode_sub_message(pos, buffer, {self.number}, {element});" return ( f"ProtoEncode::{self._ti.encode_func}(pos, {self.number}, {element}, true);" @@ -1633,8 +1802,19 @@ class FixedArrayRepeatedType(TypeInfo): ] return f"if ({non_zero_checks}) {{\n" + "\n".join(size_lines) + "\n}" + is_inline = isinstance(self._ti, MessageType) and _is_inline_encode( + self._ti.cpp_type + ) + # When using a define, always use loop-based approach if self.is_define: + if is_inline: + o = f"for (const auto &it : {name}) {{\n" + o += indent( + _generate_inline_size_block(self.number, self._ti.cpp_type, "it") + ) + o += "\n}" + return o o = f"for (const auto &it : {name}) {{\n" o += f" {self._ti.get_size_calculation('it', True)}\n" o += "}" @@ -1642,6 +1822,14 @@ class FixedArrayRepeatedType(TypeInfo): # For fixed arrays, we always encode all elements + if is_inline: + o = f"for (const auto &it : {name}) {{\n" + o += indent( + _generate_inline_size_block(self.number, self._ti.cpp_type, "it") + ) + o += "\n}" + return o + # Special case for single-element arrays - no loop needed if self.array_size == 1: return self._ti.get_size_calculation(f"{name}[0]", True) @@ -1714,6 +1902,15 @@ class FixedArrayWithLengthRepeatedType(FixedArrayRepeatedType): def get_size_calculation(self, name: str, force: bool = False) -> str: # Calculate size only for active elements + if isinstance(self._ti, MessageType) and _is_inline_encode(self._ti.cpp_type): + o = f"for (uint16_t i = 0; i < {name}_len; i++) {{\n" + o += indent( + _generate_inline_size_block( + self.number, self._ti.cpp_type, f"{name}[i]" + ) + ) + o += "\n}" + return o o = f"for (uint16_t i = 0; i < {name}_len; i++) {{\n" o += f" {self._ti.get_size_calculation(f'{name}[i]', True)}\n" o += "}" @@ -2222,6 +2419,28 @@ def calculate_message_estimated_size(desc: descriptor.DescriptorProto) -> int: return total_size +def calculate_message_max_size(desc: descriptor.DescriptorProto) -> int | None: + """Calculate the maximum possible encoded size for a message. + + Returns None if any field has unbounded size (e.g., variable-length strings). + Used to validate that (inline_encode) messages fit in a single-byte length varint. + """ + total_size = 0 + + for field in desc.field: + if field.options.deprecated: + continue + + ti = create_field_type_info(field, needs_decode=False, needs_encode=True) + max_size = ti.get_max_encoded_size() + if max_size is None: + return None + + total_size += max_size + + return total_size + + def build_message_type( desc: descriptor.DescriptorProto, base_class_fields: dict[str, list[descriptor.FieldDescriptorProto]], @@ -2451,11 +2670,23 @@ def build_message_type( prot = "void decode(const uint8_t *buffer, size_t length);" public_content.append(prot) + # Check if this message uses inline_encode — if so, skip generating standalone + # encode/calculate_size methods since the encoding is inlined into the parent. + inline_opt = getattr(pb, "inline_encode", None) + is_inline_only = ( + message_id is None # Not a service message (no id) + and inline_opt is not None + and get_opt(desc, inline_opt, False) + ) + # Only generate encode method if this message needs encoding and has fields - if needs_encode and encode: + if needs_encode and encode and not is_inline_only: # Add PROTO_ENCODE_DEBUG_ARG after pos in all proto_* calls encode_debug = [ - line.replace("(pos,", "(pos PROTO_ENCODE_DEBUG_ARG,") for line in encode + line.replace("(pos,", "(pos PROTO_ENCODE_DEBUG_ARG,").replace( + "(pos)", "(pos PROTO_ENCODE_DEBUG_ARG)" + ) + for line in encode ] o = f"uint8_t *{desc.name}::encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const {{\n" o += " uint8_t *__restrict__ pos = buffer.get_pos();\n" @@ -2470,7 +2701,7 @@ def build_message_type( # If no fields to encode or message doesn't need encoding, the default implementation in ProtoMessage will be used # Add calculate_size method only if this message needs encoding and has fields - if needs_encode and size_calc: + if needs_encode and size_calc and not is_inline_only: o = f"uint32_t {desc.name}::calculate_size() const {{\n" o += " uint32_t size = 0;\n" o += indent("\n".join(size_calc)) + "\n" @@ -2830,6 +3061,32 @@ def main() -> None: if not enum.options.deprecated and enum.value: _enum_max_values[f".{enum.name}"] = max(v.number for v in enum.value) + # Build message descriptor map for inline_encode lookups + mt = file.message_type + _message_desc_map.update({m.name: m for m in mt if not m.options.deprecated}) + + # Validate inline_encode messages fit in single-byte length varint + inline_encode_opt = getattr(pb, "inline_encode", None) + if inline_encode_opt is not None: + for m in mt: + if m.options.deprecated: + continue + if not get_opt(m, inline_encode_opt, False): + continue + max_size = calculate_message_max_size(m) + if max_size is None: + raise ValueError( + f"Message '{m.name}' has (inline_encode) = true but contains " + f"fields with unbounded size. Inline encoding requires all " + f"fields to have bounded maximum size." + ) + if max_size >= 128: + raise ValueError( + f"Message '{m.name}' has (inline_encode) = true but max " + f"encoded size is {max_size} bytes (>= 128). Inline encoding " + f"requires sub-messages that fit in a single-byte length varint." + ) + # Build dynamic ifdef mappings early so we can emit USE_API_VARINT64 before includes enum_ifdef_map, message_ifdef_map, message_source_map, used_messages = ( build_type_usage_map(file) @@ -3048,8 +3305,6 @@ static void dump_bytes_field(DumpBuffer &out, const char *field_name, const uint content += "\n} // namespace enums\n\n" - mt = file.message_type - # Identify empty SOURCE_CLIENT messages that don't need class generation for m in mt: if m.options.deprecated: From df51f4e37110a21c94f58949d6518f2bf9d8f7d0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 9 Apr 2026 17:42:33 -1000 Subject: [PATCH 7/7] [rp2040] Fix W5500 Ethernet pbuf corruption by mirroring LWIPMutex semantics The previous LwIPLock for RP2040+Ethernet only called ethernet_arch_lwip_begin/end, taking the async_context lock but never bumping arduino-pico's __inLWIP counter. The W5500 GPIO IRQ path (LwipIntfDev::_irq) checks __inLWIP to decide whether to defer packet processing; with __inLWIP == 0 it would proceed to run _lwipCallback immediately from IRQ context even while the main thread held the async_context lock, re-entering lwIP and corrupting pbuf chains. The corruption surfaced later as the pbuf_cat assertion "p->tot_len == p->len (of last pbuf in chain)" panicking the device on wiznet-w5500-evb-pico boards configured with an interrupt pin. Mirror arduino-pico's LWIPMutex (cores/rp2040/lwip_wrap.h) exactly: bump __inLWIP, take the lock, and on release re-unmask any GPIO IRQs that were deferred while we held it (__needsIRQEN). We can't write `using LwIPLock = LWIPMutex;` in core/helpers.h because pulling lwip_wrap.h there transitively poisons many translation units with lwIP types in an order that breaks err_t resolution, so the body is inlined in the rp2040 helpers.cpp where the include is scoped. --- esphome/components/rp2040/helpers.cpp | 31 +++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/esphome/components/rp2040/helpers.cpp b/esphome/components/rp2040/helpers.cpp index 8cb5f7c18df..3a53eba61b9 100644 --- a/esphome/components/rp2040/helpers.cpp +++ b/esphome/components/rp2040/helpers.cpp @@ -9,7 +9,7 @@ #include #include // For cyw43_arch_lwip_begin/end (LwIPLock) #elif defined(USE_ETHERNET) -#include // For ethernet_arch_lwip_begin/end (LwIPLock) +#include // For LWIPMutex — LwIPLock delegates to it (see below) #include "esphome/components/ethernet/ethernet_component.h" #endif #include @@ -43,9 +43,18 @@ IRAM_ATTR InterruptLock::~InterruptLock() { restore_interrupts(state_); } // main loop, corrupting the shared rx_buf_ pbuf chain (use-after-free, pbuf_cat // assertion failures). See esphome#10681. // -// WiFi uses cyw43_arch_lwip_begin/end; Ethernet uses ethernet_arch_lwip_begin/end. -// Both acquire the async_context recursive mutex to prevent IRQ callbacks from -// firing during critical sections. +// WiFi uses cyw43_arch_lwip_begin/end. +// +// For wired Ethernet, taking only the async_context lock is NOT enough. The +// W5500 GPIO IRQ path (LwipIntfDev::_irq) checks arduino-pico's `__inLWIP` +// counter to decide whether to defer packet processing. If we hold the +// async_context lock without bumping `__inLWIP`, an interrupt-driven packet +// arrival re-enters lwIP from IRQ context and corrupts pbufs (the `pbuf_cat` +// assertion crash on wiznet-w5500-evb-pico). We mirror arduino-pico's +// LWIPMutex (cores/rp2040/lwip_wrap.h) exactly: bump `__inLWIP`, take the +// lock, and on release re-unmask any GPIO IRQs that were deferred while we +// held it. We can't `using LwIPLock = LWIPMutex;` in helpers.h because +// pulling lwip_wrap.h there poisons many TUs with lwIP types. // // When neither WiFi nor Ethernet is configured, this is a no-op since // there's no network stack and no lwip callbacks to race with. @@ -53,8 +62,18 @@ IRAM_ATTR InterruptLock::~InterruptLock() { restore_interrupts(state_); } LwIPLock::LwIPLock() { cyw43_arch_lwip_begin(); } LwIPLock::~LwIPLock() { cyw43_arch_lwip_end(); } #elif defined(USE_ETHERNET) -LwIPLock::LwIPLock() { ethernet_arch_lwip_begin(); } -LwIPLock::~LwIPLock() { ethernet_arch_lwip_end(); } +LwIPLock::LwIPLock() { + __inLWIP++; + ethernet_arch_lwip_begin(); +} +LwIPLock::~LwIPLock() { + ethernet_arch_lwip_end(); + __inLWIP--; + if (__needsIRQEN && !__inLWIP) { + __needsIRQEN = false; + ethernet_arch_lwip_gpio_unmask(); + } +} #else LwIPLock::LwIPLock() {} LwIPLock::~LwIPLock() {}