Răsfoiți Sursa

Drop history/misstep narrative from component comments

Clean the comments future readers see: remove the FreeRTOS-task/old-component
backstory, the "while chasing the solid-black symptom / rolled back" asides,
the hand-derivation framing, and stale label wording, keeping the technical
rationale. Also normalize the 213 Z98c busy_timeout (was a "temporarily
generous" 60s) to 30s. The history lives in git, not in the headers.
ZiB 3 săptămâni în urmă
părinte
comite
5e03652283

+ 6 - 11
components/gxepd2/cycle_engine.h

@@ -20,17 +20,12 @@ namespace esphome::gxepd2 {
 using EpdSPIDevice = spi::SPIDevice<spi::BIT_ORDER_MSB_FIRST, spi::CLOCK_POLARITY_LOW, spi::CLOCK_PHASE_LEADING,
                                      spi::DATA_RATE_20MHZ>;
 
-// Mechanically replicates one GxEPD2 driver's refresh cycle - reset pulse,
-// init command table, RAM-window set, per-plane byte burst, refresh
-// trigger, busy-pin wait - as a state machine stepped from loop(), instead
-// of GxEPD2's own synchronous top-to-bottom call stack. See the plan this
-// was built from (and gxepd2_task_watchdog_status in project memory) for
-// why: that call stack has no yield points, so making it non-blocking used
-// to require running the whole thing on a separate FreeRTOS task, which
-// intermittently tripped the ESP-IDF watchdog anyway. step() never blocks -
-// every state does a small bounded unit of work and returns - so there is
-// nothing here that can starve another task or need a watchdog subscription
-// at all.
+// Replays one GxEPD2 driver's refresh cycle - reset pulse, init command table,
+// RAM-window set, per-plane byte burst, refresh trigger, busy-pin wait - as a
+// state machine stepped from loop(), instead of GxEPD2's own synchronous
+// top-to-bottom call stack. step() never blocks: every state does a small
+// bounded unit of work and returns, so nothing here can starve another task or
+// need a watchdog subscription.
 class CycleEngine {
  public:
   void begin(const EpdPanelDescriptor *desc, GPIOPin *cs, GPIOPin *dc, GPIOPin *rst, GPIOPin *busy,

+ 14 - 23
components/gxepd2/display.py

@@ -1,10 +1,8 @@
-"""ESPHome display platform driving a GxEPD2 panel via a native chunked
-refresh engine (see cycle_engine.h) instead of GxEPD2's own C++ classes.
+"""ESPHome display platform driving a GxEPD2 panel via the native replay
+engine (see cycle_engine.h) instead of GxEPD2's own C++ classes.
 
-Panels are selected via `model:`, each backed by a hand-written (later,
-extracted) EpdPanelDescriptor - see panel_213_z98c.h / panel_426_gdeq0426t82.h
-/ panel_583_gdeq0583t31.h. catalog.json/vendor/arduino_compat stay in the
-tree as reference material for panels not yet ported to this engine.
+Panels are selected via `model:`, each backed by an EpdPanelDescriptor
+(panel_*.h) generated by the capture emitter - see claude/DESIGN.md.
 """
 
 import re
@@ -94,12 +92,10 @@ CONFIG_SCHEMA = cv.All(
             cv.Optional(CONF_INVERT_MONO): cv.boolean,
         }
     ),
-    # IDF-first, but the runtime C++ uses only framework-portable ESPHome
-    # abstractions (spi::SPIDevice, GPIOPin, hal.h, RAMAllocator, ESP_LOG*) -
-    # the old IDF-only gate was a leftover from the superseded FreeRTOS-task
-    # design. ESP32-Arduino is a supported accommodation; ESP8266-Arduino
-    # compiles too but is resolution-restricted at 1bpp and lacks the PSRAM
-    # that >1bpp (color) panels need, so those fail allocation at runtime.
+    # The runtime C++ uses only framework-portable ESPHome abstractions, so
+    # both IDF and ESP32-Arduino are supported. ESP8266-Arduino compiles too
+    # but is resolution-restricted at 1bpp and lacks the PSRAM that >1bpp
+    # (color) panels need, so those fail allocation at runtime.
     cv.only_with_framework([Framework.ESP_IDF, Framework.ARDUINO]),
 )
 
@@ -123,19 +119,14 @@ async def to_code(config: ConfigType) -> None:
     # will compute it from the (aliased) model. An explicit invert_mono wins.
     cg.add(var.set_invert_mono(config.get(CONF_INVERT_MONO, False)))
 
-    # Panel label for test_pattern(): the Good Display part number in the
-    # vendor's dashed silkscreen form (GDEQ-0583T31), so a tested panel shows
-    # what it is on the shelf, not the GxEPD2 C++ class id.
+    # Panel label for test_pattern(): the Good Display part number minus its
+    # series prefix (GDEQ0583T31 -> 0583T31), so a tested panel shows what it
+    # is on the shelf, not the GxEPD2 C++ class id.
     cg.add(var.set_model_name(_panel_label(part_number)))
 
-    # display.register_display() only wires up `pages:` - a bare top-level
-    # `lambda:` (what our CONFIG_SCHEMA actually supports) has to be
-    # processed and attached via set_writer() by the platform itself, same
-    # as every other display component's to_code() (see e.g. epaper_spi's
-    # display.py). Missing this meant the configured lambda never ran at
-    # all - the only thing ever drawn was auto_clear's default
-    # fill(COLOR_OFF), which is why every real-hardware run came out solid
-    # black regardless of any protocol-level change.
+    # A top-level `lambda:` must be processed and attached via set_writer()
+    # here - register_display() only wires up `pages:` (same pattern as every
+    # other display component's to_code(), e.g. epaper_spi).
     if CONF_LAMBDA in config:
         lambda_ = await cg.process_lambda(config[CONF_LAMBDA], [(display.DisplayRef, "it")], return_type=cg.void)
         cg.add(var.set_writer(lambda_))

+ 6 - 8
components/gxepd2/gxepd2_display.h

@@ -19,11 +19,9 @@ namespace esphome::gxepd2 {
 
 static const char *const TAG = "gxepd2";
 
-// Native chunked-engine replacement for the old GxEPD2-template/FreeRTOS-task
-// component - see the plan this was built from (and project memory entry
-// gxepd2_task_watchdog_status) for why. Panels arrive as hand-written
-// (later, extracted) descriptors selected at runtime via set_descriptor(),
-// not by templating this class over GxEPD2's own driver classes.
+// ESPHome display component for a GxEPD2 panel. Panels arrive as
+// EpdPanelDescriptors selected at runtime via set_descriptor() and driven by
+// CycleEngine, not by templating this class over GxEPD2's own driver classes.
 class GxEPD2Display : public display::DisplayBuffer {
  public:
   void set_cs_pin(GPIOPin *pin) { this->cs_ = pin; }
@@ -289,9 +287,9 @@ class GxEPD2Display : public display::DisplayBuffer {
     bool bright = color.red > 0x7f || color.green > 0x7f || color.blue > 0x7f;
     uint32_t idx = (static_cast<uint32_t>(y) * this->desc_->width + x) / 8;
     uint8_t mask = 0x80 >> (x % 8);
-    // Same classification the old task-based component used: red/accent
-    // pixels go to the color plane (when the panel has one), everything
-    // else is black/white on the black plane. get_display_type() reports
+    // Red/accent pixels go to the color plane (when the panel has one),
+    // everything else is black/white on the black plane. get_display_type()
+    // reports
     // DISPLAY_TYPE_COLOR, so callers draw literal RGB colors, not the
     // on/off is_on() convention, even on a plane_count==1 (BW-only) panel -
     // there just is no color plane for a "red" classification to land on.

+ 18 - 48
components/gxepd2/panel_213_z98c.h

@@ -1,33 +1,20 @@
 #pragma once
 
-// Hand-derived from vendor/src/epd3c/GxEPD2_213_Z98c.{h,cpp} (GDEY0213Z98,
-// SSD1680 controller, 128x250 RAM window (122 columns visible), black +
-// red). This is the panel actually on the bench for this first slice of the
-// native chunked engine; see the plan this was implemented from (and
-// gxepd2_task_watchdog_status in project memory) for why GxEPD2's own
-// control flow is being dropped in favor of just its per-panel data.
+// GDEY0213Z98, SSD1680 controller, 128x250 RAM window (122 columns visible),
+// black + red. Derived from vendor/src/epd3c/GxEPD2_213_Z98c.{h,cpp}.
 
 #include "epd_command.h"
 
 namespace esphome::gxepd2 {
 
-// GxEPD2_213_Z98c::_InitDisplay(). Unlike GxEPD2_154_Z90c (the panel this
-// slice originally targeted), this controller's SWRESET is bracketed by
-// fixed 10ms delays rather than a busy-pin wait, and it has an extra
-// "Display update control" command (0x21) 154_Z90c doesn't. Minus the
-// trailing _setPartialRamArea() - CycleEngine's SET_WINDOW state (used for
-// every write, not just init) covers that.
+// _InitDisplay(): SWRESET (0x12) bracketed by fixed 10ms delays (not a
+// busy-pin wait), driver output control, data entry mode, border waveform,
+// temp sensor, and display update control (0x21). The trailing
+// _setPartialRamArea() is dropped - CycleEngine's SET_WINDOW covers it.
 inline const uint8_t PANEL_213_Z98C_DRIVER_OUTPUT_CONTROL[] = {0xF9, 0x00, 0x00};
 inline const uint8_t PANEL_213_Z98C_DATA_ENTRY_MODE[] = {0x03};
 inline const uint8_t PANEL_213_Z98C_BORDER_WAVEFORM[] = {0x05};
 inline const uint8_t PANEL_213_Z98C_TEMP_SENSOR[] = {0x80};
-// Stock GxEPD2 vendor value. Was swapped to {0x80, 0x80} (matching the
-// epaper_spi reference) while chasing the solid-black/solid-red hardware
-// symptom - that turned out to be an unrelated bug (display.py never
-// wired the configured lambda to set_writer() at all, so nothing but
-// auto_clear's default fill ever rendered). Rolled back now that the real
-// cause is fixed, to test whether the stock GxEPD2 protocol works as-is
-// on this panel or genuinely needs epaper_spi's deviations.
 inline const uint8_t PANEL_213_Z98C_DISPLAY_UPDATE_CONTROL[] = {0x00, 0x80};
 
 inline const EpdCommand PANEL_213_Z98C_INIT[] = {
@@ -39,13 +26,8 @@ inline const EpdCommand PANEL_213_Z98C_INIT[] = {
     {0x21, PANEL_213_Z98C_DISPLAY_UPDATE_CONTROL, 2},
 };
 
-// GxEPD2_213_Z98c::_Update_Full()/_Update_Part() - stock vendor sequence,
-// a single combined trigger that assumes it handles power internally.
-// Bracketed this with explicit power-on/off (matching epaper_spi) while
-// chasing the solid-black/solid-red symptom, which turned out to be
-// unrelated (see PANEL_213_Z98C_DISPLAY_UPDATE_CONTROL's comment). Rolled
-// back to the stock single-trigger sequence to test it now that the real
-// bug is fixed.
+// _Update_Full(): a single combined trigger (0x22 update control + 0x20) that
+// handles power internally.
 inline const uint8_t PANEL_213_Z98C_UPDATE_MODE[] = {0xF7};
 
 inline const EpdCommand PANEL_213_Z98C_REFRESH[] = {
@@ -53,12 +35,8 @@ inline const EpdCommand PANEL_213_Z98C_REFRESH[] = {
     {0x20, nullptr, 0, /*delay_before_ms=*/0, /*delay_after_ms=*/0, /*wait_busy_after=*/true},
 };
 
-// GxEPD2_EPD::_setPartialRamArea(0, 0, WIDTH, HEIGHT) - SSD1680's
-// byte-column addressing (0x44's operands are RAM byte columns, not raw
-// pixels), no Y-reversal. Was hardcoded into CycleEngine as the assumed-
-// universal window layout until GxEPD2_426_GDEQ0426T82/GxEPD2_583_GDEQ0583T31
-// showed two other controllers do this differently - moved here as plain
-// descriptor data, computed for this panel's fixed 128x250 RAM window.
+// _setPartialRamArea(0, 0, WIDTH, HEIGHT): SSD1680 byte-column X addressing
+// (0x44's operands are RAM byte columns, not raw pixels), no Y-reversal.
 inline const uint8_t PANEL_213_Z98C_WINDOW_X[] = {0x00, 0x0F};              // xe/8 = 127/8
 inline const uint8_t PANEL_213_Z98C_WINDOW_Y[] = {0x00, 0x00, 0xF9, 0x00};  // ye = 249
 inline const uint8_t PANEL_213_Z98C_WINDOW_XC[] = {0x00};
@@ -71,31 +49,23 @@ inline const EpdCommand PANEL_213_Z98C_WINDOW[] = {
     {0x4F, PANEL_213_Z98C_WINDOW_YC, 2},
 };
 
-// full_refresh_time/partial_refresh_time from the vendor header are both
-// 15000ms; used as the ceiling for every busy wait in this slice, not just
-// the refresh one it was measured for - simpler than carrying a separate
-// timeout per wait site, and erring long only delays reporting a genuine
-// timeout, it doesn't cause one.
+// Vendor full_refresh_time is 15000ms; used as the ceiling for every busy
+// wait (erring long only delays reporting a genuine timeout).
 inline const EpdPanelDescriptor PANEL_213_Z98C = {
-    .width = 128,   // RAM window width - _setPartialRamArea works in this, not WIDTH_VISIBLE (122)
+    .width = 128,   // RAM window width (addressing works in this, not WIDTH_VISIBLE=122)
     .height = 250,
     .busy_active_level = true,  // GxEPD2_EPD(..., /*busy_level=*/HIGH, ...)
-    .busy_timeout_ms = 60000,  // temporarily generous - diagnosing a busy-never-clears hang, see commit message
+    .busy_timeout_ms = 30000,
     .init_sequence = PANEL_213_Z98C_INIT,
     .init_sequence_len = sizeof(PANEL_213_Z98C_INIT) / sizeof(PANEL_213_Z98C_INIT[0]),
     .window_sequence = {PANEL_213_Z98C_WINDOW, nullptr},
     .window_sequence_len = {sizeof(PANEL_213_Z98C_WINDOW) / sizeof(PANEL_213_Z98C_WINDOW[0]), 0},
     .plane_count = 2,
     .plane_write_cmd = {0x24, 0x26},
-    // Stock GxEPD2 wire convention for the color plane, matched to
-    // DISPLAY_UPDATE_CONTROL={0x00, 0x80} above: vendor's writeImage()
-    // does _writeData(~data) where data's own "blank" default is 0xFF, so
-    // wire 0x00 = blank/not-red there - the opposite of epaper_spi's
-    // {0x80, 0x80} + inverted-buffer pairing. These two are coupled, not
-    // independent facts - confirmed by testing {0x00,0x80} register with
-    // invert=true still on: it cross-inverted (background/black text came
-    // out red, drawn-red text came out white). invert=false here matches
-    // this register value's actual polarity instead.
+    // Color-plane wire convention: vendor writeImage() does _writeData(~data)
+    // with a 0xFF "blank" default, so wire 0x00 = not-red; our buffer's 1=red
+    // bit matches that, so no inversion. (DISPLAY_UPDATE_CONTROL={0x00,0x80}
+    // above sets the matching plane polarity.) Bench-confirmed.
     .plane_invert = {false, false},
     .refresh_sequence = PANEL_213_Z98C_REFRESH,
     .refresh_sequence_len = sizeof(PANEL_213_Z98C_REFRESH) / sizeof(PANEL_213_Z98C_REFRESH[0]),