From 37945e3a29f992445e659678f62ae34bd2c79244 Mon Sep 17 00:00:00 2001 From: occheung Date: Thu, 22 Aug 2024 13:09:04 +0800 Subject: [PATCH] drtio_mgmt: remove satellite errors in mgmt --- src/satman/src/main.rs | 78 +++++++++++++++++------------------------- src/satman/src/mgmt.rs | 76 +++++++++++++--------------------------- 2 files changed, 55 insertions(+), 99 deletions(-) diff --git a/src/satman/src/main.rs b/src/satman/src/main.rs index 9f6b3db..278e597 100644 --- a/src/satman/src/main.rs +++ b/src/satman/src/main.rs @@ -1075,16 +1075,12 @@ fn process_aux_packet( timer ); - match mgmt::byte_to_level_filter(log_level) { - Ok(level_filter) => { - info!("Changing log level to {}", log_level); - log::set_max_level(level_filter); - drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: true }) - } - Err(_) => { - error!("Unknown log level: {}", log_level); - drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: false }) - } + if let Ok(level_filter) = mgmt::byte_to_level_filter(log_level) { + info!("Changing log level to {}", log_level); + log::set_max_level(level_filter); + drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: true }) + } else { + drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: false }) } } drtioaux::Packet::CoreMgmtSetUartLogLevelRequest { @@ -1102,21 +1098,17 @@ fn process_aux_packet( timer ); - match mgmt::byte_to_level_filter(log_level) { - Ok(level_filter) => { - info!("Changing UART log level to {}", log_level); - unsafe { - logger::BufferLogger::get_logger() - .as_ref() - .unwrap() - .set_uart_log_level(level_filter); - } - drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: true }) - } - Err(_) => { - error!("Unknown log level: {}", log_level); - drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: false }) + if let Ok(level_filter) = mgmt::byte_to_level_filter(log_level) { + info!("Changing log level to {}", log_level); + unsafe { + logger::BufferLogger::get_logger() + .as_ref() + .unwrap() + .set_uart_log_level(level_filter); } + drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: true }) + } else { + drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: false }) } } drtioaux::Packet::CoreMgmtConfigReadRequest { @@ -1151,18 +1143,7 @@ fn process_aux_packet( ) } else { let key = core::str::from_utf8(key_slice).unwrap(); - if !core_manager.fetch_config_value(key) { - warn!("read error: no such key"); - drtioaux::send( - 0, - &drtioaux::Packet::CoreMgmtConfigReadReply { - succeeded: false, - length: 0, - last: true, - value: value_slice, - }, - ) - } else { + if core_manager.fetch_config_value(key).is_ok() { let meta = core_manager.get_config_value_slice(&mut value_slice); drtioaux::send( 0, @@ -1173,6 +1154,16 @@ fn process_aux_packet( value: value_slice, }, ) + } else { + drtioaux::send( + 0, + &drtioaux::Packet::CoreMgmtConfigReadReply { + succeeded: false, + length: 0, + last: true, + value: value_slice, + }, + ) } } } @@ -1223,9 +1214,8 @@ fn process_aux_packet( let mut succeeded = true; if last { - if !core_manager.write_config() { - succeeded = false; - } + succeeded = core_manager.write_config().is_ok(); + core_manager.clear_data(); } drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: succeeded })?; @@ -1254,14 +1244,8 @@ fn process_aux_packet( drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: false }) } else { let key = core::str::from_utf8(key_slice).unwrap(); - debug!("erase key: {}", key); - if core_manager.remove_config(key) { - debug!("erase success"); - drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: true }) - } else { - warn!("erase failed"); - drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: false }) - } + let succeeded = core_manager.remove_config(key).is_ok(); + drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded }) } } drtioaux::Packet::CoreMgmtConfigEraseRequest { diff --git a/src/satman/src/mgmt.rs b/src/satman/src/mgmt.rs index b20c8c8..340784a 100644 --- a/src/satman/src/mgmt.rs +++ b/src/satman/src/mgmt.rs @@ -3,32 +3,12 @@ use alloc::vec::Vec; use io::{Cursor, ProtoRead, ProtoWrite}; use libboard_artiq::{drtioaux_proto::CORE_MGMT_PAYLOAD_MAX_SIZE, logger::{BufferLogger, LogBufferRef}}; -use libboard_zynq::smoltcp; use libconfig::Config; use log::{self, debug, error, info, warn, LevelFilter}; use crate::routing::{SliceMeta, Sliceable}; -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum Error { - NetworkError(smoltcp::Error), - UnknownLogLevel(u8), - UnexpectedPattern, - UnrecognizedPacket, -} - -type Result = core::result::Result; - -impl core::fmt::Display for Error { - fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { - match self { - &Error::NetworkError(error) => write!(f, "network error: {}", error), - &Error::UnknownLogLevel(lvl) => write!(f, "unknown log level {}", lvl), - &Error::UnexpectedPattern => write!(f, "unexpected pattern"), - &Error::UnrecognizedPacket => write!(f, "unrecognized packet"), - } - } -} +type Result = core::result::Result; pub fn byte_to_level_filter(level_byte: u8) -> Result { Ok(match level_byte { @@ -38,7 +18,10 @@ pub fn byte_to_level_filter(level_byte: u8) -> Result { 3 => log::LevelFilter::Info, 4 => log::LevelFilter::Debug, 5 => log::LevelFilter::Trace, - lv => return Err(Error::UnknownLogLevel(lv)), + lv => { + error!("unknown log level: {}", lv); + return Err(()); + }, }) } @@ -86,16 +69,15 @@ impl<'a> Manager<'_> { self.last_log.get_slice_core_mgmt(data_slice) } - pub fn fetch_config_value(&mut self, key: &str) -> bool { - let value = self.cfg.read(&key); - if let Ok(value) = value { - debug!("got value"); - self.last_value = Sliceable::new(0, value); - true - } else { - warn!("read error: no such key"); - false - } + pub fn fetch_config_value(&mut self, key: &str) -> Result<()> { + self.cfg.read(&key) + .map(|value| { + debug!("got value"); + self.last_value = Sliceable::new(0, value) + }) + .map_err(|_| { + warn!("read error: no such key") + }) } pub fn get_config_value_slice(&mut self, data_slice: &mut [u8; CORE_MGMT_PAYLOAD_MAX_SIZE]) -> SliceMeta { @@ -111,30 +93,20 @@ impl<'a> Manager<'_> { self.current_payload.set_position(0); } - pub fn write_config(&mut self) -> bool { - let key = self.current_payload.read_string().unwrap(); + pub fn write_config(&mut self) -> Result<()> { + let key = self.current_payload.read_string().map_err( + |_err| error!("error on reading key"))?; let value = self.current_payload.read_bytes().unwrap(); - let status = self.cfg.write(&key, value); - if status.is_ok() { - debug!("write success"); - } else { - // this is an error because we do not expect write to fail - error!("failed to write: {:?}", status); - } - - self.clear_data(); - status.is_ok() + self.cfg.write(&key, value) + .map(|()| debug!("write success")) + .map_err(|err| error!("failed to write: {:?}", err)) } - pub fn remove_config(&mut self, key: &str) -> bool { + pub fn remove_config(&mut self, key: &str) -> Result<()> { debug!("erase key: {}", key); - let status = self.cfg.remove(&key); - if status.is_ok() { - debug!("erase success"); - } else { - warn!("erase failed"); - } - status.is_ok() + self.cfg.remove(&key) + .map(|()| debug!("erase success")) + .map_err(|err| warn!("failed to erase: {:?}", err)) } }