From 065b0f34af4215cff81a799044f73980ed751120 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 17 Aug 2023 00:11:15 +0200 Subject: [PATCH 1/3] net-esp-hosted: sane error handling in control requests. --- embassy-net-esp-hosted/src/control.rs | 126 +++++++++++-------- embassy-net-esp-hosted/src/lib.rs | 9 +- examples/nrf52840/src/bin/wifi_esp_hosted.rs | 4 +- tests/nrf/src/bin/wifi_esp_hosted_perf.rs | 4 +- 4 files changed, 79 insertions(+), 64 deletions(-) diff --git a/embassy-net-esp-hosted/src/control.rs b/embassy-net-esp-hosted/src/control.rs index 37f220da..b722bdd7 100644 --- a/embassy-net-esp-hosted/src/control.rs +++ b/embassy-net-esp-hosted/src/control.rs @@ -5,9 +5,12 @@ use heapless::String; use crate::ioctl::Shared; use crate::proto::{self, CtrlMsg}; -#[derive(Debug)] -pub struct Error { - pub status: u32, +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum Error { + Failed(u32), + Timeout, + Internal, } pub struct Control<'a> { @@ -23,58 +26,68 @@ enum WifiMode { ApSta = 3, } +macro_rules! ioctl { + ($self:ident, $req_variant:ident, $resp_variant:ident, $req:ident, $resp:ident) => { + let mut msg = proto::CtrlMsg { + msg_id: proto::CtrlMsgId::$req_variant as _, + msg_type: proto::CtrlMsgType::Req as _, + payload: Some(proto::CtrlMsgPayload::$req_variant($req)), + }; + $self.ioctl(&mut msg).await?; + let Some(proto::CtrlMsgPayload::$resp_variant($resp)) = msg.payload else { + warn!("unexpected response variant"); + return Err(Error::Internal); + }; + if $resp.resp != 0 { + return Err(Error::Failed($resp.resp)); + } + }; +} + impl<'a> Control<'a> { pub(crate) fn new(state_ch: ch::StateRunner<'a>, shared: &'a Shared) -> Self { Self { state_ch, shared } } - pub async fn init(&mut self) { + pub async fn init(&mut self) -> Result<(), Error> { debug!("wait for init event..."); self.shared.init_wait().await; debug!("set wifi mode"); - self.set_wifi_mode(WifiMode::Sta as _).await; + self.set_wifi_mode(WifiMode::Sta as _).await?; - let mac_addr = self.get_mac_addr().await; + let mac_addr = self.get_mac_addr().await?; debug!("mac addr: {:02x}", mac_addr); self.state_ch.set_ethernet_address(mac_addr); + + Ok(()) } - pub async fn join(&mut self, ssid: &str, password: &str) { - let req = proto::CtrlMsg { - msg_id: proto::CtrlMsgId::ReqConnectAp as _, - msg_type: proto::CtrlMsgType::Req as _, - payload: Some(proto::CtrlMsgPayload::ReqConnectAp(proto::CtrlMsgReqConnectAp { - ssid: String::from(ssid), - pwd: String::from(password), - bssid: String::new(), - listen_interval: 3, - is_wpa3_supported: false, - })), + pub async fn connect(&mut self, ssid: &str, password: &str) -> Result<(), Error> { + let req = proto::CtrlMsgReqConnectAp { + ssid: String::from(ssid), + pwd: String::from(password), + bssid: String::new(), + listen_interval: 3, + is_wpa3_supported: false, }; - let resp = self.ioctl(req).await; - let proto::CtrlMsgPayload::RespConnectAp(resp) = resp.payload.unwrap() else { - panic!("unexpected resp") - }; - assert_eq!(resp.resp, 0); + ioctl!(self, ReqConnectAp, RespConnectAp, req, resp); self.state_ch.set_link_state(LinkState::Up); + Ok(()) } - async fn get_mac_addr(&mut self) -> [u8; 6] { - let req = proto::CtrlMsg { - msg_id: proto::CtrlMsgId::ReqGetMacAddress as _, - msg_type: proto::CtrlMsgType::Req as _, - payload: Some(proto::CtrlMsgPayload::ReqGetMacAddress( - proto::CtrlMsgReqGetMacAddress { - mode: WifiMode::Sta as _, - }, - )), + pub async fn disconnect(&mut self) -> Result<(), Error> { + let req = proto::CtrlMsgReqGetStatus {}; + ioctl!(self, ReqDisconnectAp, RespDisconnectAp, req, resp); + self.state_ch.set_link_state(LinkState::Up); + Ok(()) + } + + async fn get_mac_addr(&mut self) -> Result<[u8; 6], Error> { + let req = proto::CtrlMsgReqGetMacAddress { + mode: WifiMode::Sta as _, }; - let resp = self.ioctl(req).await; - let proto::CtrlMsgPayload::RespGetMacAddress(resp) = resp.payload.unwrap() else { - panic!("unexpected resp") - }; - assert_eq!(resp.resp, 0); + ioctl!(self, ReqGetMacAddress, RespGetMacAddress, req, resp); // WHY IS THIS A STRING? WHYYYY fn nibble_from_hex(b: u8) -> u8 { @@ -88,32 +101,32 @@ impl<'a> Control<'a> { let mac = resp.mac.as_bytes(); let mut res = [0; 6]; - assert_eq!(mac.len(), 17); + if mac.len() != 17 { + warn!("unexpected MAC respnse length"); + return Err(Error::Internal); + } for (i, b) in res.iter_mut().enumerate() { *b = (nibble_from_hex(mac[i * 3]) << 4) | nibble_from_hex(mac[i * 3 + 1]) } - res + Ok(res) } - async fn set_wifi_mode(&mut self, mode: u32) { - let req = proto::CtrlMsg { - msg_id: proto::CtrlMsgId::ReqSetWifiMode as _, - msg_type: proto::CtrlMsgType::Req as _, - payload: Some(proto::CtrlMsgPayload::ReqSetWifiMode(proto::CtrlMsgReqSetMode { mode })), - }; - let resp = self.ioctl(req).await; - let proto::CtrlMsgPayload::RespSetWifiMode(resp) = resp.payload.unwrap() else { - panic!("unexpected resp") - }; - assert_eq!(resp.resp, 0); + async fn set_wifi_mode(&mut self, mode: u32) -> Result<(), Error> { + let req = proto::CtrlMsgReqSetMode { mode }; + ioctl!(self, ReqSetWifiMode, RespSetWifiMode, req, resp); + + Ok(()) } - async fn ioctl(&mut self, req: CtrlMsg) -> CtrlMsg { - debug!("ioctl req: {:?}", &req); + async fn ioctl(&mut self, msg: &mut CtrlMsg) -> Result<(), Error> { + debug!("ioctl req: {:?}", &msg); let mut buf = [0u8; 128]; - let req_len = noproto::write(&req, &mut buf).unwrap(); + let req_len = noproto::write(msg, &mut buf).map_err(|_| { + warn!("failed to serialize control request"); + Error::Internal + })?; struct CancelOnDrop<'a>(&'a Shared); @@ -135,9 +148,12 @@ impl<'a> Control<'a> { ioctl.defuse(); - let res = noproto::read(&buf[..resp_len]).unwrap(); - debug!("ioctl resp: {:?}", &res); + *msg = noproto::read(&buf[..resp_len]).map_err(|_| { + warn!("failed to serialize control request"); + Error::Internal + })?; + debug!("ioctl resp: {:?}", msg); - res + Ok(()) } } diff --git a/embassy-net-esp-hosted/src/lib.rs b/embassy-net-esp-hosted/src/lib.rs index 96fddce5..b1fa775c 100644 --- a/embassy-net-esp-hosted/src/lib.rs +++ b/embassy-net-esp-hosted/src/lib.rs @@ -1,17 +1,14 @@ #![no_std] -use control::Control; use embassy_futures::select::{select3, Either3}; use embassy_net_driver_channel as ch; use embassy_time::{Duration, Instant, Timer}; use embedded_hal::digital::{InputPin, OutputPin}; use embedded_hal_async::digital::Wait; use embedded_hal_async::spi::SpiDevice; -use ioctl::Shared; -use proto::CtrlMsg; -use crate::ioctl::PendingIoctl; -use crate::proto::CtrlMsgPayload; +use crate::ioctl::{PendingIoctl, Shared}; +use crate::proto::{CtrlMsg, CtrlMsgPayload}; mod proto; @@ -21,6 +18,8 @@ mod fmt; mod control; mod ioctl; +pub use control::*; + const MTU: usize = 1514; macro_rules! impl_bytes { diff --git a/examples/nrf52840/src/bin/wifi_esp_hosted.rs b/examples/nrf52840/src/bin/wifi_esp_hosted.rs index e114e50b..a60822fd 100644 --- a/examples/nrf52840/src/bin/wifi_esp_hosted.rs +++ b/examples/nrf52840/src/bin/wifi_esp_hosted.rs @@ -72,8 +72,8 @@ async fn main(spawner: Spawner) { unwrap!(spawner.spawn(wifi_task(runner))); - control.init().await; - control.join(WIFI_NETWORK, WIFI_PASSWORD).await; + unwrap!(control.init().await); + unwrap!(control.connect(WIFI_NETWORK, WIFI_PASSWORD).await); let config = embassy_net::Config::dhcpv4(Default::default()); // let config = embassy_net::Config::ipv4_static(embassy_net::StaticConfigV4 { diff --git a/tests/nrf/src/bin/wifi_esp_hosted_perf.rs b/tests/nrf/src/bin/wifi_esp_hosted_perf.rs index 16055fae..4b221a81 100644 --- a/tests/nrf/src/bin/wifi_esp_hosted_perf.rs +++ b/tests/nrf/src/bin/wifi_esp_hosted_perf.rs @@ -76,8 +76,8 @@ async fn main(spawner: Spawner) { unwrap!(spawner.spawn(wifi_task(runner))); - control.init().await; - control.join(WIFI_NETWORK, WIFI_PASSWORD).await; + unwrap!(control.init().await); + unwrap!(control.connect(WIFI_NETWORK, WIFI_PASSWORD).await); // Generate random seed let mut rng = Rng::new(p.RNG, Irqs); From ef7523e5b7a36f0f15b77d4e673afdda0df51f3a Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 17 Aug 2023 00:53:25 +0200 Subject: [PATCH 2/3] net-esp-hosted: put link down on wifi disconnect. --- embassy-net-esp-hosted/src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/embassy-net-esp-hosted/src/lib.rs b/embassy-net-esp-hosted/src/lib.rs index b1fa775c..c2d9d609 100644 --- a/embassy-net-esp-hosted/src/lib.rs +++ b/embassy-net-esp-hosted/src/lib.rs @@ -128,6 +128,7 @@ where let mut runner = Runner { ch: ch_runner, + state_ch, shared: &state.shared, next_seq: 1, handshake, @@ -142,6 +143,7 @@ where pub struct Runner<'a, SPI, IN, OUT> { ch: ch::Runner<'a, MTU>, + state_ch: ch::StateRunner<'a>, shared: &'a Shared, next_seq: u16, @@ -322,6 +324,10 @@ where match payload { CtrlMsgPayload::EventEspInit(_) => self.shared.init_done(), + CtrlMsgPayload::EventStationDisconnectFromAp(e) => { + info!("disconnected, code {}", e.resp); + self.state_ch.set_link_state(LinkState::Down); + } _ => {} } } From 1cb76e0d99f8da12891491b31176c8247a7a81a4 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 17 Aug 2023 00:57:54 +0200 Subject: [PATCH 3/3] net-esp-hosted: enable heartbeats from esp32 to detect if it crashes. --- embassy-net-esp-hosted/src/control.rs | 10 ++++++++++ embassy-net-esp-hosted/src/lib.rs | 21 +++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/embassy-net-esp-hosted/src/control.rs b/embassy-net-esp-hosted/src/control.rs index b722bdd7..6cd57f68 100644 --- a/embassy-net-esp-hosted/src/control.rs +++ b/embassy-net-esp-hosted/src/control.rs @@ -53,6 +53,9 @@ impl<'a> Control<'a> { debug!("wait for init event..."); self.shared.init_wait().await; + debug!("set heartbeat"); + self.set_heartbeat(10).await?; + debug!("set wifi mode"); self.set_wifi_mode(WifiMode::Sta as _).await?; @@ -83,6 +86,13 @@ impl<'a> Control<'a> { Ok(()) } + /// duration in seconds, clamped to [10, 3600] + async fn set_heartbeat(&mut self, duration: u32) -> Result<(), Error> { + let req = proto::CtrlMsgReqConfigHeartbeat { enable: true, duration }; + ioctl!(self, ReqConfigHeartbeat, RespConfigHeartbeat, req, resp); + Ok(()) + } + async fn get_mac_addr(&mut self) -> Result<[u8; 6], Error> { let req = proto::CtrlMsgReqGetMacAddress { mode: WifiMode::Sta as _, diff --git a/embassy-net-esp-hosted/src/lib.rs b/embassy-net-esp-hosted/src/lib.rs index c2d9d609..4a318b20 100644 --- a/embassy-net-esp-hosted/src/lib.rs +++ b/embassy-net-esp-hosted/src/lib.rs @@ -1,7 +1,8 @@ #![no_std] -use embassy_futures::select::{select3, Either3}; +use embassy_futures::select::{select4, Either4}; use embassy_net_driver_channel as ch; +use embassy_net_driver_channel::driver::LinkState; use embassy_time::{Duration, Instant, Timer}; use embedded_hal::digital::{InputPin, OutputPin}; use embedded_hal_async::digital::Wait; @@ -94,6 +95,7 @@ enum InterfaceType { } const MAX_SPI_BUFFER_SIZE: usize = 1600; +const HEARTBEAT_MAX_GAP: Duration = Duration::from_secs(20); pub struct State { shared: Shared, @@ -135,6 +137,7 @@ where ready, reset, spi, + heartbeat_deadline: Instant::now() + HEARTBEAT_MAX_GAP, }; runner.init().await; @@ -147,6 +150,7 @@ pub struct Runner<'a, SPI, IN, OUT> { shared: &'a Shared, next_seq: u16, + heartbeat_deadline: Instant, spi: SPI, handshake: IN, @@ -178,9 +182,10 @@ where let ioctl = self.shared.ioctl_wait_pending(); let tx = self.ch.tx_buf(); let ev = async { self.ready.wait_for_high().await.unwrap() }; + let hb = Timer::at(self.heartbeat_deadline); - match select3(ioctl, tx, ev).await { - Either3::First(PendingIoctl { buf, req_len }) => { + match select4(ioctl, tx, ev, hb).await { + Either4::First(PendingIoctl { buf, req_len }) => { tx_buf[12..24].copy_from_slice(b"\x01\x08\x00ctrlResp\x02"); tx_buf[24..26].copy_from_slice(&(req_len as u16).to_le_bytes()); tx_buf[26..][..req_len].copy_from_slice(&unsafe { &*buf }[..req_len]); @@ -199,7 +204,7 @@ where header.checksum = checksum(&tx_buf[..26 + req_len]); tx_buf[0..12].copy_from_slice(&header.to_bytes()); } - Either3::Second(packet) => { + Either4::Second(packet) => { tx_buf[12..][..packet.len()].copy_from_slice(packet); let mut header = PayloadHeader { @@ -218,9 +223,12 @@ where self.ch.tx_done(); } - Either3::Third(()) => { + Either4::Third(()) => { tx_buf[..PayloadHeader::SIZE].fill(0); } + Either4::Fourth(()) => { + panic!("heartbeat from esp32 stopped") + } } if tx_buf[0] != 0 { @@ -309,7 +317,7 @@ where } } - fn handle_event(&self, data: &[u8]) { + fn handle_event(&mut self, data: &[u8]) { let Ok(event) = noproto::read::(data) else { warn!("failed to parse event"); return; @@ -324,6 +332,7 @@ where match payload { CtrlMsgPayload::EventEspInit(_) => self.shared.init_done(), + CtrlMsgPayload::EventHeartbeat(_) => self.heartbeat_deadline = Instant::now() + HEARTBEAT_MAX_GAP, CtrlMsgPayload::EventStationDisconnectFromAp(e) => { info!("disconnected, code {}", e.resp); self.state_ch.set_link_state(LinkState::Down);