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) { 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 } } 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..db96b14 100644 --- a/esp32-wroom-rp/src/protocol.rs +++ b/esp32-wroom-rp/src/protocol.rs @@ -13,9 +13,22 @@ 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 = 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)] @@ -34,13 +47,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 new(data: &str) -> Result; - fn from_bytes(bytes: &[u8]) -> Self; + fn from_bytes(bytes: &[u8]) -> Result; fn data(&self) -> &[u8]; @@ -49,35 +66,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 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]; @@ -86,34 +74,39 @@ pub(crate) trait NinaParam { } // Used for single byte params +#[derive(PartialEq, Debug)] pub(crate) struct NinaByteParam { length: u8, - data: Vec, + data: ::DataBuffer, } // Used for 2-byte params +#[derive(PartialEq, Debug)] pub(crate) struct NinaWordParam { length: u8, - data: Vec, + data: ::DataBuffer, } // Used for params that are smaller than 255 bytes +#[derive(PartialEq, Debug)] 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 +#[derive(PartialEq, Debug)] pub(crate) struct NinaLargeArrayParam { length: u16, - data: Vec, + data: ::DataBuffer, } +#[derive(PartialEq, Debug)] 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 @@ -139,17 +132,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 { @@ -195,23 +177,32 @@ 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(); - 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(); + Ok(Self { length: data_as_bytes.len() as u8, data: data_as_bytes, - } + }) } - fn from_bytes(bytes: &[u8]) -> Self { - let mut data_as_bytes: Vec = Vec::new(); + fn from_bytes(bytes: &[u8]) -> Result { + if bytes.len() > MAX_NINA_BYTE_PARAM_BUFFER_LENGTH { + return Err(ProtocolError::PayloadTooLarge.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] { @@ -227,24 +218,42 @@ 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]; - fn new(data: &str) -> Self { - let data_as_bytes: Vec = String::from(data).into_bytes(); - 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(); + Ok(Self { length: data_as_bytes.len() as u8, data: data_as_bytes, - } + }) } - fn from_bytes(bytes: &[u8]) -> Self { - let mut data_as_bytes: Vec = Vec::new(); + fn from_bytes(bytes: &[u8]) -> Result { + if bytes.len() > MAX_NINA_WORD_PARAM_BUFFER_LENGTH { + return Err(ProtocolError::PayloadTooLarge.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] { @@ -260,24 +269,42 @@ 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]; - fn new(data: &str) -> Self { - let data_as_bytes: Vec = String::from(data).into_bytes(); - 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(); + Ok(Self { length: data_as_bytes.len() as u8, data: data_as_bytes, - } + }) } - fn from_bytes(bytes: &[u8]) -> Self { - let mut data_as_bytes: Vec = Vec::new(); + fn from_bytes(bytes: &[u8]) -> Result { + if bytes.len() > MAX_NINA_SMALL_ARRAY_PARAM_BUFFER_LENGTH { + return Err(ProtocolError::PayloadTooLarge.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] { @@ -293,24 +320,42 @@ 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]; - fn new(data: &str) -> Self { - let data_as_bytes: Vec = String::from(data).into_bytes(); - 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(); + Ok(Self { length: data_as_bytes.len() as u16, data: data_as_bytes, - } + }) } - fn from_bytes(bytes: &[u8]) -> Self { - let mut data_as_bytes: Vec = Vec::new(); + fn from_bytes(bytes: &[u8]) -> Result { + if bytes.len() > MAX_NINA_LARGE_ARRAY_PARAM_BUFFER_LENGTH { + return Err(ProtocolError::PayloadTooLarge.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] { @@ -329,6 +374,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); @@ -338,7 +392,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( @@ -350,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; ARRAY_LENGTH_PLACEHOLDER], Error>; + fn send_data(&mut self, data: &str, socket: Socket) -> Result<[u8; 1], Error>; } #[derive(Debug)] @@ -381,6 +431,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 { @@ -390,7 +443,106 @@ 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."), } } } + +#[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) + ) + } +} 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 1b689bc..76205fe 100644 --- a/esp32-wroom-rp/src/spi.rs +++ b/esp32-wroom-rp/src/spi.rs @@ -15,12 +15,12 @@ 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, + NinaProtocolHandler, NinaResponseBuffer, 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,14 +52,15 @@ 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> { let operation = Operation::new(NinaCommand::SetPassphrase) - .param(NinaSmallArrayParam::new(ssid).into()) - .param(NinaSmallArrayParam::new(passphrase).into()); + .param(NinaSmallArrayParam::new(ssid)?) + .param(NinaSmallArrayParam::new(passphrase)?); self.execute(&operation)?; @@ -79,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()); self.execute(&operation)?; @@ -92,9 +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]).into()) - .param(NinaSmallArrayParam::from_bytes(&ip1).into()) - .param(NinaSmallArrayParam::from_bytes(&ip2.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)?; @@ -104,8 +106,8 @@ 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)?); self.execute(&operation)?; @@ -118,7 +120,7 @@ where Ok(result[0]) } - fn get_host_by_name(&mut self) -> Result<[u8; 8], Error> { + fn get_host_by_name(&mut self) -> Result { let operation = Operation::new(NinaCommand::GetHostByName); self.execute(&operation)?; @@ -165,10 +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).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)?) + .param(NinaWordParam::from_bytes(&port_as_bytes)?) + .param(NinaByteParam::from_bytes(&[socket])?) + .param(NinaByteParam::from_bytes(&[*mode as u8])?); self.execute(&operation)?; @@ -183,8 +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]).into()); + let operation = + Operation::new(NinaCommand::StopClientTcp).param(NinaByteParam::from_bytes(&[socket])?); self.execute(&operation)?; @@ -198,7 +200,7 @@ where fn get_client_state_tcp(&mut self, socket: Socket) -> Result { let operation = Operation::new(NinaCommand::GetClientStateTcp) - .param(NinaByteParam::from_bytes(&[socket]).into()); + .param(NinaByteParam::from_bytes(&[socket])?); self.execute(&operation)?; @@ -208,20 +210,16 @@ where Ok(ConnectionState::from(result[0])) } - fn send_data( - &mut self, - data: &str, - socket: Socket, - ) -> Result<[u8; ARRAY_LENGTH_PLACEHOLDER], 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]).into()) - .param(NinaLargeArrayParam::new(data).into()); + .param(NinaLargeArrayParam::from_bytes(&[socket])?) + .param(NinaLargeArrayParam::new(data)?); self.execute(&operation)?; let result = self.receive(&operation, 1)?; - Ok(result) + Ok([result[0]]) } } @@ -270,14 +268,16 @@ where &mut self, operation: &Operation

