[image] Restore defaults:/files: support for platform entries (#18032)

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: J. Nick Koston <nick@koston.org>
This commit is contained in:
Clyde Stubbs
2026-08-20 09:33:03 +12:00
committed by Jesse Hills
co-authored by Claude Sonnet 5 Copilot Autofix powered by AI J. Nick Koston
parent 4a85c98285
commit b3fda9973e
9 changed files with 657 additions and 25 deletions
+325 -3
View File
@@ -21,16 +21,20 @@ from esphome.components.image import (
CONF_OPAQUE,
CONF_TRANSPARENCY,
PLATFORM_FILE,
_expand_platform_entry,
_flatten_legacy_image_config,
_is_legacy_image_format,
_is_new_image_format,
_migrate_legacy_image_config,
expand_platform_config,
get_all_image_metadata,
get_image_metadata,
)
from esphome.const import (
CONF_DEFAULTS,
CONF_DITHER,
CONF_FILE,
CONF_FILES,
CONF_ID,
CONF_PLATFORM,
CONF_RAW_DATA_ID,
@@ -259,6 +263,15 @@ def test_flatten_keeps_byte_order_for_endian_type() -> None:
assert out[0][CONF_BYTE_ORDER] == "little_endian"
def test_flatten_drops_byte_order_written_directly_on_legacy_entry() -> None:
"""The legacy flattener drops an incompatible byte_order even when written directly on the entry."""
out = _flatten_legacy_image_config(
{"binary": [{"id": "a", "file": "x.png", "byte_order": "little_endian"}]}
)
assert out == [{"id": "a", "file": "x.png", "type": "binary"}]
assert CONF_BYTE_ORDER not in out[0]
def test_flatten_skips_meta_and_unknown_keys() -> None:
out = _flatten_legacy_image_config(
{
@@ -342,6 +355,42 @@ def test_migrate_legacy_warns_and_prepends_platform(
),
pytest.param({"foo": 1}, False, id="dict_unknown_keys"),
pytest.param("a string", False, id="scalar"),
# A `platform:`-tagged dict is the new format written without list brackets.
pytest.param(
{CONF_PLATFORM: "file", "id": "a", "file": "x.png"},
False,
id="platform_tagged_flat_dict",
),
pytest.param(
{
CONF_PLATFORM: "file",
"defaults": {"type": "rgb565"},
"files": [{"id": "a", "file": "x.png"}],
},
False,
id="platform_tagged_defaults_files_dict",
),
# `files:` without `platform:` is not legacy either -- the flattener has no branch for it.
pytest.param(
{
"defaults": {"type": "rgb565"},
"files": [{"id": "a", "file": "x.png"}],
},
False,
id="defaults_files_dict_without_platform",
),
# Same as above in a list -- without this exclusion it would be silently
# migrated to a hard-coded `platform: file` instead of raising the error.
pytest.param(
[
{
"defaults": {"type": "rgb565"},
"files": [{"id": "a", "file": "x.png"}],
}
],
False,
id="defaults_files_list_entry_without_platform",
),
],
)
def test_is_legacy_image_format(config: object, expected: bool) -> None:
@@ -359,17 +408,290 @@ def test_is_legacy_image_format(config: object, expected: bool) -> None:
def test_migrate_returns_none_for_invalid_legacy_shapes(
config: object, caplog: pytest.LogCaptureFixture
) -> None:
"""Unrecognised shapes are not migrated (and emit no warning) so normal
platform validation surfaces a proper error instead of silently dropping
the offending input."""
"""Unrecognised shapes are not migrated (and emit no warning), so normal platform validation reports them."""
with caplog.at_level(logging.WARNING):
assert _migrate_legacy_image_config(config) is None
assert "deprecated" not in caplog.text
def test_migrate_returns_none_for_mapping_form_defaults_files() -> None:
"""A `platform:`-tagged `defaults:`/`files:` mapping must not be swallowed by the legacy migrator."""
config = {
CONF_PLATFORM: "file",
"defaults": {"type": "rgb565"},
"files": [{"id": "a", "file": "a.png"}],
}
assert _migrate_legacy_image_config(config) is None
def test_migrate_returns_none_for_defaults_files_dict_without_platform() -> None:
"""`defaults:`/`files:` without `platform:` must not be swallowed either -- the flattener has
no `files:` branch and would silently return `[]`."""
config = {
"defaults": {"type": "rgb565"},
"files": [{"id": "a", "file": "a.png"}],
}
assert _migrate_legacy_image_config(config) is None
def test_migrate_returns_none_for_defaults_files_list_entry_without_platform() -> None:
"""Same, in a list -- previously the list branch migrated it to a hard-coded
`platform: file` instead of raising a missing-platform error."""
config = [
{
"defaults": {"type": "rgb565"},
"files": [{"id": "a", "file": "a.png"}],
}
]
assert _migrate_legacy_image_config(config) is None
# --------------------------- end legacy migration --------------------------
def test_expand_platform_entry_passes_through_plain_entry() -> None:
entry = {CONF_PLATFORM: "file", "id": "a", "file": "x.png"}
assert _expand_platform_entry(0, entry) == [entry]
def test_expand_platform_entry_expands_files_with_defaults() -> None:
entry = {
CONF_PLATFORM: "file",
CONF_DEFAULTS: {"type": "RGB565", "transparency": "opaque"},
CONF_FILES: [
{"id": "img1", "file": "foo.png"},
{"id": "img2", "file": "bar.png", "type": "GRAYSCALE"},
],
}
assert _expand_platform_entry(0, entry) == [
{
CONF_PLATFORM: "file",
"id": "img1",
"file": "foo.png",
"type": "RGB565",
"transparency": "opaque",
},
{
CONF_PLATFORM: "file",
"id": "img2",
"file": "bar.png",
"type": "GRAYSCALE",
"transparency": "opaque",
},
]
def test_expand_platform_entry_files_without_defaults() -> None:
entry = {
CONF_PLATFORM: "file",
CONF_FILES: [{"id": "img1", "file": "foo.png"}],
}
assert _expand_platform_entry(0, entry) == [
{CONF_PLATFORM: "file", "id": "img1", "file": "foo.png"}
]
def test_expand_platform_entry_preserves_source_range() -> None:
"""A merged entry keeps the source range of its `files:` item so whole-entry errors anchor there."""
from esphome import yaml_util
file_entry = yaml_util.make_data_base({"id": "img1", "file": "foo.png"})
file_entry._esp_range = "sentinel-range"
entry = {
CONF_PLATFORM: "file",
CONF_DEFAULTS: {"type": "RGB565"},
CONF_FILES: [file_entry],
}
[out] = _expand_platform_entry(0, entry)
assert isinstance(out, yaml_util.ESPHomeDataBase)
assert out.esp_range == "sentinel-range"
def test_expand_platform_entry_plain_dict_file_entry_has_no_source_range() -> None:
"""Plain-dict `files:` items must not crash -- `from_database` reads `.esp_range` unconditionally."""
entry = {
CONF_PLATFORM: "file",
CONF_FILES: [{"id": "img1", "file": "foo.png"}],
}
[out] = _expand_platform_entry(0, entry)
assert out == {CONF_PLATFORM: "file", "id": "img1", "file": "foo.png"}
def test_expand_platform_entry_per_file_overrides_win() -> None:
entry = {
CONF_PLATFORM: "file",
CONF_DEFAULTS: {"type": "RGB565"},
CONF_FILES: [{"id": "img1", "file": "foo.png", "type": "BINARY"}],
}
[out] = _expand_platform_entry(0, entry)
assert out["type"] == "BINARY"
def test_expand_platform_entry_drops_byte_order_for_non_endian_override() -> None:
"""A `byte_order` default merged into a non-endian override is dropped, as the legacy flattener did."""
entry = {
CONF_PLATFORM: "file",
CONF_DEFAULTS: {"type": "rgb565", "byte_order": "little_endian"},
CONF_FILES: [
{"id": "a", "file": "x.png"},
{"id": "b", "file": "y.png", "type": "binary"},
],
}
out = _expand_platform_entry(0, entry)
assert out[0]["byte_order"] == "little_endian"
assert "byte_order" not in out[1]
def test_expand_platform_entry_invalid_byte_order_in_defaults_raises() -> None:
"""A dropped `byte_order` inherited from `defaults:` is still validated, so a typo raises."""
entry = {
CONF_PLATFORM: "file",
CONF_DEFAULTS: {"type": "rgb565", "byte_order": "little_andian"},
CONF_FILES: [{"id": "a", "file": "x.png", "type": "binary"}],
}
with pytest.raises(cv.Invalid, match="did you mean") as excinfo:
_expand_platform_entry(0, entry)
assert excinfo.value.path == [0]
def test_expand_platform_entry_keeps_byte_order_for_endian_override() -> None:
entry = {
CONF_PLATFORM: "file",
CONF_DEFAULTS: {"type": "rgb565", "byte_order": "big_endian"},
CONF_FILES: [{"id": "a", "file": "x.png", "type": "rgb565"}],
}
[out] = _expand_platform_entry(0, entry)
assert out["byte_order"] == "big_endian"
def test_expand_platform_entry_keeps_explicit_byte_order_conflict() -> None:
"""A `byte_order` written directly on the entry is kept so validate_settings raises the normal error."""
entry = {
CONF_PLATFORM: "file",
CONF_DEFAULTS: {"type": "rgb565"},
CONF_FILES: [
{
"id": "a",
"file": "x.png",
"type": "binary",
"byte_order": "little_endian",
}
],
}
[out] = _expand_platform_entry(0, entry)
assert out["byte_order"] == "little_endian"
def test_expand_platform_entry_defaults_without_files_raises() -> None:
entry = {CONF_PLATFORM: "file", CONF_DEFAULTS: {"type": "RGB565"}}
with pytest.raises(cv.Invalid, match="may only be used together with") as excinfo:
_expand_platform_entry(0, entry)
assert excinfo.value.path == [0]
def test_expand_platform_entry_null_files_raises_not_empty() -> None:
"""A `files:` key with no value parses to `None` and must be reported clearly."""
entry = {CONF_PLATFORM: "file", CONF_DEFAULTS: {"type": "RGB565"}, CONF_FILES: None}
with pytest.raises(cv.Invalid, match="must not be empty"):
_expand_platform_entry(0, entry)
def test_expand_platform_entry_empty_files_list_raises_not_empty() -> None:
"""An explicit `files: []` must not silently drop the whole platform entry."""
entry = {CONF_PLATFORM: "file", CONF_FILES: []}
with pytest.raises(cv.Invalid, match="must not be empty"):
_expand_platform_entry(0, entry)
def test_expand_platform_entry_files_with_stray_key_raises() -> None:
entry = {
CONF_PLATFORM: "file",
CONF_FILES: [{"id": "a", "file": "x.png"}],
"extra": 1,
}
with pytest.raises(cv.Invalid, match="cannot be combined with"):
_expand_platform_entry(0, entry)
def test_expand_platform_entry_id_in_defaults_raises() -> None:
entry = {
CONF_PLATFORM: "file",
CONF_DEFAULTS: {CONF_ID: "a"},
CONF_FILES: [{"file": "x.png"}],
}
with pytest.raises(cv.Invalid, match="not allowed inside"):
_expand_platform_entry(0, entry)
def test_expand_platform_entry_platform_in_defaults_raises() -> None:
"""`platform:` inside `defaults:` would silently reassign every file's platform."""
entry = {
CONF_PLATFORM: "file",
CONF_DEFAULTS: {CONF_PLATFORM: "animation"},
CONF_FILES: [{"id": "a", "file": "x.png"}],
}
with pytest.raises(cv.Invalid, match="not allowed inside"):
_expand_platform_entry(0, entry)
def test_expand_platform_entry_platform_in_file_entry_raises() -> None:
"""`platform:` on a `files:` item must not silently override the entry's platform."""
entry = {
CONF_PLATFORM: "file",
CONF_FILES: [{"id": "a", "file": "x.png", CONF_PLATFORM: "animation"}],
}
with pytest.raises(cv.Invalid, match="not allowed inside"):
_expand_platform_entry(0, entry)
def test_expand_platform_entry_files_not_list_raises() -> None:
entry = {CONF_PLATFORM: "file", CONF_FILES: "not-a-list"}
with pytest.raises(cv.Invalid, match="must be a list"):
_expand_platform_entry(0, entry)
def test_expand_platform_entry_defaults_not_mapping_raises() -> None:
entry = {
CONF_PLATFORM: "file",
CONF_DEFAULTS: "not-a-mapping",
CONF_FILES: [{"id": "a", "file": "x.png"}],
}
with pytest.raises(cv.Invalid, match="must be a mapping"):
_expand_platform_entry(0, entry)
def test_expand_platform_entry_file_item_not_mapping_raises() -> None:
entry = {CONF_PLATFORM: "file", CONF_FILES: [1, 2]}
with pytest.raises(cv.Invalid, match="must be a mapping"):
_expand_platform_entry(0, entry)
def test_expand_platform_config_mixes_plain_and_expanded_entries() -> None:
config = [
{
CONF_PLATFORM: "file",
CONF_DEFAULTS: {"type": "RGB565"},
CONF_FILES: [
{"id": "img1", "file": "foo.png"},
{"id": "img2", "file": "bar.png"},
],
},
{CONF_PLATFORM: "file", "id": "plain", "file": "baz.png", "type": "BINARY"},
]
out = expand_platform_config(config)
assert [entry["id"] for entry in out] == ["img1", "img2", "plain"]
def test_expand_platform_config_ignores_non_platform_entries() -> None:
# Not expanded here -- legacy_config_migrate runs before this hook and is
# responsible for tagging/flattening pre-platform shapes.
config = ["not-a-platform-entry"]
assert expand_platform_config(config) == config
# --------------------- end defaults/files expansion -------------------------
def test_validate_image_final_defaults_to_little_endian() -> None:
out = validate_image_final({CONF_FILE: "x.png"})
assert out[CONF_BYTE_ORDER] == "LITTLE_ENDIAN"
@@ -0,0 +1,21 @@
# `platform: animation` entry exercising the shared `defaults:`/`files:` expansion.
display:
- platform: sdl
id: animation_display
auto_clear_enabled: false
dimensions:
width: 480
height: 480
image:
- platform: animation
defaults:
type: rgb565
transparency: opaque
resize: 50x50
files:
- id: platform_defaults_animation
file: $component_dir/anim.gif
- id: platform_defaults_animation_rgb
file: $component_dir/anim.apng
type: rgb
@@ -0,0 +1,24 @@
# `platform: file` entry using the `defaults:`/`files:` shape, including the
# per-type byte_order drop when an entry overrides to a non-endian type.
display:
- platform: sdl
id: image_display
auto_clear_enabled: false
dimensions:
width: 480
height: 480
image:
- platform: file
defaults:
type: rgb565
transparency: opaque
byte_order: little_endian
resize: 50x50
dither: FloydSteinberg
files:
- id: platform_defaults_image
file: ../../pnglogo.png
- id: platform_defaults_binary
file: ../../pnglogo.png
type: binary
+122 -2
View File
@@ -7,7 +7,7 @@ from unittest.mock import MagicMock, Mock, patch
import pytest
from esphome import config, yaml_util
from esphome import config, config_validation as cv, yaml_util
from esphome.core import CORE, AutoLoad
from esphome.types import ConfigType
@@ -127,12 +127,14 @@ def _run_load_step(
domain: str,
conf: object,
migrate: Callable[[ConfigType], list | None] | None,
expand: Callable[[list], list] | None = None,
) -> config.Config:
"""Run a LoadValidationStep for a platform component with a given migrate hook."""
"""Run a LoadValidationStep for a platform component with given hooks."""
component = Mock()
component.is_platform_component = True
component.multi_conf_no_default = False
component.legacy_config_migrate = migrate
component.expand_platform_config = expand
result = config.Config()
with (
@@ -197,6 +199,124 @@ def test_legacy_migrate_skipped_for_autoload() -> None:
assert result["image"] == [auto]
# ---------------------------------------------------------------------------
# EXPAND_PLATFORM_CONFIG hook on LoadValidationStep -- permanent counterpart
# to legacy_config_migrate; runs after legacy migration/list normalization.
# ---------------------------------------------------------------------------
def test_expand_hook_rewrites_conf() -> None:
"""A config the expand hook rewrites is replaced with the expanded list."""
expanded = [{"platform": "file", "id": "a"}, {"platform": "file", "id": "b"}]
expand = Mock(return_value=expanded)
result = _run_load_step("image", [{"platform": "file", "id": "a"}], None, expand)
expand.assert_called_once_with([{"platform": "file", "id": "a"}])
assert result["image"] == expanded
def test_expand_hook_absent_is_noop() -> None:
"""A platform component without the hook is left as normalized by the
existing list-wrapping logic."""
result = _run_load_step("image", [{"platform": "file", "id": "a"}], None, None)
assert result["image"] == [{"platform": "file", "id": "a"}]
def test_expand_hook_runs_after_legacy_migrate() -> None:
"""The expand hook sees the already-migrated list, not the raw legacy conf."""
migrated = [{"platform": "file", "id": "a"}]
migrate = Mock(return_value=migrated)
expand = Mock(side_effect=lambda conf: conf)
_run_load_step("image", [{"id": "a", "file": "x.png"}], migrate, expand)
expand.assert_called_once_with(migrated)
def test_expand_hook_skipped_for_non_dict_entry() -> None:
"""Malformed entries are left alone; the hook only sees `platform:`-tagged dicts."""
expand = Mock(side_effect=lambda conf: conf)
result = _run_load_step("image", ["not-a-dict"], None, expand)
expand.assert_not_called()
assert result["image"] == ["not-a-dict"]
def test_expand_hook_skipped_for_entry_missing_platform_key() -> None:
"""A dict entry missing the `platform:` key is left alone -- the normal
per-entry error reporting further down catches this case instead."""
expand = Mock(side_effect=lambda conf: conf)
result = _run_load_step("image", [{"id": "a"}], None, expand)
expand.assert_not_called()
assert result["image"] == [{"id": "a"}]
def test_expand_hook_skipped_for_autoload() -> None:
"""A non-empty AutoLoad reaching the hook stage is left alone."""
expand = Mock(side_effect=lambda conf: conf)
auto = AutoLoad()
auto["id"] = "a"
result = _run_load_step("image", auto, None, expand)
expand.assert_not_called()
assert result["image"] == [auto]
def test_expand_hook_runs_when_all_entries_are_platform_tagged_dicts() -> None:
"""The guard does not block the normal, well-formed case."""
expand = Mock(side_effect=lambda conf: conf)
conf = [{"platform": "file", "id": "a"}, {"platform": "animation", "id": "b"}]
result = _run_load_step("image", conf, None, expand)
expand.assert_called_once_with(conf)
assert result["image"] == conf
def test_expand_hook_invalid_reports_single_error_at_domain_path() -> None:
"""A `cv.Invalid` from the hook is reported once with the domain path prepended; no further validation runs."""
expand = Mock(side_effect=cv.Invalid("bad shape"))
pre_expand_conf = [{"platform": "file", "id": "a"}]
result = _run_load_step("image", pre_expand_conf, None, expand)
assert len(result.errors) == 1
assert result.errors[0].path == ["image"]
assert "bad shape" in str(result.errors[0])
assert result["image"] == pre_expand_conf
def test_expand_hook_final_external_invalid_reports_without_path_prepend() -> None:
"""`cv.FinalExternalInvalid` keeps its already-resolved path (no domain path prepended)."""
already_resolved_error = cv.FinalExternalInvalid(
"bad shape", path=["image", 3, "files"]
)
expand = Mock(side_effect=already_resolved_error)
pre_expand_conf = [{"platform": "file", "id": "a"}]
result = _run_load_step("image", pre_expand_conf, None, expand)
assert len(result.errors) == 1
assert result.errors[0] is already_resolved_error
assert result.errors[0].path == ["image", 3, "files"]
assert result["image"] == pre_expand_conf
def test_expand_hook_non_list_return_raises_type_error() -> None:
"""A non-list return is a component bug: it escapes as an uncaught TypeError
(explicit raise survives -O/-OO)."""
expand = Mock(return_value={"not": "a list"})
with pytest.raises(TypeError, match="must return a list"):
_run_load_step("image", [{"platform": "file", "id": "a"}], None, expand)
def _write_merge_conflict_config(tmp_path: Path, *, suppress: bool) -> Path:
"""Create a config where two `<<` includes both define `logger:`.