Shared-core maintenance: BFF font RLE guard, AMOLED mirrored rotation, LEDC guard fixes - #248
Merged
Merged
Conversation
… intr_type guard ledc_channel_config_t.intr_type carries a deprecated attribute from ESP-IDF v6 (interrupts are handled inside the driver), so it is now assigned only when building against an earlier IDF. The Arduino and IDF code paths are consolidated behind LGFX_LEDCINIT / LGFX_LEDC_SPEED_MODE macros selected once at the top of the file.
There was a problem hiding this comment.
Pull request overview
This PR batches four maintenance fixes to the shared LovyanGFX core, targeting correctness and portability across font decoding, AMOLED rotation handling, ESP32 PWM backlight configuration, and ESP32 SoC header availability.
Changes:
- Fixes the BFF RLE font decoder’s “first pixel” run-guard condition.
- Adds missing MADCTL values for mirrored AMOLED rotations (modes 4–7).
- Refactors ESP32
Light_PWMto consolidate Arduino/ESP-IDF LEDC handling and gatesintr_typeassignment for ESP-IDF < v6. - Guards optional ESP32
soc/*headers with__has_includefor better toolchain/target robustness.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/lgfx/v1/platforms/esp32/Light_PWM.cpp | Consolidates Arduino LEDC init/write macros and ESP-IDF speed-mode handling; adjusts intr_type assignment guard. |
| src/lgfx/v1/platforms/esp32/common.hpp | Splits SoC header includes into individual __has_include guards. |
| src/lgfx/v1/panel/Panel_AMOLED.cpp | Adds MADCTL mappings for mirrored rotation modes 4–7. |
| src/lgfx/v1/lgfx_fonts.cpp | Corrects RLE repeated-run detection to avoid misfiring on the first output pixel. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+35
to
+43
| #if defined ESP_ARDUINO_VERSION // use ledc* functions shipped with arduino-esp32 core | ||
| #if ESP_ARDUINO_VERSION >= ESP_ARDUINO_VERSION_VAL(3, 0, 0) | ||
| #define LGFX_LEDCINIT(pin_bl, freq, bits, pwm_channel) ledcAttach(pin_bl, freq, bits) | ||
| #define LGFX_LEDCWRITE(pin_bl, pwm_channel, duty) ledcWrite(pin_bl, duty) | ||
| #elif ESP_ARDUINO_VERSION >= ESP_ARDUINO_VERSION_VAL(2, 0, 0) | ||
| #define LGFX_LEDCINIT(pin_bl, freq, bits, pwm_channel) { ledcSetup(pwm_channel, freq, bits); ledcAttachPin(pin_bl, pwm_channel); } | ||
| #define LGFX_LEDCWRITE(pin_bl, pwm_channel, duty) ledcWrite(pwm_channel, duty) | ||
| #endif | ||
| #endif |
Contributor
Author
There was a problem hiding this comment.
Addressed in 83d0b75: the new-API branch now applies only on cores that provably report 3.0.0+, and everything else (2.x, and 1.x where the version macro doesn't exist) falls back to the legacy ledcSetup/ledcAttachPin/ledcWrite(channel) API. Verified the branch selection against the esp32-hal-ledc.h headers of 1.0.6 / 2.0.0 / 2.0.17 / 3.0.0 / 3.3.x.
…ro is absent ESP_ARDUINO_VERSION was introduced with arduino-esp32 2.0.0, so on the 1.x cores neither branch defined LGFX_LEDCINIT / LGFX_LEDCWRITE and the calls below failed to compile. Define the new API variant only when the version is known to be 3.0.0 or later, and fall back to ledcSetup/ledcAttachPin/ledcWrite(channel) everywhere else, which is the only API those old cores provide.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four pending fixes for the shared graphics core, batched into one sync PR:
BFF font: correct first-pixel guard in
decode_rle_bitmap(cherry-picked, original author credited)The guard for reusing the previous run value keyed off
bs->bit_pos != bpp, which misfires on the first pixel of a glyph; it now keys offout != 0.Panel_AMOLED: mirrored rotation support (modes 4..7) (cherry-picked, original author credited)
Adds the missing MADCTL values for the four mirrored orientations.
Light_PWM: consolidate LEDC conditional blocks and correct the IDF v6
intr_typeguardledc_channel_config_t.intr_typecarries__attribute__((deprecated))from ESP-IDF v6 (components/esp_driver_ledc/include/driver/ledc.h) — interrupts are handled inside the driver — so the field is now assigned only when building against an earlier IDF. On IDF 5.x the field is still current and the assignment is kept. The Arduino / ESP-IDF code paths are consolidated behind macros selected once at the top of the file; behavior on the success path is unchanged (Arduino 2.x keepsledcSetup/ledcAttachPin, 3.x+ keepsledcAttach).common.hpp: guard optional soc headers with
__has_includeRobustness for toolchains/targets where
soc/i2s_reg.hetc. are absent.Testing
lgfx_fonts.cpp/Panel_AMOLED.cpp); the ESP32 paths are exercised by this repository's CI matrix.