Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `rustyfarian-esp-idf-mqtt`: non-blocking `MqttHandle::try_publish`, `try_publish_retained`, and `try_publish_with` with `TryPublishError` for time-critical loops
- `wifi-pure`: `WifiPowerSave` enum (`None`, `MinModem`, `MaxModem`) and `WiFiConfig::with_power_save()` builder method
- `rustyfarian-esp-idf-wifi`: applies configured power save mode via `esp_wifi_set_ps()` after Wi-Fi start
- `rustyfarian-esp-idf-espnow`: `EspIdfEspNow::init_with_radio()` starts and owns the Wi-Fi radio for ESP-NOW-only devices (ADR 008)
- `rustyfarian-esp-idf-espnow`: `EspIdfEspNow::default_interface()` returns the correct `WifiInterface` based on init mode
- `espnow-pure`: `PeerConfig::with_ap_interface()` builder method for ESP-NOW-only devices

## [0.1.0] - 2026-03-16

Expand Down
15 changes: 11 additions & 4 deletions crates/espnow-pure/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,14 @@ impl PeerConfig {
interface: WifiInterface::Sta,
}
}

/// Sets the Wi-Fi interface to [`WifiInterface::Ap`].
///
/// Use this for peers on ESP-NOW-only devices that have no STA connection.
pub fn with_ap_interface(mut self) -> Self {
self.interface = WifiInterface::Ap;
self
}
}

