diff --git a/esphome/config.py b/esphome/config.py index 7a6feea3d3..641b6ec1b4 100644 --- a/esphome/config.py +++ b/esphome/config.py @@ -958,6 +958,23 @@ class FinalValidateValidationStep(ConfigValidationStep): fv.full_config.reset(token) +class CoreFinalValidateStep(ConfigValidationStep): + """Run final validation on core esphome config (area/device hash collisions).""" + + # Same priority as component final validate steps + priority = -20.0 + + def run(self, result: Config) -> None: + if result.errors: + return + + token = fv.full_config.set(result) + with result.catch_error([CONF_ESPHOME]): + if CONF_ESPHOME in result: + core_config.validate_ids_and_references(result[CONF_ESPHOME]) + fv.full_config.reset(token) + + class PinUseValidationCheck(ConfigValidationStep): """Check for pin reuse""" @@ -1085,6 +1102,7 @@ def validate_config( for domain, conf in config.items(): result.add_validation_step(LoadValidationStep(domain, conf)) result.add_validation_step(IDPassValidationStep()) + result.add_validation_step(CoreFinalValidateStep()) result.add_validation_step(PinUseValidationCheck()) result.add_validation_step(RemoveReferenceValidationStep()) diff --git a/esphome/core/config.py b/esphome/core/config.py index e02c6ec75f..c47693c783 100644 --- a/esphome/core/config.py +++ b/esphome/core/config.py @@ -156,22 +156,22 @@ def validate_ids_and_references(config: ConfigType) -> ConfigType: hash_dict[hash_val] = id_obj.id # Collect all areas - all_areas: list[dict[str, str | core.ID]] = [] + all_areas: list[tuple[dict[str, str | core.ID], str]] = [] if CONF_AREA in config: - all_areas.append(config[CONF_AREA]) - all_areas.extend(config[CONF_AREAS]) + all_areas.append((config[CONF_AREA], CONF_AREA)) + all_areas.extend((area, CONF_AREAS) for area in config.get(CONF_AREAS, [])) # Validate area hash collisions and collect IDs area_hashes: dict[int, str] = {} area_ids: set[str] = set() - for area in all_areas: + for area, key in all_areas: area_id: core.ID = area[CONF_ID] - check_hash_collision(area_id, area_hashes, "Area", [CONF_AREAS, area_id.id]) + check_hash_collision(area_id, area_hashes, "Area", [key, area_id.id]) area_ids.add(area_id.id) # Validate device hash collisions and area references device_hashes: dict[int, str] = {} - for device in config[CONF_DEVICES]: + for device in config.get(CONF_DEVICES, []): device_id: core.ID = device[CONF_ID] check_hash_collision( device_id, device_hashes, "Device", [CONF_DEVICES, device_id.id] @@ -329,9 +329,6 @@ CONFIG_SCHEMA = cv.All( ) -FINAL_VALIDATE_SCHEMA = cv.All(validate_ids_and_references) - - PRELOAD_CONFIG_SCHEMA = cv.Schema( { cv.Required(CONF_NAME): cv.valid_name, diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 231312dd27..ba5fcffa9a 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -22,6 +22,19 @@ namespace esphome { static const char *const TAG = "helpers"; +__attribute__((noinline, cold)) void *callback_manager_grow(void *data, uint16_t size, uint16_t &capacity, + size_t elem_size) { + ESPHOME_DEBUG_ASSERT(size < UINT16_MAX); + uint16_t new_cap = size + 1; + auto *new_data = ::operator new(new_cap *elem_size); + if (data) { + __builtin_memcpy(new_data, data, size * elem_size); + ::operator delete(data); + } + capacity = new_cap; + return new_data; +} + static const uint16_t CRC16_A001_LE_LUT_L[] = {0x0000, 0xc0c1, 0xc181, 0x0140, 0xc301, 0x03c0, 0x0280, 0xc241, 0xc601, 0x06c0, 0x0780, 0xc741, 0x0500, 0xc5c1, 0xc481, 0x0440}; static const uint16_t CRC16_A001_LE_LUT_H[] = {0x0000, 0xcc01, 0xd801, 0x1400, 0xf001, 0x3c00, 0x2800, 0xe401, diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index ce730e1aed..6c124116b8 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -1806,33 +1806,84 @@ template struct Callback { } }; +/// Grow a CallbackManager's backing array to exactly size+1. Defined in helpers.cpp. +void *callback_manager_grow(void *data, uint16_t size, uint16_t &capacity, size_t elem_size); + template class CallbackManager; /** Helper class to allow having multiple subscribers to a callback. + * + * Uses a trivial-copyable-specialized container instead of std::vector to avoid + * template bloat (_M_realloc_insert, exception-safe copies). Since Callback is + * trivially copyable (just {fn_ptr, ctx_ptr}), reallocation is a plain memcpy. + * Uses uint16_t for size/capacity (8 bytes on 32-bit vs 12 for std::vector). + * Grows to exact size on each add — callbacks are registered during setup() + * and most instances have only 1-2 callbacks, so slack capacity is wasteful. * * @tparam Ts The arguments for the callbacks, wrapped in void(). */ template class CallbackManager { + using CbType = Callback; + static_assert(std::is_trivially_copyable_v, "Callback must be trivially copyable"); + public: + CallbackManager() = default; + ~CallbackManager() { ::operator delete(this->data_); } + + // Non-copyable (would alias data_), movable (for std::map support) + CallbackManager(const CallbackManager &) = delete; + CallbackManager &operator=(const CallbackManager &) = delete; + CallbackManager(CallbackManager &&other) noexcept + : data_(other.data_), size_(other.size_), capacity_(other.capacity_) { + other.data_ = nullptr; + other.size_ = 0; + other.capacity_ = 0; + } + CallbackManager &operator=(CallbackManager &&other) noexcept { + if (this != &other) { + ::operator delete(this->data_); + this->data_ = other.data_; + this->size_ = other.size_; + this->capacity_ = other.capacity_; + other.data_ = nullptr; + other.size_ = 0; + other.capacity_ = 0; + } + return *this; + } + /// Add any callable. Small trivially-copyable callables (like [this] lambdas) /// are stored inline without heap allocation or std::function. - template void add(F &&callback) { this->add_(Callback::create(std::forward(callback))); } - - /// Call all callbacks in this manager. No null check on invoke. - void call(Ts... args) { - for (auto &cb : this->callbacks_) - cb.call(args...); - } - size_t size() const { return this->callbacks_.size(); } + template void add(F &&callback) { this->add_(CbType::create(std::forward(callback))); } /// Call all callbacks in this manager. - void operator()(Ts... args) { call(args...); } + inline void ESPHOME_ALWAYS_INLINE call(Ts... args) { + if (this->size_ == 0) { + return; + } + for (auto *it = this->data_, *end = it + this->size_; it != end; ++it) { + it->call(args...); + } + } + uint16_t size() const { return this->size_; } + + /// Call all callbacks in this manager. + void operator()(Ts... args) { this->call(args...); } protected: template friend class LazyCallbackManager; /// Non-template core to avoid code duplication per lambda type. - void add_(Callback cb) { this->callbacks_.push_back(cb); } - std::vector> callbacks_; + /// Inline fast path; cold growth path is in helpers.cpp via callback_manager_grow(). + void add_(CbType cb) { + if (this->size_ == this->capacity_) { + this->data_ = + static_cast(callback_manager_grow(this->data_, this->size_, this->capacity_, sizeof(CbType))); + } + this->data_[this->size_++] = cb; + } + CbType *data_{nullptr}; + uint16_t size_{0}; + uint16_t capacity_{0}; }; template class LazyCallbackManager; @@ -1844,7 +1895,7 @@ template class LazyCallbackManager; * from API and web_server components). * * Memory overhead comparison (32-bit systems): - * - CallbackManager: 12 bytes (empty std::vector) + * - CallbackManager: 8 bytes (pointer + uint16 size + uint16 capacity) * - LazyCallbackManager: 4 bytes (nullptr pointer) * * Uses plain pointer instead of unique_ptr to avoid template instantiation overhead. diff --git a/script/ci-custom.py b/script/ci-custom.py index 7d0680a491..ad39f92005 100755 --- a/script/ci-custom.py +++ b/script/ci-custom.py @@ -1006,6 +1006,18 @@ def lint_log_in_header(fname, line, col, content): ) +@lint_content_find_check( + "FINAL_VALIDATE_SCHEMA", + include=["esphome/core/*.py"], + exclude=["esphome/core/entity_helpers.py"], +) +def lint_final_validate_in_core(fname, line, col, content): + return ( + "FINAL_VALIDATE_SCHEMA in esphome/core/ is not picked up by the component loader. " + "Use CoreFinalValidateStep in esphome/config.py instead." + ) + + def main(): colorama.init() diff --git a/tests/unit_tests/core/test_config.py b/tests/unit_tests/core/test_config.py index 474d31a90a..6fa8f7ed43 100644 --- a/tests/unit_tests/core/test_config.py +++ b/tests/unit_tests/core/test_config.py @@ -248,6 +248,24 @@ def test_area_id_hash_collision( ) +def test_area_singular_hash_collision( + yaml_file: Callable[[str], str], capsys: pytest.CaptureFixture[str] +) -> None: + """Test that area hash collisions between singular area: and areas: list are detected.""" + result = load_config_from_fixture( + yaml_file, "area_singular_hash_collision.yaml", FIXTURES_DIR + ) + assert result is None + + captured = capsys.readouterr() + assert ( + "Area ID 'd6ka' with hash 3082558663 collides with existing area ID 'test_2258'" + in captured.out + ) + # Error path should point to 'areas' (where the colliding entry is), not 'area' + assert "areas" in captured.out + + def test_device_duplicate_id( yaml_file: Callable[[str], str], capsys: pytest.CaptureFixture[str] ) -> None: diff --git a/tests/unit_tests/fixtures/core/config/area_singular_hash_collision.yaml b/tests/unit_tests/fixtures/core/config/area_singular_hash_collision.yaml new file mode 100644 index 0000000000..6e137f5f6e --- /dev/null +++ b/tests/unit_tests/fixtures/core/config/area_singular_hash_collision.yaml @@ -0,0 +1,10 @@ +esphome: + name: test + area: + id: test_2258 + name: "Area 1" + areas: + - id: d6ka + name: "Area 2" + +host: