Skip to content

Commit dac43a7

Browse files
ivmarkovCopilot
ivmarkov
and
Copilot
committed
Apply code review feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 1535b91 commit dac43a7

File tree

6 files changed

+106
-38
lines changed

6 files changed

+106
-38
lines changed

‎Cargo.toml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@ members = [
88
exclude = ["examples", "xtask"]
99

1010
[patch.crates-io]
11-
esp-hal = { git = "https://github.com/ivmarkov/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
11+
esp-hal = { git = "https://github.com/esp-rs/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
12+
esp-radio = { git = "https://github.com/esp-rs/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }

‎examples/esp/Cargo.toml‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,13 @@ esp32h2 = ["esp-rtos/esp32h2", "esp-radio/esp32h2", "esp-backtrace/esp32h2", "es
3333
force-generate-bindings = ["openthread/force-generate-bindings"]
3434

3535
[patch.crates-io]
36-
esp-hal = { git = "https://github.com/ivmarkov/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
37-
esp-rtos = { git = "https://github.com/ivmarkov/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
38-
esp-alloc = { git = "https://github.com/ivmarkov/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
39-
esp-backtrace = { git = "https://github.com/ivmarkov/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
40-
esp-println = { git = "https://github.com/ivmarkov/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
41-
esp-radio = { git = "https://github.com/ivmarkov/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
42-
esp-bootloader-esp-idf = { git = "https://github.com/ivmarkov/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
36+
esp-hal = { git = "https://github.com/esp-rs/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
37+
esp-rtos = { git = "https://github.com/esp-rs/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
38+
esp-alloc = { git = "https://github.com/esp-rs/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
39+
esp-backtrace = { git = "https://github.com/esp-rs/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
40+
esp-println = { git = "https://github.com/esp-rs/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
41+
esp-radio = { git = "https://github.com/esp-rs/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
42+
esp-bootloader-esp-idf = { git = "https://github.com/esp-rs/esp-hal", rev = "e156b5453a8adc123b7a8b6aa08e0698ec87dfe8" }
4343

4444
[dependencies]
4545
embassy-executor = "0.9"

‎openthread/src/esp.rs‎

Lines changed: 68 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ impl<'a> EspRadio<'a> {
6363
pan_id: config.pan_id,
6464
short_addr: config.short_addr,
6565
ext_addr: config.ext_addr,
66+
// The default of 10 is too small for OpenThread,
67+
// which can have bursts of incoming frames, so we increase it to 50.
68+
// TODO: See if we can get by with a smaller number to save memory.
6669
rx_queue_size: 50,
6770
..Default::default()
6871
};
@@ -86,7 +89,10 @@ impl<'a> EspRadio<'a> {
8689
impl Radio for EspRadio<'_> {
8790
type Error = RadioErrorKind;
8891

89-
const CAPS: Capabilities = Capabilities::ACK_TIMEOUT.union(Capabilities::CSMA_BACKOFF) /* TODO: Depends on coex being off .union(Capabilities::RX_WHEN_IDLE) */;
92+
const CAPS: Capabilities = Capabilities::ACK_TIMEOUT
93+
.union(Capabilities::CSMA_BACKOFF)
94+
// .union(Capabilities::RX_ON_WHEN_IDLE) TODO: Depends on coex being off in ESP-IDF
95+
;
9096

9197
const MAC_CAPS: MacCapabilities = MacCapabilities::all();
9298

@@ -110,7 +116,7 @@ impl Radio for EspRadio<'_> {
110116
TX_SIGNAL.reset();
111117

112118
trace!(
113-
"802.15.4 TX: {} bytes ch{}",
119+
"802.15.4: About to TX {} bytes ch{}",
114120
psdu.len(),
115121
self.config.channel
116122
);
@@ -122,48 +128,61 @@ impl Radio for EspRadio<'_> {
122128
let success = TX_SIGNAL.wait().await;
123129

124130
if success {
125-
trace!("ESP Radio, transmission done");
131+
trace!("802.15.4: TX done");
126132

127133
if let Some(ack_psdu_buf) = ack_psdu_buf {
128134
// After tx_done signal received, get the ACK frame:
129135
if let Some(ack_frame) = self.driver.get_ack_frame() {
130-
let ack_psdu_len =
131-
(ack_frame.data.len() - 1).min((ack_frame.data[0] & 0x7f) as usize);
132-
ack_psdu_buf[..ack_psdu_len]
133-
.copy_from_slice(&ack_frame.data[1..][..ack_psdu_len]);
134-
135-
trace!(
136-
"ESP Radio, received ACK: {} on channel {}",
137-
Bytes(&ack_psdu_buf[..ack_psdu_len]),
138-
ack_frame.channel
139-
);
140-
141-
let rssi = ack_frame.data[1..][ack_psdu_len] as i8;
142-
143-
return Ok(Some(PsduMeta {
144-
len: ack_psdu_len,
145-
channel: ack_frame.channel,
146-
rssi: Some(rssi),
147-
}));
136+
if ack_frame.data.len() >= 1 {
137+
// Must have at least 1 byte for PSDU
138+
let ack_psdu_len =
139+
(ack_frame.data.len() - 1).min((ack_frame.data[0] & 0x7f) as usize);
140+
141+
if ack_psdu_len <= ack_psdu_buf.len() {
142+
ack_psdu_buf[..ack_psdu_len]
143+
.copy_from_slice(&ack_frame.data[1..][..ack_psdu_len]);
144+
145+
trace!(
146+
"802.15.4: ACK: {} on ch{}",
147+
Bytes(&ack_psdu_buf[..ack_psdu_len]),
148+
ack_frame.channel
149+
);
150+
151+
// Only read RSSI if there is at least one byte after the PSDU.
152+
let rssi = if ack_frame.data.len() > 1 + ack_psdu_len {
153+
Some(ack_frame.data[1..][ack_psdu_len] as i8)
154+
} else {
155+
None
156+
};
157+
158+
return Ok(Some(PsduMeta {
159+
len: ack_psdu_len,
160+
channel: ack_frame.channel,
161+
rssi,
162+
}));
163+
} else {
164+
trace!(
165+
"802.15.4: ACK frame too large for provided buffer: {} bytes",
166+
ack_psdu_len
167+
);
168+
}
169+
}
148170
}
149171
}
150172

151173
Ok(None)
152174
} else {
153-
trace!("ESP Radio, transmission failed");
175+
trace!("802.15.4: TX failed");
154176

155-
// Report as NoAck error so OpenThread SubMac retries
177+
// Report as a failure so OpenThread SubMac retries
156178
Err(RadioErrorKind::TxFailed)
157179
}
158180
}
159181

160182
async fn receive(&mut self, psdu_buf: &mut [u8]) -> Result<PsduMeta, Self::Error> {
161183
RX_SIGNAL.reset();
162184

163-
trace!(
164-
"ESP Radio, about to receive on channel {}",
165-
self.config.channel
166-
);
185+
trace!("802.15.4: About to RX on ch{}", self.config.channel);
167186

168187
self.driver.start_receive();
169188

@@ -175,13 +194,32 @@ impl Radio for EspRadio<'_> {
175194
RX_SIGNAL.wait().await;
176195
};
177196

197+
if raw.data.len() < 1 {
198+
// Must have at least 1 byte for PSDU
199+
return Err(RadioErrorKind::Other);
200+
}
201+
178202
let psdu_len = (raw.data.len() - 1).min((raw.data[0] & 0x7f) as usize);
203+
if psdu_len > psdu_buf.len() {
204+
// PSDU length is larger than the provided buffer
205+
trace!(
206+
"802.15.4: Received frame too large for provided buffer: {} bytes",
207+
psdu_len
208+
);
209+
return Err(RadioErrorKind::Other);
210+
}
211+
179212
psdu_buf[..psdu_len].copy_from_slice(&raw.data[1..][..psdu_len]);
180213

181-
let rssi = raw.data[1..][psdu_len] as i8;
214+
// Only read RSSI if there is at least one byte after the PSDU.
215+
let rssi = if raw.data.len() > 1 + psdu_len {
216+
Some(raw.data[1..][psdu_len] as i8)
217+
} else {
218+
None
219+
};
182220

183221
trace!(
184-
"802.15.4 RX: {} bytes ch{} rssi={}",
222+
"802.15.4: RX {} bytes ch{} rssi={:?}",
185223
psdu_len,
186224
raw.channel,
187225
rssi
@@ -190,7 +228,7 @@ impl Radio for EspRadio<'_> {
190228
Ok(PsduMeta {
191229
len: psdu_len,
192230
channel: raw.channel,
193-
rssi: Some(rssi),
231+
rssi,
194232
})
195233
}
196234
}

‎openthread/src/lib.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1708,6 +1708,18 @@ impl<'a> OtContext<'a> {
17081708
Ok(())
17091709
}
17101710

1711+
fn plat_radio_set_rx_on_when_idle(&mut self, on: bool) {
1712+
info!("Plat radio set RX on when idle callback, on: {}", on);
1713+
1714+
let state = self.state();
1715+
1716+
if state.ot.radio_conf.rx_when_idle != on {
1717+
// Defer applying the new rx_when_idle value until the next role change event, to avoid
1718+
// unnecessary otThreadGetDeviceRole calls on every state change.
1719+
state.ot.pending_rx_when_idle = Some(on);
1720+
}
1721+
}
1722+
17111723
fn plat_settings_init(&mut self, sensitive_keys: &[u16]) {
17121724
info!(
17131725
"Plat settings init callback, sensitive keys: {:?}",

‎openthread/src/platform.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,11 @@ extern "C" fn otPlatRadioSetPanId(instance: *const otInstance, pan_id: u16) {
153153
OtContext::callback(instance).plat_radio_set_pan_id(pan_id);
154154
}
155155

156+
#[no_mangle]
157+
extern "C" fn otPlatRadioSetRxOnWhenIdle(instance: *const otInstance, enable: bool) {
158+
OtContext::callback(instance).plat_radio_set_rx_on_when_idle(enable);
159+
}
160+
156161
#[no_mangle]
157162
extern "C" fn otPlatRadioTransmit(
158163
instance: *const otInstance,

‎openthread/src/radio.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ use embassy_sync::zerocopy_channel::{Channel, Receiver, Sender};
1717
use embassy_time::Instant;
1818

1919
use mac::MacHeader;
20+
use openthread_sys::{
21+
OT_RADIO_CAPS_ALT_SHORT_ADDR, OT_RADIO_CAPS_RX_ON_WHEN_IDLE, OT_RADIO_CAPS_TRANSMIT_FRAME_POWER,
22+
};
2023

2124
use crate::fmt::{bitflags, Bytes};
2225
use crate::sys::{
@@ -110,6 +113,12 @@ bitflags! {
110113
const TRANSMIT_TIMING = OT_RADIO_CAPS_TRANSMIT_TIMING as u16;
111114
/// Radio supports precise RX timing.
112115
const RECEIVE_TIMING = OT_RADIO_CAPS_RECEIVE_TIMING as u16;
116+
/// Radio supports receiving when idle.
117+
const RX_ON_WHEN_IDLE = OT_RADIO_CAPS_RX_ON_WHEN_IDLE as u16;
118+
/// Radio supports setting the transmit frame power.
119+
const TRANSMIT_FRAME_POWER = OT_RADIO_CAPS_TRANSMIT_FRAME_POWER as u16;
120+
/// Radio supports alternative short address.
121+
const ALT_SHORT_ADDR = OT_RADIO_CAPS_ALT_SHORT_ADDR as u16;
113122
}
114123
}
115124

@@ -168,10 +177,13 @@ impl Config {
168177
pub const fn new() -> Self {
169178
Self {
170179
channel: 11,
180+
// Run with max power by default
181+
// TODO: Figure out how to have this specified by the user
171182
power: 20,
172183
cca: Cca::Carrier,
173184
sfd: 0,
174185
promiscuous: false,
186+
// OpenThread can have bursts of incoming frames, so we need to receive during idle state to not miss them.
175187
rx_when_idle: true,
176188
pan_id: None,
177189
short_addr: None,

0 commit comments

Comments
 (0)