From c010fa05225358f77f4497b8906b26c6aa3a15bf Mon Sep 17 00:00:00 2001 From: George Cosma Date: Mon, 30 Jun 2025 22:07:34 +0300 Subject: [PATCH 01/11] wip: add TODO markers Signed-off-by: George Cosma --- .../src/attributes/app_attributes.rs | 2 + tockloader-lib/src/board_settings.rs | 2 + tockloader-lib/src/connection.rs | 1 + tockloader-lib/src/errors.rs | 6 ++- tockloader-lib/src/lib.rs | 42 ++++++++++++++----- 5 files changed, 41 insertions(+), 12 deletions(-) diff --git a/tockloader-lib/src/attributes/app_attributes.rs b/tockloader-lib/src/attributes/app_attributes.rs index d204de9b..5cdc1709 100644 --- a/tockloader-lib/src/attributes/app_attributes.rs +++ b/tockloader-lib/src/attributes/app_attributes.rs @@ -30,6 +30,8 @@ impl TbfFooter { } } +// TODO(george-cosma): Could take advantages of the trait rework + impl AppAttributes { pub(crate) fn new(header_data: TbfHeader, footers_data: Vec) -> AppAttributes { AppAttributes { diff --git a/tockloader-lib/src/board_settings.rs b/tockloader-lib/src/board_settings.rs index f0a69552..d7f67ab8 100644 --- a/tockloader-lib/src/board_settings.rs +++ b/tockloader-lib/src/board_settings.rs @@ -3,6 +3,8 @@ pub struct BoardSettings { pub start_address: u64, } +// TODO(george-cosma): Does a default implementation make sense for this? Is a +// 'None' architechture a sane idea? impl Default for BoardSettings { fn default() -> Self { Self { diff --git a/tockloader-lib/src/connection.rs b/tockloader-lib/src/connection.rs index e714f88f..c7404d50 100644 --- a/tockloader-lib/src/connection.rs +++ b/tockloader-lib/src/connection.rs @@ -46,6 +46,7 @@ impl Default for SerialTargetInfo { } } +// TODO(george-cosma): trait? and 2 struct ProbeConnection and SerialConection? #[allow(clippy::large_enum_variant)] pub enum Connection { ProbeRS { diff --git a/tockloader-lib/src/errors.rs b/tockloader-lib/src/errors.rs index 6f8f20cf..ad5b058a 100644 --- a/tockloader-lib/src/errors.rs +++ b/tockloader-lib/src/errors.rs @@ -3,9 +3,13 @@ // Copyright OXIDOS AUTOMOTIVE 2024. use std::io; - use thiserror::Error; +// TODO(george-cosma): Split this. Possibly each meta-function (install, list, +// ...) should have its own error type + error type for connection. We can have +// more detailed errors as "sources" for more generic error types, if we TRULY +// need to know why probe-rs failed. Or serial. + #[derive(Debug, Error)] pub enum TockloaderError { #[error("Error occurred while trying to access core: {0}")] diff --git a/tockloader-lib/src/lib.rs b/tockloader-lib/src/lib.rs index 3779c76e..8b6abddd 100644 --- a/tockloader-lib/src/lib.rs +++ b/tockloader-lib/src/lib.rs @@ -36,6 +36,18 @@ pub fn list_serial_ports() -> Result, TockloaderError> { tokio_serial::available_ports().map_err(TockloaderError::SerialInitializationError) } +// TODO(george-cosma): These functions can all be turned into traits (Listable, +// Installable, etc.) and be implemented on the Connection enum. Or, if we also +// split Connection into trait + structs for each type of connection type, we +// can implement these traits on the individual connections types, and not have +// this mega-massive file. + +// TODO(george-cosma): Examine if we need to split these functions into smaller +// parts (reading - processing - writing) for mocking. Could also involve adding +// functions to the proposed 'Connection' trait. + +// TODO(george-cosma): General housekeeping in these functions. + pub async fn list( connection: &mut Connection, settings: &BoardSettings, @@ -107,12 +119,12 @@ pub async fn install_app( .core(info.core) .map_err(|e| TockloaderError::CoreAccessError(info.core, e))?; - // TODO: extract these informations without bootloader - // TODO: extract board name and kernelr version to verify app compatability + // TODO(george-cosma): extract these informations without bootloader + // TODO(george-cosma): extract board name and kernel version to verify app compatability let mut address = settings.start_address; - // TODO: double-check/rework this + // TODO(george-cosma): double-check/rework this // Read a block of 200 8-bit words// Loop to check if there are another apps installed loop { @@ -141,7 +153,7 @@ pub async fn install_app( "No architecture found.".to_owned(), ))?; - let mut binary = tab_file.extract_binary(&arch.clone())?; + let mut binary = tab_file.extract_binary(&arch)?; let size = binary.len() as u64; // Make sure the app is aligned to a multiple of its size @@ -155,12 +167,18 @@ pub async fn install_app( (address, 0) }; - // No more need of core + // TODO(george-cosma): This point MIGHT mark a good point to split + // this function (for probe-rs). + + // At this point we no longer need to hold the probe-rs connection + // to the core, as the flashing is done without it. drop(core); // Make sure the binary is a multiple of the page size by padding 0xFFs - // TODO(Micu Ana): check if the page-size differs - // TODO: also use page_size given or known. This works for now + + // TODO(george-cosma): check if the page-size differs + support + // multiple types of page sizes. Possibly make page size a board + // setting. let page_size = 512; let needs_padding = binary.len() % page_size != 0; @@ -186,14 +204,16 @@ pub async fn install_app( } } - // If there are no pages valid, all pages would have been removed, so we write them all + // If there are no pages valid, all pages would have been removed, + // so we write them all if valid_pages.is_empty() { for i in 0..(size as usize / page_size) { valid_pages.push(i.try_into().unwrap()); } } - // Include a blank page (if exists) after the end of a valid page. There might be a usable 0 on the next page + // Include a blank page (if exists) after the end of a valid page. + // There might be a usable 0 on the next page let mut ending_pages: Vec = Vec::new(); for &i in &valid_pages { let mut iter = valid_pages.iter(); @@ -208,8 +228,8 @@ pub async fn install_app( for i in valid_pages { println!("Writing page number {}", i); - // Create the packet that we send to the bootloader - // First four bytes are the address of the page + // Create the packet that we send to the bootloader. First four + // bytes are the address of the page let mut pkt = Vec::new(); // Then the bytes that go into the page From 0aef16b7ef49dde49b14918d9c19d2e7960091ce Mon Sep 17 00:00:00 2001 From: George Cosma Date: Mon, 30 Jun 2025 22:39:11 +0300 Subject: [PATCH 02/11] wip: refactor board-conenction Signed-off-by: George Cosma --- tockloader-lib/src/connection.rs | 168 ++++++++++++++++++++----------- 1 file changed, 107 insertions(+), 61 deletions(-) diff --git a/tockloader-lib/src/connection.rs b/tockloader-lib/src/connection.rs index c7404d50..49611603 100644 --- a/tockloader-lib/src/connection.rs +++ b/tockloader-lib/src/connection.rs @@ -2,14 +2,15 @@ use std::time::Duration; use probe_rs::probe::DebugProbeInfo; use probe_rs::{Permissions, Session}; +use tokio::io::AsyncWriteExt; use tokio_serial::{FlowControl, Parity, SerialPort, SerialStream, StopBits}; use crate::errors::TockloaderError; -pub enum ConnectionInfo { - SerialInfo(String, SerialTargetInfo), - ProbeInfo(DebugProbeInfo, ProbeTargetInfo), -} +// pub enum ConnectionInfo { +// SerialInfo(String, SerialTargetInfo), +// ProbeInfo(DebugProbeInfo, ProbeTargetInfo), +// } pub struct ProbeTargetInfo { pub chip: String, @@ -46,65 +47,110 @@ impl Default for SerialTargetInfo { } } -// TODO(george-cosma): trait? and 2 struct ProbeConnection and SerialConection? -#[allow(clippy::large_enum_variant)] -pub enum Connection { - ProbeRS { - session: Session, - info: ProbeTargetInfo, - }, - Serial { - stream: SerialStream, - info: SerialTargetInfo, - }, +pub trait Connection { + async fn open(&mut self) -> Result<(), TockloaderError>; + /// Closes the connection, if it is open. If it is not open, it does + /// nothing. On error the state of the connection is unknown and calling + /// `open` or any other method is undefined behavior. + async fn close(&mut self) -> Result<(), TockloaderError>; + fn is_open(&self) -> bool; +} + +pub struct ProbeRSConnection { + session: Option, + /// Used both to open new conections but also used during the session to + /// provide information about the target + target_info: ProbeTargetInfo, + /// Only used for opening a new connection + debug_probe: DebugProbeInfo, } -impl Connection { - pub fn open(info: ConnectionInfo) -> Result { - match info { - ConnectionInfo::SerialInfo(path, target_info) => { - let builder = tokio_serial::new(path, target_info.baud_rate); - match SerialStream::open(&builder) { - Ok(mut stream) => { - stream - .set_parity(target_info.parity) - .map_err(TockloaderError::SerialInitializationError)?; - stream - .set_stop_bits(target_info.stop_bits) - .map_err(TockloaderError::SerialInitializationError)?; - stream - .set_flow_control(target_info.flow_control) - .map_err(TockloaderError::SerialInitializationError)?; - stream - .set_timeout(target_info.timeout) - .map_err(TockloaderError::SerialInitializationError)?; - stream - .write_request_to_send(target_info.request_to_send) - .map_err(TockloaderError::SerialInitializationError)?; - stream - .write_data_terminal_ready(target_info.data_terminal_ready) - .map_err(TockloaderError::SerialInitializationError)?; - Ok(Connection::Serial { - stream, - info: target_info, - }) - } - Err(e) => Err(TockloaderError::SerialInitializationError(e)), - } - } - ConnectionInfo::ProbeInfo(probe_info, target_info) => { - let probe = probe_info - .open() - .map_err(TockloaderError::ProbeRsInitializationError)?; - - match probe.attach(&target_info.chip, Permissions::default()) { - Ok(session) => Ok(Connection::ProbeRS { - session, - info: target_info, - }), - Err(e) => Err(TockloaderError::ProbeRsCommunicationError(e)), - } - } +impl ProbeRSConnection { + pub fn new(debug_probe: DebugProbeInfo, target_info: ProbeTargetInfo) -> Self { + Self { + session: None, + target_info, + debug_probe, } } } + +impl Connection for ProbeRSConnection { + async fn open(&mut self) -> Result<(), TockloaderError> { + let probe = self + .debug_probe + .open() + .map_err(TockloaderError::ProbeRsInitializationError)?; + + self.session = Some( + probe + .attach(&self.target_info.chip, Permissions::default()) + .map_err(TockloaderError::ProbeRsCommunicationError)?, + ); + + Ok(()) + } + + async fn close(&mut self) -> Result<(), TockloaderError> { + // Session implements Drop, so we don't need to explicitly close it. + self.session = None; + Ok(()) + } + + fn is_open(&self) -> bool { + self.session.is_some() + } +} + +pub struct SerialConnection { + stream: Option, + /// Used both to open new connections but also used during the session to + /// provide information about the target + target_info: SerialTargetInfo, + /// Path to the serial port. This is only used for opening a new connection. + port: String, +} + +impl SerialConnection { + pub fn new(port: String, target_info: SerialTargetInfo) -> Self { + Self { + stream: None, + target_info, + port, + } + } +} + +impl Connection for SerialConnection { + async fn open(&mut self) -> Result<(), TockloaderError> { + let builder = tokio_serial::new(&self.port, self.target_info.baud_rate) + .parity(self.target_info.parity) + .stop_bits(self.target_info.stop_bits) + .flow_control(self.target_info.flow_control) + .timeout(self.target_info.timeout); + + let mut stream = + SerialStream::open(&builder).map_err(TockloaderError::SerialInitializationError)?; + + stream + .write_request_to_send(self.target_info.request_to_send) + .map_err(TockloaderError::SerialInitializationError)?; + stream + .write_data_terminal_ready(self.target_info.data_terminal_ready) + .map_err(TockloaderError::SerialInitializationError)?; + + self.stream = Some(stream); + Ok(()) + } + + async fn close(&mut self) -> Result<(), TockloaderError> { + if let Some(mut stream) = self.stream.take() { + stream.shutdown().await; + } + Ok(()) + } + + fn is_open(&self) -> bool { + self.stream.is_some() + } +} From df34e677241da40b834eecf9d0772007ff1d9abf Mon Sep 17 00:00:00 2001 From: George Cosma Date: Mon, 30 Jun 2025 22:39:54 +0300 Subject: [PATCH 03/11] wip: scaffold new command structure Signed-off-by: George Cosma --- tockloader-lib/src/command_impl/mod.rs | 3 +++ .../src/command_impl/probers/info.rs | 0 .../src/command_impl/probers/install.rs | 1 + .../src/command_impl/probers/list.rs | 1 + .../src/command_impl/probers/mod.rs | 3 +++ .../src/command_impl/serial/info.rs | 0 .../src/command_impl/serial/install.rs | 0 .../src/command_impl/serial/list.rs | 1 + tockloader-lib/src/command_impl/serial/mod.rs | 3 +++ tockloader-lib/src/lib.rs | 23 +++++++++++++++++++ 10 files changed, 35 insertions(+) create mode 100644 tockloader-lib/src/command_impl/mod.rs create mode 100644 tockloader-lib/src/command_impl/probers/info.rs create mode 100644 tockloader-lib/src/command_impl/probers/install.rs create mode 100644 tockloader-lib/src/command_impl/probers/list.rs create mode 100644 tockloader-lib/src/command_impl/probers/mod.rs create mode 100644 tockloader-lib/src/command_impl/serial/info.rs create mode 100644 tockloader-lib/src/command_impl/serial/install.rs create mode 100644 tockloader-lib/src/command_impl/serial/list.rs create mode 100644 tockloader-lib/src/command_impl/serial/mod.rs diff --git a/tockloader-lib/src/command_impl/mod.rs b/tockloader-lib/src/command_impl/mod.rs new file mode 100644 index 00000000..755a681b --- /dev/null +++ b/tockloader-lib/src/command_impl/mod.rs @@ -0,0 +1,3 @@ + +pub mod probers; +pub mod serial; diff --git a/tockloader-lib/src/command_impl/probers/info.rs b/tockloader-lib/src/command_impl/probers/info.rs new file mode 100644 index 00000000..e69de29b diff --git a/tockloader-lib/src/command_impl/probers/install.rs b/tockloader-lib/src/command_impl/probers/install.rs new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/tockloader-lib/src/command_impl/probers/install.rs @@ -0,0 +1 @@ + diff --git a/tockloader-lib/src/command_impl/probers/list.rs b/tockloader-lib/src/command_impl/probers/list.rs new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/tockloader-lib/src/command_impl/probers/list.rs @@ -0,0 +1 @@ + diff --git a/tockloader-lib/src/command_impl/probers/mod.rs b/tockloader-lib/src/command_impl/probers/mod.rs new file mode 100644 index 00000000..f820a6a8 --- /dev/null +++ b/tockloader-lib/src/command_impl/probers/mod.rs @@ -0,0 +1,3 @@ +pub mod info; +pub mod install; +pub mod list; diff --git a/tockloader-lib/src/command_impl/serial/info.rs b/tockloader-lib/src/command_impl/serial/info.rs new file mode 100644 index 00000000..e69de29b diff --git a/tockloader-lib/src/command_impl/serial/install.rs b/tockloader-lib/src/command_impl/serial/install.rs new file mode 100644 index 00000000..e69de29b diff --git a/tockloader-lib/src/command_impl/serial/list.rs b/tockloader-lib/src/command_impl/serial/list.rs new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/tockloader-lib/src/command_impl/serial/list.rs @@ -0,0 +1 @@ + diff --git a/tockloader-lib/src/command_impl/serial/mod.rs b/tockloader-lib/src/command_impl/serial/mod.rs new file mode 100644 index 00000000..f820a6a8 --- /dev/null +++ b/tockloader-lib/src/command_impl/serial/mod.rs @@ -0,0 +1,3 @@ +pub mod info; +pub mod install; +pub mod list; diff --git a/tockloader-lib/src/lib.rs b/tockloader-lib/src/lib.rs index 8b6abddd..e176e1ed 100644 --- a/tockloader-lib/src/lib.rs +++ b/tockloader-lib/src/lib.rs @@ -5,6 +5,7 @@ pub mod attributes; pub mod board_settings; pub(crate) mod bootloader_serial; +pub mod command_impl; pub mod connection; mod errors; pub mod known_boards; @@ -48,6 +49,28 @@ pub fn list_serial_ports() -> Result, TockloaderError> { // TODO(george-cosma): General housekeeping in these functions. +pub trait CommandList { + async fn list( + &mut self, + settings: &BoardSettings, + ) -> Result, TockloaderError>; +} + +pub trait CommandInfo { + async fn info( + &mut self, + settings: &BoardSettings, + ) -> Result; +} + +pub trait CommandInstall { + async fn install_app( + &mut self, + settings: &BoardSettings, + tab_file: Tab, + ) -> Result<(), TockloaderError>; +} + pub async fn list( connection: &mut Connection, settings: &BoardSettings, From 7df2a2405e8da2dd93ed2c5880646b67a6a438e8 Mon Sep 17 00:00:00 2001 From: George Cosma Date: Mon, 30 Jun 2025 22:47:23 +0300 Subject: [PATCH 04/11] wip: refactor list command Signed-off-by: George Cosma --- .../src/command_impl/probers/list.rs | 22 ++++++++++++++ .../src/command_impl/serial/list.rs | 29 +++++++++++++++++++ tockloader-lib/src/connection.rs | 8 ++--- tockloader-lib/src/lib.rs | 26 ----------------- 4 files changed, 55 insertions(+), 30 deletions(-) diff --git a/tockloader-lib/src/command_impl/probers/list.rs b/tockloader-lib/src/command_impl/probers/list.rs index 8b137891..ab20f34b 100644 --- a/tockloader-lib/src/command_impl/probers/list.rs +++ b/tockloader-lib/src/command_impl/probers/list.rs @@ -1 +1,23 @@ +use crate::attributes::app_attributes::AppAttributes; +use crate::board_settings::BoardSettings; +use crate::connection::{Connection, ProbeRSConnection}; +use crate::errors::TockloaderError; +use crate::CommandList; +impl CommandList for ProbeRSConnection { + async fn list( + &mut self, + settings: &BoardSettings, + ) -> Result, TockloaderError> { + if !self.is_open() { + todo!("Error here"); + } + 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))?; + + AppAttributes::read_apps_data_probe(&mut core, settings.start_address) + } +} diff --git a/tockloader-lib/src/command_impl/serial/list.rs b/tockloader-lib/src/command_impl/serial/list.rs index 8b137891..00dabcd2 100644 --- a/tockloader-lib/src/command_impl/serial/list.rs +++ b/tockloader-lib/src/command_impl/serial/list.rs @@ -1 +1,30 @@ +use std::time::Duration; +use crate::attributes::app_attributes::AppAttributes; +use crate::board_settings::BoardSettings; +use crate::bootloader_serial::{ping_bootloader_and_wait_for_response, Response}; +use crate::connection::{Connection, SerialConnection}; +use crate::errors::TockloaderError; +use crate::CommandList; + +impl CommandList for SerialConnection { + async fn list( + &mut self, + settings: &BoardSettings, + ) -> Result, TockloaderError> { + if !self.is_open() { + todo!("Error here"); + } + let stream = self.stream.as_mut().expect("Board must be open"); + + let response = ping_bootloader_and_wait_for_response(stream).await?; + + if response as u8 != Response::Pong as u8 { + tokio::time::sleep(Duration::from_millis(100)).await; + // TODO: more robust retry system (and configurable) + let _ = ping_bootloader_and_wait_for_response(stream).await?; + } + + AppAttributes::read_apps_data_serial(stream, settings.start_address).await + } +} diff --git a/tockloader-lib/src/connection.rs b/tockloader-lib/src/connection.rs index 49611603..1cd4b477 100644 --- a/tockloader-lib/src/connection.rs +++ b/tockloader-lib/src/connection.rs @@ -57,10 +57,10 @@ pub trait Connection { } pub struct ProbeRSConnection { - session: Option, + pub(crate) session: Option, /// Used both to open new conections but also used during the session to /// provide information about the target - target_info: ProbeTargetInfo, + pub(crate) target_info: ProbeTargetInfo, /// Only used for opening a new connection debug_probe: DebugProbeInfo, } @@ -103,10 +103,10 @@ impl Connection for ProbeRSConnection { } pub struct SerialConnection { - stream: Option, + pub(crate) stream: Option, /// Used both to open new connections but also used during the session to /// provide information about the target - target_info: SerialTargetInfo, + pub(crate) target_info: SerialTargetInfo, /// Path to the serial port. This is only used for opening a new connection. port: String, } diff --git a/tockloader-lib/src/lib.rs b/tockloader-lib/src/lib.rs index e176e1ed..afb77aa4 100644 --- a/tockloader-lib/src/lib.rs +++ b/tockloader-lib/src/lib.rs @@ -71,32 +71,6 @@ pub trait CommandInstall { ) -> Result<(), TockloaderError>; } -pub async fn list( - connection: &mut Connection, - settings: &BoardSettings, -) -> Result, TockloaderError> { - match connection { - Connection::ProbeRS { session, info } => { - let mut core = session - .core(info.core) - .map_err(|e| TockloaderError::CoreAccessError(info.core, e))?; - - AppAttributes::read_apps_data_probe(&mut core, settings.start_address) - } - Connection::Serial { stream, info: _ } => { - let response = ping_bootloader_and_wait_for_response(stream).await?; - - if response as u8 != Response::Pong as u8 { - tokio::time::sleep(Duration::from_millis(100)).await; - // TODO: more robust retry system (and configurable) - let _ = ping_bootloader_and_wait_for_response(stream).await?; - } - - AppAttributes::read_apps_data_serial(stream, settings.start_address).await - } - } -} - pub async fn info( connection: &mut Connection, settings: &BoardSettings, From 37fde6fe2284e984615a1c6b5c87e9c9d357bccc Mon Sep 17 00:00:00 2001 From: George Cosma Date: Mon, 30 Jun 2025 22:51:49 +0300 Subject: [PATCH 05/11] wip: refactor info command Signed-off-by: George Cosma --- .../src/command_impl/probers/info.rs | 30 ++++++++++++++++ .../src/command_impl/serial/info.rs | 35 +++++++++++++++++++ tockloader-lib/src/lib.rs | 34 ------------------ 3 files changed, 65 insertions(+), 34 deletions(-) diff --git a/tockloader-lib/src/command_impl/probers/info.rs b/tockloader-lib/src/command_impl/probers/info.rs index e69de29b..426158f2 100644 --- a/tockloader-lib/src/command_impl/probers/info.rs +++ b/tockloader-lib/src/command_impl/probers/info.rs @@ -0,0 +1,30 @@ +use crate::attributes::app_attributes::AppAttributes; +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::CommandInfo; + +impl CommandInfo for ProbeRSConnection { + async fn info( + &mut self, + settings: &BoardSettings, + ) -> Result { + if !self.is_open() { + todo!("Error here"); + } + 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))?; + + // TODO(george-cosma): extract these informations without bootloader + let system_attributes = SystemAttributes::read_system_attributes_probe(&mut core)?; + let app_attributes = + AppAttributes::read_apps_data_probe(&mut core, settings.start_address)?; + + Ok(GeneralAttributes::new(system_attributes, app_attributes)) + } +} diff --git a/tockloader-lib/src/command_impl/serial/info.rs b/tockloader-lib/src/command_impl/serial/info.rs index e69de29b..0eccf7e6 100644 --- a/tockloader-lib/src/command_impl/serial/info.rs +++ b/tockloader-lib/src/command_impl/serial/info.rs @@ -0,0 +1,35 @@ +use std::time::Duration; + +use crate::attributes::app_attributes::AppAttributes; +use crate::attributes::general_attributes::GeneralAttributes; +use crate::attributes::system_attributes::SystemAttributes; +use crate::board_settings::BoardSettings; +use crate::bootloader_serial::{ping_bootloader_and_wait_for_response, Response}; +use crate::connection::{Connection, SerialConnection}; +use crate::errors::TockloaderError; +use crate::CommandInfo; + +impl CommandInfo for SerialConnection { + async fn info( + &mut self, + settings: &BoardSettings, + ) -> Result { + if !self.is_open() { + todo!("Error here"); + } + let stream = self.stream.as_mut().expect("Board must be open"); + + let response = ping_bootloader_and_wait_for_response(stream).await?; + + if response as u8 != Response::Pong as u8 { + tokio::time::sleep(Duration::from_millis(100)).await; + let _ = ping_bootloader_and_wait_for_response(stream).await?; + } + + let system_attributes = SystemAttributes::read_system_attributes_serial(stream).await?; + let app_attributes = + AppAttributes::read_apps_data_serial(stream, settings.start_address).await?; + + Ok(GeneralAttributes::new(system_attributes, app_attributes)) + } +} diff --git a/tockloader-lib/src/lib.rs b/tockloader-lib/src/lib.rs index afb77aa4..559269d7 100644 --- a/tockloader-lib/src/lib.rs +++ b/tockloader-lib/src/lib.rs @@ -71,40 +71,6 @@ pub trait CommandInstall { ) -> Result<(), TockloaderError>; } -pub async fn info( - connection: &mut Connection, - settings: &BoardSettings, -) -> Result { - match connection { - Connection::ProbeRS { session, info } => { - let mut core = session - .core(info.core) - .map_err(|e| TockloaderError::CoreAccessError(info.core, e))?; - - // TODO: extract these informations without bootloader - let system_attributes = SystemAttributes::read_system_attributes_probe(&mut core)?; - let app_attributes = - AppAttributes::read_apps_data_probe(&mut core, settings.start_address)?; - - Ok(GeneralAttributes::new(system_attributes, app_attributes)) - } - Connection::Serial { stream, info: _ } => { - let response = ping_bootloader_and_wait_for_response(stream).await?; - - if response as u8 != Response::Pong as u8 { - tokio::time::sleep(Duration::from_millis(100)).await; - let _ = ping_bootloader_and_wait_for_response(stream).await?; - } - - let system_attributes = SystemAttributes::read_system_attributes_serial(stream).await?; - let app_attributes = - AppAttributes::read_apps_data_serial(stream, settings.start_address).await?; - - Ok(GeneralAttributes::new(system_attributes, app_attributes)) - } - } -} - pub async fn install_app( connection: &mut Connection, settings: &BoardSettings, From 5e02614676ea5cb7757ab4ae481bb42aa6e4e961 Mon Sep 17 00:00:00 2001 From: George Cosma Date: Mon, 30 Jun 2025 22:56:16 +0300 Subject: [PATCH 06/11] wip: refactor install command Signed-off-by: George Cosma --- .../src/command_impl/probers/install.rs | 163 ++++++++++++ .../src/command_impl/serial/install.rs | 236 ++++++++++++++++++ tockloader-lib/src/lib.rs | 155 ------------ 3 files changed, 399 insertions(+), 155 deletions(-) diff --git a/tockloader-lib/src/command_impl/probers/install.rs b/tockloader-lib/src/command_impl/probers/install.rs index 8b137891..516ae8c6 100644 --- a/tockloader-lib/src/command_impl/probers/install.rs +++ b/tockloader-lib/src/command_impl/probers/install.rs @@ -1 +1,164 @@ +use probe_rs::flashing::DownloadOptions; +use probe_rs::MemoryInterface; +use tbf_parser::parse::parse_tbf_header_lengths; +use crate::board_settings::BoardSettings; +use crate::connection::{Connection, ProbeRSConnection}; +use crate::errors::TockloaderError; +use crate::tabs::tab::Tab; +use crate::CommandInstall; + +impl CommandInstall for ProbeRSConnection { + async fn install_app( + &mut self, + settings: &BoardSettings, + tab_file: Tab, + ) -> Result<(), TockloaderError> { + if !self.is_open() { + todo!("Error here"); + } + 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))?; + + // TODO(george-cosma): extract these informations without bootloader + // TODO(george-cosma): extract board name and kernel version to verify app compatability + + let mut address = settings.start_address; + + // TODO(george-cosma): double-check/rework this + + // Read a block of 200 8-bit words// Loop to check if there are another apps installed + loop { + let mut buff = vec![0u8; 200]; + core.read(address, &mut buff) + .map_err(TockloaderError::ProbeRsReadError)?; + + let (_ver, _header_len, whole_len) = match parse_tbf_header_lengths( + &buff[0..8] + .try_into() + .expect("Buffer length must be at least 8 bytes long."), + ) { + Ok((ver, header_len, whole_len)) if header_len != 0 => (ver, header_len, whole_len), + _ => break, // No more apps + }; + address += whole_len as u64; + } + + // TODO: extract arch(?) + let arch = settings + .arch + .clone() + .ok_or(TockloaderError::MisconfiguredBoard( + "No architecture found.".to_owned(), + ))?; + + let mut binary = tab_file.extract_binary(&arch)?; + let size = binary.len() as u64; + + // Make sure the app is aligned to a multiple of its size + let multiple = address / size; + + let (new_address, _gap_size) = if multiple * size != address { + let new_address = ((address + size) / size) * size; + let gap_size = new_address - address; + (new_address, gap_size) + } else { + (address, 0) + }; + + // TODO(george-cosma): This point MIGHT mark a good point to split + // this function (for probe-rs). + + // At this point we no longer need to hold the probe-rs connection + // to the core, as the flashing is done without it. + drop(core); + + // Make sure the binary is a multiple of the page size by padding 0xFFs + + // TODO(george-cosma): check if the page-size differs + support + // multiple types of page sizes. Possibly make page size a board + // setting. + let page_size = 512; + let needs_padding = binary.len() % page_size != 0; + + if needs_padding { + let remaining = page_size - (binary.len() % page_size); + dbg!(remaining); + for _i in 0..remaining { + binary.push(0xFF); + } + } + + // Get indices of pages that have valid data to write + let mut valid_pages: Vec = Vec::new(); + for i in 0..(size as usize / page_size) { + for b in binary[(i * page_size)..((i + 1) * page_size)] + .iter() + .copied() + { + if b != 0 { + valid_pages.push(i.try_into().unwrap()); + break; + } + } + } + + // If there are no pages valid, all pages would have been removed, + // so we write them all + if valid_pages.is_empty() { + for i in 0..(size as usize / page_size) { + valid_pages.push(i.try_into().unwrap()); + } + } + + // Include a blank page (if exists) after the end of a valid page. + // There might be a usable 0 on the next page + let mut ending_pages: Vec = Vec::new(); + for &i in &valid_pages { + let mut iter = valid_pages.iter(); + if !iter.any(|&x| x == (i + 1)) && (i + 1) < (size as usize / page_size) as u8 { + ending_pages.push(i + 1); + } + } + + for i in ending_pages { + valid_pages.push(i); + } + + for i in valid_pages { + println!("Writing page number {}", i); + // Create the packet that we send to the bootloader. First four + // bytes are the address of the page + let mut pkt = Vec::new(); + + // Then the bytes that go into the page + for b in binary[(i as usize * page_size)..((i + 1) as usize * page_size)] + .iter() + .copied() + { + pkt.push(b); + } + let mut loader = session.target().flash_loader(); + + loader + .add_data( + (new_address as u32 + (i as usize * page_size) as u32).into(), + &pkt, + ) + .map_err(TockloaderError::ProbeRsWriteError)?; + + let mut options = DownloadOptions::default(); + options.keep_unwritten_bytes = true; + + // Finally, the data can be programmed + loader + .commit(session, options) + .map_err(TockloaderError::ProbeRsWriteError)?; + } + + Ok(()) + } +} diff --git a/tockloader-lib/src/command_impl/serial/install.rs b/tockloader-lib/src/command_impl/serial/install.rs index e69de29b..bdd7154e 100644 --- a/tockloader-lib/src/command_impl/serial/install.rs +++ b/tockloader-lib/src/command_impl/serial/install.rs @@ -0,0 +1,236 @@ +use crate::board_settings::BoardSettings; +use crate::connection::SerialConnection; +use crate::errors::TockloaderError; +use crate::tabs::tab::Tab; +use crate::CommandInstall; + +impl CommandInstall for SerialConnection { + async fn install_app( + &mut self, + _settings: &BoardSettings, + _tab_file: Tab, + ) -> Result<(), TockloaderError> { + todo!() + } +} + +// pub async fn install_app( +// choice: Connection, +// core_index: Option<&usize>, +// tab_file: Tab, +// ) -> Result<(), TockloaderError> { +// match choice { +// Connection::ProbeRS(mut session) => { +// // *snip* +// } +// Connection::Serial(mut port) => { +// let response = ping_bootloader_and_wait_for_response(&mut port).await?; +// +// if response as u8 != Response::Pong as u8 { +// tokio::time::sleep(Duration::from_millis(100)).await; +// let _ = ping_bootloader_and_wait_for_response(&mut port).await?; +// } + +// let system_attributes = +// SystemAttributes::read_system_attributes_serial(&mut port).await?; + +// let board = system_attributes +// .board +// .ok_or("No board name found.".to_owned()); +// let kernel_version = system_attributes +// .kernel_version +// .ok_or("No kernel version found.".to_owned()); + +// match board { +// Ok(board) => { +// // Verify if the specified app is compatible with board +// // TODO(Micu Ana): Replace the print with log messages +// if tab_file.is_compatible_with_board(&board) { +// println!("Specified tab is compatible with board."); +// } else { +// panic!("Specified tab is not compatible with board."); +// } +// } +// Err(e) => { +// return Err(TockloaderError::MisconfiguredBoard(e)); +// } +// } + +// match kernel_version { +// Ok(kernel_version) => { +// // Verify if the specified app is compatible with kernel version +// // TODO(Micu Ana): Replace the prints with log messages +// if tab_file.is_compatible_with_kernel_verison(kernel_version as u32) { +// println!("Specified tab is compatible with your kernel version."); +// } else { +// println!("Specified tab is not compatible with your kernel version."); +// } +// } +// Err(e) => { +// return Err(TockloaderError::MisconfiguredBoard(e)); +// } +// } + +// let mut address = +// system_attributes +// .appaddr +// .ok_or(TockloaderError::MisconfiguredBoard( +// "No start address found.".to_owned(), +// ))?; +// loop { +// // Read a block of 200 8-bit words +// let mut pkt = (address as u32).to_le_bytes().to_vec(); +// let length = (200_u16).to_le_bytes().to_vec(); +// for i in length { +// pkt.push(i); +// } + +// let (_, message) = issue_command( +// &mut port, +// Command::ReadRange, +// pkt, +// true, +// 200, +// Response::ReadRange, +// ) +// .await?; + +// let (_ver, _header_len, whole_len) = match parse_tbf_header_lengths( +// &message[0..8] +// .try_into() +// .expect("Buffer length must be at least 8 bytes long."), +// ) { +// Ok((ver, header_len, whole_len)) if header_len != 0 => { +// (ver, header_len, whole_len) +// } +// _ => break, // No more apps +// }; + +// address += whole_len as u64; +// } + +// let arch = system_attributes +// .arch +// .ok_or("No architecture found.".to_owned()); + +// match arch { +// Ok(arch) => { +// let binary = tab_file.extract_binary(&arch.clone()); + +// match binary { +// Ok(mut binary) => { +// let size = binary.len() as u64; + +// let multiple = address / size; + +// let (mut new_address, _gap_size) = if multiple * size != address { +// let new_address = ((address + size) / size) * size; +// let gap_size = new_address - address; +// (new_address, gap_size) +// } else { +// (address, 0) +// }; + +// // Make sure the binary is a multiple of the page size by padding 0xFFs +// // TODO(Micu Ana): check if the page-size differs +// let page_size = 512; +// let needs_padding = binary.len() % page_size != 0; + +// if needs_padding { +// let remaining = page_size - (binary.len() % page_size); +// for _i in 0..remaining { +// binary.push(0xFF); +// } +// } + +// let binary_len = binary.len(); + +// // Get indices of pages that have valid data to write +// let mut valid_pages: Vec = Vec::new(); +// for i in 0..(binary_len / page_size) { +// for b in binary[(i * page_size)..((i + 1) * page_size)] +// .iter() +// .copied() +// { +// if b != 0 { +// valid_pages.push(i as u8); +// break; +// } +// } +// } + +// // If there are no pages valid, all pages would have been removed, so we write them all +// if valid_pages.is_empty() { +// for i in 0..(binary_len / page_size) { +// valid_pages.push(i as u8); +// } +// } + +// // Include a blank page (if exists) after the end of a valid page. There might be a usable 0 on the next page +// let mut ending_pages: Vec = Vec::new(); +// for &i in &valid_pages { +// let mut iter = valid_pages.iter(); +// if !iter.any(|&x| x == (i + 1)) +// && (i + 1) < (binary_len / page_size) as u8 +// { +// ending_pages.push(i + 1); +// } +// } + +// for i in ending_pages { +// valid_pages.push(i); +// } + +// for i in valid_pages { +// // Create the packet that we send to the bootloader +// // First four bytes are the address of the page +// let mut pkt = (new_address as u32 +// + (i as usize * page_size) as u32) +// .to_le_bytes() +// .to_vec(); +// // Then the bytes that go into the page +// for b in binary +// [(i as usize * page_size)..((i + 1) as usize * page_size)] +// .iter() +// .copied() +// { +// pkt.push(b); +// } + +// // Write to bootloader +// let (_, _) = issue_command( +// &mut port, +// Command::WritePage, +// pkt, +// true, +// 0, +// Response::OK, +// ) +// .await?; +// } + +// new_address += binary.len() as u64; + +// let pkt = (new_address as u32).to_le_bytes().to_vec(); + +// let _ = issue_command( +// &mut port, +// Command::ErasePage, +// pkt, +// true, +// 0, +// Response::OK, +// ) +// .await?; +// } +// Err(e) => { +// return Err(e); +// } +// } +// Ok(()) +// } +// Err(e) => Err(TockloaderError::MisconfiguredBoard(e)), +// } +// } +// } +// } diff --git a/tockloader-lib/src/lib.rs b/tockloader-lib/src/lib.rs index 559269d7..ba330fc9 100644 --- a/tockloader-lib/src/lib.rs +++ b/tockloader-lib/src/lib.rs @@ -71,161 +71,6 @@ pub trait CommandInstall { ) -> Result<(), TockloaderError>; } -pub async fn install_app( - connection: &mut Connection, - settings: &BoardSettings, - tab_file: Tab, -) -> Result<(), TockloaderError> { - match connection { - Connection::ProbeRS { session, info } => { - let mut core = session - .core(info.core) - .map_err(|e| TockloaderError::CoreAccessError(info.core, e))?; - - // TODO(george-cosma): extract these informations without bootloader - // TODO(george-cosma): extract board name and kernel version to verify app compatability - - let mut address = settings.start_address; - - // TODO(george-cosma): double-check/rework this - - // Read a block of 200 8-bit words// Loop to check if there are another apps installed - loop { - let mut buff = vec![0u8; 200]; - core.read(address, &mut buff) - .map_err(TockloaderError::ProbeRsReadError)?; - - let (_ver, _header_len, whole_len) = match parse_tbf_header_lengths( - &buff[0..8] - .try_into() - .expect("Buffer length must be at least 8 bytes long."), - ) { - Ok((ver, header_len, whole_len)) if header_len != 0 => { - (ver, header_len, whole_len) - } - _ => break, // No more apps - }; - address += whole_len as u64; - } - - // TODO: extract arch(?) - let arch = settings - .arch - .clone() - .ok_or(TockloaderError::MisconfiguredBoard( - "No architecture found.".to_owned(), - ))?; - - let mut binary = tab_file.extract_binary(&arch)?; - let size = binary.len() as u64; - - // Make sure the app is aligned to a multiple of its size - let multiple = address / size; - - let (new_address, _gap_size) = if multiple * size != address { - let new_address = ((address + size) / size) * size; - let gap_size = new_address - address; - (new_address, gap_size) - } else { - (address, 0) - }; - - // TODO(george-cosma): This point MIGHT mark a good point to split - // this function (for probe-rs). - - // At this point we no longer need to hold the probe-rs connection - // to the core, as the flashing is done without it. - drop(core); - - // Make sure the binary is a multiple of the page size by padding 0xFFs - - // TODO(george-cosma): check if the page-size differs + support - // multiple types of page sizes. Possibly make page size a board - // setting. - let page_size = 512; - let needs_padding = binary.len() % page_size != 0; - - if needs_padding { - let remaining = page_size - (binary.len() % page_size); - dbg!(remaining); - for _i in 0..remaining { - binary.push(0xFF); - } - } - - // Get indices of pages that have valid data to write - let mut valid_pages: Vec = Vec::new(); - for i in 0..(size as usize / page_size) { - for b in binary[(i * page_size)..((i + 1) * page_size)] - .iter() - .copied() - { - if b != 0 { - valid_pages.push(i.try_into().unwrap()); - break; - } - } - } - - // If there are no pages valid, all pages would have been removed, - // so we write them all - if valid_pages.is_empty() { - for i in 0..(size as usize / page_size) { - valid_pages.push(i.try_into().unwrap()); - } - } - - // Include a blank page (if exists) after the end of a valid page. - // There might be a usable 0 on the next page - let mut ending_pages: Vec = Vec::new(); - for &i in &valid_pages { - let mut iter = valid_pages.iter(); - if !iter.any(|&x| x == (i + 1)) && (i + 1) < (size as usize / page_size) as u8 { - ending_pages.push(i + 1); - } - } - - for i in ending_pages { - valid_pages.push(i); - } - - for i in valid_pages { - println!("Writing page number {}", i); - // Create the packet that we send to the bootloader. First four - // bytes are the address of the page - let mut pkt = Vec::new(); - - // Then the bytes that go into the page - for b in binary[(i as usize * page_size)..((i + 1) as usize * page_size)] - .iter() - .copied() - { - pkt.push(b); - } - let mut loader = session.target().flash_loader(); - - loader - .add_data( - (new_address as u32 + (i as usize * page_size) as u32).into(), - &pkt, - ) - .map_err(TockloaderError::ProbeRsWriteError)?; - - let mut options = DownloadOptions::default(); - options.keep_unwritten_bytes = true; - - // Finally, the data can be programmed - loader - .commit(session, options) - .map_err(TockloaderError::ProbeRsWriteError)?; - } - - Ok(()) - } - Connection::Serial { stream: _, info: _ } => todo!(), - } -} - // pub async fn install_app( // choice: Connection, // core_index: Option<&usize>, From 703916ceaad03704231d819e3aa964fa6a43f380 Mon Sep 17 00:00:00 2001 From: George Cosma Date: Mon, 30 Jun 2025 23:02:14 +0300 Subject: [PATCH 07/11] wip: fix compilation erros and introduce async trait Signed-off-by: George Cosma --- tockloader-lib/Cargo.toml | 1 + .../src/command_impl/probers/info.rs | 3 + .../src/command_impl/probers/install.rs | 2 + .../src/command_impl/probers/list.rs | 3 + .../src/command_impl/serial/info.rs | 3 + .../src/command_impl/serial/install.rs | 3 + .../src/command_impl/serial/list.rs | 3 + tockloader-lib/src/connection.rs | 6 +- tockloader-lib/src/lib.rs | 419 +----------------- 9 files changed, 33 insertions(+), 410 deletions(-) diff --git a/tockloader-lib/Cargo.toml b/tockloader-lib/Cargo.toml index 3f357752..107c5747 100644 --- a/tockloader-lib/Cargo.toml +++ b/tockloader-lib/Cargo.toml @@ -19,3 +19,4 @@ bytes = "1.7.1" toml = "0.8.19" serde = { version = "1.0.210", features = ["derive"] } thiserror = "1.0.63" +async-trait = "0.1.88" diff --git a/tockloader-lib/src/command_impl/probers/info.rs b/tockloader-lib/src/command_impl/probers/info.rs index 426158f2..8843c214 100644 --- a/tockloader-lib/src/command_impl/probers/info.rs +++ b/tockloader-lib/src/command_impl/probers/info.rs @@ -1,3 +1,5 @@ +use async_trait::async_trait; + use crate::attributes::app_attributes::AppAttributes; use crate::attributes::general_attributes::GeneralAttributes; use crate::attributes::system_attributes::SystemAttributes; @@ -6,6 +8,7 @@ use crate::connection::{Connection, ProbeRSConnection}; use crate::errors::TockloaderError; use crate::CommandInfo; +#[async_trait] impl CommandInfo for ProbeRSConnection { async fn info( &mut self, diff --git a/tockloader-lib/src/command_impl/probers/install.rs b/tockloader-lib/src/command_impl/probers/install.rs index 516ae8c6..05ce3c62 100644 --- a/tockloader-lib/src/command_impl/probers/install.rs +++ b/tockloader-lib/src/command_impl/probers/install.rs @@ -1,3 +1,4 @@ +use async_trait::async_trait; use probe_rs::flashing::DownloadOptions; use probe_rs::MemoryInterface; use tbf_parser::parse::parse_tbf_header_lengths; @@ -8,6 +9,7 @@ use crate::errors::TockloaderError; use crate::tabs::tab::Tab; use crate::CommandInstall; +#[async_trait] impl CommandInstall for ProbeRSConnection { async fn install_app( &mut self, diff --git a/tockloader-lib/src/command_impl/probers/list.rs b/tockloader-lib/src/command_impl/probers/list.rs index ab20f34b..2262182b 100644 --- a/tockloader-lib/src/command_impl/probers/list.rs +++ b/tockloader-lib/src/command_impl/probers/list.rs @@ -1,9 +1,12 @@ +use async_trait::async_trait; + use crate::attributes::app_attributes::AppAttributes; use crate::board_settings::BoardSettings; use crate::connection::{Connection, ProbeRSConnection}; use crate::errors::TockloaderError; use crate::CommandList; +#[async_trait] impl CommandList for ProbeRSConnection { async fn list( &mut self, diff --git a/tockloader-lib/src/command_impl/serial/info.rs b/tockloader-lib/src/command_impl/serial/info.rs index 0eccf7e6..2732b3b9 100644 --- a/tockloader-lib/src/command_impl/serial/info.rs +++ b/tockloader-lib/src/command_impl/serial/info.rs @@ -1,5 +1,7 @@ use std::time::Duration; +use async_trait::async_trait; + use crate::attributes::app_attributes::AppAttributes; use crate::attributes::general_attributes::GeneralAttributes; use crate::attributes::system_attributes::SystemAttributes; @@ -9,6 +11,7 @@ use crate::connection::{Connection, SerialConnection}; use crate::errors::TockloaderError; use crate::CommandInfo; +#[async_trait] impl CommandInfo for SerialConnection { async fn info( &mut self, diff --git a/tockloader-lib/src/command_impl/serial/install.rs b/tockloader-lib/src/command_impl/serial/install.rs index bdd7154e..c9ba88c7 100644 --- a/tockloader-lib/src/command_impl/serial/install.rs +++ b/tockloader-lib/src/command_impl/serial/install.rs @@ -1,9 +1,12 @@ +use async_trait::async_trait; + use crate::board_settings::BoardSettings; use crate::connection::SerialConnection; use crate::errors::TockloaderError; use crate::tabs::tab::Tab; use crate::CommandInstall; +#[async_trait] impl CommandInstall for SerialConnection { async fn install_app( &mut self, diff --git a/tockloader-lib/src/command_impl/serial/list.rs b/tockloader-lib/src/command_impl/serial/list.rs index 00dabcd2..a7bade2e 100644 --- a/tockloader-lib/src/command_impl/serial/list.rs +++ b/tockloader-lib/src/command_impl/serial/list.rs @@ -1,5 +1,7 @@ use std::time::Duration; +use async_trait::async_trait; + use crate::attributes::app_attributes::AppAttributes; use crate::board_settings::BoardSettings; use crate::bootloader_serial::{ping_bootloader_and_wait_for_response, Response}; @@ -7,6 +9,7 @@ use crate::connection::{Connection, SerialConnection}; use crate::errors::TockloaderError; use crate::CommandList; +#[async_trait] impl CommandList for SerialConnection { async fn list( &mut self, diff --git a/tockloader-lib/src/connection.rs b/tockloader-lib/src/connection.rs index 1cd4b477..8160470f 100644 --- a/tockloader-lib/src/connection.rs +++ b/tockloader-lib/src/connection.rs @@ -1,5 +1,6 @@ use std::time::Duration; +use async_trait::async_trait; use probe_rs::probe::DebugProbeInfo; use probe_rs::{Permissions, Session}; use tokio::io::AsyncWriteExt; @@ -47,6 +48,7 @@ impl Default for SerialTargetInfo { } } +#[async_trait] pub trait Connection { async fn open(&mut self) -> Result<(), TockloaderError>; /// Closes the connection, if it is open. If it is not open, it does @@ -75,6 +77,7 @@ impl ProbeRSConnection { } } +#[async_trait] impl Connection for ProbeRSConnection { async fn open(&mut self) -> Result<(), TockloaderError> { let probe = self @@ -121,6 +124,7 @@ impl SerialConnection { } } +#[async_trait] impl Connection for SerialConnection { async fn open(&mut self) -> Result<(), TockloaderError> { let builder = tokio_serial::new(&self.port, self.target_info.baud_rate) @@ -145,7 +149,7 @@ impl Connection for SerialConnection { async fn close(&mut self) -> Result<(), TockloaderError> { if let Some(mut stream) = self.stream.take() { - stream.shutdown().await; + stream.shutdown().await?; } Ok(()) } diff --git a/tockloader-lib/src/lib.rs b/tockloader-lib/src/lib.rs index ba330fc9..ab69c92b 100644 --- a/tockloader-lib/src/lib.rs +++ b/tockloader-lib/src/lib.rs @@ -11,24 +11,16 @@ mod errors; pub mod known_boards; pub mod tabs; -use std::time::Duration; - -use attributes::app_attributes::AppAttributes; -use attributes::general_attributes::GeneralAttributes; -use attributes::system_attributes::SystemAttributes; - -use board_settings::BoardSettings; -use bootloader_serial::{ping_bootloader_and_wait_for_response, Response}; -use connection::Connection; -use probe_rs::flashing::DownloadOptions; +use async_trait::async_trait; use probe_rs::probe::DebugProbeInfo; -use probe_rs::MemoryInterface; - -use errors::TockloaderError; -use tabs::tab::Tab; -use tbf_parser::parse::parse_tbf_header_lengths; use tokio_serial::SerialPortInfo; +use crate::attributes::app_attributes::AppAttributes; +use crate::attributes::general_attributes::GeneralAttributes; +use crate::board_settings::BoardSettings; +use crate::errors::TockloaderError; +use crate::tabs::tab::Tab; + pub fn list_debug_probes() -> Vec { probe_rs::probe::list::Lister::new().list_all() } @@ -37,18 +29,13 @@ pub fn list_serial_ports() -> Result, TockloaderError> { tokio_serial::available_ports().map_err(TockloaderError::SerialInitializationError) } -// TODO(george-cosma): These functions can all be turned into traits (Listable, -// Installable, etc.) and be implemented on the Connection enum. Or, if we also -// split Connection into trait + structs for each type of connection type, we -// can implement these traits on the individual connections types, and not have -// this mega-massive file. - // TODO(george-cosma): Examine if we need to split these functions into smaller // parts (reading - processing - writing) for mocking. Could also involve adding // functions to the proposed 'Connection' trait. // TODO(george-cosma): General housekeeping in these functions. +#[async_trait] pub trait CommandList { async fn list( &mut self, @@ -56,6 +43,7 @@ pub trait CommandList { ) -> Result, TockloaderError>; } +#[async_trait] pub trait CommandInfo { async fn info( &mut self, @@ -63,6 +51,7 @@ pub trait CommandInfo { ) -> Result; } +#[async_trait] pub trait CommandInstall { async fn install_app( &mut self, @@ -70,391 +59,3 @@ pub trait CommandInstall { tab_file: Tab, ) -> Result<(), TockloaderError>; } - -// pub async fn install_app( -// choice: Connection, -// core_index: Option<&usize>, -// tab_file: Tab, -// ) -> Result<(), TockloaderError> { -// match choice { -// Connection::ProbeRS(mut session) => { -// // Get core - if not specified, by default is 0 -// let mut core = session -// .core(*core_index.unwrap()) -// .map_err(|e| TockloaderError::CoreAccessError(*core_index.unwrap(), e))?; -// // Get board data -// let system_attributes = SystemAttributes::read_system_attributes_probe(&mut core)?; - -// let board = system_attributes -// .board -// .ok_or(TockloaderError::MisconfiguredBoard( -// "No board name found.".to_owned(), -// ))?; -// let kernel_version = -// system_attributes -// .kernel_version -// .ok_or(TockloaderError::MisconfiguredBoard( -// "No kernel version found.".to_owned(), -// ))?; - -// // Verify if the specified app is compatible with board -// // TODO(Micu Ana): Replace the print with log messages -// if tab_file.is_compatible_with_board(&board) { -// println!("Specified tab is compatible with board."); -// } else { -// panic!("Specified tab is not compatible with board."); -// } - -// // Verify if the specified app is compatible with kernel version -// // TODO(Micu Ana): Replace the prints with log messages -// if tab_file.is_compatible_with_kernel_verison(kernel_version as u32) { -// println!("Specified tab is compatible with your kernel version."); -// } else { -// println!("Specified tab is not compatible with your kernel version."); -// } - -// // Get the address from which we start writing the new app -// // TODO: change appaddr to 32 bit -// // TODO for the future: support 64 bit arhitecture -// let mut address = -// system_attributes -// .appaddr -// .ok_or(TockloaderError::MisconfiguredBoard( -// "No start address found.".to_owned(), -// ))?; - -// // Loop to check if there are another apps installed -// loop { -// // Read a block of 200 8-bit words -// let mut buff = vec![0u8; 200]; -// core.read(address, &mut buff) -// .map_err(TockloaderError::ProbeRsReadError)?; - -// let (_ver, _header_len, whole_len) = match parse_tbf_header_lengths( -// &buff[0..8] -// .try_into() -// .expect("Buffer length must be at least 8 bytes long."), -// ) { -// Ok((ver, header_len, whole_len)) if header_len != 0 => { -// (ver, header_len, whole_len) -// } -// _ => break, // No more apps -// }; -// address += whole_len as u64; -// } - -// let arch = system_attributes -// .arch -// .ok_or(TockloaderError::MisconfiguredBoard( -// "No architecture found.".to_owned(), -// ))?; - -// let mut binary = tab_file.extract_binary(&arch.clone())?; // use the system_attributes arch or the provided one? - -// let size = binary.len() as u64; - -// // Make sure the app is aligned to a multiple of its size -// let multiple = address / size; - -// let (new_address, _gap_size) = if multiple * size != address { -// let new_address = ((address + size) / size) * size; -// let gap_size = new_address - address; -// (new_address, gap_size) -// } else { -// (address, 0) -// }; - -// // No more need of core -// drop(core); - -// // Make sure the binary is a multiple of the page size by padding 0xFFs -// // TODO(Micu Ana): check if the page-size differs -// let page_size = 512; -// let needs_padding = binary.len() % page_size != 0; - -// if needs_padding { -// let remaining = page_size - (binary.len() % page_size); -// dbg!(remaining); -// for _i in 0..remaining { -// binary.push(0xFF); -// } -// } - -// // Get indices of pages that have valid data to write -// let mut valid_pages: Vec = Vec::new(); -// for i in 0..(size as usize / page_size) { -// for b in binary[(i * page_size)..((i + 1) * page_size)] -// .iter() -// .copied() -// { -// if b != 0 { -// valid_pages.push(i.try_into().unwrap()); -// break; -// } -// } -// } - -// // If there are no pages valid, all pages would have been removed, so we write them all -// if valid_pages.is_empty() { -// for i in 0..(size as usize / page_size) { -// valid_pages.push(i.try_into().unwrap()); -// } -// } - -// // Include a blank page (if exists) after the end of a valid page. There might be a usable 0 on the next page -// let mut ending_pages: Vec = Vec::new(); -// for &i in &valid_pages { -// let mut iter = valid_pages.iter(); -// if !iter.any(|&x| x == (i + 1)) && (i + 1) < (size as usize / page_size) as u8 { -// ending_pages.push(i + 1); -// } -// } - -// for i in ending_pages { -// valid_pages.push(i); -// } - -// for i in valid_pages { -// println!("Writing page number {}", i); -// // Create the packet that we send to the bootloader -// // First four bytes are the address of the page -// let mut pkt = Vec::new(); - -// // Then the bytes that go into the page -// for b in binary[(i as usize * page_size)..((i + 1) as usize * page_size)] -// .iter() -// .copied() -// { -// pkt.push(b); -// } -// let mut loader = session.target().flash_loader(); - -// loader -// .add_data( -// (new_address as u32 + (i as usize * page_size) as u32).into(), -// &pkt, -// ) -// .map_err(TockloaderError::ProbeRsWriteError)?; - -// let mut options = DownloadOptions::default(); -// options.keep_unwritten_bytes = true; - -// // Finally, the data can be programmed -// loader -// .commit(&mut session, options) -// .map_err(TockloaderError::ProbeRsWriteError)?; -// } - -// Ok(()) -// } -// Connection::Serial(mut port) => { -// let response = ping_bootloader_and_wait_for_response(&mut port).await?; - -// if response as u8 != Response::Pong as u8 { -// tokio::time::sleep(Duration::from_millis(100)).await; -// let _ = ping_bootloader_and_wait_for_response(&mut port).await?; -// } - -// let system_attributes = -// SystemAttributes::read_system_attributes_serial(&mut port).await?; - -// let board = system_attributes -// .board -// .ok_or("No board name found.".to_owned()); -// let kernel_version = system_attributes -// .kernel_version -// .ok_or("No kernel version found.".to_owned()); - -// match board { -// Ok(board) => { -// // Verify if the specified app is compatible with board -// // TODO(Micu Ana): Replace the print with log messages -// if tab_file.is_compatible_with_board(&board) { -// println!("Specified tab is compatible with board."); -// } else { -// panic!("Specified tab is not compatible with board."); -// } -// } -// Err(e) => { -// return Err(TockloaderError::MisconfiguredBoard(e)); -// } -// } - -// match kernel_version { -// Ok(kernel_version) => { -// // Verify if the specified app is compatible with kernel version -// // TODO(Micu Ana): Replace the prints with log messages -// if tab_file.is_compatible_with_kernel_verison(kernel_version as u32) { -// println!("Specified tab is compatible with your kernel version."); -// } else { -// println!("Specified tab is not compatible with your kernel version."); -// } -// } -// Err(e) => { -// return Err(TockloaderError::MisconfiguredBoard(e)); -// } -// } - -// let mut address = -// system_attributes -// .appaddr -// .ok_or(TockloaderError::MisconfiguredBoard( -// "No start address found.".to_owned(), -// ))?; -// loop { -// // Read a block of 200 8-bit words -// let mut pkt = (address as u32).to_le_bytes().to_vec(); -// let length = (200_u16).to_le_bytes().to_vec(); -// for i in length { -// pkt.push(i); -// } - -// let (_, message) = issue_command( -// &mut port, -// Command::ReadRange, -// pkt, -// true, -// 200, -// Response::ReadRange, -// ) -// .await?; - -// let (_ver, _header_len, whole_len) = match parse_tbf_header_lengths( -// &message[0..8] -// .try_into() -// .expect("Buffer length must be at least 8 bytes long."), -// ) { -// Ok((ver, header_len, whole_len)) if header_len != 0 => { -// (ver, header_len, whole_len) -// } -// _ => break, // No more apps -// }; - -// address += whole_len as u64; -// } - -// let arch = system_attributes -// .arch -// .ok_or("No architecture found.".to_owned()); - -// match arch { -// Ok(arch) => { -// let binary = tab_file.extract_binary(&arch.clone()); - -// match binary { -// Ok(mut binary) => { -// let size = binary.len() as u64; - -// let multiple = address / size; - -// let (mut new_address, _gap_size) = if multiple * size != address { -// let new_address = ((address + size) / size) * size; -// let gap_size = new_address - address; -// (new_address, gap_size) -// } else { -// (address, 0) -// }; - -// // Make sure the binary is a multiple of the page size by padding 0xFFs -// // TODO(Micu Ana): check if the page-size differs -// let page_size = 512; -// let needs_padding = binary.len() % page_size != 0; - -// if needs_padding { -// let remaining = page_size - (binary.len() % page_size); -// for _i in 0..remaining { -// binary.push(0xFF); -// } -// } - -// let binary_len = binary.len(); - -// // Get indices of pages that have valid data to write -// let mut valid_pages: Vec = Vec::new(); -// for i in 0..(binary_len / page_size) { -// for b in binary[(i * page_size)..((i + 1) * page_size)] -// .iter() -// .copied() -// { -// if b != 0 { -// valid_pages.push(i as u8); -// break; -// } -// } -// } - -// // If there are no pages valid, all pages would have been removed, so we write them all -// if valid_pages.is_empty() { -// for i in 0..(binary_len / page_size) { -// valid_pages.push(i as u8); -// } -// } - -// // Include a blank page (if exists) after the end of a valid page. There might be a usable 0 on the next page -// let mut ending_pages: Vec = Vec::new(); -// for &i in &valid_pages { -// let mut iter = valid_pages.iter(); -// if !iter.any(|&x| x == (i + 1)) -// && (i + 1) < (binary_len / page_size) as u8 -// { -// ending_pages.push(i + 1); -// } -// } - -// for i in ending_pages { -// valid_pages.push(i); -// } - -// for i in valid_pages { -// // Create the packet that we send to the bootloader -// // First four bytes are the address of the page -// let mut pkt = (new_address as u32 -// + (i as usize * page_size) as u32) -// .to_le_bytes() -// .to_vec(); -// // Then the bytes that go into the page -// for b in binary -// [(i as usize * page_size)..((i + 1) as usize * page_size)] -// .iter() -// .copied() -// { -// pkt.push(b); -// } - -// // Write to bootloader -// let (_, _) = issue_command( -// &mut port, -// Command::WritePage, -// pkt, -// true, -// 0, -// Response::OK, -// ) -// .await?; -// } - -// new_address += binary.len() as u64; - -// let pkt = (new_address as u32).to_le_bytes().to_vec(); - -// let _ = issue_command( -// &mut port, -// Command::ErasePage, -// pkt, -// true, -// 0, -// Response::OK, -// ) -// .await?; -// } -// Err(e) => { -// return Err(e); -// } -// } -// Ok(()) -// } -// Err(e) => Err(TockloaderError::MisconfiguredBoard(e)), -// } -// } -// } -// } From 26f50db1eb7cbcace35b88442fbebe6792a2d94e Mon Sep 17 00:00:00 2001 From: George Cosma Date: Mon, 30 Jun 2025 23:18:49 +0300 Subject: [PATCH 08/11] wip: refactor tockloader-cli Signed-off-by: George Cosma --- tockloader-cli/src/main.rs | 51 +++++++++++-------- .../src/command_impl/generalized.rs | 49 ++++++++++++++++++ tockloader-lib/src/command_impl/mod.rs | 2 +- tockloader-lib/src/connection.rs | 48 +++++++++++++++-- 4 files changed, 123 insertions(+), 27 deletions(-) create mode 100644 tockloader-lib/src/command_impl/generalized.rs diff --git a/tockloader-cli/src/main.rs b/tockloader-cli/src/main.rs index 0297bbe3..76408a55 100644 --- a/tockloader-cli/src/main.rs +++ b/tockloader-cli/src/main.rs @@ -11,10 +11,15 @@ use clap::ArgMatches; use cli::make_cli; use known_boards::KnownBoardNames; use tockloader_lib::board_settings::BoardSettings; -use tockloader_lib::connection::{Connection, ConnectionInfo, ProbeTargetInfo, SerialTargetInfo}; +use tockloader_lib::connection::{ + Connection, ProbeRSConnection, ProbeTargetInfo, SerialConnection, SerialTargetInfo, + TockloaderConnection, +}; use tockloader_lib::known_boards::KnownBoard; use tockloader_lib::tabs::tab::Tab; -use tockloader_lib::{list_debug_probes, list_serial_ports}; +use tockloader_lib::{ + list_debug_probes, list_serial_ports, CommandInfo, CommandInstall, CommandList, +}; fn get_serial_target_info(user_options: &ArgMatches) -> SerialTargetInfo { let board = get_known_board(user_options); @@ -83,7 +88,7 @@ fn get_known_board(user_options: &ArgMatches) -> Option> { }) } -fn open_connection(user_options: &ArgMatches) -> Result { +async fn open_connection(user_options: &ArgMatches) -> Result { if using_serial(user_options) { let path = if let Some(path) = user_options.get_one::("port") { path.clone() @@ -96,22 +101,27 @@ fn open_connection(user_options: &ArgMatches) -> Result { .context("No device is connected.")? }; - Connection::open(ConnectionInfo::SerialInfo( - path, - get_serial_target_info(user_options), - )) - .context("Failed to open serial connection.") + let mut conn: TockloaderConnection = + SerialConnection::new(path, get_serial_target_info(user_options)).into(); + conn.open() + .await + .context("Failed to open serial connection.")?; + + Ok(conn) } else { let ans = inquire::Select::new("Which debug probe do you want to use?", list_debug_probes()) .prompt() .context("No debug probe is connected.")?; - Connection::open(ConnectionInfo::ProbeInfo( - ans, - get_probe_target_info(user_options), - )) - .context("Failed to open probe connection.") + let mut conn: TockloaderConnection = + ProbeRSConnection::new(ans, get_probe_target_info(user_options)).into(); + + conn.open() + .await + .context("Failed to open probe connection.")?; + + Ok(conn) } } @@ -130,21 +140,20 @@ async fn main() -> Result<()> { Some(("list", sub_matches)) => { cli::validate(&mut cmd, sub_matches); - let mut conn = open_connection(sub_matches)?; + let mut conn = open_connection(sub_matches).await?; let settings = get_board_settings(sub_matches); - let app_details = tockloader_lib::list(&mut conn, &settings) - .await - .context("Failed to list apps.")?; + let app_details = conn.list(&settings).await.context("Failed to list apps.")?; display::print_list(&app_details).await; } Some(("info", sub_matches)) => { cli::validate(&mut cmd, sub_matches); - let mut conn = open_connection(sub_matches)?; + let mut conn = open_connection(sub_matches).await?; let settings = get_board_settings(sub_matches); - let mut attributes = tockloader_lib::info(&mut conn, &settings) + let mut attributes = conn + .info(&settings) .await .context("Failed to get data from the board.")?; @@ -155,10 +164,10 @@ async fn main() -> Result<()> { let tab_file = Tab::open(sub_matches.get_one::("tab").unwrap().to_string()) .context("Failed to use provided tab file.")?; - let mut conn = open_connection(sub_matches)?; + let mut conn = open_connection(sub_matches).await?; let settings = get_board_settings(sub_matches); - tockloader_lib::install_app(&mut conn, &settings, tab_file) + conn.install_app(&settings, tab_file) .await .context("Failed to install app.")?; } diff --git a/tockloader-lib/src/command_impl/generalized.rs b/tockloader-lib/src/command_impl/generalized.rs new file mode 100644 index 00000000..9ad11b70 --- /dev/null +++ b/tockloader-lib/src/command_impl/generalized.rs @@ -0,0 +1,49 @@ +use async_trait::async_trait; + +use crate::attributes::app_attributes::AppAttributes; +use crate::attributes::general_attributes::GeneralAttributes; +use crate::board_settings::BoardSettings; +use crate::connection::TockloaderConnection; +use crate::errors::TockloaderError; +use crate::tabs::tab::Tab; +use crate::{CommandInfo, CommandInstall, CommandList}; + +#[async_trait] +impl CommandList for TockloaderConnection { + async fn list( + &mut self, + settings: &BoardSettings, + ) -> Result, TockloaderError> { + match self { + TockloaderConnection::ProbeRS(conn) => conn.list(settings).await, + TockloaderConnection::Serial(conn) => conn.list(settings).await, + } + } +} + +#[async_trait] +impl CommandInfo for TockloaderConnection { + async fn info( + &mut self, + settings: &BoardSettings, + ) -> Result { + match self { + TockloaderConnection::ProbeRS(conn) => conn.info(settings).await, + TockloaderConnection::Serial(conn) => conn.info(settings).await, + } + } +} + +#[async_trait] +impl CommandInstall for TockloaderConnection { + async fn install_app( + &mut self, + settings: &BoardSettings, + tab_file: Tab, + ) -> Result<(), TockloaderError> { + match self { + TockloaderConnection::ProbeRS(conn) => conn.install_app(settings, tab_file).await, + TockloaderConnection::Serial(conn) => conn.install_app(settings, tab_file).await, + } + } +} diff --git a/tockloader-lib/src/command_impl/mod.rs b/tockloader-lib/src/command_impl/mod.rs index 755a681b..46aa06ea 100644 --- a/tockloader-lib/src/command_impl/mod.rs +++ b/tockloader-lib/src/command_impl/mod.rs @@ -1,3 +1,3 @@ - +pub mod generalized; pub mod probers; pub mod serial; diff --git a/tockloader-lib/src/connection.rs b/tockloader-lib/src/connection.rs index 8160470f..5d8dd4ce 100644 --- a/tockloader-lib/src/connection.rs +++ b/tockloader-lib/src/connection.rs @@ -8,11 +8,6 @@ use tokio_serial::{FlowControl, Parity, SerialPort, SerialStream, StopBits}; use crate::errors::TockloaderError; -// pub enum ConnectionInfo { -// SerialInfo(String, SerialTargetInfo), -// ProbeInfo(DebugProbeInfo, ProbeTargetInfo), -// } - pub struct ProbeTargetInfo { pub chip: String, pub core: usize, @@ -158,3 +153,46 @@ impl Connection for SerialConnection { self.stream.is_some() } } + +/// This is an utility enum to make your life easier when you want to abstract +/// away the underlying connection type. Use with caution, not all connection +/// types must implement every command. +pub enum TockloaderConnection { + ProbeRS(ProbeRSConnection), + Serial(SerialConnection), +} + +impl From for TockloaderConnection { + fn from(conn: ProbeRSConnection) -> Self { + TockloaderConnection::ProbeRS(conn) + } +} + +impl From for TockloaderConnection { + fn from(conn: SerialConnection) -> Self { + TockloaderConnection::Serial(conn) + } +} +#[async_trait] +impl Connection for TockloaderConnection { + async fn open(&mut self) -> Result<(), TockloaderError> { + match self { + TockloaderConnection::ProbeRS(conn) => conn.open().await, + TockloaderConnection::Serial(conn) => conn.open().await, + } + } + + async fn close(&mut self) -> Result<(), TockloaderError> { + match self { + TockloaderConnection::ProbeRS(conn) => conn.close().await, + TockloaderConnection::Serial(conn) => conn.close().await, + } + } + + fn is_open(&self) -> bool { + match self { + TockloaderConnection::ProbeRS(conn) => conn.is_open(), + TockloaderConnection::Serial(conn) => conn.is_open(), + } + } +} From 05e4a22cd0f16406a2b4806469c1f197bd970ada Mon Sep 17 00:00:00 2001 From: George Cosma Date: Mon, 30 Jun 2025 23:23:43 +0300 Subject: [PATCH 09/11] wip: implement connection not open error Signed-off-by: George Cosma --- tockloader-lib/src/command_impl/probers/info.rs | 2 +- tockloader-lib/src/command_impl/probers/install.rs | 2 +- tockloader-lib/src/command_impl/probers/list.rs | 2 +- tockloader-lib/src/command_impl/serial/info.rs | 2 +- tockloader-lib/src/command_impl/serial/list.rs | 2 +- tockloader-lib/src/errors.rs | 3 +++ 6 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tockloader-lib/src/command_impl/probers/info.rs b/tockloader-lib/src/command_impl/probers/info.rs index 8843c214..e69bb2b4 100644 --- a/tockloader-lib/src/command_impl/probers/info.rs +++ b/tockloader-lib/src/command_impl/probers/info.rs @@ -15,7 +15,7 @@ impl CommandInfo for ProbeRSConnection { settings: &BoardSettings, ) -> Result { if !self.is_open() { - todo!("Error here"); + return Err(TockloaderError::ConnectionNotOpen); } let session = self.session.as_mut().expect("Board must be open"); diff --git a/tockloader-lib/src/command_impl/probers/install.rs b/tockloader-lib/src/command_impl/probers/install.rs index 05ce3c62..de7bb756 100644 --- a/tockloader-lib/src/command_impl/probers/install.rs +++ b/tockloader-lib/src/command_impl/probers/install.rs @@ -17,7 +17,7 @@ impl CommandInstall for ProbeRSConnection { tab_file: Tab, ) -> Result<(), TockloaderError> { if !self.is_open() { - todo!("Error here"); + return Err(TockloaderError::ConnectionNotOpen); } let session = self.session.as_mut().expect("Board must be open"); diff --git a/tockloader-lib/src/command_impl/probers/list.rs b/tockloader-lib/src/command_impl/probers/list.rs index 2262182b..2295fc4d 100644 --- a/tockloader-lib/src/command_impl/probers/list.rs +++ b/tockloader-lib/src/command_impl/probers/list.rs @@ -13,7 +13,7 @@ impl CommandList for ProbeRSConnection { settings: &BoardSettings, ) -> Result, TockloaderError> { if !self.is_open() { - todo!("Error here"); + return Err(TockloaderError::ConnectionNotOpen); } let session = self.session.as_mut().expect("Board must be open"); diff --git a/tockloader-lib/src/command_impl/serial/info.rs b/tockloader-lib/src/command_impl/serial/info.rs index 2732b3b9..c5be85b0 100644 --- a/tockloader-lib/src/command_impl/serial/info.rs +++ b/tockloader-lib/src/command_impl/serial/info.rs @@ -18,7 +18,7 @@ impl CommandInfo for SerialConnection { settings: &BoardSettings, ) -> Result { if !self.is_open() { - todo!("Error here"); + return Err(TockloaderError::ConnectionNotOpen); } let stream = self.stream.as_mut().expect("Board must be open"); diff --git a/tockloader-lib/src/command_impl/serial/list.rs b/tockloader-lib/src/command_impl/serial/list.rs index a7bade2e..bcbd92c3 100644 --- a/tockloader-lib/src/command_impl/serial/list.rs +++ b/tockloader-lib/src/command_impl/serial/list.rs @@ -16,7 +16,7 @@ impl CommandList for SerialConnection { settings: &BoardSettings, ) -> Result, TockloaderError> { if !self.is_open() { - todo!("Error here"); + return Err(TockloaderError::ConnectionNotOpen); } let stream = self.stream.as_mut().expect("Board must be open"); diff --git a/tockloader-lib/src/errors.rs b/tockloader-lib/src/errors.rs index ad5b058a..ec3b7995 100644 --- a/tockloader-lib/src/errors.rs +++ b/tockloader-lib/src/errors.rs @@ -12,6 +12,9 @@ use thiserror::Error; #[derive(Debug, Error)] pub enum TockloaderError { + #[error("Failed due to the connection not being open.")] + ConnectionNotOpen, + #[error("Error occurred while trying to access core: {0}")] CoreAccessError(usize, probe_rs::Error), From ede0cc952a6a7ce6b19015366062e4a71425a303 Mon Sep 17 00:00:00 2001 From: George Cosma Date: Mon, 30 Jun 2025 23:30:10 +0300 Subject: [PATCH 10/11] wip: fix clippy warning Signed-off-by: George Cosma --- tockloader-lib/src/connection.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tockloader-lib/src/connection.rs b/tockloader-lib/src/connection.rs index 5d8dd4ce..43cc80a9 100644 --- a/tockloader-lib/src/connection.rs +++ b/tockloader-lib/src/connection.rs @@ -157,6 +157,7 @@ impl Connection for SerialConnection { /// This is an utility enum to make your life easier when you want to abstract /// away the underlying connection type. Use with caution, not all connection /// types must implement every command. +#[allow(clippy::large_enum_variant)] pub enum TockloaderConnection { ProbeRS(ProbeRSConnection), Serial(SerialConnection), From 8b45dc3370a8453b0c88d4bbefd35becdef8afcb Mon Sep 17 00:00:00 2001 From: George Cosma Date: Mon, 30 Jun 2025 23:50:50 +0300 Subject: [PATCH 11/11] fix: clippy warnings from update of clippy Signed-off-by: George Cosma --- tbf-parser/src/types.rs | 10 ++++------ .../pages/setup_page/setup_page_structure.rs | 10 +++++----- .../ui_management/pages/terminal_page/section/usage.rs | 2 +- tockloader-cli/src/display.rs | 4 ++++ tockloader-lib/src/command_impl/probers/install.rs | 2 +- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/tbf-parser/src/types.rs b/tbf-parser/src/types.rs index 12a0d978..15229307 100644 --- a/tbf-parser/src/types.rs +++ b/tbf-parser/src/types.rs @@ -94,21 +94,19 @@ impl fmt::Debug for TbfParseError { match self { TbfParseError::NotEnoughFlash => write!(f, "Buffer too short to parse TBF header"), TbfParseError::UnsupportedVersion(version) => { - write!(f, "TBF version {} unsupported", version) + write!(f, "TBF version {version} unsupported") } TbfParseError::ChecksumMismatch(app, calc) => write!( f, - "Checksum verification failed: app:{:#x}, calc:{:#x}", - app, calc + "Checksum verification failed: app:{app:#x}, calc:{calc:#x}" ), - TbfParseError::BadTlvEntry(tipe) => write!(f, "TLV entry type {} is invalid", tipe), + TbfParseError::BadTlvEntry(tipe) => write!(f, "TLV entry type {tipe} is invalid"), TbfParseError::BadProcessName => write!(f, "Process name not UTF-8"), TbfParseError::InternalError => write!(f, "Internal kernel error. This is a bug."), TbfParseError::TooManyEntries(tipe) => { write!( f, - "There are too many variable entries of {} for Tock to parse", - tipe + "There are too many variable entries of {tipe} for Tock to parse" ) } TbfParseError::PackageNameTooLong => write!(f, "The package name is too long."), diff --git a/tock-process-console/src/ui_management/pages/setup_page/setup_page_structure.rs b/tock-process-console/src/ui_management/pages/setup_page/setup_page_structure.rs index 01dfa671..8472f703 100644 --- a/tock-process-console/src/ui_management/pages/setup_page/setup_page_structure.rs +++ b/tock-process-console/src/ui_management/pages/setup_page/setup_page_structure.rs @@ -207,7 +207,7 @@ impl ComponentRender<()> for SetupPage { let available_ports = match tokio_serial::available_ports() { Ok(ports) => ports, - Err(error) => panic!("Error while searching for ports: {}", error), + Err(error) => panic!("Error while searching for ports: {error}"), }; let mut vec_serial: Vec = vec![]; @@ -261,7 +261,7 @@ impl ComponentRender<()> for SetupPage { .borders(Borders::ALL) .border_type(BorderType::Rounded) .fg(Color::Yellow) - .title(format!(" Number of boards found: {} ", boards_found)) + .title(format!(" Number of boards found: {boards_found} ")) .title_style(Style::default().fg(Color::Blue)), ) .highlight_style(Style::default().add_modifier(Modifier::BOLD)) @@ -275,12 +275,12 @@ impl ComponentRender<()> for SetupPage { if self.show_state == ShowState::ShowBoardsOnly { match self.probeinfo_sender.send(board_ports) { Ok(data) => data, - Err(error) => println!("{}", error), + Err(error) => println!("{error}"), }; } else if self.show_state == ShowState::ShowAllSerialPorts { match self.probeinfo_sender.send(serial_ports) { Ok(data) => data, - Err(error) => println!("{}", error), + Err(error) => println!("{error}"), }; } @@ -374,7 +374,7 @@ impl ComponentRender<()> for SetupPage { frame.render_widget(help_text, help_text_h); let error = if let Some(error) = &self.properties.error_message { - Text::from(format!("Error: {}", error)) + Text::from(format!("Error: {error}")) } else { Text::from("") }; diff --git a/tock-process-console/src/ui_management/pages/terminal_page/section/usage.rs b/tock-process-console/src/ui_management/pages/terminal_page/section/usage.rs index b84e95e7..8587a72c 100644 --- a/tock-process-console/src/ui_management/pages/terminal_page/section/usage.rs +++ b/tock-process-console/src/ui_management/pages/terminal_page/section/usage.rs @@ -16,7 +16,7 @@ pub struct UsageInfo { } fn key_to_span<'a>(key: &String) -> Span<'a> { - Span::from(format!("( {} )", key)).bold() + Span::from(format!("( {key} )")).bold() } pub fn widget_usage_to_text<'a>(usage: UsageInfo) -> Text<'a> { diff --git a/tockloader-cli/src/display.rs b/tockloader-cli/src/display.rs index 236c6230..8d96635d 100644 --- a/tockloader-cli/src/display.rs +++ b/tockloader-cli/src/display.rs @@ -5,6 +5,8 @@ use tockloader_lib::attributes::app_attributes::AppAttributes; use tockloader_lib::attributes::system_attributes::SystemAttributes; +// TODO(george-cosma): Fix this +#[allow(clippy::uninlined_format_args)] pub async fn print_list(app_details: &[AppAttributes]) { for (i, details) in app_details.iter().enumerate() { println!("\n\x1b[0m\x1b[1;35m ┏━━━━━━━━━━━━━━━━┓"); @@ -40,6 +42,8 @@ pub async fn print_list(app_details: &[AppAttributes]) { } } +// TODO(george-cosma): Fix this +#[allow(clippy::uninlined_format_args)] pub async fn print_info(app_details: &mut [AppAttributes], system_details: &mut SystemAttributes) { for (i, details) in app_details.iter().enumerate() { println!("\n\x1b[0m\x1b[1;35m ┏━━━━━━━━━━━━━━━━┓"); diff --git a/tockloader-lib/src/command_impl/probers/install.rs b/tockloader-lib/src/command_impl/probers/install.rs index de7bb756..503527e1 100644 --- a/tockloader-lib/src/command_impl/probers/install.rs +++ b/tockloader-lib/src/command_impl/probers/install.rs @@ -131,7 +131,7 @@ impl CommandInstall for ProbeRSConnection { } for i in valid_pages { - println!("Writing page number {}", i); + println!("Writing page number {i}"); // Create the packet that we send to the bootloader. First four // bytes are the address of the page let mut pkt = Vec::new();