Fix board misidentification when OPI-PSRAM is disabled - #254
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes ESP32S3 board autodetection so boards that require OPI-PSRAM for their display buffer remain correctly identified even when OPI-PSRAM is disabled, instead of falling through to board_unknown and being mis-guessed by the caller. It also removes an unnecessary OPI-PSRAM requirement for M5StopWatch, relying on direct drawing when the optional framebuffer allocation fails.
Changes:
- Removed the OPI-PSRAM compile-time guard for M5StopWatch panel setup (direct drawing remains usable without a PSRAM framebuffer).
- For EPD boards, kept the OPI-PSRAM requirement but added early exits (
goto init_clear) so board identification is preserved even when display init is skipped.
Suppressed comments (3)
src/M5GFX.cpp:2107
- These PSRAM-missing early-exit branches jump to init_clear without creating a new panel/touch. Since init_clear unconditionally applies
_panel_last(panel(_panel_last.get())), a subsequentautodetect()call could accidentally reattach a stale panel/touch from a previous init. Clear_panel_last(and_touch_last) beforegoto init_clearto guarantee the display is actually unavailable in this path.
ESP_LOGE(LIBRARY_NAME, "M5ChainCaptain needs OPI-PSRAM enabled");
goto init_clear; // keep the board identification; the display stays unavailable
#elif !defined (CONFIG_SPIRAM_MODE_OCT)
ESP_LOGE(LIBRARY_NAME, "M5ChainCaptain needs OPI-PSRAM enabled");
goto init_clear; // keep the board identification; the display stays unavailable
src/M5GFX.cpp:2192
- These PSRAM-missing early-exit branches jump to init_clear without creating a new panel/touch. Since init_clear unconditionally applies
_panel_last(panel(_panel_last.get())), a subsequentautodetect()call could accidentally reattach a stale panel/touch from a previous init. Clear_panel_last(and_touch_last) beforegoto init_clearto guarantee the display is actually unavailable in this path.
ESP_LOGE(LIBRARY_NAME, "M5PaperColor need OPI-PSRAM enabled");
goto init_clear; // keep the board identification; the display stays unavailable
#elif !defined (CONFIG_SPIRAM_MODE_OCT)
ESP_LOGE(LIBRARY_NAME, "M5PaperColor need OPI-PSRAM enabled");
goto init_clear; // keep the board identification; the display stays unavailable
src/M5GFX.cpp:2320
- These PSRAM-missing early-exit branches jump to init_clear without creating a new panel/touch. Since init_clear unconditionally applies
_panel_last(panel(_panel_last.get())), a subsequentautodetect()call could accidentally reattach a stale panel/touch from a previous init. Clear_panel_last(and_touch_last) beforegoto init_clearto guarantee the display is actually unavailable in this path.
ESP_LOGE(LIBRARY_NAME, "%s need OPI-PSRAM enabled", board == board_t::board_M5PaperDIY ? "M5PaperDIY" : "M5PaperS3");
goto init_clear; // keep the board identification; the display stays unavailable
#elif !defined (CONFIG_SPIRAM_MODE_OCT)
ESP_LOGE(LIBRARY_NAME, "%s need OPI-PSRAM enabled", board == board_t::board_M5PaperDIY ? "M5PaperDIY" : "M5PaperS3");
goto init_clear; // keep the board identification; the display stays unavailable
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ESP_LOGE(LIBRARY_NAME, "M5PaperMono need OPI-PSRAM enabled"); | ||
| goto init_clear; // keep the board identification; the display stays unavailable | ||
| #elif !defined (CONFIG_SPIRAM_MODE_OCT) | ||
| ESP_LOGE(LIBRARY_NAME, "M5PaperMono need OPI-PSRAM enabled"); | ||
| goto init_clear; // keep the board identification; the display stays unavailable |
Panel_CO5300 supports direct drawing: the optional PSRAM frame buffer (initPanelFb) may fail to allocate and is not required, so the PSRAM guard (introduced alongside the EPD boards, which genuinely need the buffer) is unnecessary here. Worse, with PSRAM disabled the guard skipped the panel setup after the board had already been identified, and the autodetection then fell through and misidentified the device as an AtomS3Lite. Direct drawing has a limitation: drawing whose origin is at an odd coordinate may render incorrectly, so a warning is logged when the build lacks OPI-PSRAM. Verified on the real device: with PSRAM disabled the board is now detected as M5StopWatch and the display works via direct drawing; builds with and without OPI-PSRAM are both green.
When PSRAM was disabled in the build, the EPD board blocks (PaperMono, PaperS3/PaperDIY, PaperColor, ChainCaptain) logged the requirement but skipped the panel setup and fell through the rest of the autodetection, which ended with board_unknown and let the caller misidentify the device as a display-less model. Jump to init_clear instead so the board identification is kept; the display simply stays unavailable. Clear _panel_last and _touch_last before the jump: autodetect() only detaches the active panel at entry, so a stale panel object from a previous detection round would otherwise be re-attached at init_clear.
3ba0292 to
12002e9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (4)
src/M5GFX.cpp:2115
- The PSRAM guard for M5ChainCaptain repeats the same log/reset/goto in both the SPIRAM support and SPI mode checks. Consider merging these into a single
#if !(...) || !defined(...)condition to avoid duplicated maintenance.
#if !(defined(CONFIG_ESP32S3_SPIRAM_SUPPORT))
ESP_LOGE(LIBRARY_NAME, "M5ChainCaptain needs OPI-PSRAM enabled");
_panel_last.reset();
_touch_last.reset();
goto init_clear; // keep the board identification; the display stays unavailable
src/M5GFX.cpp:2204
- The PSRAM guard for M5PaperColor contains two identical error-handling branches for the SPIRAM-support and OCT-mode checks. This can be collapsed into a single combined preprocessor condition to reduce duplication.
#if !(defined(CONFIG_ESP32S3_SPIRAM_SUPPORT))
ESP_LOGE(LIBRARY_NAME, "M5PaperColor need OPI-PSRAM enabled");
_panel_last.reset();
_touch_last.reset();
goto init_clear; // keep the board identification; the display stays unavailable
src/M5GFX.cpp:2336
- The PSRAM guard for M5PaperDIY/M5PaperS3 duplicates the same error-handling block across two preprocessor branches. Combining the checks into one
#ifreduces duplication and keeps the guard consistent with other boards.
#if !(defined(CONFIG_ESP32S3_SPIRAM_SUPPORT))
ESP_LOGE(LIBRARY_NAME, "%s need OPI-PSRAM enabled", board == board_t::board_M5PaperDIY ? "M5PaperDIY" : "M5PaperS3");
_panel_last.reset();
_touch_last.reset();
goto init_clear; // keep the board identification; the display stays unavailable
src/M5GFX.cpp:2005
- The PSRAM guard for PaperMono duplicates the same error/cleanup block in both the SPIRAM support and SPI mode checks. This duplication makes future edits error-prone; the two branches can be combined into a single preprocessor condition like the M5StopWatch warning block.
This issue also appears in the following locations of the same file:
- line 2111
- line 2200
- line 2332
#if !(defined(CONFIG_ESP32S3_SPIRAM_SUPPORT))
ESP_LOGE(LIBRARY_NAME, "M5PaperMono need OPI-PSRAM enabled");
_panel_last.reset();
_touch_last.reset();
goto init_clear; // keep the board identification; the display stays unavailable
Overview
When a build lacks OPI-PSRAM, boards whose display requires it were misidentified as display-less models: the detection block had already identified the board, but skipping the panel setup let the autodetection fall through to
board_unknown, and the caller then guessed a wrong model (e.g. M5StopWatch was reported as an AtomS3Lite).Changes
Panel_CO5300supports direct drawing — the optional PSRAM frame buffer (initPanelFb) may fail to allocate and is not required — so the guard (introduced alongside the EPD boards, which genuinely need the buffer) was unnecessary.init_clearafter logging so the board identification is kept and only the display stays unavailable.Verification (real hardware)