From cd9c3d2eee8eaecbecd8f5db8ee7bb25b0f39fca Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Tue, 25 Aug 2026 16:22:07 +1200 Subject: [PATCH] [ethernet] Validate the shared spi bus via the standard spi device schema Replaces the hand-rolled bus lookup in _final_validate_spi with spi.final_validate_device_schema, which also enforces mosi_pin on the shared bus (previously only miso_pin was checked), plus an id_declaration_match_schema check that the bus uses a hardware interface. Errors now anchor to the referenced spi: block. --- esphome/components/ethernet/__init__.py | 39 +++++++----- tests/component_tests/ethernet/test_spi_id.py | 62 ++++++++++++------- 2 files changed, 62 insertions(+), 39 deletions(-) diff --git a/esphome/components/ethernet/__init__.py b/esphome/components/ethernet/__init__.py index 17efe745b3..0454440f14 100644 --- a/esphome/components/ethernet/__init__.py +++ b/esphome/components/ethernet/__init__.py @@ -569,23 +569,28 @@ def _final_validate_spi(config: ConfigType) -> None: return from esphome.components.spi import CONF_INTERFACE_INDEX, get_spi_interface - if (spi_id := config.get(CONF_SPI_ID)) is not None: - # Sharing the bus: the referenced spi component must own a hardware - # host (the IDF ethernet drivers require one) and expose MISO so the - # ethernet chip can be read. - spi_conf = next( - c for c in fv.full_config.get()[CONF_SPI] if c[CONF_ID] == spi_id - ) - if CONF_INTERFACE_INDEX not in spi_conf: - raise cv.Invalid( - f"The 'spi' bus referenced by '{CONF_SPI_ID}' must use a hardware " - f"'{CONF_INTERFACE}' to be shared with 'ethernet'." - ) - if CONF_MISO_PIN not in spi_conf: - raise cv.Invalid( - f"The 'spi' bus referenced by '{CONF_SPI_ID}' must declare a " - f"'{CONF_MISO_PIN}' to be shared with 'ethernet'." - ) + if CONF_SPI_ID in config: + # Sharing the bus: the standard spi device schema enforces that the + # referenced bus declares both data lines. The IDF ethernet drivers + # additionally need a hardware host, which shows as an interface index + # on the validated bus config. + spi.final_validate_device_schema( + "ethernet", require_mosi=True, require_miso=True + )(config) + cv.Schema( + { + cv.Required(CONF_SPI_ID): fv.id_declaration_match_schema( + { + cv.Required( + CONF_INTERFACE_INDEX, + msg="Component ethernet requires this spi bus to use " + "a hardware interface", + ): cv.valid + } + ) + }, + extra=cv.ALLOW_EXTRA, + )(config) return if spi_configs := fv.full_config.get().get(CONF_SPI): diff --git a/tests/component_tests/ethernet/test_spi_id.py b/tests/component_tests/ethernet/test_spi_id.py index 7f40563e6d..cf87045751 100644 --- a/tests/component_tests/ethernet/test_spi_id.py +++ b/tests/component_tests/ethernet/test_spi_id.py @@ -134,24 +134,39 @@ def _eth_spi_id_final_config() -> dict: return {CONF_TYPE: "W5500", CONF_SPI_ID: ID("spi_bus")} -def test_final_validate_accepts_hardware_bus_with_miso( +class _FakeFinalConfig(dict): + """Dict-backed FinalValidateConfig with just enough ID resolution for + fv.id_declaration_match_schema to find an spi bus fragment.""" + + def get_path_for_id(self, id: ID) -> list: + for index, conf in enumerate(self[CONF_SPI]): + if conf[CONF_ID] == id: + return [CONF_SPI, index, CONF_ID] + raise KeyError(id) + + def get_config_for_path(self, path: list) -> dict: + return self[path[0]][path[1]] + + +def _set_spi_buses(*buses: dict) -> None: + fv.full_config.set(_FakeFinalConfig({CONF_SPI: list(buses)})) + + +_SHAREABLE_BUS = { + CONF_ID: ID("spi_bus"), + CONF_INTERFACE_INDEX: 0, + CONF_MISO_PIN: {}, + CONF_MOSI_PIN: {}, +} + + +def test_final_validate_accepts_hardware_bus_with_data_pins( set_core_config: SetCoreConfigCallable, ) -> None: - """A hardware spi bus that declares miso_pin may be shared.""" + """A hardware spi bus that declares miso_pin and mosi_pin may be shared.""" _set_esp32_s3(set_core_config) - fv.full_config.set( - { - CONF_SPI: [ - # An unrelated bus first: the lookup must skip past it. - {CONF_ID: ID("other_bus"), CONF_INTERFACE_INDEX: 1}, - { - CONF_ID: ID("spi_bus"), - CONF_INTERFACE_INDEX: 0, - CONF_MISO_PIN: {}, - }, - ] - } - ) + # An unrelated bus first: the ID lookup must skip past it. + _set_spi_buses({CONF_ID: ID("other_bus"), CONF_INTERFACE_INDEX: 1}, _SHAREABLE_BUS) _final_validate(_eth_spi_id_final_config()) @@ -160,18 +175,21 @@ def test_final_validate_rejects_software_bus( ) -> None: """A software spi bus (no hardware interface index) cannot be shared.""" _set_esp32_s3(set_core_config) - fv.full_config.set({CONF_SPI: [{CONF_ID: ID("spi_bus"), CONF_MISO_PIN: {}}]}) - with pytest.raises(Invalid, match="must use a hardware 'interface'"): + bus = {k: v for k, v in _SHAREABLE_BUS.items() if k != CONF_INTERFACE_INDEX} + _set_spi_buses(bus) + with pytest.raises(Invalid, match="requires this spi bus to use a hardware"): _final_validate(_eth_spi_id_final_config()) -def test_final_validate_rejects_bus_without_miso( - set_core_config: SetCoreConfigCallable, +@pytest.mark.parametrize("pin_key", [CONF_MISO_PIN, CONF_MOSI_PIN]) +def test_final_validate_rejects_bus_without_data_pin( + set_core_config: SetCoreConfigCallable, pin_key: str ) -> None: - """The shared bus must declare miso_pin; the ethernet chip needs to read.""" + """The shared bus must declare both data pins to drive the ethernet chip.""" _set_esp32_s3(set_core_config) - fv.full_config.set({CONF_SPI: [{CONF_ID: ID("spi_bus"), CONF_INTERFACE_INDEX: 0}]}) - with pytest.raises(Invalid, match="must declare a 'miso_pin'"): + bus = {k: v for k, v in _SHAREABLE_BUS.items() if k != pin_key} + _set_spi_buses(bus) + with pytest.raises(Invalid, match=f"requires this spi bus to declare a {pin_key}"): _final_validate(_eth_spi_id_final_config())