-
Notifications
You must be signed in to change notification settings - Fork 229
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
Some code assumes that a critical-section impl is available #657
Comments
This could be worked around by making the example explicitly require the feature to be enabled: The down side is that one would still need to pass it to the command line. At least it makes the error caught earlier and leaves a much less cryptic error message. |
For the examples, that's exactly what I did in jannic@3d17af0 (I will send a PR once the PAC update is released) However it doesn't help users who write their own firmware based on rp2040-hal without using a BSP. They still need to remember to activate |
We can probably do something similar to what IIRC defmt does with a symbol in linker file that's attempted to be linked if there's something missing. |
I had the following idea, which unfortunately didn't work:
So why didn't this work? Well, besides defining the two symbols, one also needs to tell But we also can't just set the feature flag, because that may collide with a custom impl which needs a different restore state. |
Checking out rp-hal and attempting to build pio_audio.rs from the examples directory yields error: rust-lld: error: undefined symbol: _critical_section_1_0_acquire >>> referenced by pio_audio.db0b02402204ccb1-cgu.1 I found rp-rs/rp-hal#657 and discovered that enabling the required feature enables compilation.
Checking out rp-hal and attempting to build pio_audio.rs from the examples directory yields error: rust-lld: error: undefined symbol: _critical_section_1_0_acquire >>> referenced by pio_audio.db0b02402204ccb1-cgu.1 I found rp-rs/rp-hal#657 and discovered that enabling the required feature enables compilation.
While
critical-section
is a non-optional dependency ofrp2040-hal
, it's not sufficient to make critical sections work: There also needs to be an implementation of critical sections, which can be provided by activating the non-default featurecritical-section-impl
.We don't want to make it a default feature, because users might want to provide their own implementation, and disabling a default feature of a crate that might be a transitive dependency of several other crates in the dependency tree is difficult.
But if it is not enabled, and some code uses a critical section, compilation fails with an ugly error message:
From this error message, it's not obvious how to solve the issue.
This is mitigated a bit by the fact that we do activate
critical-section-impl
from the BSP crates. (See #444 for details.)Still, it would be nice if we could improve that error message, avoid the situation altogether, or at least describe the requirement to activate
critical-section-impl
(or provide an alternative implementation) in the docs.The text was updated successfully, but these errors were encountered: