Skip to content

Commit 1e508e2

Browse files
authored
Refactor esp_radio::init() (#4482)
* Refactor esp_radio::init() * Introduce BleInitError and WifiInitError enums and call RadioRefGuard new and drop in critical section * reviews (wifi::new returns WifiError) * Changelog and MG * Redo tests * Add coex qa-test that drops ble and test if wifi still works
1 parent 85a962b commit 1e508e2

File tree

31 files changed

+567
-250
lines changed

31 files changed

+567
-250
lines changed

esp-radio/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212

1313
- `ble::mac` to get the MAC address of the device (#4485)
1414
- `last_calibration_result` to get the result of the last calibration (#4479)
15+
- `BleInitError` for BLE init failures and `Internal`, `WrongClockConfig`, `SchedulerNotInitialized` and `Adc2IsUsed` variants to `WifiError (#4482)
1516

1617
### Changed
1718

1819
- `phy_calibration_data` and `set_phy_calibration_data` are now marked as `unstable`. (#4463)
1920
- The `InitializationError::General` is replaced by `InitializationError::Internal` (#4467)
2021
- The `WifiError::InternalError` is removed and `InternalWifiError` was folded into `WifiError`, some variants renamed for clarity (#4467)
2122
- Refactor: extract submodules from `esp_radio::wifi` module (#4460)
23+
- `BleConnector::new` returns `BleInitError` instead of `InvalidConfigError` (#4482)
24+
- `esp_radio::init()` is not public anymore (#4482)
25+
- `esp_radio::wifi::new` and `BleConnector::new` no longer take a `Controller` (#4482)
2226

2327
### Fixed
2428

@@ -29,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2933
### Removed
3034

3135
- The `serde` feature has been removed (#4435)
36+
- `Controller` struct and `InitializationError::InterruptsDisabled` enum variant have been removed (#4482)
3237

3338
## [v0.17.0] - 2025-10-30
3439

esp-radio/Cargo.toml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ cfg-if = "1"
4444
esp-hal = { version = "1.0.0", path = "../esp-hal", default-features = false, features = ["requires-unstable"] }
4545
portable-atomic = { version = "1.11", default-features = false }
4646
enumset = { version = "1.1", default-features = false, optional = true }
47+
critical-section = { version = "1", default-features = false, optional = true }
4748

4849
# ⚠️ Unstable dependencies
4950
esp-radio-rtos-driver = { version = "0.2.0", path = "../esp-radio-rtos-driver" }
@@ -137,6 +138,7 @@ esp32 = [
137138
"dep:esp-wifi-sys-esp32",
138139
"esp-metadata-generated/esp32",
139140
"xtensa-lx-rt/float-save-restore",
141+
"critical-section",
140142
]
141143
##
142144
esp32c2 = [
@@ -145,6 +147,7 @@ esp32c2 = [
145147
"esp-phy/esp32c2",
146148
"dep:esp-wifi-sys-esp32c2",
147149
"esp-metadata-generated/esp32c2",
150+
"critical-section",
148151
]
149152
##
150153
esp32c3 = [
@@ -153,6 +156,7 @@ esp32c3 = [
153156
"esp-phy/esp32c3",
154157
"dep:esp-wifi-sys-esp32c3",
155158
"esp-metadata-generated/esp32c3",
159+
"critical-section",
156160
]
157161
##
158162
esp32c6 = [
@@ -161,6 +165,7 @@ esp32c6 = [
161165
"esp-phy/esp32c6",
162166
"dep:esp-wifi-sys-esp32c6",
163167
"esp-metadata-generated/esp32c6",
168+
"critical-section",
164169
]
165170
##
166171
esp32h2 = [
@@ -169,6 +174,7 @@ esp32h2 = [
169174
"esp-phy/esp32h2",
170175
"dep:esp-wifi-sys-esp32h2",
171176
"esp-metadata-generated/esp32h2",
177+
"critical-section",
172178
]
173179
##
174180
esp32s2 = [
@@ -178,6 +184,7 @@ esp32s2 = [
178184
"dep:esp-wifi-sys-esp32s2",
179185
"esp-metadata-generated/esp32s2",
180186
"xtensa-lx-rt/float-save-restore",
187+
"critical-section",
181188
]
182189
##
183190
esp32s3 = [
@@ -187,6 +194,7 @@ esp32s3 = [
187194
"dep:esp-wifi-sys-esp32s3",
188195
"esp-metadata-generated/esp32s3",
189196
"xtensa-lx-rt/float-save-restore",
197+
"critical-section",
190198
]
191199

192200
#! ### Wireless Feature Flags

esp-radio/MIGRATING-0.17.0.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,21 @@ impl From<ScanMethod> for esp_radio::wifi::ScanMethod {
3636
- `EapFastConfig`, `TlsPhase2Method` and `EapClientConfig` are now located in `wifi::ap::eap`.
3737

3838
You will need to update any imports in your project accordingly.
39+
40+
## `esp_radio::init()` and `Controller` is no longer available
41+
42+
WiFi initialization:
43+
```diff
44+
- let esp_radio_ctrl = esp_radio::init().unwrap();
45+
- let (mut controller, interfaces) =
46+
- esp_radio::wifi::new(&esp_radio_ctrl, wifi, Default::default()).unwrap();
47+
+ let (mut controller, interfaces) = esp_radio::wifi::new(wifi, Default::default()).unwrap();
48+
```
49+
50+
BLE initialization:
51+
```diff
52+
- static RADIO: StaticCell<esp_radio::Controller<'static>> = StaticCell::new();
53+
- let radio = RADIO.init(esp_radio::init().unwrap());
54+
- let connector = BleConnector::new(radio, bluetooth, Default::default()).unwrap();
55+
+ let connector = BleConnector::new(bluetooth, Default::default()).unwrap();
56+
```

esp-radio/src/ble/controller/mod.rs

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,37 @@ use esp_hal::asynch::AtomicWaker;
1212
use esp_phy::PhyInitGuard;
1313

1414
use crate::{
15-
Controller,
15+
InitializationError,
16+
RadioRefGuard,
1617
ble::{Config, InvalidConfigError, have_hci_read_data, read_hci, read_next, send_hci},
1718
};
1819

20+
#[derive(Debug)]
21+
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
22+
/// Error enum for BLE initialization failures.
23+
pub enum BleInitError {
24+
/// Failure during initial validation of the provided configuration.
25+
Config(InvalidConfigError),
26+
27+
/// Failure during the acquisition or initialization of the global radio hardware.
28+
RadioInit(InitializationError),
29+
}
30+
31+
// Implement the From trait for cleaner error mapping
32+
impl From<InvalidConfigError> for BleInitError {
33+
fn from(err: InvalidConfigError) -> Self {
34+
BleInitError::Config(err)
35+
}
36+
}
37+
1938
/// A blocking HCI connector
2039
#[instability::unstable]
2140
pub struct BleConnector<'d> {
2241
_phy_init_guard: PhyInitGuard<'d>,
2342
_device: crate::hal::peripherals::BT<'d>,
43+
_guard: RadioRefGuard,
2444
}
45+
2546
impl Drop for BleConnector<'_> {
2647
fn drop(&mut self) {
2748
crate::ble::ble_deinit();
@@ -31,14 +52,17 @@ impl<'d> BleConnector<'d> {
3152
/// Create and init a new BLE connector.
3253
#[instability::unstable]
3354
pub fn new(
34-
_init: &'d Controller<'d>,
3555
device: crate::hal::peripherals::BT<'d>,
3656
config: Config,
37-
) -> Result<BleConnector<'d>, InvalidConfigError> {
57+
) -> Result<BleConnector<'d>, BleInitError> {
58+
let _guard = RadioRefGuard::new().map_err(BleInitError::RadioInit)?;
59+
3860
config.validate()?;
61+
3962
Ok(Self {
4063
_phy_init_guard: crate::ble::ble_init(&config),
4164
_device: device,
65+
_guard,
4266
})
4367
}
4468

esp-radio/src/lib.rs

Lines changed: 72 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,6 @@ extern crate alloc;
136136
// MUST be the first module
137137
mod fmt;
138138

139-
use core::marker::PhantomData;
140-
141139
use esp_hal::{
142140
self as hal,
143141

@@ -226,6 +224,9 @@ pub(crate) mod memory_fence;
226224

227225
pub(crate) static ESP_RADIO_LOCK: RawMutex = RawMutex::new();
228226

227+
static RADIO_REFCOUNT: critical_section::Mutex<core::cell::Cell<u32>> =
228+
critical_section::Mutex::new(core::cell::Cell::new(0));
229+
229230
// this is just to verify that we use the correct defaults in `build.rs`
230231
#[allow(clippy::assertions_on_constants)] // TODO: try assert_eq once it's usable in const context
231232
const _: () = {
@@ -243,30 +244,6 @@ const _: () = {
243244
};
244245
};
245246

246-
#[derive(Debug, PartialEq, PartialOrd)]
247-
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
248-
/// Controller for the ESP Radio driver.
249-
pub struct Controller<'d> {
250-
_inner: PhantomData<&'d ()>,
251-
}
252-
253-
impl Drop for Controller<'_> {
254-
fn drop(&mut self) {
255-
// Disable coexistence
256-
#[cfg(coex)]
257-
{
258-
unsafe { crate::wifi::os_adapter::coex_disable() };
259-
unsafe { crate::wifi::os_adapter::coex_deinit() };
260-
}
261-
262-
shutdown_radio_isr();
263-
264-
#[cfg(esp32)]
265-
// Allow using `ADC2` again
266-
release_adc2(unsafe { esp_hal::Internal::conjure() });
267-
}
268-
}
269-
270247
#[procmacros::doc_replace]
271248
/// Initialize for using Wi-Fi and or BLE.
272249
///
@@ -289,35 +266,12 @@ impl Drop for Controller<'_> {
289266
)]
290267
/// - The function may return an error if interrupts are disabled.
291268
/// - The function may return an error if initializing the underlying driver fails.
292-
///
293-
/// ## Example
294-
///
295-
/// For examples of the necessary setup, see your RTOS's documentation. If you are
296-
/// using the `esp-rtos` crate, you will need to initialize the scheduler before calling this
297-
/// function:
298-
///
299-
/// ```rust, no_run
300-
/// # {before_snippet}
301-
/// use esp_hal::{interrupt::software::SoftwareInterruptControl, timer::timg::TimerGroup};
302-
///
303-
/// let timg0 = TimerGroup::new(peripherals.TIMG0);
304-
/// let software_interrupt = SoftwareInterruptControl::new(peripherals.SW_INTERRUPT);
305-
/// esp_rtos::start(timg0.timer0, software_interrupt.software_interrupt0);
306-
///
307-
/// // You can now start esp-radio:
308-
/// let esp_radio_controller = esp_radio::init().unwrap();
309-
/// # {after_snippet}
310-
/// ```
311-
pub fn init<'d>() -> Result<Controller<'d>, InitializationError> {
269+
pub(crate) fn init() -> Result<(), InitializationError> {
312270
#[cfg(esp32)]
313271
if try_claim_adc2(unsafe { hal::Internal::conjure() }).is_err() {
314272
return Err(InitializationError::Adc2IsUsed);
315273
}
316274

317-
if crate::is_interrupts_disabled() {
318-
return Err(InitializationError::InterruptsDisabled);
319-
}
320-
321275
if !preempt::initialized() {
322276
return Err(InitializationError::SchedulerNotInitialized);
323277
}
@@ -339,12 +293,75 @@ pub fn init<'d>() -> Result<Controller<'d>, InitializationError> {
339293
#[cfg(coex)]
340294
match crate::wifi::coex_initialize() {
341295
0 => {}
342-
error => return Err(InitializationError::Internal),
296+
error => panic!("Failed to initialize coexistence, error code: {}", error),
297+
}
298+
299+
debug!("Radio initialized");
300+
301+
Ok(())
302+
}
303+
304+
pub(crate) fn deinit() {
305+
// Disable coexistence
306+
#[cfg(coex)]
307+
{
308+
unsafe { crate::wifi::os_adapter::coex_disable() };
309+
unsafe { crate::wifi::os_adapter::coex_deinit() };
343310
}
344311

345-
Ok(Controller {
346-
_inner: PhantomData,
347-
})
312+
shutdown_radio_isr();
313+
314+
#[cfg(esp32)]
315+
// Allow using `ADC2` again
316+
release_adc2(unsafe { esp_hal::Internal::conjure() });
317+
318+
debug!("Radio deinitialized");
319+
}
320+
321+
/// Management of the global reference count
322+
/// and conditional hardware initialization/deinitialization.
323+
#[derive(Debug)]
324+
pub(crate) struct RadioRefGuard;
325+
326+
impl RadioRefGuard {
327+
/// Increments the refcount. If the old count was 0, it performs hardware init.
328+
/// If hardware init fails, it rolls back the refcount only once.
329+
fn new() -> Result<Self, InitializationError> {
330+
critical_section::with(|cs| {
331+
debug!("Creating RadioRefGuard");
332+
let rc = RADIO_REFCOUNT.borrow(cs);
333+
334+
let prev = rc.get();
335+
rc.set(prev + 1);
336+
337+
if prev == 0
338+
&& let Err(e) = init()
339+
{
340+
rc.set(prev);
341+
return Err(e);
342+
}
343+
344+
Ok(RadioRefGuard)
345+
})
346+
}
347+
}
348+
349+
impl Drop for RadioRefGuard {
350+
/// Decrements the refcount. If the count drops to 0, it performs hardware de-init.
351+
fn drop(&mut self) {
352+
critical_section::with(|cs| {
353+
debug!("Dropping RadioRefGuard");
354+
let rc = RADIO_REFCOUNT.borrow(cs);
355+
356+
let prev = rc.get();
357+
rc.set(prev - 1);
358+
359+
if prev == 1 {
360+
// Last user dropped, run de-initialization
361+
deinit();
362+
}
363+
});
364+
}
348365
}
349366

350367
/// Returns true if at least some interrupt levels are disabled.
@@ -360,19 +377,14 @@ fn is_interrupts_disabled() -> bool {
360377

361378
#[derive(Debug, Clone, Copy)]
362379
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
363-
/// Error which can be returned during [`init`].
380+
/// Error which can be returned during radio initialization.
364381
#[non_exhaustive]
365382
pub enum InitializationError {
366-
/// An internal error occurred.
367-
Internal,
368383
/// An error from the Wi-Fi driver.
369384
#[cfg(feature = "wifi")]
370385
WifiError(WifiError),
371386
/// The current CPU clock frequency is too low.
372387
WrongClockConfig,
373-
/// Tried to initialize while interrupts are disabled.
374-
/// This is not supported.
375-
InterruptsDisabled,
376388
/// The scheduler is not initialized.
377389
SchedulerNotInitialized,
378390
#[cfg(esp32)]
@@ -385,18 +397,13 @@ impl core::error::Error for InitializationError {}
385397
impl core::fmt::Display for InitializationError {
386398
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
387399
match self {
388-
InitializationError::Internal => write!(f, "An internal error occurred"),
389400
#[cfg(feature = "wifi")]
390401
InitializationError::WifiError(e) => {
391402
write!(f, "Wi-Fi driver related error occurred: {e}")
392403
}
393404
InitializationError::WrongClockConfig => {
394405
write!(f, "The current CPU clock frequency is too low")
395406
}
396-
InitializationError::InterruptsDisabled => write!(
397-
f,
398-
"Attempted to initialize while interrupts are disabled (Unsupported)"
399-
),
400407
InitializationError::SchedulerNotInitialized => {
401408
write!(f, "The scheduler is not initialized")
402409
}

0 commit comments

Comments
 (0)