From 7a3af36293c30be0b1fd4f215ff08eee5df777b2 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Mon, 13 Feb 2023 10:02:14 -0600 Subject: [PATCH 01/11] Use core::hint to implement friendlier spin loop for gpio level-checking --- esp32-wroom-rp/src/gpio.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/esp32-wroom-rp/src/gpio.rs b/esp32-wroom-rp/src/gpio.rs index a1189e6..1c9404e 100644 --- a/esp32-wroom-rp/src/gpio.rs +++ b/esp32-wroom-rp/src/gpio.rs @@ -30,6 +30,8 @@ //! }; //! ``` +use core::hint; + use embedded_hal::blocking::delay::DelayMs; use embedded_hal::digital::v2::{InputPin, OutputPin}; @@ -131,13 +133,13 @@ where fn wait_for_esp_ready(&self) { while !self.get_esp_ready() { - //cortex_m::asm::nop(); // Make sure rustc doesn't optimize this loop out + hint::spin_loop(); // Make sure rustc doesn't optimize this loop out } } fn wait_for_esp_ack(&self) { while !self.get_esp_ack() { - //cortex_m::asm::nop(); // Make sure rustc doesn't optimize this loop out + hint::spin_loop(); // Make sure rustc doesn't optimize this loop out } } From bbbe78a9ab9d0ec51a383d09df9f557d55f1dc17 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Mon, 13 Feb 2023 18:20:17 -0600 Subject: [PATCH 02/11] Allows for different size param buffer lengths that takes advantage of associated types. Also begins to make wait_response_cmd() a bit more robust --- esp32-wroom-rp/src/lib.rs | 17 ++++------- esp32-wroom-rp/src/protocol.rs | 52 +++++++++++++++++++++----------- esp32-wroom-rp/src/spi.rs | 32 +++++++++++--------- esp32-wroom-rp/src/tcp_client.rs | 6 ++-- 4 files changed, 61 insertions(+), 46 deletions(-) diff --git a/esp32-wroom-rp/src/lib.rs b/esp32-wroom-rp/src/lib.rs index e9043f2..872b0e5 100644 --- a/esp32-wroom-rp/src/lib.rs +++ b/esp32-wroom-rp/src/lib.rs @@ -163,8 +163,6 @@ use network::NetworkError; use protocol::ProtocolError; -const ARRAY_LENGTH_PLACEHOLDER: usize = 8; - /// Highest level error types for this crate. #[derive(Debug, Eq, PartialEq)] pub enum Error { @@ -212,17 +210,15 @@ pub struct FirmwareVersion { } impl FirmwareVersion { - fn new(version: [u8; ARRAY_LENGTH_PLACEHOLDER]) -> FirmwareVersion { + fn new(version: &[u8]) -> FirmwareVersion { Self::parse(version) } // Takes in 8 bytes (e.g. 1.7.4) and returns a FirmwareVersion instance - fn parse(version: [u8; ARRAY_LENGTH_PLACEHOLDER]) -> FirmwareVersion { - let major_version: u8; - let minor_version: u8; - let patch_version: u8; - - [major_version, _, minor_version, _, patch_version, _, _, _] = version; + fn parse(version: &[u8]) -> FirmwareVersion { + let major_version: u8 = version[0]; + let minor_version: u8 = version[2]; + let patch_version: u8 = version[4]; FirmwareVersion { major: major_version, @@ -248,8 +244,7 @@ mod tests { #[test] fn firmware_new_returns_a_populated_firmware_struct() { - let firmware_version: FirmwareVersion = - FirmwareVersion::new([0x1, 0x2e, 0x7, 0x2e, 0x4, 0x0, 0x0, 0x0]); + let firmware_version: FirmwareVersion = FirmwareVersion::new(&[0x1, 0x2e, 0x7, 0x2e, 0x4]); assert_eq!( firmware_version, diff --git a/esp32-wroom-rp/src/protocol.rs b/esp32-wroom-rp/src/protocol.rs index 583d960..99defbc 100644 --- a/esp32-wroom-rp/src/protocol.rs +++ b/esp32-wroom-rp/src/protocol.rs @@ -13,9 +13,18 @@ use heapless::{String, Vec}; use super::network::{ConnectionState, IpAddress, Port, Socket, TransportMode}; use super::wifi::ConnectionStatus; -use super::{Error, FirmwareVersion, ARRAY_LENGTH_PLACEHOLDER}; +use super::{Error, FirmwareVersion}; -pub(crate) const MAX_NINA_PARAM_LENGTH: usize = 4096; +// The maximum number of NINA param u8 bytes in a command send/receive byte stream +pub(crate) const MAX_NINA_PARAMS: usize = 8; + +pub(crate) const MAX_NINA_BYTE_PARAM_BUFFER_LENGTH: usize = 1; +pub(crate) const MAX_NINA_WORD_PARAM_BUFFER_LENGTH: usize = 2; +pub(crate) const MAX_NINA_SMALL_ARRAY_PARAM_BUFFER_LENGTH: usize = 255; +pub(crate) const MAX_NINA_LARGE_ARRAY_PARAM_BUFFER_LENGTH: usize = 1024; + +// The maximum length that a 2-byte length NINA response can be +pub(crate) const MAX_NINA_RESPONSE_LENGTH: usize = 32; #[repr(u8)] #[derive(Copy, Clone, Debug)] @@ -35,6 +44,7 @@ pub(crate) enum NinaCommand { } pub(crate) trait NinaConcreteParam { + type DataBuffer; // Length of parameter in bytes type LengthAsBytes: IntoIterator; @@ -55,6 +65,7 @@ pub(crate) struct NinaNoParams { } impl NinaConcreteParam for NinaNoParams { + type DataBuffer = [u8; 0]; type LengthAsBytes = [u8; 0]; fn new(_data: &str) -> Self { @@ -88,32 +99,32 @@ pub(crate) trait NinaParam { // Used for single byte params pub(crate) struct NinaByteParam { length: u8, - data: Vec, + data: ::DataBuffer, } // Used for 2-byte params pub(crate) struct NinaWordParam { length: u8, - data: Vec, + data: ::DataBuffer, } // Used for params that are smaller than 255 bytes pub(crate) struct NinaSmallArrayParam { length: u8, - data: Vec, + data: ::DataBuffer, } // Used for params that can be larger than 255 bytes up to MAX_NINA_PARAM_LENGTH pub(crate) struct NinaLargeArrayParam { length: u16, - data: Vec, + data: ::DataBuffer, } pub(crate) struct NinaAbstractParam { // Byte representation of length of data length_as_bytes: [u8; 2], - // Data to be transfered over SPI bus - data: Vec, + // Data to be transferred over SPI bus + data: ::DataBuffer, // Number of bytes in data length: u16, // The number of bytes needed to represent @@ -195,10 +206,11 @@ impl From for NinaAbstractParam { } impl NinaConcreteParam for NinaByteParam { + type DataBuffer = Vec; type LengthAsBytes = [u8; 1]; fn new(data: &str) -> Self { - let data_as_bytes: Vec = String::from(data).into_bytes(); + let data_as_bytes: Self::DataBuffer = String::from(data).into_bytes(); Self { length: data_as_bytes.len() as u8, data: data_as_bytes, @@ -206,7 +218,8 @@ impl NinaConcreteParam for NinaByteParam { } fn from_bytes(bytes: &[u8]) -> Self { - let mut data_as_bytes: Vec = Vec::new(); + let mut data_as_bytes: Self::DataBuffer = Vec::new(); + data_as_bytes.extend_from_slice(bytes).unwrap_or_default(); Self { length: data_as_bytes.len() as u8, @@ -228,10 +241,11 @@ impl NinaConcreteParam for NinaByteParam { } impl NinaConcreteParam for NinaWordParam { + type DataBuffer = Vec; type LengthAsBytes = [u8; 1]; fn new(data: &str) -> Self { - let data_as_bytes: Vec = String::from(data).into_bytes(); + let data_as_bytes: Self::DataBuffer = String::from(data).into_bytes(); Self { length: data_as_bytes.len() as u8, data: data_as_bytes, @@ -239,7 +253,7 @@ impl NinaConcreteParam for NinaWordParam { } fn from_bytes(bytes: &[u8]) -> Self { - let mut data_as_bytes: Vec = Vec::new(); + let mut data_as_bytes: Self::DataBuffer = Vec::new(); data_as_bytes.extend_from_slice(bytes).unwrap_or_default(); Self { length: data_as_bytes.len() as u8, @@ -261,10 +275,11 @@ impl NinaConcreteParam for NinaWordParam { } impl NinaConcreteParam for NinaSmallArrayParam { + type DataBuffer = Vec; type LengthAsBytes = [u8; 1]; fn new(data: &str) -> Self { - let data_as_bytes: Vec = String::from(data).into_bytes(); + let data_as_bytes: Self::DataBuffer = String::from(data).into_bytes(); Self { length: data_as_bytes.len() as u8, data: data_as_bytes, @@ -272,7 +287,7 @@ impl NinaConcreteParam for NinaSmallArrayParam { } fn from_bytes(bytes: &[u8]) -> Self { - let mut data_as_bytes: Vec = Vec::new(); + let mut data_as_bytes: Self::DataBuffer = Vec::new(); data_as_bytes.extend_from_slice(bytes).unwrap_or_default(); Self { length: data_as_bytes.len() as u8, @@ -294,10 +309,11 @@ impl NinaConcreteParam for NinaSmallArrayParam { } impl NinaConcreteParam for NinaLargeArrayParam { + type DataBuffer = Vec; type LengthAsBytes = [u8; 2]; fn new(data: &str) -> Self { - let data_as_bytes: Vec = String::from(data).into_bytes(); + let data_as_bytes: Self::DataBuffer = String::from(data).into_bytes(); Self { length: data_as_bytes.len() as u16, data: data_as_bytes, @@ -305,7 +321,7 @@ impl NinaConcreteParam for NinaLargeArrayParam { } fn from_bytes(bytes: &[u8]) -> Self { - let mut data_as_bytes: Vec = Vec::new(); + let mut data_as_bytes: Self::DataBuffer = Vec::new(); data_as_bytes.extend_from_slice(bytes).unwrap_or_default(); Self { length: data_as_bytes.len() as u16, @@ -338,7 +354,7 @@ pub(crate) trait ProtocolInterface { fn get_conn_status(&mut self) -> Result; fn set_dns_config(&mut self, dns1: IpAddress, dns2: Option) -> Result<(), Error>; fn req_host_by_name(&mut self, hostname: &str) -> Result; - fn get_host_by_name(&mut self) -> Result<[u8; 8], Error>; + fn get_host_by_name(&mut self) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error>; fn resolve(&mut self, hostname: &str) -> Result; fn get_socket(&mut self) -> Result; fn start_client_tcp( @@ -354,7 +370,7 @@ pub(crate) trait ProtocolInterface { &mut self, data: &str, socket: Socket, - ) -> Result<[u8; ARRAY_LENGTH_PLACEHOLDER], Error>; + ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error>; } #[derive(Debug)] diff --git a/esp32-wroom-rp/src/spi.rs b/esp32-wroom-rp/src/spi.rs index 1b689bc..e3d15fb 100644 --- a/esp32-wroom-rp/src/spi.rs +++ b/esp32-wroom-rp/src/spi.rs @@ -16,11 +16,11 @@ use super::protocol::operation::Operation; use super::protocol::{ NinaByteParam, NinaCommand, NinaConcreteParam, NinaLargeArrayParam, NinaParam, NinaProtocolHandler, NinaSmallArrayParam, NinaWordParam, ProtocolError, ProtocolInterface, + MAX_NINA_PARAMS, MAX_NINA_RESPONSE_LENGTH, }; use super::wifi::ConnectionStatus; -use super::{Error, FirmwareVersion, ARRAY_LENGTH_PLACEHOLDER}; +use super::{Error, FirmwareVersion}; -// TODO: this should eventually move into NinaCommandHandler #[repr(u8)] #[derive(Debug)] enum ControlByte { @@ -52,8 +52,9 @@ where self.execute(&operation)?; let result = self.receive(&operation, 1)?; + let (version, _) = result.split_at(5); - Ok(FirmwareVersion::new(result)) // e.g. 1.7.4 + Ok(FirmwareVersion::new(version)) // e.g. 1.7.4 } fn set_passphrase(&mut self, ssid: &str, passphrase: &str) -> Result<(), Error> { @@ -118,7 +119,7 @@ where Ok(result[0]) } - fn get_host_by_name(&mut self) -> Result<[u8; 8], Error> { + fn get_host_by_name(&mut self) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { let operation = Operation::new(NinaCommand::GetHostByName); self.execute(&operation)?; @@ -212,7 +213,7 @@ where &mut self, data: &str, socket: Socket, - ) -> Result<[u8; ARRAY_LENGTH_PLACEHOLDER], Error> { + ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { let operation = Operation::new(NinaCommand::SendDataTcp) .param(NinaLargeArrayParam::from_bytes(&[socket]).into()) .param(NinaLargeArrayParam::new(data).into()); @@ -270,7 +271,7 @@ where &mut self, operation: &Operation

