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

When the latest versions of esp-hal and slint are present simultaneously, a compilation error occurs #5057

Open
Song-aff opened this issue Apr 15, 2024 · 6 comments
Labels
a:platform-mcu Microcontrollers (mO,bS) bug Something isn't working priority:low Lowest priority. The issue is kept open for tracking purpose, but noone is actively working on this

Comments

@Song-aff
Copy link

When the latest versions of esp-hal and slint are present simultaneously, a compilation error occurs:

[dependencies]
esp-hal = { version = "0.16.1", features = ["esp32c3"] }
slint ={version = "1.5.1", default-features = false, features = [
    "compat-1-2",
    "unsafe-single-threaded",
    "libm",
    "renderer-software"]}

The error message is as follows:

cargo build
   Compiling portable-atomic v1.6.0
error: you may not enable feature `critical-section` and cfg(portable_atomic_unsafe_assume_single_core) at the same time
   --> /Users/admin/.cargo/registry/src/mirrors.tuna.tsinghua.edu.cn-df7c3c540f42cdbd/portable-atomic-1.6.0/src/lib.rs:409:1
    |
409 | / compile_error!(
410 | |     "you may not enable feature `critical-section` and cfg(portable_...
411 | | );
    | |_^

error: could not compile `portable-atomic` (lib) due to 1 previous error
@Song-aff
Copy link
Author

esp-rs/esp-hal#1434 (comment)
I also raised this question in ESP-HAL

@ogoffart
Copy link
Member

Looks like we shouldn't unconditionally require critical-section in

portable-atomic = { version = "1", features = ["critical-section"] }
and maybe also in once_cell, but we then need to enable it in some case. I wonder what's best here.

@KortanZ
Copy link

KortanZ commented May 12, 2024

Any plan to fix this? It's really annoying that need to patch esp-hal as a workaround in order to use slint. Really thankful if this could be fixed 🙂.

@ogoffart
Copy link
Member

ogoffart commented Jun 3, 2024

Not sure how to fix this.
We can't just remove the features = ["critical-section"] from our Cargo.toml, otherwise things will stop compiling.
Even if we add unsafe-single-threaded = ["portable-atomic/unsafe-assume-single-core"] in our own Cargo.toml this won't work on stm32

@KortanZ
Copy link

KortanZ commented Jun 4, 2024

@ogoffart what if we follow the portable-atomic crate recommendation? Leave the end user to determine which feature to use, features = ["critical-section"] or features = ["unsafe-assume-single-core"]. We remove features = ["critical-section"], and add an slint feature let's say critical-section, and bind portable-atomic/critical-section. Full change may like:

unsafe-single-threaded = ["portable-atomic/unsafe-assume-single-core"]
critical-section = ["portable-atomic/critical-section"]

And user could choose one of them to use.

Update:
There may some HAL use critical-seciton in protable-atomic, but user may still need use unsafe-single-threaded in slint. So we may need change like:

unsafe-single-threaded = []
portable-atomic-single-core = ["portable-atomic/unsafe-assume-single-core"]
portable-atomic-critical-section = ["portable-atomic/critical-section"]

A breaking and clumsy change, but I can't see any other solution. : (

@ogoffart
Copy link
Member

ogoffart commented Jun 4, 2024

I guess you're right. We can make it non-breaking by using the compat-1-2 feature.

In api/rs/slint/Cargo.toml

unsafe-single-threaded = []
portable-atomic-single-core = ["portable-atomic/unsafe-assume-single-core"]
portable-atomic-critical-section = ["portable-atomic/critical-section"]
compat-1-2 = ["portable-atomic-critical-section", "compat-1-7"]
compat-1-7 = []

in internal/core/Cargo.toml

portable-atomic = { version = "1", features = ["require-cas"] }

But we also need to document that properly.

(This just makes it even more complicated to setup for MCU apps)

@ogoffart ogoffart added bug Something isn't working a:platform-mcu Microcontrollers (mO,bS) priority:low Lowest priority. The issue is kept open for tracking purpose, but noone is actively working on this labels Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:platform-mcu Microcontrollers (mO,bS) bug Something isn't working priority:low Lowest priority. The issue is kept open for tracking purpose, but noone is actively working on this
Projects
None yet
Development

No branches or pull requests

3 participants