esp-idf: Fix build against latest esp-idf and for esp32-p4 #4810
Conversation
|
Ah sigh, the change to esp-println isn't good for non-esp builds :( |
| @@ -230,12 +230,14 @@ void EspPlatform::run_event_loop() | |||
| std::swap(buffer1, buffer2.value()); | |||
| } else { | |||
| for (int y = o.y; y < o.y + s.height; y++) { | |||
| #if BSP_LCD_COLOR_SPACE == ESP_LCD_COLOR_SPACE_BGR | |||
There was a problem hiding this comment.
We'll have to check if this still works on the S3 box (that this is properly defined at this location.
(I'm not entirely sure how this gets defined if we don't include any bsp header)
There was a problem hiding this comment.
I was curious about this, too, and added an #error inside this #ifdef and did builds for s3-usb-otb/s2-kaluga and s3-box, and it triggered for all of them. I'll double check to see where exactly it comes from.
There was a problem hiding this comment.
maybe this is equivalent to 0 == 0 if none are defined?
There was a problem hiding this comment.
ESP_LCD_COLOR_SPACE_BGR is an enum value:
https://github.com/espressif/esp-idf/blob/60a2bf6a68867fd468a250966230a067f8d97ea2/components/esp_lcd/include/esp_lcd_types.h#L50
So i think this should somehow be a runtime check and not a compile time check.
btw, i think i looked for a way back in the days to check that but couldn't find a proper way. I think you'll need to pass an option to the init function of something like that.
| @@ -6,7 +6,9 @@ | |||
| #include "slint-esp.h" | |||
| #include "slint-platform.h" | |||
| #include "esp_lcd_panel_ops.h" | |||
| #include "esp_lcd_panel_rgb.h" | |||
| #if SOC_LCD_RGB_SUPPORTED && ESP_IDF_VERSION_MAJOR >= 5 | |||
There was a problem hiding this comment.
SOC_LCD_RGB_SUPPORTED is defined indirectly from esp_lcd_panel_rgb.h via "soc/soc_caps.h". I'm not sure if the other header we include includes this.
There was a problem hiding this comment.
You're right, this isn't a good change. We should include "soc/soc_caps.h" directly. I see it's documented at https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/system/soc_caps.html but it's not strictly public API. I'll change the include for now to be on the safe side.
There was a problem hiding this comment.
Good idea, can't harm :)
There was a problem hiding this comment.
Ohh, or do you mean for esp_lcd_panel_rgb.h and then make everything else conditional to that?
There was a problem hiding this comment.
yes, i meant that: check for __has_include("esp_lcd_panel_rgb.h") and leave everything as is.
But this is ok too.
tronical
force-pushed
the
simon/esp32-p4
branch
from
af4c336 to
721d9db
Compare
|
Simplified down to just the two changes needed for this particular board. |
It looks like the esp_lcd_rgb_panel.h header file is not unconditionally available anymore. Guard it with the same #ifdef that's surrounding the calls to esp_lcd_rgb_panel_* and include the header that provides it.
- Bump esp-backtrace for esp32-p4 support, which removes the uart feature and delegates to esp-println - Added esp-println dependency - Add mapping for ESP32P4 and other risc-v esp-idf targets to the corresponding rust target triplet. Unfortunately there's no generic pattern like for xtensa, as can be seen on https://github.com/esp-rs/esp-idf-sys?tab=readme-ov-file#examples
tronical
force-pushed
the
simon/esp32-p4
branch
from
721d9db to
75155c6
Compare
No description provided.