, expected_num_params: u8, - ) -> Result<[u8; ARRAY_LENGTH_PLACEHOLDER], Error> { + ) -> Result { 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> { @@ -298,11 +298,26 @@ where Ok(()) } - fn wait_response_cmd( - &mut self, - cmd: &NinaCommand, - num_params: u8, - ) -> Result<[u8; ARRAY_LENGTH_PLACEHOLDER], 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: 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)?; + } + + 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(); @@ -316,22 +331,21 @@ where if !result { return Err(ProtocolError::InvalidNumberOfParameters.into()); } + Ok(()) + } - let num_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 { - 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() + fn read_response_bytes( + &mut self, + mut response_param_buffer: NinaResponseBuffer, + response_length_in_bytes: usize, + ) -> Result { + for byte in response_param_buffer + .iter_mut() + .take(response_length_in_bytes) + { + *byte = 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> { @@ -352,6 +366,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); @@ -392,3 +409,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 bf7939c..1df8aeb 100644 --- a/esp32-wroom-rp/src/tcp_client.rs +++ b/esp32-wroom-rp/src/tcp_client.rs @@ -52,7 +52,7 @@ use super::network::{ }; use super::protocol::{NinaProtocolHandler, ProtocolInterface}; 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; 1], Error> { self.protocol_handler .send_data(data, self.socket.unwrap_or_default()) } 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);