-
Notifications
You must be signed in to change notification settings - Fork 222
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
Hide most of the unstable peripherals #2667
Conversation
@@ -556,6 +556,7 @@ macro_rules! as_mut_byte_array { | |||
unsafe { &mut *($name.as_mut_ptr() as *mut [u8; $size]) } | |||
}; | |||
} | |||
pub use as_mut_byte_array; // TODO: can be removed as soon as DMA is stabilized |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The weird pub use
on macros is a solution to the following error:
error: macro-expanded `macro_export` macros from the current crate cannot be referred to by absolute paths
--> C:\_Espressif\esp-hal\esp-hal\src\spi\master.rs:969:21
|
969 | crate::as_mut_byte_array!(BUFFERS[id], 4)
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #52234 <https://github.com/rust-lang/rust/issues/52234>
note: the macro is defined here
--> C:\_Espressif\esp-hal\esp-hal\src\dma\mod.rs:554:1
|
554 | / macro_rules! as_mut_byte_array {
555 | | ($name:expr, $size:expr) => {
556 | | unsafe { &mut *($name.as_mut_ptr() as *mut [u8; $size]) }
557 | | };
558 | | }
| |_^
= note: `#[deny(macro_expanded_macro_exports_accessed_by_absolute_paths)]` on by default
d25c188
to
b8f3cd4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - LGTM
let mut builder = if chip.is_xtensa() { | ||
// We only overwrite Xtensas so that externally set nightly/stable toolchains | ||
// are not overwritten. | ||
builder.toolchain("esp") | ||
} else { | ||
builder | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this mean that we will be linting with different compiler versions depending on which chip is being targeted? The esp
toolchain is based on nightly
, which will have a different set of lint rules than the stable
toolchain used for RISC-V. Perhaps we should always use esp
? Or am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit weird, because Xtensa is technically nightly, but a nightly that is built from the latest stable. But this isn't important here, I think.
We need to use the esp toolchain to lint Xtensa MCUs, otherwise the xtask just fails with a lot of "unknown cpu" sadness. So this part is essentially a QoL change so that we don't need to always invoke it using cargo +esp xtask
.
This is not changing the invocation for RISC-V chips, so that the CI can still have a choice between the stable/nightly upstream compilers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left one comment regarding the xtask changes, but otherwise LGTM.
There are a few annoying details, like how
#[macro_export]
refuses to work (workaround), and satellite crates, as well as examples will requireunstable
.I guess we also want to hide
peripherals.UNSTABLE_PERIPHERAL
as wellThis PR edits the xtask to enable the unstable feature where needed, and adds notes to esp-hal-embassy and esp-wifi (becaus they don't compile without timers, and maybe rng and more). We should also probably lint the stable subset of esp-hal. Be warned that it currently triggers weird clippy lints that we should have seen before.
cc #2499