chore: refactor errors

Signed-off-by: George Cosma <george.cosma@wyliodrin.com>

Author: George Cosma

tockloader-lib/src/attributes/app_attributes.rs

@@ -10,7 +10,7 @@ use tbf_parser::{self};
 use tokio_serial::SerialStream;
 
 use crate::bootloader_serial::{issue_command, Command, Response};
-use crate::errors::TockloaderError;
+use crate::errors::{TockError, TockloaderError};
 
 /// This structure contains all relevant information about a tock application.
 ///
@@ -74,9 +74,7 @@ impl AppAttributes {
         loop {
             let mut appdata = vec![0u8; 8];
 
-            board_core
-                .read(appaddr, &mut appdata)
-                .map_err(TockloaderError::ProbeRsReadError)?;
+            board_core.read(appaddr, &mut appdata)?;
 
             let tbf_version: u16;
             let header_size: u16;
@@ -102,11 +100,9 @@ impl AppAttributes {
 
             let mut header_data = vec![0u8; header_size as usize];
 
-            board_core
-                .read(appaddr, &mut header_data)
-                .map_err(TockloaderError::ProbeRsReadError)?;
+            board_core.read(appaddr, &mut header_data)?;
             let header = parse_tbf_header(&header_data, tbf_version)
-                .map_err(TockloaderError::ParsingError)?;
+                .map_err(TockError::InvalidAppTbfHeader)?;
 
             // The end of the application binary marks the beginning of the
             // footer.
@@ -129,12 +125,10 @@ impl AppAttributes {
                 let mut appfooter =
                     vec![0u8; (total_footers_size - (footer_offset - binary_end_offset)) as usize];
 
-                board_core
-                    .read(appaddr + footer_offset as u64, &mut appfooter)
-                    .map_err(TockloaderError::ProbeRsReadError)?;
+                board_core.read(appaddr + footer_offset as u64, &mut appfooter)?;
 
                 let footer_info =
-                    parse_tbf_footer(&appfooter).map_err(TockloaderError::ParsingError)?;
+                    parse_tbf_footer(&appfooter).map_err(TockError::InvalidAppTbfHeader)?;
 
                 footers.insert(footer_number, TbfFooter::new(footer_info.0, footer_info.1));
 
@@ -231,7 +225,7 @@ impl AppAttributes {
             .await?;
 
             let header = parse_tbf_header(&header_data, tbf_version)
-                .map_err(TockloaderError::ParsingError)?;
+                .map_err(TockError::InvalidAppTbfHeader)?;
             let binary_end_offset = header.get_binary_end();
 
             let mut footers: Vec<TbfFooter> = vec![];
@@ -264,7 +258,7 @@ impl AppAttributes {
                 .await?;
 
                 let footer_info =
-                    parse_tbf_footer(&appfooter).map_err(TockloaderError::ParsingError)?;
+                    parse_tbf_footer(&appfooter).map_err(TockError::InvalidAppTbfHeader)?;
 
                 footers.insert(footer_number, TbfFooter::new(footer_info.0, footer_info.1));
 

tockloader-lib/src/attributes/system_attributes.rs

@@ -7,7 +7,7 @@ use probe_rs::{Core, MemoryInterface};
 use tokio_serial::SerialStream;
 
 use crate::bootloader_serial::{issue_command, Command, Response};
-use crate::errors::TockloaderError;
+use crate::errors::{AttributeParseError, TockError, TockloaderError};
 
 use super::decode::{bytes_to_string, decode_attribute};
 
@@ -72,7 +72,7 @@ impl SystemAttributes {
         // Each attribute is 64 bytes exactly, and there are 16 slots
         let mut buf = [0u8; 64 * 16];
 
-        let _ = board_core.read(address, &mut buf);
+        board_core.read(address, &mut buf)?;
 
         let mut data = buf.chunks(64);
 
@@ -107,10 +107,8 @@ impl SystemAttributes {
                                     .trim_start_matches("0x"),
                                 16,
                             )
-                            .map_err(|_| {
-                                TockloaderError::MisconfiguredBoard(
-                                    "Invalid start address.".to_owned(),
-                                )
+                            .map_err(|e| {
+                                TockError::AttributeParsing(AttributeParseError::InvalidNumber(e))
                             })?,
                         );
                     }
@@ -129,28 +127,22 @@ impl SystemAttributes {
         let address = 0x40E;
         let mut buf = [0u8; 8];
 
-        let _ = board_core.read_8(address, &mut buf);
+        board_core.read_8(address, &mut buf)?;
 
-        let string = String::from_utf8(buf.to_vec()).map_err(|_| {
-            TockloaderError::MisconfiguredBoard(
-                "Data may be corrupted. System attribure is not UTF-8.".to_owned(),
-            )
-        })?;
+        let string = String::from_utf8(buf.to_vec())
+            .map_err(|e| TockError::AttributeParsing(AttributeParseError::InvalidString(e)))?;
 
         let string = string.trim_matches(char::from(0));
 
         result.bootloader_version = Some(string.to_owned());
 
         // The 100 bytes prior to the application start address are reserved for the kernel attributes and flags
         let mut kernel_attr_binary = [0u8; 100];
-        board_core
-            .read(
-                result.appaddr.ok_or(TockloaderError::MisconfiguredBoard(
-                    "No start address found.".to_owned(),
-                ))? - 100,
-                &mut kernel_attr_binary,
-            )
-            .map_err(TockloaderError::ProbeRsReadError)?;
+        let kernel_attr_addr = result
+            .appaddr
+            .ok_or(TockError::MissingAttribute("appaddr".to_owned()))?
+            - 100;
+        board_core.read(kernel_attr_addr, &mut kernel_attr_binary)?;
 
         let sentinel = bytes_to_string(&kernel_attr_binary[96..100]);
         let kernel_version = LittleEndian::read_uint(&kernel_attr_binary[95..96], 1);
@@ -234,10 +226,8 @@ impl SystemAttributes {
                                     .trim_start_matches("0x"),
                                 16,
                             )
-                            .map_err(|_| {
-                                TockloaderError::MisconfiguredBoard(
-                                    "Invalid start address.".to_owned(),
-                                )
+                            .map_err(|e| {
+                                TockError::AttributeParsing(AttributeParseError::InvalidNumber(e))
                             })?,
                         );
                     }
@@ -263,24 +253,20 @@ impl SystemAttributes {
         let (_, buf) =
             issue_command(port, Command::ReadRange, pkt, true, 8, Response::ReadRange).await?;
 
-        let string = String::from_utf8(buf).map_err(|_| {
-            TockloaderError::MisconfiguredBoard(
-                "Data may be corrupted. System attribure is not UTF-8.".to_owned(),
-            )
-        })?;
+        let string = String::from_utf8(buf)
+            .map_err(|e| TockError::AttributeParsing(AttributeParseError::InvalidString(e)))?;
 
         // Strip null bytes
         let string = string.trim_matches(char::from(0));
         result.bootloader_version = Some(string.to_owned());
 
         // The 100 bytes prior to the application start address are reserved
         // for the kernel attributes and flags
-        let mut pkt = ((result.appaddr.ok_or(TockloaderError::MisconfiguredBoard(
-            "No start address found.".to_owned(),
-        ))? - 100) as u32)
-            .to_le_bytes()
-            .to_vec();
-
+        let kernel_attr_addr = (result
+            .appaddr
+            .ok_or(TockError::MissingAttribute("appaddr".to_owned()))?
+            - 100) as u32;
+        let mut pkt = kernel_attr_addr.to_le_bytes().to_vec();
         let length = (100_u16).to_le_bytes().to_vec();
         for i in length {
             pkt.push(i);

tockloader-lib/src/bootloader_serial.rs

@@ -4,7 +4,7 @@
 
 // The "X" commands are for external flash
 
-use crate::errors;
+use crate::errors::{self, InternalError, TockError};
 use bytes::BytesMut;
 use errors::TockloaderError;
 use std::time::Duration;
@@ -17,6 +17,8 @@ pub const SYNC_MESSAGE: [u8; 3] = [0x00, 0xFC, 0x05];
 // "This was chosen as it is infrequent in .bin files" - immesys
 pub const ESCAPE_CHAR: u8 = 0xFC;
 
+pub const DEFAULT_TIMEOUT: Duration = Duration::from_millis(500);
+
 #[allow(dead_code)]
 pub enum Command {
     // Commands from this tool to the bootloader
@@ -99,46 +101,77 @@ impl From<u8> for Response {
 pub async fn toggle_bootloader_entry_dtr_rts(
     port: &mut SerialStream,
 ) -> Result<(), TockloaderError> {
-    port.write_data_terminal_ready(true)
-        .map_err(TockloaderError::SerialInitializationError)?;
-    port.write_request_to_send(true)
-        .map_err(TockloaderError::SerialInitializationError)?;
+    port.write_data_terminal_ready(true)?;
+    port.write_request_to_send(true)?;
 
     tokio::time::sleep(Duration::from_millis(100)).await;
 
-    port.write_data_terminal_ready(false)
-        .map_err(TockloaderError::SerialInitializationError)?;
+    port.write_data_terminal_ready(false)?;
 
     tokio::time::sleep(Duration::from_millis(500)).await;
 
-    port.write_request_to_send(false)
-        .map_err(TockloaderError::SerialInitializationError)?;
+    port.write_request_to_send(false)?;
 
     Ok(())
 }
 
+async fn read_bytes(
+    port: &mut SerialStream,
+    bytes_to_read: usize,
+    timeout: Duration,
+) -> Result<BytesMut, TockloaderError> {
+    let mut ret = BytesMut::with_capacity(bytes_to_read);
+    let mut read_bytes = 0;
+
+    tokio::time::timeout(timeout, async {
+        while read_bytes < bytes_to_read {
+            read_bytes += port
+                .read_buf(&mut ret)
+                .await
+                .map_err(|e| TockloaderError::Serial(e.into()))?;
+        }
+        Ok(ret)
+    })
+    .await
+    .map_err(|_| TockError::BootloaderTimeout)?
+}
+
+async fn write_bytes(
+    port: &mut SerialStream,
+    bytes: &[u8],
+    timeout: Duration,
+) -> Result<(), TockloaderError> {
+    let mut bytes_written = 0;
+
+    tokio::time::timeout(timeout, async {
+        while bytes_written != bytes.len() {
+            bytes_written += port
+                .write_buf(&mut &bytes[bytes_written..])
+                .await
+                .map_err(|e| TockloaderError::Serial(e.into()))?;
+        }
+        Ok(())
+    })
+    .await
+    .map_err(|_| TockError::BootloaderTimeout)?
+}
+
 #[allow(dead_code)]
 pub async fn ping_bootloader_and_wait_for_response(
     port: &mut SerialStream,
-) -> Result<Response, TockloaderError> {
+) -> Result<(), TockloaderError> {
     let ping_pkt = [ESCAPE_CHAR, Command::Ping as u8];
 
-    let mut ret = BytesMut::with_capacity(200);
-
     for _ in 0..30 {
-        let mut bytes_written = 0;
-        while bytes_written != ping_pkt.len() {
-            bytes_written += port.write_buf(&mut &ping_pkt[bytes_written..]).await?;
-        }
-        let mut read_bytes = 0;
-        while read_bytes < 2 {
-            read_bytes += port.read_buf(&mut ret).await?;
-        }
+        write_bytes(port, &ping_pkt, DEFAULT_TIMEOUT).await?;
+        let ret = read_bytes(port, 2, DEFAULT_TIMEOUT).await?;
+
         if ret[1] == Response::Pong as u8 {
-            return Ok(Response::from(ret[1]));
+            return Ok(());
         }
     }
-    Ok(Response::from(ret[1]))
+
+    Err(InternalError::BootloaderNotPresent.into())
 }
 
 #[allow(dead_code)]
@@ -174,46 +207,37 @@ pub async fn issue_command(
     }
 
     // Write the command message
-    let mut bytes_written = 0;
-    while bytes_written != message.len() {
-        bytes_written += port.write_buf(&mut &message[bytes_written..]).await?;
-    }
+    write_bytes(port, &message, DEFAULT_TIMEOUT).await?;
 
     // Response has a two byte header, then response_len bytes
-    let bytes_to_read = 2 + response_len;
-    let mut ret = BytesMut::with_capacity(2);
+    let header = read_bytes(port, 2, DEFAULT_TIMEOUT).await?;
 
-    // We are waiting for 2 bytes to be read
-    let mut read_bytes = 0;
-    while read_bytes < 2 {
-        read_bytes += port.read_buf(&mut ret).await?;
+    if header[0..2] != [ESCAPE_CHAR, response_code as u8] {
+        return Err(TockError::BootloaderBadHeader(header[0], header[1]).into());
     }
 
-    if ret[0] != ESCAPE_CHAR {
-        return Err(TockloaderError::BootloaderError(ret[0]));
-    }
-
-    if ret[1] != response_code.clone() as u8 {
-        return Err(TockloaderError::BootloaderError(ret[1]));
-    }
-
-    let mut new_data: Vec<u8> = Vec::new();
-    let mut value = 2;
-
     if response_len != 0 {
-        while bytes_to_read > value {
-            value += port.read_buf(&mut new_data).await?;
-        }
+        let input = read_bytes(port, response_len, DEFAULT_TIMEOUT).await?;
+        let mut result = Vec::with_capacity(input.len());
 
         // De-escape and add array of read in the bytes
-        for i in 0..(new_data.len() - 1) {
-            if new_data[i] == ESCAPE_CHAR && new_data[i + 1] == ESCAPE_CHAR {
-                new_data.remove(i + 1);
+
+        // TODO(george-cosma): Extract this into a function and unit test this.
+        let mut i = 0;
+        while i < input.len() {
+            if i + 1 < input.len() && input[i] == ESCAPE_CHAR && input[i + 1] == ESCAPE_CHAR {
+                // Found consecutive ESCAPE_CHAR bytes, add only one
+                result.push(ESCAPE_CHAR);
+                i += 2; // Skip both bytes
+            } else {
+                // Not consecutive ESCAPE_CHAR, add the current byte
+                result.push(input[i]);
+                i += 1;
             }
         }
 
-        ret.extend_from_slice(&new_data);
+        Ok((Response::from(header[1]), result))
+    } else {
+        Ok((Response::from(header[1]), vec![]))
     }
-
-    Ok((Response::from(ret[1]), ret[2..].to_vec()))
 }

tockloader-lib/src/command_impl/probers/info.rs

@@ -5,7 +5,7 @@ use crate::attributes::general_attributes::GeneralAttributes;
 use crate::attributes::system_attributes::SystemAttributes;
 use crate::board_settings::BoardSettings;
 use crate::connection::{Connection, ProbeRSConnection};
-use crate::errors::TockloaderError;
+use crate::errors::{InternalError, TockloaderError};
 use crate::CommandInfo;
 
 #[async_trait]
@@ -15,13 +15,11 @@ impl CommandInfo for ProbeRSConnection {
         settings: &BoardSettings,
     ) -> Result<GeneralAttributes, TockloaderError> {
         if !self.is_open() {
-            return Err(TockloaderError::ConnectionNotOpen);
+            return Err(InternalError::ConnectionNotOpen.into());
         }
         let session = self.session.as_mut().expect("Board must be open");
 
-        let mut core = session
-            .core(self.target_info.core)
-            .map_err(|e| TockloaderError::CoreAccessError(self.target_info.core, e))?;
+        let mut core = session.core(self.target_info.core)?;
 
         // TODO(george-cosma): extract these informations without bootloader
         let system_attributes = SystemAttributes::read_system_attributes_probe(&mut core)?;