10000 Fix ``captive_portal`` loading entire ``web_server`` by bdraco · Pull Request #9066 · esphome/esphome · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix captive_portal loading entire web_server #9066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Jun 14, 2025
Merged

Fix captive_portal loading entire web_server #9066

merged 7 commits into from
Jun 14, 2025

Conversation

bdraco
Copy link
Member
@bdraco bdraco commented Jun 13, 2025 •
edited
Loading

What does this implement/fix?

Quick description

This PR fixes an issue where the captive_portal component automatically includes the full web_server component when using the ESP-IDF framework, causing unnecessary binary size increase.

This PR fixes a regression introduced in PR #7538 where the web_server_idf component was set to automatically load the full web_server component. This caused the captive_portal component to inadvertently include the web server functionality when using ESP-IDF framework, significantly increasing binary size.

Root Cause

The captive_portal component depends on web_server_base, which conditionally loads web_server_idf when using ESP-IDF. The web_server_idf component had AUTO_LOAD = ["web_server"], creating an unwanted dependency chain:

captive_portal → web_server_base → web_server_idf → web_server

Solution

  1. Removed the AUTO_LOAD = ["web_server"] directive from web_server_idf/__init__.py
  2. Added conditional compilation guards (#ifdef USE_WEBSERVER) around web_server-specific code in web_server_idf to ensure it compiles correctly both with and without the web_server component

Changes Made

  • esphome/components/web_server_idf/init.py: Removed AUTO_LOAD = ["web_server"]
  • esphome/components/web_server_idf/web_server_idf.h: Added #ifdef USE_WEBSERVER guards around:
    • Forward declarations for web_server::WebServer and web_server::ListEntitiesIterator
    • AsyncEventSource and AsyncEventSourceResponse class declarations
    • DeferredEvent struct and message_generator_t type definition
    • Friend declaration in DefaultHeaders class
  • esphome/components/web_server_idf/web_server_idf.cpp: Added #ifdef USE_WEBSERVER guards around:
    • Include statements for web_server headers
    • All AsyncEventSource and AsyncEventSourceResponse method implementations

Testing

  • Verified that captive_portal alone no longer includes web_server component
  • Verified that explicitly including web_server still works correctly
  • Confirmed USE_WEBSERVER is only defined when web_server component is explicitly included
  • Both configurations compile successfully on ESP-IDF framework

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Related issue or feature (if applicable): fixes esphome/issues#6883 fixes esphome/issues#7112

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • 8000 BK72xx
  • RTL87xx

Example entry for config.yaml:

# Test configuration showing captive_portal no longer pulls in web_server
esphome:
  name: captive-portal-test

esp32:
  board: esp32doit-devkit-v1
  framework:
    type: esp-idf

wifi:
  ssid: "test_network"
  password: "test1234"

# This should NOT automatically include web_server component
captive_portal:

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@bdraco bdraco marked this pull request as ready for review June 13, 2025 23:33
@Copilot Copilot AI review requested due to automatic review settings June 13, 2025 23:33
@probot-esphome
Copy link

Hey there @dentra, mind taking a look at this pull request as it has been labeled with an integration (web_server_idf) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

Copilot
Copilot AI reviewed Jun 13, 2025
View reviewed changes
Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR prevents the captive_portal component from unintentionally pulling in the full web_server by removing automatic loading and guarding web-server–specific code behind a compile flag.

  • Removed AUTO_LOAD = ["web_server"] from the IDF wrapper to break the unwanted dependency.
  • Wrapped all web-server–dependent declarations and implementations in #ifdef USE_WEBSERVER.
  • Added defines.h include to support the new compile-time flag.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
esphome/components/web_server_idf/init.py Removed the AUTO_LOAD directive that caused web_server to load by default.
esphome/components/web_server_idf/web_server_idf.h Added #ifdef USE_WEBSERVER guards around web-server–only declarations; included defines.h.
esphome/components/web_server_idf/web_server_idf.cpp Wrapped web-server includes and AsyncEventSource/response implementations in #ifdef USE_WEBSERVER.
Comments suppressed due to low confidence (2)

esphome/components/web_server_idf/init.py:10

  • Add tests to verify that web_server_idf compiles and operates correctly both with and without the web_server component (i.e., when USE_WEBSERVER is undefined) to prevent future regressions.
-AUTO_LOAD = ["web_server"]

esphome/components/web_server_idf/web_server_idf.h:1

  • [nitpick] Consider adding a brief file-level comment explaining that parts of this header are conditionally compiled under USE_WEBSERVER, so maintainers understand the purpose of the guards.
#pragma once

bdraco and others added 3 commits June 13, 2025 18:35
@bdraco @Copilot
Update esphome/components/web_server_idf/web_server_idf.h
1d562b9 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@bdraco @Copilot
Update esphome/components/web_server_idf/web_server_idf.cpp
90a2dc6 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@bdraco
lint
b3ce9a7
@bdraco bdraco added this to the 2025.6.0b2 milestone Jun 14, 2025
kbx81
kbx81 approved these changes Jun 14, 2025
View reviewed changes
Copy link
Member
@kbx81 kbx81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks! 🍻

@bdraco
Copy link
Member Author
bdraco commented Jun 14, 2025

Thanks

@bdraco bdraco merged commit 92ea697 into dev Jun 14, 2025
29 checks passed
@bdraco bdraco deleted the webserver_fix branch June 14, 2025 13:19
jesserockz pushed a commit that referenced this pull request Jun 15, 2025
@bdraco @jesserockz
Fix captive_portal loading entire web_server (#9066)
ebecf70
@jesserockz jesserockz mentioned this pull request Jun 15, 2025
Merged
@jesserockz jesserockz changed the title Fix captive_portal loading entire web_server Fix captive_portal loading entire web_server Jun 16, 2025
@jesserockz jesserockz mentioned this pull request Jun 18, 2025
Merged
@Roving-Ronin Roving-Ronin mentioned this pull request Jun 19, 2025
Open
@github-actions github-actions bot locked and limited conversation to collaborators Jun 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

Copilot code review Copilot Copilot left review comments

@kbx81 kbx81 kbx81 approved these changes

Assignees
No one assigned
Labels
cherry-picked integration: web_server_idf small-pr
Projects
None yet
Milestone
2025.6.0b2
3 participants
@bdraco @kbx81 @jesserockz
0