Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esp-idf: Fix build against latest esp-idf and for esp32-p4 #4810

Merged
merged 3 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .github/workflows/nightly_snapshot.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -471,8 +471,13 @@ jobs:

# Purely quality-assurance related jobs, not relevant for release artefacts
qa-esp-idf:
strategy:
matrix:
esp-idf-target:
- release-v5.2
- latest
runs-on: ubuntu-22.04
container: espressif/idf:release-v5.2
container: espressif/idf:${{ matrix.esp-idf-target }}
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
Expand Down
1 change: 1 addition & 0 deletions api/cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ if (SLINT_BUILD_RUNTIME)
if (SLINT_FEATURE_FREESTANDING)
if (ESP_PLATFORM)
list(APPEND features "esp-backtrace/${IDF_TARGET}")
list(APPEND features "esp-println/${IDF_TARGET}")
endif()
else (SLINT_FEATURE_FREESTANDING)
list(APPEND features std)
Expand Down
3 changes: 2 additions & 1 deletion api/cpp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ raw-window-handle = { version = "0.5", optional = true }
# Enable image-rs' default features to make all image formats to C++ users
image = { version = "0.24.0", optional = true }

esp-backtrace = { version = "0.9.0", features = ["panic-handler", "print-uart"], optional = true }
esp-backtrace = { version = "0.11.0", features = ["panic-handler", "println"], optional = true }
esp-println = { version = "0.9.0", default-features = false, features = ["uart"], optional = true }

[build-dependencies]
anyhow = "1.0"
Expand Down
12 changes: 10 additions & 2 deletions api/cpp/esp-idf/slint/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,17 @@ idf_component_register(

if (CONFIG_IDF_TARGET_ARCH_XTENSA)
set(Rust_CARGO_TARGET "xtensa-${IDF_TARGET}-none-elf")
else(CONFIG_IDF_TARGET_ARCH_XTENSA)
elseif(CONFIG_IDF_TARGET_ARCH_RISCV)
if (CONFIG_IDF_TARGET_ESP32C6 OR CONFIG_IDF_TARGET_ESP32C5 OR CONFIG_IDF_TARGET_ESP32H2)
set(Rust_CARGO_TARGET "riscv32imac-esp-espidf")
elseif (CONFIG_IDF_TARGET_ESP32P4)
set(Rust_CARGO_TARGET "riscv32imafc-esp-espidf")
else ()
set(Rust_CARGO_TARGET "riscv32imc-esp-espidf")
endif()
else()
message(FATAL_ERROR "Architecture currently not supported")
endif(CONFIG_IDF_TARGET_ARCH_XTENSA)
endif()

set(SLINT_FEATURE_FREESTANDING ON)
set(SLINT_FEATURE_RENDERER_SOFTWARE ON)
Expand Down
7 changes: 6 additions & 1 deletion api/cpp/esp-idf/slint/src/slint-esp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
#include "slint-esp.h"
#include "slint-platform.h"
#include "esp_lcd_panel_ops.h"
#include "esp_lcd_panel_rgb.h"
#if __has_include("soc/soc_caps.h")
# include "soc/soc_caps.h"
#endif
#if SOC_LCD_RGB_SUPPORTED && ESP_IDF_VERSION_MAJOR >= 5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe with __has_include ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, can't harm :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, or do you mean for esp_lcd_panel_rgb.h and then make everything else conditional to that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, i meant that: check for __has_include("esp_lcd_panel_rgb.h") and leave everything as is.

But this is ok too.

# include "esp_lcd_panel_rgb.h"
#endif
#include "esp_log.h"

static const char *TAG = "slint_platform";
Expand Down
Loading