improve internal ergonomics

Author: calebbourg

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<ConnectionState, Error>;
-    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)]

esp32-wroom-rp/src/protocol/operation.rs

@@ -24,9 +24,9 @@ impl Operation<NinaAbstractParam> {
     // 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<P: Into<NinaAbstractParam>>(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
     }
 }

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<u8, Error> {
-        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<NinaResponseBuffer, Error> {
         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<ConnectionState, Error> {
-        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<P>,
         expected_num_params: u8,
-    ) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> {
+    ) -> Result<NinaResponseBuffer, Error> {
         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<NinaResponseBuffer, 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];
+        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<NinaResponseBuffer, Error> {
         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<u8> 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<bool, Self::Error> {
+            Ok(true)
+        }
+
+        fn is_low(&self) -> Result<bool, Self::Error> {
+            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)
+        )
+    }
+}

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())
     }