// ─── EspNowDriver trait ──────────────────────────────────────────────────────
Expand Down Expand Up @@ -220,12 +228,11 @@ mod tests {
#[test]
fn peer_config_with_ap_interface() {
let mac = [0x01, 0x02, 0x03, 0x04, 0x05, 0x06];
let config = PeerConfig {
interface: WifiInterface::Ap,
..PeerConfig::new(mac)
};
let config = PeerConfig::new(mac).with_ap_interface();
assert_eq!(config.interface, WifiInterface::Ap);
assert_eq!(config.mac, mac);
assert_eq!(config.channel, 0);
assert!(!config.encrypt);
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions crates/rustyfarian-esp-idf-espnow/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ embuild.workspace = true
espnow-pure.workspace = true
anyhow.workspace = true
esp-idf-svc.workspace = true
esp-idf-hal.workspace = true
log.workspace = true
63 changes: 61 additions & 2 deletions crates/rustyfarian-esp-idf-espnow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use std::sync::mpsc::{sync_channel, Receiver, SyncSender};

use anyhow::Context as _;
use esp_idf_svc::espnow::{EspNow, PeerInfo};
use esp_idf_svc::hal::modem::Modem;
use esp_idf_svc::wifi::EspWifi;
pub use espnow_pure::{
EspNowDriver, EspNowEvent, MacAddress, PeerConfig, WifiInterface, BROADCAST_MAC,
DEFAULT_RX_CHANNEL_CAPACITY, MAX_DATA_LEN,
Expand All @@ -25,22 +27,75 @@ pub use espnow_pure::{
///
/// Wraps [`EspNow<'static>`] and bridges the C receive callback into a
/// [`std::sync::mpsc::sync_channel`] for non-blocking polling.
///
/// # Radio management
///
/// - [`init()`](EspIdfEspNow::init) — caller owns the Wi-Fi radio;
/// the radio must already be started before calling this.
/// - [`init_with_radio()`](EspIdfEspNow::init_with_radio) — the driver
/// starts and owns the radio internally. Use this for ESP-NOW-only devices
/// that do not connect to a Wi-Fi AP.
pub struct EspIdfEspNow {
esp_now: EspNow<'static>,
rx: Receiver<EspNowEvent>,
_wifi: Option<EspWifi<'static>>,
}

impl EspIdfEspNow {
/// Initialise ESP-NOW with the default receive-queue capacity of
/// [`DEFAULT_RX_CHANNEL_CAPACITY`] frames.
///
/// The Wi-Fi radio must already be started by the caller.
/// For ESP-NOW-only devices, use [`init_with_radio()`](Self::init_with_radio) instead.
pub fn init() -> anyhow::Result<Self> {
Self::init_with_capacity(DEFAULT_RX_CHANNEL_CAPACITY)
Self::init_inner(DEFAULT_RX_CHANNEL_CAPACITY, None)
}

/// Initialise ESP-NOW with a custom receive-queue capacity.
///
/// The Wi-Fi radio must already be started by the caller.
/// Frames received while the queue is full are dropped with a warning log.
pub fn init_with_capacity(capacity: usize) -> anyhow::Result<Self> {
Self::init_inner(capacity, None)
}

/// Initialise ESP-NOW and start the Wi-Fi radio internally.
///
/// Use this for devices that need ESP-NOW without connecting to a Wi-Fi AP.
/// The radio is kept alive for the lifetime of the returned driver.
///
/// Peers should use [`WifiInterface::Ap`] (or call
/// [`PeerConfig::with_ap_interface()`]) since there is no STA connection.
/// See [`default_interface()`](Self::default_interface).
pub fn init_with_radio(
modem: Modem<'static>,
sys_loop: esp_idf_svc::eventloop::EspSystemEventLoop,
nvs: Option<esp_idf_svc::nvs::EspDefaultNvsPartition>,
) -> anyhow::Result<Self> {
let mut wifi = EspWifi::new(modem, sys_loop, nvs)
.context("failed to create EspWifi for ESP-NOW radio")?;
wifi.start()
.context("failed to start Wi-Fi radio for ESP-NOW")?;
log::info!("Wi-Fi radio started for ESP-NOW (no AP connection)");

Self::init_inner(DEFAULT_RX_CHANNEL_CAPACITY, Some(wifi))
}

/// Returns the recommended [`WifiInterface`] for peer configuration.
///
/// - [`WifiInterface::Ap`] when the driver owns the radio
/// (created via [`init_with_radio()`](Self::init_with_radio) — no STA connection)
/// - [`WifiInterface::Sta`] when the caller manages Wi-Fi
/// (created via [`init()`](Self::init) — STA is assumed)
pub fn default_interface(&self) -> WifiInterface {
if self._wifi.is_some() {
WifiInterface::Ap
} else {
WifiInterface::Sta
}
}

fn init_inner(capacity: usize, wifi: Option<EspWifi<'static>>) -> anyhow::Result<Self> {
let esp_now = EspNow::take().context("failed to acquire EspNow singleton")?;

let (tx, rx): (SyncSender<EspNowEvent>, Receiver<EspNowEvent>) = sync_channel(capacity);
Expand All @@ -58,7 +113,11 @@ impl EspIdfEspNow {
})
.context("failed to register ESP-NOW receive callback")?;

Ok(Self { esp_now, rx })
Ok(Self {
esp_now,
rx,
_wifi: wifi,
})
}
}

Expand Down
89 changes: 89 additions & 0 deletions docs/adr/008-espnow-radio-only-init.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# ADR 008: ESP-NOW Radio-Only Initialisation

## Status

Accepted

## Context

Devices that use ESP-NOW without connecting to a Wi-Fi access point (e.g. LED matrix, nunchuck controller) must manually initialise the Wi-Fi radio before calling `EspIdfEspNow::init()`.
Every consumer writes the same boilerplate:

```rust
let mut wifi = EspWifi::new(peripherals.modem, sys_loop, Some(nvs))?;
wifi.start()?;
let espnow = EspIdfEspNow::init()?;
```

Additionally, ESP-NOW-only devices must manually override `PeerConfig::interface` from the default `WifiInterface::Sta` to `WifiInterface::Ap`, because there is no STA connection.
This is a known ESP-NOW quirk that catches every new consumer and wastes debugging time.

Evidence from downstream firmware:
- `firmware-rgb-matrix/src/main.rs` — raw `EspWifi::new()` + `.start()`
- `firmware-rgb-nunchuck/src/main.rs` — same pattern + manual `brain_peer.interface = WifiInterface::Ap`

### Design decisions under evaluation

**Where to store the radio**

`init_with_radio()` must keep the `EspWifi` instance alive — if dropped, the radio shuts down and ESP-NOW stops working.
Two options: a separate wrapper struct, or an `Option<EspWifi<'static>>` field on the existing `EspIdfEspNow`.
A separate struct would duplicate the `EspNowDriver` trait implementation.

**WifiInterface auto-detection**

Consumers who use `init_with_radio()` (no STA connection) always need `WifiInterface::Ap` on their peers.
The driver knows whether it owns the radio, so it can expose a `default_interface()` method to guide callers.

**PeerConfig ergonomics**

`PeerConfig` in `espnow-pure` defaults `interface` to `Sta`.
A builder method for the AP case would eliminate the field-level override that every ESP-NOW-only device needs.

## Decision

Add `init_with_radio()` to `EspIdfEspNow` and a `with_ap_interface()` builder to `PeerConfig`.
Leave the existing `init()` unchanged for devices that manage Wi-Fi themselves.

### Changes to `rustyfarian-esp-idf-espnow`

- Add `_wifi: Option<EspWifi<'static>>` field to `EspIdfEspNow`.
Existing `init()` sets it to `None`; `init_with_radio()` stores `Some(wifi)`.

- Add `init_with_radio(modem, sys_loop, nvs)` method that:
1. Creates `EspWifi::new(modem, sys_loop, nvs)` and calls `.start()`
2. Stores the `EspWifi` instance to keep the radio alive
3. Delegates to the existing `init_with_capacity()` logic for ESP-NOW setup

- Add `default_interface()` method returning `WifiInterface::Ap` when the driver owns the radio, `WifiInterface::Sta` otherwise.

- Add `esp-idf-hal` workspace dependency to `Cargo.toml` (for `Modem` type).

### Changes to `espnow-pure`

- Add `PeerConfig::with_ap_interface()` builder method — sets `interface` to `WifiInterface::Ap`.

### What stays unchanged

- `EspNowDriver` trait — radio init is a platform concern, not a driver operation.
- `WifiInterface` enum — no new variants needed.
- Existing `init()` / `init_with_capacity()` signatures — fully backwards compatible.

## Consequences

### Positive

- **Eliminates boilerplate** — ESP-NOW-only firmware drops ~8 lines of manual radio init per crate
- **Eliminates the `WifiInterface::Ap` gotcha** — `default_interface()` + `with_ap_interface()` make the correct choice obvious
- **Backwards compatible** — existing `init()` callers are unaffected
- **Radio lifetime is safe** — `EspWifi` stored in the struct prevents accidental drop

### Negative

- **`EspIdfEspNow` grows an `Option` field** — minor size increase; `None` path has no overhead
- **New dependency** — `esp-idf-hal` added to `rustyfarian-esp-idf-espnow` (already a transitive dep via `esp-idf-svc`)

## References

- [ADR 007 — ESP-NOW Abstraction Layer](007-espnow-abstraction.md)
- Feature request: `review-queue/espnow-radio-only-init.md`
Loading