, expected_num_params: u8, - ) -> Result<[u8; ARRAY_LENGTH_PLACEHOLDER], Error> { + ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { self.control_pins.wait_for_esp_select(); let result = self.wait_response_cmd(&operation.command, expected_num_params); @@ -302,7 +303,7 @@ where &mut self, cmd: &NinaCommand, num_params: u8, - ) -> Result<[u8; ARRAY_LENGTH_PLACEHOLDER], Error> { + ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { self.check_start_cmd()?; let byte_to_check: u8 = *cmd as u8 | ControlByte::Reply as u8; let result = self.read_and_check_byte(&byte_to_check).ok().unwrap(); @@ -317,21 +318,24 @@ where return Err(ProtocolError::InvalidNumberOfParameters.into()); } - let num_params_to_read = self.get_byte().ok().unwrap() as usize; + let number_of_params_to_read = self.get_byte().ok().unwrap() as usize; - // TODO: use a constant instead of inline params max == 8 - if num_params_to_read > 8 { + if number_of_params_to_read > MAX_NINA_PARAMS { return Err(ProtocolError::TooManyParameters.into()); } - let mut params: [u8; ARRAY_LENGTH_PLACEHOLDER] = [0; ARRAY_LENGTH_PLACEHOLDER]; - for (index, _param) in params.into_iter().enumerate() { - params[index] = self.get_byte().ok().unwrap() + let mut response_param_buffer: [u8; MAX_NINA_RESPONSE_LENGTH] = + [0; MAX_NINA_RESPONSE_LENGTH]; + if number_of_params_to_read > 0 { + for (index, _) in response_param_buffer.into_iter().enumerate() { + response_param_buffer[index] = self.get_byte().ok().unwrap() + } } + let control_byte: u8 = ControlByte::End as u8; self.read_and_check_byte(&control_byte).ok(); - Ok(params) + Ok(response_param_buffer) } fn send_end_cmd(&mut self) -> Result<(), Infallible> { diff --git a/esp32-wroom-rp/src/tcp_client.rs b/esp32-wroom-rp/src/tcp_client.rs index bf7939c..8b1297e 100644 --- a/esp32-wroom-rp/src/tcp_client.rs +++ b/esp32-wroom-rp/src/tcp_client.rs @@ -50,9 +50,9 @@ use super::gpio::EspControlInterface; use super::network::{ ConnectionState, Hostname, IpAddress, NetworkError, Port, Socket, TransportMode, }; -use super::protocol::{NinaProtocolHandler, ProtocolInterface}; +use super::protocol::{NinaProtocolHandler, ProtocolInterface, MAX_NINA_RESPONSE_LENGTH}; use super::wifi::Wifi; -use super::{Error, ARRAY_LENGTH_PLACEHOLDER}; +use super::Error; const MAX_HOSTNAME_LENGTH: usize = 255; @@ -176,7 +176,7 @@ where } /// Send a string slice of data to a connected server. - pub fn send_data(&mut self, data: &str) -> Result<[u8; ARRAY_LENGTH_PLACEHOLDER], Error> { + pub fn send_data(&mut self, data: &str) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { self.protocol_handler .send_data(data, self.socket.unwrap_or_default()) } From f5320f1e621cc3b5c92c97faa2f993e06ac0467a Mon Sep 17 00:00:00 2001 From: Caleb Bourg Date: Wed, 15 Feb 2023 06:29:05 -0800 Subject: [PATCH 03/11] remove NinaNoParams --- esp32-wroom-rp/src/protocol.rs | 41 ---------------------------------- 1 file changed, 41 deletions(-) diff --git a/esp32-wroom-rp/src/protocol.rs b/esp32-wroom-rp/src/protocol.rs index 99defbc..a016f9c 100644 --- a/esp32-wroom-rp/src/protocol.rs +++ b/esp32-wroom-rp/src/protocol.rs @@ -59,36 +59,6 @@ pub(crate) trait NinaConcreteParam { fn length(&self) -> u16; } -// Used for Nina protocol commands with no parameters -pub(crate) struct NinaNoParams { - _placeholder: u8, -} - -impl NinaConcreteParam for NinaNoParams { - type DataBuffer = [u8; 0]; - type LengthAsBytes = [u8; 0]; - - fn new(_data: &str) -> Self { - Self { _placeholder: 0 } - } - - fn from_bytes(_bytes: &[u8]) -> Self { - Self { _placeholder: 0 } - } - - fn data(&self) -> &[u8] { - &[0u8] - } - - fn length_as_bytes(&self) -> Self::LengthAsBytes { - [] - } - - fn length(&self) -> u16 { - 0u16 - } -} - pub(crate) trait NinaParam { fn length_as_bytes(&self) -> [u8; 2]; fn data(&self) -> &[u8]; @@ -150,17 +120,6 @@ impl NinaParam for NinaAbstractParam { } } -impl From for NinaAbstractParam { - fn from(concrete_param: NinaNoParams) -> NinaAbstractParam { - NinaAbstractParam { - length_as_bytes: [0, 0], - data: Vec::from_slice(concrete_param.data()).unwrap(), - length: concrete_param.length(), - length_size: 0, - } - } -} - impl From for NinaAbstractParam { fn from(concrete_param: NinaByteParam) -> NinaAbstractParam { NinaAbstractParam { From 46f038964287676acce324e2337cae5f5cd53212 Mon Sep 17 00:00:00 2001 From: Caleb Bourg Date: Wed, 15 Feb 2023 07:21:57 -0800 Subject: [PATCH 04/11] only read bytes equal to what NINA Firmware tells us --- esp32-wroom-rp/src/protocol.rs | 2 +- esp32-wroom-rp/src/spi.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/esp32-wroom-rp/src/protocol.rs b/esp32-wroom-rp/src/protocol.rs index a016f9c..f9d1667 100644 --- a/esp32-wroom-rp/src/protocol.rs +++ b/esp32-wroom-rp/src/protocol.rs @@ -24,7 +24,7 @@ pub(crate) const MAX_NINA_SMALL_ARRAY_PARAM_BUFFER_LENGTH: usize = 255; pub(crate) const MAX_NINA_LARGE_ARRAY_PARAM_BUFFER_LENGTH: usize = 1024; // The maximum length that a 2-byte length NINA response can be -pub(crate) const MAX_NINA_RESPONSE_LENGTH: usize = 32; +pub(crate) const MAX_NINA_RESPONSE_LENGTH: usize = 1064; #[repr(u8)] #[derive(Copy, Clone, Debug)] diff --git a/esp32-wroom-rp/src/spi.rs b/esp32-wroom-rp/src/spi.rs index e3d15fb..a1f971a 100644 --- a/esp32-wroom-rp/src/spi.rs +++ b/esp32-wroom-rp/src/spi.rs @@ -327,8 +327,8 @@ where let mut response_param_buffer: [u8; MAX_NINA_RESPONSE_LENGTH] = [0; MAX_NINA_RESPONSE_LENGTH]; if number_of_params_to_read > 0 { - for (index, _) in response_param_buffer.into_iter().enumerate() { - response_param_buffer[index] = self.get_byte().ok().unwrap() + for i in 0..number_of_params_to_read { + response_param_buffer[i] = self.get_byte().ok().unwrap() } } From 04537612c02491a505aa95aad8b580adc0272392 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Wed, 15 Feb 2023 15:21:28 -0600 Subject: [PATCH 05/11] Make sure that bytes passed into creating a new NinaParam* do not exceed the type's max size. --- esp32-wroom-rp/src/protocol.rs | 90 ++++++++++++++++++++++++++++------ esp32-wroom-rp/src/spi.rs | 61 ++++++++++++++++++----- 2 files changed, 122 insertions(+), 29 deletions(-) diff --git a/esp32-wroom-rp/src/protocol.rs b/esp32-wroom-rp/src/protocol.rs index f9d1667..05477d2 100644 --- a/esp32-wroom-rp/src/protocol.rs +++ b/esp32-wroom-rp/src/protocol.rs @@ -43,14 +43,17 @@ pub(crate) enum NinaCommand { SendDataTcp = 0x44, } -pub(crate) trait NinaConcreteParam { +pub(crate) trait NinaConcreteParam +where + Self: core::marker::Sized, +{ type DataBuffer; // Length of parameter in bytes type LengthAsBytes: IntoIterator; fn new(data: &str) -> Self; - fn from_bytes(bytes: &[u8]) -> Self; + fn from_bytes(bytes: &[u8]) -> Result; fn data(&self) -> &[u8]; @@ -176,14 +179,17 @@ impl NinaConcreteParam for NinaByteParam { } } - fn from_bytes(bytes: &[u8]) -> Self { - let mut data_as_bytes: Self::DataBuffer = Vec::new(); + fn from_bytes(bytes: &[u8]) -> Result { + if bytes.len() > MAX_NINA_BYTE_PARAM_BUFFER_LENGTH { + return Err(ProtocolError::TooManyParameters.into()); + } + let mut data_as_bytes: Self::DataBuffer = Vec::new(); data_as_bytes.extend_from_slice(bytes).unwrap_or_default(); - Self { + Ok(Self { length: data_as_bytes.len() as u8, data: data_as_bytes, - } + }) } fn data(&self) -> &[u8] { @@ -199,6 +205,15 @@ impl NinaConcreteParam for NinaByteParam { } } +impl Default for NinaByteParam { + fn default() -> Self { + Self { + length: 0, + data: Vec::new(), + } + } +} + impl NinaConcreteParam for NinaWordParam { type DataBuffer = Vec; type LengthAsBytes = [u8; 1]; @@ -211,13 +226,17 @@ impl NinaConcreteParam for NinaWordParam { } } - fn from_bytes(bytes: &[u8]) -> Self { + fn from_bytes(bytes: &[u8]) -> Result { + if bytes.len() > MAX_NINA_WORD_PARAM_BUFFER_LENGTH { + return Err(ProtocolError::TooManyParameters.into()); + } + let mut data_as_bytes: Self::DataBuffer = Vec::new(); data_as_bytes.extend_from_slice(bytes).unwrap_or_default(); - Self { + Ok(Self { length: data_as_bytes.len() as u8, data: data_as_bytes, - } + }) } fn data(&self) -> &[u8] { @@ -233,6 +252,15 @@ impl NinaConcreteParam for NinaWordParam { } } +impl Default for NinaWordParam { + fn default() -> Self { + Self { + length: 0, + data: Vec::new(), + } + } +} + impl NinaConcreteParam for NinaSmallArrayParam { type DataBuffer = Vec; type LengthAsBytes = [u8; 1]; @@ -245,13 +273,17 @@ impl NinaConcreteParam for NinaSmallArrayParam { } } - fn from_bytes(bytes: &[u8]) -> Self { + fn from_bytes(bytes: &[u8]) -> Result { + if bytes.len() > MAX_NINA_SMALL_ARRAY_PARAM_BUFFER_LENGTH { + return Err(ProtocolError::TooManyParameters.into()); + } + let mut data_as_bytes: Self::DataBuffer = Vec::new(); data_as_bytes.extend_from_slice(bytes).unwrap_or_default(); - Self { + Ok(Self { length: data_as_bytes.len() as u8, data: data_as_bytes, - } + }) } fn data(&self) -> &[u8] { @@ -267,6 +299,15 @@ impl NinaConcreteParam for NinaSmallArrayParam { } } +impl Default for NinaSmallArrayParam { + fn default() -> Self { + Self { + length: 0, + data: Vec::new(), + } + } +} + impl NinaConcreteParam for NinaLargeArrayParam { type DataBuffer = Vec; type LengthAsBytes = [u8; 2]; @@ -279,13 +320,17 @@ impl NinaConcreteParam for NinaLargeArrayParam { } } - fn from_bytes(bytes: &[u8]) -> Self { + fn from_bytes(bytes: &[u8]) -> Result { + if bytes.len() > MAX_NINA_LARGE_ARRAY_PARAM_BUFFER_LENGTH { + return Err(ProtocolError::TooManyParameters.into()); + } + let mut data_as_bytes: Self::DataBuffer = Vec::new(); data_as_bytes.extend_from_slice(bytes).unwrap_or_default(); - Self { + Ok(Self { length: data_as_bytes.len() as u16, data: data_as_bytes, - } + }) } fn data(&self) -> &[u8] { @@ -304,6 +349,15 @@ impl NinaConcreteParam for NinaLargeArrayParam { } } +impl Default for NinaLargeArrayParam { + fn default() -> Self { + Self { + length: 0, + data: Vec::new(), + } + } +} + pub(crate) trait ProtocolInterface { fn init(&mut self); fn reset>(&mut self, delay: &mut D); @@ -356,6 +410,9 @@ pub enum ProtocolError { InvalidNumberOfParameters, /// Too many parameters sent over the data bus. TooManyParameters, + /// Payload is larger than the maximum buffer size allowed for transmission over + /// the data bus. + PayloadTooLarge, } impl Format for ProtocolError { @@ -365,7 +422,8 @@ impl Format for ProtocolError { ProtocolError::CommunicationTimeout => write!(fmt, "Communication with ESP32 target timed out."), ProtocolError::InvalidCommand => write!(fmt, "Encountered an invalid command while communicating with ESP32 target."), ProtocolError::InvalidNumberOfParameters => write!(fmt, "Encountered an unexpected number of parameters for a NINA command while communicating with ESP32 target."), - ProtocolError::TooManyParameters => write!(fmt, "Encountered too many parameters for a NINA command while communicating with ESP32 target.") + ProtocolError::TooManyParameters => write!(fmt, "Encountered too many parameters for a NINA command while communicating with ESP32 target."), + ProtocolError::PayloadTooLarge => write!(fmt, "The payload is larger than the max buffer size allowed for a NINA parameter while communicating with ESP32 target."), } } } diff --git a/esp32-wroom-rp/src/spi.rs b/esp32-wroom-rp/src/spi.rs index a1f971a..9c839e6 100644 --- a/esp32-wroom-rp/src/spi.rs +++ b/esp32-wroom-rp/src/spi.rs @@ -80,7 +80,8 @@ where fn disconnect(&mut self) -> Result<(), Error> { let dummy_param = NinaByteParam::from_bytes(&[ControlByte::Dummy as u8]); - let operation = Operation::new(NinaCommand::Disconnect).param(dummy_param.into()); + let operation = + Operation::new(NinaCommand::Disconnect).param(dummy_param.unwrap_or_default().into()); self.execute(&operation)?; @@ -93,9 +94,17 @@ where // FIXME: refactor Operation so it can take different NinaParam types let operation = Operation::new(NinaCommand::SetDNSConfig) // FIXME: first param should be able to be a NinaByteParam: - .param(NinaByteParam::from_bytes(&[1]).into()) - .param(NinaSmallArrayParam::from_bytes(&ip1).into()) - .param(NinaSmallArrayParam::from_bytes(&ip2.unwrap_or_default()).into()); + .param(NinaByteParam::from_bytes(&[1]).unwrap_or_default().into()) + .param( + NinaSmallArrayParam::from_bytes(&ip1) + .unwrap_or_default() + .into(), + ) + .param( + NinaSmallArrayParam::from_bytes(&ip2.unwrap_or_default()) + .unwrap_or_default() + .into(), + ); self.execute(&operation)?; @@ -166,10 +175,26 @@ where ) -> Result<(), Error> { let port_as_bytes = [((port & 0xff00) >> 8) as u8, (port & 0xff) as u8]; let operation = Operation::new(NinaCommand::StartClientTcp) - .param(NinaSmallArrayParam::from_bytes(&ip).into()) - .param(NinaWordParam::from_bytes(&port_as_bytes).into()) - .param(NinaByteParam::from_bytes(&[socket]).into()) - .param(NinaByteParam::from_bytes(&[*mode as u8]).into()); + .param( + NinaSmallArrayParam::from_bytes(&ip) + .unwrap_or_default() + .into(), + ) + .param( + NinaWordParam::from_bytes(&port_as_bytes) + .unwrap_or_default() + .into(), + ) + .param( + NinaByteParam::from_bytes(&[socket]) + .unwrap_or_default() + .into(), + ) + .param( + NinaByteParam::from_bytes(&[*mode as u8]) + .unwrap_or_default() + .into(), + ); self.execute(&operation)?; @@ -184,8 +209,11 @@ where // TODO: passing in TransportMode but not using, for now. It will become a way // of stopping the right kind of client (e.g. TCP, vs UDP) fn stop_client_tcp(&mut self, socket: Socket, _mode: &TransportMode) -> Result<(), Error> { - let operation = Operation::new(NinaCommand::StopClientTcp) - .param(NinaByteParam::from_bytes(&[socket]).into()); + let operation = Operation::new(NinaCommand::StopClientTcp).param( + NinaByteParam::from_bytes(&[socket]) + .unwrap_or_default() + .into(), + ); self.execute(&operation)?; @@ -198,8 +226,11 @@ where } fn get_client_state_tcp(&mut self, socket: Socket) -> Result { - let operation = Operation::new(NinaCommand::GetClientStateTcp) - .param(NinaByteParam::from_bytes(&[socket]).into()); + let operation = Operation::new(NinaCommand::GetClientStateTcp).param( + NinaByteParam::from_bytes(&[socket]) + .unwrap_or_default() + .into(), + ); self.execute(&operation)?; @@ -215,7 +246,11 @@ where socket: Socket, ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { let operation = Operation::new(NinaCommand::SendDataTcp) - .param(NinaLargeArrayParam::from_bytes(&[socket]).into()) + .param( + NinaLargeArrayParam::from_bytes(&[socket]) + .unwrap_or_default() + .into(), + ) .param(NinaLargeArrayParam::new(data).into()); self.execute(&operation)?; From bbfcef4f035282802d68a5858e8a49afac340b4e Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Wed, 15 Feb 2023 15:29:52 -0600 Subject: [PATCH 06/11] Make sure that bytes passed into creating a new NinaParam* do not exceed the type's max siz (for new())e. --- esp32-wroom-rp/src/protocol.rs | 50 ++++++++++++++++++++++------------ esp32-wroom-rp/src/spi.rs | 17 ++++++++---- 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/esp32-wroom-rp/src/protocol.rs b/esp32-wroom-rp/src/protocol.rs index 05477d2..2088554 100644 --- a/esp32-wroom-rp/src/protocol.rs +++ b/esp32-wroom-rp/src/protocol.rs @@ -51,7 +51,7 @@ where // Length of parameter in bytes type LengthAsBytes: IntoIterator; - fn new(data: &str) -> Self; + fn new(data: &str) -> Result; fn from_bytes(bytes: &[u8]) -> Result; @@ -171,17 +171,21 @@ impl NinaConcreteParam for NinaByteParam { type DataBuffer = Vec; type LengthAsBytes = [u8; 1]; - fn new(data: &str) -> Self { + fn new(data: &str) -> Result { + if data.len() > MAX_NINA_BYTE_PARAM_BUFFER_LENGTH { + return Err(ProtocolError::PayloadTooLarge.into()); + } + let data_as_bytes: Self::DataBuffer = String::from(data).into_bytes(); - Self { + Ok(Self { length: data_as_bytes.len() as u8, data: data_as_bytes, - } + }) } fn from_bytes(bytes: &[u8]) -> Result { if bytes.len() > MAX_NINA_BYTE_PARAM_BUFFER_LENGTH { - return Err(ProtocolError::TooManyParameters.into()); + return Err(ProtocolError::PayloadTooLarge.into()); } let mut data_as_bytes: Self::DataBuffer = Vec::new(); @@ -218,17 +222,21 @@ impl NinaConcreteParam for NinaWordParam { type DataBuffer = Vec; type LengthAsBytes = [u8; 1]; - fn new(data: &str) -> Self { + fn new(data: &str) -> Result { + if data.len() > MAX_NINA_WORD_PARAM_BUFFER_LENGTH { + return Err(ProtocolError::PayloadTooLarge.into()); + } + let data_as_bytes: Self::DataBuffer = String::from(data).into_bytes(); - Self { + Ok(Self { length: data_as_bytes.len() as u8, data: data_as_bytes, - } + }) } fn from_bytes(bytes: &[u8]) -> Result { if bytes.len() > MAX_NINA_WORD_PARAM_BUFFER_LENGTH { - return Err(ProtocolError::TooManyParameters.into()); + return Err(ProtocolError::PayloadTooLarge.into()); } let mut data_as_bytes: Self::DataBuffer = Vec::new(); @@ -265,17 +273,21 @@ impl NinaConcreteParam for NinaSmallArrayParam { type DataBuffer = Vec; type LengthAsBytes = [u8; 1]; - fn new(data: &str) -> Self { + fn new(data: &str) -> Result { + if data.len() > MAX_NINA_SMALL_ARRAY_PARAM_BUFFER_LENGTH { + return Err(ProtocolError::PayloadTooLarge.into()); + } + let data_as_bytes: Self::DataBuffer = String::from(data).into_bytes(); - Self { + Ok(Self { length: data_as_bytes.len() as u8, data: data_as_bytes, - } + }) } fn from_bytes(bytes: &[u8]) -> Result { if bytes.len() > MAX_NINA_SMALL_ARRAY_PARAM_BUFFER_LENGTH { - return Err(ProtocolError::TooManyParameters.into()); + return Err(ProtocolError::PayloadTooLarge.into()); } let mut data_as_bytes: Self::DataBuffer = Vec::new(); @@ -312,17 +324,21 @@ impl NinaConcreteParam for NinaLargeArrayParam { type DataBuffer = Vec; type LengthAsBytes = [u8; 2]; - fn new(data: &str) -> Self { + fn new(data: &str) -> Result { + if data.len() > MAX_NINA_LARGE_ARRAY_PARAM_BUFFER_LENGTH { + return Err(ProtocolError::PayloadTooLarge.into()); + } + let data_as_bytes: Self::DataBuffer = String::from(data).into_bytes(); - Self { + Ok(Self { length: data_as_bytes.len() as u16, data: data_as_bytes, - } + }) } fn from_bytes(bytes: &[u8]) -> Result { if bytes.len() > MAX_NINA_LARGE_ARRAY_PARAM_BUFFER_LENGTH { - return Err(ProtocolError::TooManyParameters.into()); + return Err(ProtocolError::PayloadTooLarge.into()); } let mut data_as_bytes: Self::DataBuffer = Vec::new(); diff --git a/esp32-wroom-rp/src/spi.rs b/esp32-wroom-rp/src/spi.rs index 9c839e6..2c9bdf7 100644 --- a/esp32-wroom-rp/src/spi.rs +++ b/esp32-wroom-rp/src/spi.rs @@ -59,8 +59,12 @@ where fn set_passphrase(&mut self, ssid: &str, passphrase: &str) -> Result<(), Error> { let operation = Operation::new(NinaCommand::SetPassphrase) - .param(NinaSmallArrayParam::new(ssid).into()) - .param(NinaSmallArrayParam::new(passphrase).into()); + .param(NinaSmallArrayParam::new(ssid).unwrap_or_default().into()) + .param( + NinaSmallArrayParam::new(passphrase) + .unwrap_or_default() + .into(), + ); self.execute(&operation)?; @@ -114,8 +118,11 @@ where } fn req_host_by_name(&mut self, hostname: &str) -> Result { - let operation = Operation::new(NinaCommand::ReqHostByName) - .param(NinaSmallArrayParam::new(hostname).into()); + let operation = Operation::new(NinaCommand::ReqHostByName).param( + NinaSmallArrayParam::new(hostname) + .unwrap_or_default() + .into(), + ); self.execute(&operation)?; @@ -251,7 +258,7 @@ where .unwrap_or_default() .into(), ) - .param(NinaLargeArrayParam::new(data).into()); + .param(NinaLargeArrayParam::new(data).unwrap_or_default().into()); self.execute(&operation)?; From bd570f4af6e3cccae61f8c0eea8084ab6a847ba6 Mon Sep 17 00:00:00 2001 From: Caleb Bourg Date: Wed, 15 Feb 2023 17:10:25 -0800 Subject: [PATCH 07/11] add tests to protocol.rs --- esp32-wroom-rp/src/protocol.rs | 103 +++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/esp32-wroom-rp/src/protocol.rs b/esp32-wroom-rp/src/protocol.rs index 2088554..c2da671 100644 --- a/esp32-wroom-rp/src/protocol.rs +++ b/esp32-wroom-rp/src/protocol.rs @@ -70,29 +70,34 @@ pub(crate) trait NinaParam { } // Used for single byte params +#[derive(PartialEq, Debug)] pub(crate) struct NinaByteParam { length: u8, data: ::DataBuffer, } // Used for 2-byte params +#[derive(PartialEq, Debug)] pub(crate) struct NinaWordParam { length: u8, data: ::DataBuffer, } // Used for params that are smaller than 255 bytes +#[derive(PartialEq, Debug)] pub(crate) struct NinaSmallArrayParam { length: u8, data: ::DataBuffer, } // Used for params that can be larger than 255 bytes up to MAX_NINA_PARAM_LENGTH +#[derive(PartialEq, Debug)] pub(crate) struct NinaLargeArrayParam { length: u16, data: ::DataBuffer, } +#[derive(PartialEq, Debug)] pub(crate) struct NinaAbstractParam { // Byte representation of length of data length_as_bytes: [u8; 2], @@ -443,3 +448,101 @@ impl Format for ProtocolError { } } } + +#[cfg(test)] +mod protocol_tests { + use super::*; + use core::str; + + #[test] + fn nina_byte_param_new_returns_payload_too_large_error_when_given_too_many_bytes() { + let str_slice: &str = "too many bytes"; + let result = NinaByteParam::new(str_slice); + + assert_eq!( + result.unwrap_err(), + Error::Protocol(ProtocolError::PayloadTooLarge) + ) + } + + #[test] + fn nina_byte_param_from_bytes_returns_payload_too_large_error_when_given_too_many_bytes() { + let bytes: [u8; 2] = [0; 2]; + let result = NinaByteParam::from_bytes(&bytes); + + assert_eq!( + result.unwrap_err(), + Error::Protocol(ProtocolError::PayloadTooLarge) + ) + } + + #[test] + fn nina_word_param_new_returns_payload_too_large_error_when_given_too_many_bytes() { + let str_slice: &str = "too many bytes"; + let result = NinaWordParam::new(str_slice); + + assert_eq!( + result.unwrap_err(), + Error::Protocol(ProtocolError::PayloadTooLarge) + ) + } + + #[test] + fn nina_word_param_from_bytes_returns_payload_too_large_error_when_given_too_many_bytes() { + let bytes: [u8; 3] = [0; 3]; + let result = NinaWordParam::from_bytes(&bytes); + + assert_eq!( + result.unwrap_err(), + Error::Protocol(ProtocolError::PayloadTooLarge) + ) + } + + #[test] + fn nina_small_array_param_new_returns_payload_too_large_error_when_given_too_many_bytes() { + let bytes = [0xA; 256]; + let str_slice: &str = str::from_utf8(&bytes).unwrap(); + let result = NinaSmallArrayParam::new(str_slice); + + assert_eq!( + result.unwrap_err(), + Error::Protocol(ProtocolError::PayloadTooLarge) + ) + } + + #[test] + fn nina_small_array_param_from_bytes_returns_payload_too_large_error_when_given_too_many_bytes() + { + let bytes: [u8; 256] = [0xA; 256]; + let result = NinaSmallArrayParam::from_bytes(&bytes); + + assert_eq!( + result.unwrap_err(), + Error::Protocol(ProtocolError::PayloadTooLarge) + ) + } + + #[test] + fn nina_large_array_param_new_returns_payload_too_large_error_when_given_too_many_bytes() { + let bytes = [0xA; 1025]; + let str_slice: &str = str::from_utf8(&bytes).unwrap(); + let result = NinaLargeArrayParam::new(str_slice); + + assert_eq!( + result.unwrap_err(), + Error::Protocol(ProtocolError::PayloadTooLarge) + ) + } + + #[test] + fn nina_large_array_param_from_bytes_returns_payload_too_large_error_when_given_too_many_bytes() + { + let bytes: [u8; 1025] = [0xA; 1025]; + let result = NinaLargeArrayParam::from_bytes(&bytes); + + assert_eq!( + result.unwrap_err(), + Error::Protocol(ProtocolError::PayloadTooLarge) + ) + } +} From 42a23c9f46635c3f33ee28b4061e5f0e68f9dff9 Mon Sep 17 00:00:00 2001 From: Caleb Bourg Date: Wed, 15 Feb 2023 17:11:07 -0800 Subject: [PATCH 08/11] refactor receive function --- esp32-wroom-rp/src/spi.rs | 60 ++++++++++++++++++++++++--------------- host-tests/tests/spi.rs | 3 ++ 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/esp32-wroom-rp/src/spi.rs b/esp32-wroom-rp/src/spi.rs index 2c9bdf7..0b38116 100644 --- a/esp32-wroom-rp/src/spi.rs +++ b/esp32-wroom-rp/src/spi.rs @@ -316,11 +316,13 @@ where ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { self.control_pins.wait_for_esp_select(); - let result = self.wait_response_cmd(&operation.command, expected_num_params); + self.check_response_ready(&operation.command, expected_num_params)?; + + let result = self.read_response()?; self.control_pins.esp_deselect(); - result + Ok(result) } fn send_cmd(&mut self, cmd: &NinaCommand, num_params: u8) -> Result<(), Error> { @@ -341,11 +343,27 @@ where Ok(()) } - fn wait_response_cmd( - &mut self, - cmd: &NinaCommand, - num_params: u8, - ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { + fn read_response(&mut self) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { + let response_length_in_bytes = self.get_byte().ok().unwrap() as usize; + + if response_length_in_bytes > MAX_NINA_PARAMS { + return Err(ProtocolError::TooManyParameters.into()); + } + + let mut response_param_buffer: [u8; MAX_NINA_RESPONSE_LENGTH] = + [0; MAX_NINA_RESPONSE_LENGTH]; + if response_length_in_bytes > 0 { + response_param_buffer = + self.read_response_bytes(response_param_buffer, response_length_in_bytes)?; + } + + let control_byte: u8 = ControlByte::End as u8; + self.read_and_check_byte(&control_byte).ok(); + + Ok(response_param_buffer) + } + + fn check_response_ready(&mut self, cmd: &NinaCommand, num_params: u8) -> Result<(), Error> { self.check_start_cmd()?; let byte_to_check: u8 = *cmd as u8 | ControlByte::Reply as u8; let result = self.read_and_check_byte(&byte_to_check).ok().unwrap(); @@ -359,24 +377,17 @@ where if !result { return Err(ProtocolError::InvalidNumberOfParameters.into()); } + Ok(()) + } - let number_of_params_to_read = self.get_byte().ok().unwrap() as usize; - - if number_of_params_to_read > MAX_NINA_PARAMS { - return Err(ProtocolError::TooManyParameters.into()); - } - - let mut response_param_buffer: [u8; MAX_NINA_RESPONSE_LENGTH] = - [0; MAX_NINA_RESPONSE_LENGTH]; - if number_of_params_to_read > 0 { - for i in 0..number_of_params_to_read { - response_param_buffer[i] = self.get_byte().ok().unwrap() - } + fn read_response_bytes( + &mut self, + mut response_param_buffer: [u8; MAX_NINA_RESPONSE_LENGTH], + response_length_in_bytes: usize, + ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { + for i in 0..response_length_in_bytes { + response_param_buffer[i] = self.get_byte().ok().unwrap() } - - let control_byte: u8 = ControlByte::End as u8; - self.read_and_check_byte(&control_byte).ok(); - Ok(response_param_buffer) } @@ -398,6 +409,9 @@ where for _ in 0..retry_limit { let byte_read = self.get_byte().ok().unwrap(); if byte_read == ControlByte::Error as u8 { + // consume remaining bytes after error: 0x00, 0xEE + self.get_byte().ok(); + self.get_byte().ok(); return Err(ProtocolError::NinaProtocolVersionMismatch.into()); } else if byte_read == wait_byte { return Ok(true); diff --git a/host-tests/tests/spi.rs b/host-tests/tests/spi.rs index bb1102d..7787f21 100644 --- a/host-tests/tests/spi.rs +++ b/host-tests/tests/spi.rs @@ -163,7 +163,10 @@ fn invalid_command_induces_nina_protocol_version_mismatch_error() { let mut invalid_command_expactations = vec![ // wait_response_cmd() // read start command (should be ee) + // NINA Firmware send an error byte (0xef) followed by 0x00 and end 0xee spi::Transaction::transfer(vec![0xff], vec![0xef]), + spi::Transaction::transfer(vec![0xff], vec![0x00]), + spi::Transaction::transfer(vec![0xff], vec![0xee]), ]; expectations.append(&mut invalid_command_expactations); From 33ebdfa7b377d2078c1f4f7dcf0127bb91a4a06d Mon Sep 17 00:00:00 2001 From: Caleb Bourg Date: Wed, 15 Feb 2023 17:11:12 -0800 Subject: [PATCH 09/11] cargo fmt --- cross/send_data_tcp/src/main.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/cross/send_data_tcp/src/main.rs b/cross/send_data_tcp/src/main.rs index 7936bc5..fb24e3b 100644 --- a/cross/send_data_tcp/src/main.rs +++ b/cross/send_data_tcp/src/main.rs @@ -171,11 +171,7 @@ fn main() -> ! { mode, &mut delay, &mut |tcp_client| { - defmt::info!( - "TCP connection to {:?}:{:?} successful", - hostname, - port - ); + defmt::info!("TCP connection to {:?}:{:?} successful", hostname, port); defmt::info!("Hostname: {:?}", tcp_client.server_hostname()); defmt::info!("Sending HTTP Document: {:?}", http_document.as_str()); match tcp_client.send_data(&http_document) { From ad4d6eebfd25cb3c57086dea3c4df30127d6bed6 Mon Sep 17 00:00:00 2001 From: Caleb Bourg Date: Thu, 16 Feb 2023 19:21:38 -0800 Subject: [PATCH 10/11] improve internal ergonomics --- esp32-wroom-rp/src/protocol.rs | 12 +- esp32-wroom-rp/src/protocol/operation.rs | 4 +- esp32-wroom-rp/src/spi.rs | 182 ++++++++++++++--------- esp32-wroom-rp/src/tcp_client.rs | 4 +- 4 files changed, 118 insertions(+), 84 deletions(-) diff --git a/esp32-wroom-rp/src/protocol.rs b/esp32-wroom-rp/src/protocol.rs index c2da671..db96b14 100644 --- a/esp32-wroom-rp/src/protocol.rs +++ b/esp32-wroom-rp/src/protocol.rs @@ -24,7 +24,11 @@ pub(crate) const MAX_NINA_SMALL_ARRAY_PARAM_BUFFER_LENGTH: usize = 255; pub(crate) const MAX_NINA_LARGE_ARRAY_PARAM_BUFFER_LENGTH: usize = 1024; // The maximum length that a 2-byte length NINA response can be -pub(crate) const MAX_NINA_RESPONSE_LENGTH: usize = 1064; +pub(crate) const MAX_NINA_RESPONSE_LENGTH: usize = 1024; + +// TODO: unalias this type and turn into a full wrapper struct +/// Provides a byte buffer to hold responses returned from NINA-FW +pub type NinaResponseBuffer = [u8; MAX_NINA_RESPONSE_LENGTH]; #[repr(u8)] #[derive(Copy, Clone, Debug)] @@ -400,11 +404,7 @@ pub(crate) trait ProtocolInterface { ) -> Result<(), Error>; fn stop_client_tcp(&mut self, socket: Socket, _mode: &TransportMode) -> Result<(), Error>; fn get_client_state_tcp(&mut self, socket: Socket) -> Result; - fn send_data( - &mut self, - data: &str, - socket: Socket, - ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error>; + fn send_data(&mut self, data: &str, socket: Socket) -> Result<[u8; 1], Error>; } #[derive(Debug)] diff --git a/esp32-wroom-rp/src/protocol/operation.rs b/esp32-wroom-rp/src/protocol/operation.rs index 9b10fdb..d3bcde2 100644 --- a/esp32-wroom-rp/src/protocol/operation.rs +++ b/esp32-wroom-rp/src/protocol/operation.rs @@ -24,9 +24,9 @@ impl Operation { // Pushes a new param into the internal `params` Vector which // builds up an internal byte stream representing one Nina command // on the data bus. - pub fn param(mut self, param: NinaAbstractParam) -> Self { + pub fn param>(mut self, param: P) -> Self { // FIXME: Vec::push() will return T when it is full, handle this gracefully - self.params.push(param).unwrap_or(()); + self.params.push(param.into()).unwrap_or(()); self } } diff --git a/esp32-wroom-rp/src/spi.rs b/esp32-wroom-rp/src/spi.rs index 0b38116..a217b58 100644 --- a/esp32-wroom-rp/src/spi.rs +++ b/esp32-wroom-rp/src/spi.rs @@ -15,8 +15,8 @@ use super::network::{ConnectionState, IpAddress, NetworkError, Port, Socket, Tra use super::protocol::operation::Operation; use super::protocol::{ NinaByteParam, NinaCommand, NinaConcreteParam, NinaLargeArrayParam, NinaParam, - NinaProtocolHandler, NinaSmallArrayParam, NinaWordParam, ProtocolError, ProtocolInterface, - MAX_NINA_PARAMS, MAX_NINA_RESPONSE_LENGTH, + NinaProtocolHandler, NinaResponseBuffer, NinaSmallArrayParam, NinaWordParam, ProtocolError, + ProtocolInterface, MAX_NINA_PARAMS, MAX_NINA_RESPONSE_LENGTH, }; use super::wifi::ConnectionStatus; use super::{Error, FirmwareVersion}; @@ -59,12 +59,8 @@ where fn set_passphrase(&mut self, ssid: &str, passphrase: &str) -> Result<(), Error> { let operation = Operation::new(NinaCommand::SetPassphrase) - .param(NinaSmallArrayParam::new(ssid).unwrap_or_default().into()) - .param( - NinaSmallArrayParam::new(passphrase) - .unwrap_or_default() - .into(), - ); + .param(NinaSmallArrayParam::new(ssid)?) + .param(NinaSmallArrayParam::new(passphrase)?); self.execute(&operation)?; @@ -85,7 +81,7 @@ where fn disconnect(&mut self) -> Result<(), Error> { let dummy_param = NinaByteParam::from_bytes(&[ControlByte::Dummy as u8]); let operation = - Operation::new(NinaCommand::Disconnect).param(dummy_param.unwrap_or_default().into()); + Operation::new(NinaCommand::Disconnect).param(dummy_param.unwrap_or_default()); self.execute(&operation)?; @@ -98,17 +94,9 @@ where // FIXME: refactor Operation so it can take different NinaParam types let operation = Operation::new(NinaCommand::SetDNSConfig) // FIXME: first param should be able to be a NinaByteParam: - .param(NinaByteParam::from_bytes(&[1]).unwrap_or_default().into()) - .param( - NinaSmallArrayParam::from_bytes(&ip1) - .unwrap_or_default() - .into(), - ) - .param( - NinaSmallArrayParam::from_bytes(&ip2.unwrap_or_default()) - .unwrap_or_default() - .into(), - ); + .param(NinaByteParam::from_bytes(&[1])?) + .param(NinaSmallArrayParam::from_bytes(&ip1)?) + .param(NinaSmallArrayParam::from_bytes(&ip2.unwrap_or_default())?); self.execute(&operation)?; @@ -118,11 +106,8 @@ where } fn req_host_by_name(&mut self, hostname: &str) -> Result { - let operation = Operation::new(NinaCommand::ReqHostByName).param( - NinaSmallArrayParam::new(hostname) - .unwrap_or_default() - .into(), - ); + let operation = + Operation::new(NinaCommand::ReqHostByName).param(NinaSmallArrayParam::new(hostname)?); self.execute(&operation)?; @@ -135,7 +120,7 @@ where Ok(result[0]) } - fn get_host_by_name(&mut self) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { + fn get_host_by_name(&mut self) -> Result { let operation = Operation::new(NinaCommand::GetHostByName); self.execute(&operation)?; @@ -182,26 +167,10 @@ where ) -> Result<(), Error> { let port_as_bytes = [((port & 0xff00) >> 8) as u8, (port & 0xff) as u8]; let operation = Operation::new(NinaCommand::StartClientTcp) - .param( - NinaSmallArrayParam::from_bytes(&ip) - .unwrap_or_default() - .into(), - ) - .param( - NinaWordParam::from_bytes(&port_as_bytes) - .unwrap_or_default() - .into(), - ) - .param( - NinaByteParam::from_bytes(&[socket]) - .unwrap_or_default() - .into(), - ) - .param( - NinaByteParam::from_bytes(&[*mode as u8]) - .unwrap_or_default() - .into(), - ); + .param(NinaSmallArrayParam::from_bytes(&ip)?) + .param(NinaWordParam::from_bytes(&port_as_bytes)?) + .param(NinaByteParam::from_bytes(&[socket])?) + .param(NinaByteParam::from_bytes(&[*mode as u8])?); self.execute(&operation)?; @@ -216,11 +185,8 @@ where // TODO: passing in TransportMode but not using, for now. It will become a way // of stopping the right kind of client (e.g. TCP, vs UDP) fn stop_client_tcp(&mut self, socket: Socket, _mode: &TransportMode) -> Result<(), Error> { - let operation = Operation::new(NinaCommand::StopClientTcp).param( - NinaByteParam::from_bytes(&[socket]) - .unwrap_or_default() - .into(), - ); + let operation = + Operation::new(NinaCommand::StopClientTcp).param(NinaByteParam::from_bytes(&[socket])?); self.execute(&operation)?; @@ -233,11 +199,8 @@ where } fn get_client_state_tcp(&mut self, socket: Socket) -> Result { - let operation = Operation::new(NinaCommand::GetClientStateTcp).param( - NinaByteParam::from_bytes(&[socket]) - .unwrap_or_default() - .into(), - ); + let operation = Operation::new(NinaCommand::GetClientStateTcp) + .param(NinaByteParam::from_bytes(&[socket])?); self.execute(&operation)?; @@ -247,24 +210,16 @@ where Ok(ConnectionState::from(result[0])) } - fn send_data( - &mut self, - data: &str, - socket: Socket, - ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { + fn send_data(&mut self, data: &str, socket: Socket) -> Result<[u8; 1], Error> { let operation = Operation::new(NinaCommand::SendDataTcp) - .param( - NinaLargeArrayParam::from_bytes(&[socket]) - .unwrap_or_default() - .into(), - ) - .param(NinaLargeArrayParam::new(data).unwrap_or_default().into()); + .param(NinaLargeArrayParam::from_bytes(&[socket])?) + .param(NinaLargeArrayParam::new(data)?); self.execute(&operation)?; let result = self.receive(&operation, 1)?; - Ok(result) + Ok([result[0]]) } } @@ -313,7 +268,7 @@ where &mut self, operation: &Operation

, expected_num_params: u8, - ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { + ) -> Result { self.control_pins.wait_for_esp_select(); self.check_response_ready(&operation.command, expected_num_params)?; @@ -343,15 +298,14 @@ where Ok(()) } - fn read_response(&mut self) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { + fn read_response(&mut self) -> Result { let response_length_in_bytes = self.get_byte().ok().unwrap() as usize; if response_length_in_bytes > MAX_NINA_PARAMS { return Err(ProtocolError::TooManyParameters.into()); } - let mut response_param_buffer: [u8; MAX_NINA_RESPONSE_LENGTH] = - [0; MAX_NINA_RESPONSE_LENGTH]; + let mut response_param_buffer: NinaResponseBuffer = [0; MAX_NINA_RESPONSE_LENGTH]; if response_length_in_bytes > 0 { response_param_buffer = self.read_response_bytes(response_param_buffer, response_length_in_bytes)?; @@ -382,9 +336,9 @@ where fn read_response_bytes( &mut self, - mut response_param_buffer: [u8; MAX_NINA_RESPONSE_LENGTH], + mut response_param_buffer: NinaResponseBuffer, response_length_in_bytes: usize, - ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { + ) -> Result { for i in 0..response_length_in_bytes { response_param_buffer[i] = self.get_byte().ok().unwrap() } @@ -452,3 +406,83 @@ where } } } + +#[cfg(test)] +mod spi_tests { + use super::*; + + use crate::gpio::EspControlPins; + use crate::Error; + use core::cell::RefCell; + use core::str; + use embedded_hal::blocking::spi::Transfer; + use embedded_hal::digital::v2::{InputPin, OutputPin, PinState}; + + struct TransferMock {} + + impl Transfer for TransferMock { + type Error = Error; + fn transfer<'w>(&mut self, words: &'w mut [u8]) -> Result<&'w [u8], Error> { + Ok(words) + } + } + + struct OutputPinMock {} + + impl OutputPin for OutputPinMock { + type Error = Error; + + fn set_low(&mut self) -> Result<(), Self::Error> { + Ok(()) + } + + fn set_high(&mut self) -> Result<(), Self::Error> { + Ok(()) + } + + fn set_state(&mut self, _state: PinState) -> Result<(), Self::Error> { + Ok(()) + } + } + + struct InputPinMock {} + + impl InputPin for InputPinMock { + type Error = Error; + + fn is_high(&self) -> Result { + Ok(true) + } + + fn is_low(&self) -> Result { + Ok(true) + } + } + + #[test] + fn too_large_of_a_nina_param_throws_error() { + let bytes = [0xA; 256]; + let str_slice: &str = str::from_utf8(&bytes).unwrap(); + + let control_pins = EspControlPins { + cs: OutputPinMock {}, + gpio0: OutputPinMock {}, + resetn: OutputPinMock {}, + ack: InputPinMock {}, + }; + + let transfer_mock = TransferMock {}; + + let mut protocol_handler = NinaProtocolHandler { + bus: RefCell::new(transfer_mock), + control_pins: control_pins, + }; + + let result = protocol_handler.set_passphrase(str_slice, ""); + + assert_eq!( + result.unwrap_err(), + Error::Protocol(ProtocolError::PayloadTooLarge) + ) + } +} diff --git a/esp32-wroom-rp/src/tcp_client.rs b/esp32-wroom-rp/src/tcp_client.rs index 8b1297e..1df8aeb 100644 --- a/esp32-wroom-rp/src/tcp_client.rs +++ b/esp32-wroom-rp/src/tcp_client.rs @@ -50,7 +50,7 @@ use super::gpio::EspControlInterface; use super::network::{ ConnectionState, Hostname, IpAddress, NetworkError, Port, Socket, TransportMode, }; -use super::protocol::{NinaProtocolHandler, ProtocolInterface, MAX_NINA_RESPONSE_LENGTH}; +use super::protocol::{NinaProtocolHandler, ProtocolInterface}; use super::wifi::Wifi; use super::Error; @@ -176,7 +176,7 @@ where } /// Send a string slice of data to a connected server. - pub fn send_data(&mut self, data: &str) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> { + pub fn send_data(&mut self, data: &str) -> Result<[u8; 1], Error> { self.protocol_handler .send_data(data, self.socket.unwrap_or_default()) } From ed641f793e0800921202af7838153182aa81ce6d Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Thu, 16 Feb 2023 21:48:07 -0600 Subject: [PATCH 11/11] Fix clippy warning about needless_range_loop being used --- esp32-wroom-rp/src/spi.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/esp32-wroom-rp/src/spi.rs b/esp32-wroom-rp/src/spi.rs index a217b58..76205fe 100644 --- a/esp32-wroom-rp/src/spi.rs +++ b/esp32-wroom-rp/src/spi.rs @@ -339,8 +339,11 @@ where mut response_param_buffer: NinaResponseBuffer, response_length_in_bytes: usize, ) -> Result { - for i in 0..response_length_in_bytes { - response_param_buffer[i] = self.get_byte().ok().unwrap() + for byte in response_param_buffer + .iter_mut() + .take(response_length_in_bytes) + { + *byte = self.get_byte().ok().unwrap(); } Ok(response_param_buffer) }