-
Notifications
You must be signed in to change notification settings - Fork 96
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
Rework initialization #194
Conversation
esp-wifi/src/lib.rs
Outdated
#[cfg(any(feature = "esp32c3", feature = "esp32c2", feature = "esp32c6"))] | ||
/// Initialize for using WiFi / BLE | ||
/// This will initialize internals and also initialize WiFi and BLE | ||
pub fn initialize( | ||
systimer: Alarm<Target, 0>, | ||
rng: hal::Rng, | ||
radio_clocks: hal::system::RadioClockControl, | ||
clocks: &Clocks, | ||
) -> Result<(), InitializationError> { | ||
init_heap(); | ||
type Timer = Alarm<Target, 0>; | ||
|
||
#[cfg(feature = "esp32c6")] | ||
if clocks.cpu_clock != MegahertzU32::MHz(160) { | ||
return Err(InitializationError::WrongClockConfig); | ||
} | ||
#[cfg(any(feature = "esp32", feature = "esp32s3", feature = "esp32s2"))] | ||
type Timer = hal::timer::Timer<hal::timer::Timer0<hal::peripherals::TIMG1>>; |
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.
Should we give those types a public visibility, so that if someone were to wrap initialize()
inside another function, they wouldn't have to redefine those types again.
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.
You mean just for convenience? 🤔 Sure that shouldn't be a problem. Not sure if the naming is a bit too generic then - probably better to rename this if we make it public
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.
Overall it looks good! Just a comment about the cfgs.
I hope in the future we can move towards a more implicit initialization, i.e we track the state of which drivers have been created and initialize internally, but this is a good first step towards that.
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.
LGTM!
Closes #193
initialize
function for RV and Xtensaasync
feature selection