HAL cleanups & remove example macros #261
Conversation
MabezDev
force-pushed
the
hal-cleanup
branch
10 times, most recently
from
c7d96a2 to
614fb26
Compare
|
This is now ready for review. Each commit is an individual change (+noise from example changes), so we can revert one if needed. The examples_util still exists for the last few bits, I wasn't sure what to do with them. I changed it from a separate crate to a module that is included in each example. I hope this will get us to a spot for a first release! |
| #[cfg(feature = "esp32")] | ||
| pub use esp32_hal as hal; | ||
| #[cfg(feature = "esp32c2")] | ||
| pub use esp32c2_hal as hal; | ||
| #[cfg(feature = "esp32c3")] | ||
| pub use esp32c3_hal as hal; | ||
| #[cfg(feature = "esp32c6")] | ||
| pub use esp32c6_hal as hal; | ||
| #[cfg(feature = "esp32s2")] | ||
| pub use esp32s2_hal as hal; | ||
| #[cfg(feature = "esp32s3")] | ||
| pub use esp32s3_hal as hal; |
There was a problem hiding this comment.
You can change the examples' Cargo.toml to include the hal as an alias. hal = { package = ...
There was a problem hiding this comment.
Does that work with multiple optional dependencies aliased to the same name? I think that's not valid toml
There was a problem hiding this comment.
ah - forget that - we have examples per chip 🤦♂️ yes ... good idea
There was a problem hiding this comment.
I tried this but workspace deps complicate things, it seems cargo doesn't understand this:
hal = { package = "esp32-hal", workspace = true }
and instead looks for the "hal" package Caused by: dependency.hal was not found in workspace.dependencies.
There was a problem hiding this comment.
oh interesting ... cargo workspaces are always fun to use
There was a problem hiding this comment.
wait I wrote my suggestion after I tried it... too bad I wasn't exact enough, I'll need to come up with the same thing again 😅
| pub use esp32s3_hal as hal; | ||
|
|
||
| #[cfg(any(feature = "esp32c2", feature = "esp32c3"))] | ||
| pub type BootButton = crate::hal::gpio::Gpio9<crate::hal::gpio::Input<crate::hal::gpio::PullDown>>; |
There was a problem hiding this comment.
Since the pin is somewhat special, maybe the HAL could expose these?
| pub type BootButton = crate::hal::gpio::Gpio0<crate::hal::gpio::Input<crate::hal::gpio::PullDown>>; | ||
|
|
||
| #[cfg(feature = "esp32c3")] | ||
| pub const SOC_NAME: &str = "ESP32-C3"; |
There was a problem hiding this comment.
I find these rather silly to keep around, to be honest. But maybe the HAL build.rs could expose them as an env!() var.
There was a problem hiding this comment.
LGTM
it's definitely a bit more effort to maintain the examples now but I see it's easier to copy/paste code now
I agree it would be good to have a const of the soc-name in the HAL and maybe also good to have a way to know the boot button by just using the HAL - but given we just had a HAL release and that we can change this in a follow up PR I'd rather wait with that instead of patching all the HAL dependencies here
MabezDev
force-pushed
the
hal-cleanup
branch
from
614fb26 to
695a291
Compare
MabezDev
force-pushed
the
hal-cleanup
branch
2 times, most recently
from
6dbd105 to
7345bed
Compare
MabezDev
force-pushed
the
hal-cleanup
branch
from
7345bed to
4b9d510
Compare
|
Thanks for the review @bugadani, I think I'll address them in a separate PR just so we can get this merged. |
|
Sorry I didn't have the time, I'll try and figure out why I suggested that dependency rename yesterday. |
|
Right, so as a follow-up for the record: I think I may have set |
No description provided.