[mipi_rgb] Fix SWRESET delay for st7701s (#17646)

esphome/components/mipi/__init__.py

@@ -606,11 +606,11 @@ class DriverChip:
         """
         Create the init sequence for the display.
         Use the default sequence from the model, if any, and append any custom sequence provided in the config.
-        Append SLPOUT (if not already in the sequence) and DISPON to the end of the sequence
+        Append SLPOUT (if not suppressed by the model) and DISPON to the end of the sequence
         MADCTL will be set if add_madctl is True
         If add_reset is True, a reset is prepended: a software reset when no reset pin
         is configured (and the model doesn't skip it), followed by a settling delay that
-        both a software and a hardware reset require.
+        both a software and a hardware reset require. The delay length is set via reset_delay, and defaults to 10ms.
         Returns the init sequence
         """
         sequence = list(self.initsequence or ())
@@ -620,12 +620,16 @@ class DriverChip:
         sequence = [x if isinstance(x, tuple) else (x,) for x in sequence]
 
         if add_reset:
+            # Matches the 1-255ms range map_sequence() already allows for a "delay N" entry.
+            reset_delay = self.get_default("reset_delay", 10)
+            if reset_delay < 1 or reset_delay > 255:
+                raise ValueError("reset_delay must be between 1 and 255ms")
             reset: list = []
             # A software reset is only needed when there is no hardware reset pin.
             if CONF_RESET_PIN not in config and not self.skip_command("SWRESET"):
                 reset.append((SWRESET,))
             # Both a software and a hardware reset need a settling delay before further commands.
-            reset.append(delay(10))
+            reset.append(delay(reset_delay))
             sequence = reset + sequence
 
         # Set pixel format if not already in the custom sequence

esphome/components/mipi_rgb/display.py

@@ -285,7 +285,7 @@ async def to_code(config: ConfigType) -> None:
 
     if CONF_SPI_ID in config:
         await spi.register_spi_device(var, config, write_only=True)
-        sequence = model.get_sequence(config)
+        sequence = model.get_sequence(config, add_reset=True)
         cg.add(var.set_init_sequence(sequence))
 
     cg.add(var.set_color_mode(COLOR_ORDERS[config[CONF_COLOR_ORDER]]))

esphome/components/mipi_rgb/mipi_rgb.cpp

@@ -44,8 +44,10 @@ void MipiRgb::setup_enables_() {
 void MipiRgbSpi::setup() {
   this->setup_enables_();
   this->spi_setup();
-  this->write_init_sequence_();
   this->common_setup_();
+  if (this->is_failed())
+    return;
+  this->write_init_sequence_();
 }
 void MipiRgbSpi::write_command_(uint8_t value) {
   this->enable();

esphome/components/mipi_rgb/models/st7701s.py

@@ -7,6 +7,10 @@ SDIR_CMD = 0xC7
 
 
 class ST7701S(RgbDriverChip):
+    def __init__(self, *args, reset_delay=50, **kwargs):
+        kwargs["reset_delay"] = reset_delay
+        super().__init__(*args, **kwargs)
+
     # The ST7701s does not use the standard MADCTL bits for x/y mirroring
     def add_madctl(self, sequence: list, config: dict) -> int:
         transform = self.get_transform(config)
@@ -49,7 +53,6 @@ st7701s = ST7701S(
     pclk_frequency="16MHz",
     pclk_inverted=True,
     initsequence=(
-        (0x01,),  # Software Reset
         (0xFF, 0x77, 0x01, 0x00, 0x00, 0x10),  # Page 0
         (0xC0, 0x3B, 0x00), (0xC1, 0x0D, 0x02), (0xC2, 0x31, 0x05),
         (0xB0, 0x00, 0x11, 0x18, 0x0E, 0x11, 0x06, 0x07, 0x08, 0x07, 0x22, 0x04, 0x12, 0x0F, 0xAA, 0x31, 0x18,),

tests/component_tests/mipi_rgb/test_mipi_rgb_config.py

@@ -1,5 +1,7 @@
 """Tests for mipi_rgb configuration validation."""
 
+from collections.abc import Generator
+
 import pytest
 
 from esphome import config_validation as cv
@@ -17,6 +19,7 @@ from esphome.components.esp32 import (
     VARIANT_ESP32S3,
     VARIANT_ESP32S31,
 )
+from esphome.components.mipi import DriverChip
 import esphome.components.pca9554  # noqa: F401
 import esphome.components.xl9535  # noqa: F401
 from esphome.const import (
@@ -44,6 +47,19 @@ DATA_PINS = {
 }
 
 
+@pytest.fixture(autouse=True)
+def _remove_test_models() -> Generator[None]:
+    """Unregister chips created by a test.
+
+    display.py modules drain DriverChip.models when first imported, so a
+    leftover TEST-* chip could become a selectable model there.
+    """
+    existing = set(DriverChip.models)
+    yield
+    for name in set(DriverChip.models) - existing:
+        del DriverChip.models[name]
+
+
 def _set_s3(set_core_config: SetCoreConfigCallable) -> None:
     set_core_config(
         PlatformFramework.ESP32_IDF,
@@ -176,6 +192,41 @@ def test_configuration_succeeds_on_supported_variants(
     CONFIG_SCHEMA(config)
 
 
+def test_st7701s_default_reset_delay() -> None:
+    """ST7701S instances default to a 50ms reset delay.
+
+    The datasheet's stated 5ms is too short in practice; ST7701S overrides the
+    DriverChip default of 10ms with its own default of 50ms.
+    """
+    from esphome.components.mipi_rgb.models.st7701s import st7701s
+
+    assert st7701s.get_default("reset_delay") == 50
+
+
+def test_st7701s_reset_delay_can_be_overridden() -> None:
+    """An explicit reset_delay overrides the ST7701S default of 50ms."""
+    from esphome.components.mipi_rgb.models.st7701s import ST7701S
+
+    chip = ST7701S("TEST-ST7701S-RESET-DELAY", width=480, height=480, reset_delay=99)
+
+    assert chip.get_default("reset_delay") == 99
+
+
+def test_st7701s_extend_inherits_reset_delay_default() -> None:
+    """extend() carries the 50ms default forward to derived board models.
+
+    Every shipped ST7701S variant is built via ``st7701s.extend(...)`` rather
+    than direct construction, so the override in ``ST7701S.__init__`` must
+    survive that path (see DriverChip.extend, which re-passes the copied
+    defaults as kwargs to the constructor).
+    """
+    from esphome.components.mipi_rgb.models.st7701s import st7701s
+
+    extended = st7701s.extend("TEST-ST7701S-EXTEND", width=480, height=480)
+
+    assert extended.get_default("reset_delay") == 50
+
+
 def test_only_on_variant_rejects_unsupported_variant(
     set_core_config: SetCoreConfigCallable,
 ) -> None:

tests/component_tests/mipi_rgb/test_reset_sequence.py

@@ -0,0 +1,81 @@
+"""End-to-end tests for the mipi_rgb SPI reset sequence.
+
+These exercise the actual codegen path (mipi_rgb/display.py's
+``model.get_sequence(config, add_reset=True)`` call) rather than calling
+DriverChip.get_sequence directly, so a regression that drops add_reset or
+reintroduces a hardcoded SWRESET into a model's initsequence would be caught
+here.
+"""
+
+from collections.abc import Callable
+from pathlib import Path
+
+# A model with no reset_pin default: SWRESET ({1, 0}) is prepended ahead of the
+# inherited ST7701S reset_delay ({50, 255}).
+_NO_RESET_PIN_YAML = """
+esphome:
+  name: mipi-rgb-reset-test
+esp32:
+  board: esp32-s3-devkitc-1
+  framework:
+    type: esp-idf
+psram:
+  mode: octal
+spi:
+  id: spi_bus
+  clk_pin: 10
+  mosi_pin: 11
+display:
+  - platform: mipi_rgb
+    id: no_reset_display
+    spi_id: spi_bus
+    model: MAKERFABS-4
+"""
+
+# A model with a reset_pin default: no SWRESET, just the settling delay.
+_RESET_PIN_YAML = """
+esphome:
+  name: mipi-rgb-reset-test
+esp32:
+  board: esp32-s3-devkitc-1
+  framework:
+    type: esp-idf
+psram:
+  mode: octal
+spi:
+  id: spi_bus
+  clk_pin: 6
+  mosi_pin: 7
+display:
+  - platform: mipi_rgb
+    id: has_reset_display
+    spi_id: spi_bus
+    model: WAVESHARE-3.16-320X820
+"""
+
+
+def test_swreset_and_reset_delay_without_reset_pin(
+    generate_main: Callable[[str | Path], str],
+    tmp_path: Path,
+) -> None:
+    """A model with no reset_pin gets SWRESET plus the ST7701S 50ms delay."""
+    yaml_file = tmp_path / "no_reset.yaml"
+    yaml_file.write_text(_NO_RESET_PIN_YAML)
+
+    main_cpp = generate_main(yaml_file)
+
+    assert "no_reset_display->set_init_sequence({1, 0, 50, 255," in main_cpp
+
+
+def test_reset_delay_only_with_reset_pin(
+    generate_main: Callable[[str | Path], str],
+    tmp_path: Path,
+) -> None:
+    """A model with a reset_pin default skips SWRESET but keeps the settling delay."""
+    yaml_file = tmp_path / "has_reset.yaml"
+    yaml_file.write_text(_RESET_PIN_YAML)
+
+    main_cpp = generate_main(yaml_file)
+
+    assert "has_reset_display->set_init_sequence({50, 255," in main_cpp
+    assert "has_reset_display->set_init_sequence({1, 0," not in main_cpp

tests/component_tests/mipi_spi/test_get_sequence.py

@@ -0,0 +1,64 @@
+"""Tests for DriverChip.get_sequence's reset-delay handling."""
+
+from collections.abc import Generator
+
+import pytest
+
+from esphome.components.mipi import CONF_INVERT_COLORS, CONF_PIXEL_MODE, DriverChip
+
+# A minimal config with no reset pin: enough for get_sequence(add_madctl=False) to run
+# without needing a full display configuration.
+_BASE_CONFIG = {CONF_PIXEL_MODE: "16bit", CONF_INVERT_COLORS: False}
+
+
+@pytest.fixture(autouse=True)
+def _remove_test_models() -> Generator[None]:
+    """Unregister chips created by a test."""
+    existing = set(DriverChip.models)
+    yield
+    for name in set(DriverChip.models) - existing:
+        del DriverChip.models[name]
+
+
+def test_get_sequence_defaults_to_10ms_reset_delay() -> None:
+    """A model with no reset_delay default falls back to a 10ms settling delay."""
+    chip = DriverChip("TEST-GET-SEQUENCE-DEFAULT")
+
+    sequence = chip.get_sequence(_BASE_CONFIG, add_madctl=False, add_reset=True)
+
+    # SWRESET ({1, 0}) is prepended (no reset pin configured), followed by the
+    # 10ms settling delay, flattened to {10, 255}.
+    assert sequence[:4] == (1, 0, 10, 255)
+
+
+def test_get_sequence_uses_model_reset_delay_default() -> None:
+    """A model's own reset_delay default overrides the base 10ms default."""
+    chip = DriverChip("TEST-GET-SEQUENCE-CUSTOM-DELAY", reset_delay=99)
+
+    sequence = chip.get_sequence(_BASE_CONFIG, add_madctl=False, add_reset=True)
+
+    assert sequence[:4] == (1, 0, 99, 255)
+
+
+@pytest.mark.parametrize("reset_delay", [0, 256])
+def test_get_sequence_rejects_out_of_range_reset_delay(reset_delay: int) -> None:
+    """reset_delay outside 1-255ms is rejected.
+
+    This matches the 1-255ms range map_sequence() already allows for a
+    "delay N" entry in a custom init sequence.
+    """
+    chip = DriverChip("TEST-GET-SEQUENCE-BAD-DELAY", reset_delay=reset_delay)
+
+    with pytest.raises(ValueError, match="reset_delay must be between 1 and 255ms"):
+        chip.get_sequence(_BASE_CONFIG, add_madctl=False, add_reset=True)
+
+
+def test_get_sequence_skips_reset_delay_validation_without_add_reset() -> None:
+    """An out-of-range reset_delay is only checked when add_reset is requested.
+
+    mipi_dsi calls get_sequence with add_reset=False and never uses
+    reset_delay, so an invalid default there should not raise.
+    """
+    chip = DriverChip("TEST-GET-SEQUENCE-NO-RESET", reset_delay=999)
+
+    chip.get_sequence(_BASE_CONFIG, add_madctl=False, add_reset=False)