From 207edd0522f7d84aca105e35591586c19e6c6e53 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 24 Sep 2026 17:12:32 +0100 Subject: [PATCH] [core] Tidy the controller dispatch after review Move ControllerContract next to its only user in controller_dispatch.h, collapse the contract check into one static_assert, and let codegen build the tuple with ArrayInitializer after emitting the tuple include. Share one esphome.h exclusion set between the writer and the clang-tidy all-headers file so the dispatch header is skipped by both. Add unit tests for the generated dispatch. --- esphome/components/web_server/web_server.h | 2 +- esphome/core/__init__.py | 7 +--- esphome/core/config.py | 8 +++-- esphome/core/controller_dispatch.h | 39 +++++++++++++--------- esphome/core/controller_registry.h | 22 ++---------- esphome/writer.py | 27 ++++++--------- script/helpers.py | 6 ++-- tests/unit_tests/core/test_config.py | 32 +++++++++++++++++- 8 files changed, 78 insertions(+), 65 deletions(-) diff --git a/esphome/components/web_server/web_server.h b/esphome/components/web_server/web_server.h index 27e35dfe0f..b776c955ff 100644 --- a/esphome/components/web_server/web_server.h +++ b/esphome/components/web_server/web_server.h @@ -484,7 +484,7 @@ class WebServer final : public Component, public AsyncWebHandler { #endif #ifdef USE_MEDIA_PLAYER - void on_media_player_update(media_player::MediaPlayer *obj) {} + void on_media_player_update(media_player::MediaPlayer *) {} #endif #ifdef USE_EVENT diff --git a/esphome/core/__init__.py b/esphome/core/__init__.py index 7a60e8de41..fb75285081 100644 --- a/esphome/core/__init__.py +++ b/esphome/core/__init__.py @@ -1210,12 +1210,7 @@ class EsphomeCore: self.platform_counts[platform_name] = 1 def register_controller(self, controller: "MockObj") -> None: - """Register a controller that receives every entity state update. - - Code generation defines the ControllerRegistry notify functions in - main.cpp as direct calls on each registered controller, so the C++ - class only needs the on_*_update methods, not a base class. - """ + """Register a controller that receives every entity state update.""" self.data.setdefault(KEY_CONTROLLER_REGISTRY_CONTROLLERS, []).append(controller) @property diff --git a/esphome/core/config.py b/esphome/core/config.py index 2d27ebce6f..4d3fb83312 100644 --- a/esphome/core/config.py +++ b/esphome/core/config.py @@ -676,15 +676,17 @@ async def _add_platform_defines() -> None: @coroutine_with_priority(CoroPriority.FINAL) async def _add_controller_registry_dispatch() -> None: # controller_dispatch.h defines ControllerRegistry::notify_*() as direct - # calls on the controllers returned by esphome_controllers(). + # calls on the controllers returned by esphome_controllers(), emitted as + # static auto esphome_controllers() { return std::tuple{a, b}; } controllers = CORE.data.get(KEY_CONTROLLER_REGISTRY_CONTROLLERS) if not controllers: return cg.add_define("USE_CONTROLLER_REGISTRY") - entries = ", ".join(str(var) for var in controllers) + controllers = cg.ArrayInitializer(*controllers) + cg.add_global(cg.RawStatement("#include ")) cg.add_global( cg.RawStatement( - f"static auto esphome_controllers() {{ return std::tuple{{{entries}}}; }}" + f"static auto esphome_controllers() {{ return std::tuple{controllers}; }}" ) ) cg.add_global(cg.RawStatement('#include "esphome/core/controller_dispatch.h"')) diff --git a/esphome/core/controller_dispatch.h b/esphome/core/controller_dispatch.h index 631de50236..fc6a03556d 100644 --- a/esphome/core/controller_dispatch.h +++ b/esphome/core/controller_dispatch.h @@ -6,26 +6,36 @@ // #include "esphome/core/controller_dispatch.h" // // Defines ControllerRegistry::notify_*() as direct calls on those controllers. Excluded from -// esphome.h so nothing else includes it. - -#include "esphome/core/controller_registry.h" - -#ifdef USE_CONTROLLER_REGISTRY +// esphome.h and the clang-tidy all-headers file, so nothing else includes it. #include #include +#include "esphome/core/controller_registry.h" + namespace esphome { -template constexpr bool controllers_satisfy_contract(std::tuple *) { - static_assert((ControllerContract> && ...), - "A registered controller is missing an on_*_update() callback for an entity type in this build " - "(esphome/core/controller_registry.h)"); - return true; -} -static_assert(controllers_satisfy_contract(static_cast(nullptr))); - // NOLINTBEGIN(bugprone-macro-parentheses) + +/// A controller provides a plain on_*_update() member for every entity type in the build. +template +concept ControllerContract = requires(T &controller) { + controller; // keeps the requirement list non-empty when no entity type has a callback +#define ENTITY_TYPE_(type, singular, plural, count, upper) // no controller callback +#define ENTITY_CONTROLLER_TYPE_(type, singular, plural, count, upper, callback) \ + controller.on_##callback(static_cast(nullptr)); +#include "esphome/core/entity_types.h" +#undef ENTITY_TYPE_ +#undef ENTITY_CONTROLLER_TYPE_ +}; + +template constexpr bool controllers_satisfy_contract(std::tuple *) { + return (ControllerContract> && ...); +} +static_assert(controllers_satisfy_contract(static_cast(nullptr)), + "A registered controller is missing an on_*_update() callback for an entity type in this build " + "(ControllerContract in esphome/core/controller_dispatch.h)"); + #define ENTITY_TYPE_(type, singular, plural, count, upper) // no controller callback #define ENTITY_CONTROLLER_TYPE_(type, singular, plural, count, upper, callback) \ void ControllerRegistry::notify_##callback(type *obj) { \ @@ -34,8 +44,7 @@ static_assert(controllers_satisfy_contract(static_cast -concept ControllerContract = requires(T &controller) { - controller; -#define ENTITY_TYPE_(type, singular, plural, count, upper) // no controller callback -#define ENTITY_CONTROLLER_TYPE_(type, singular, plural, count, upper, callback) \ - controller.on_##callback(static_cast(nullptr)); -#include "esphome/core/entity_types.h" -#undef ENTITY_TYPE_ -#undef ENTITY_CONTROLLER_TYPE_ -}; -// NOLINTEND(bugprone-macro-parentheses) - /** Fan-out of entity state updates to the controllers (APIServer, WebServer). * * Entities call ControllerRegistry::notify_*_update() instead of holding - * per-entity controller callbacks. The notify functions are only declared here; - * code generation defines them in main.cpp as direct calls on each controller - * that registered through CORE.register_controller(), so there is no virtual - * dispatch, no controller base class and no runtime list of controllers. + * per-entity controller callbacks. The functions are only declared here; + * controller_dispatch.h, included by the generated main.cpp, defines them as + * direct calls on the controllers registered through CORE.register_controller(). */ class ControllerRegistry { public: diff --git a/esphome/writer.py b/esphome/writer.py index 3a1dc3ecf4..9ab5d644c5 100644 --- a/esphome/writer.py +++ b/esphome/writer.py @@ -211,7 +211,16 @@ VERSION_H_TARGET = "esphome/core/version.h" BUILD_INFO_DATA_H_TARGET = "esphome/core/build_info_data.h" BUILD_INFO_DATA_CPP_TARGET = "esphome/core/build_info_data.cpp" ENTITY_TYPES_H_TARGET = "esphome/core/entity_types.h" -CONTROLLER_DISPATCH_H_TARGET = "esphome/core/controller_dispatch.h" +# Headers that must not be included bare from esphome.h or the clang-tidy +# all-headers file: X-macro files, headers main.cpp includes itself, and +# deprecated headers that only resolve when their new component is loaded. +ESPHOME_H_EXCLUDE = { + Path(ENTITY_TYPES_H_TARGET), + # main.cpp includes it after defining esphome_controllers() + Path("esphome/core/controller_dispatch.h"), + # moved to components/ring_buffer/, removed in 2026.11.0 + Path("esphome/core/ring_buffer.h"), +} ESPHOME_README_TXT = """ THIS DIRECTORY IS AUTO-GENERATED, DO NOT MODIFY @@ -237,23 +246,9 @@ def copy_src_tree(): source_files_l.sort() # Build #include list for esphome.h - # X-macro files are included multiple times with different macro definitions - # and must not be included bare in esphome.h - # Deprecated headers that re-export from a relocated component must not be - # auto-included, since their #include of the new path only resolves when the - # new component is loaded by a consumer. - esphome_h_exclude = { - Path(ENTITY_TYPES_H_TARGET), - Path( - CONTROLLER_DISPATCH_H_TARGET - ), # included by main.cpp once ESPHOME_CONTROLLERS is defined - Path( - "esphome/core/ring_buffer.h" - ), # moved to components/ring_buffer/, removed in 2026.11.0 - } include_l = [] for target, _ in source_files_l: - if target.suffix in HEADER_FILE_EXTENSIONS and target not in esphome_h_exclude: + if target.suffix in HEADER_FILE_EXTENSIONS and target not in ESPHOME_H_EXCLUDE: include_l.append(f'#include "{target}"') include_l.append("") include_s = "\n".join(include_l) diff --git a/script/helpers.py b/script/helpers.py index a8a237118f..f2516523c7 100644 --- a/script/helpers.py +++ b/script/helpers.py @@ -429,11 +429,9 @@ def build_all_include(header_files: list[str] | None = None) -> None: if line ] - from esphome.writer import ENTITY_TYPES_H_TARGET + from esphome.writer import ESPHOME_H_EXCLUDE - # X-macro files are included multiple times with different macro definitions - # and must not be included bare in the all-include header - exclude = {ENTITY_TYPES_H_TARGET} + exclude = {str(path) for path in ESPHOME_H_EXCLUDE} headers = [f'#include "{h}"' for h in header_files if h not in exclude] headers.sort() headers.append("") diff --git a/tests/unit_tests/core/test_config.py b/tests/unit_tests/core/test_config.py index 07cff003cd..0db7039668 100644 --- a/tests/unit_tests/core/test_config.py +++ b/tests/unit_tests/core/test_config.py @@ -9,6 +9,7 @@ from unittest.mock import MagicMock, Mock, patch import pytest from esphome import config_validation as cv, core +import esphome.codegen as cg from esphome.components.safe_mode import to_code as safe_mode_to_code from esphome.const import ( CONF_AREA, @@ -23,7 +24,7 @@ from esphome.const import ( KEY_TARGET_PLATFORM, Toolchain, ) -from esphome.core import CORE, config +from esphome.core import CORE, KEY_CONTROLLER_REGISTRY_CONTROLLERS, config from esphome.core.config import ( Area, make_app_name_cpp, @@ -455,6 +456,35 @@ async def test_add_looping_components_with_entries() -> None: assert "(1 * HasLoopOverride::value)" in text +@pytest.mark.asyncio +async def test_add_controller_registry_dispatch_without_controllers() -> None: + """Nothing is emitted when no controller registered.""" + CORE.data.pop(KEY_CONTROLLER_REGISTRY_CONTROLLERS, None) + + await config._add_controller_registry_dispatch() + + assert "USE_CONTROLLER_REGISTRY" not in {d.name for d in CORE.defines} + assert not [s for s in CORE.global_statements if "controller" in str(s)] + + +@pytest.mark.asyncio +async def test_add_controller_registry_dispatch_with_controllers() -> None: + """Registered controllers become one tuple plus the dispatch include.""" + CORE.register_controller(cg.MockObj("api_apiserver_id")) + CORE.register_controller(cg.MockObj("web_server_webserver_id")) + + await config._add_controller_registry_dispatch() + + assert "USE_CONTROLLER_REGISTRY" in {d.name for d in CORE.defines} + statements = [str(s) for s in CORE.global_statements] + assert "#include " in statements + assert ( + "static auto esphome_controllers() { return std::tuple{api_apiserver_id, web_server_webserver_id}; }" + in statements + ) + assert '#include "esphome/core/controller_dispatch.h"' in statements + + def test_valid_include_with_angle_brackets() -> None: """Test valid_include accepts angle bracket includes.""" assert valid_include("") == ""