drtio_mgmt: remove satellite errors in mgmt

src/satman/src/main.rs

@@ -1075,18 +1075,14 @@ fn process_aux_packet(
                 timer
             );
 
-            match mgmt::byte_to_level_filter(log_level) {
-                Ok(level_filter) => {
+            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 })
-                }
-                Err(_) => {
-                    error!("Unknown log level: {}", log_level);
+            } else {
                 drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: false })
             }
         }
-        }
         drtioaux::Packet::CoreMgmtSetUartLogLevelRequest {
             destination: _destination,
             log_level,
@@ -1102,9 +1098,8 @@ fn process_aux_packet(
                 timer
             );
 
-            match mgmt::byte_to_level_filter(log_level) {
-                Ok(level_filter) => {
-                    info!("Changing UART log level to {}", log_level);
+            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()
@@ -1112,13 +1107,10 @@ fn process_aux_packet(
                         .set_uart_log_level(level_filter);
                 }
                 drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: true })
-                }
-                Err(_) => {
-                    error!("Unknown log level: {}", log_level);
+            } else {
                 drtioaux::send(0, &drtioaux::Packet::CoreMgmtAck { succeeded: false })
             }
         }
-        }
         drtioaux::Packet::CoreMgmtConfigReadRequest {
             destination: _destination,
             length,
@@ -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 {

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<T> = core::result::Result<T, Error>;
-
-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<T> = core::result::Result<T, ()>;
 
 pub fn byte_to_level_filter(level_byte: u8) -> Result<log::LevelFilter> {
     Ok(match level_byte {
@@ -38,7 +18,10 @@ pub fn byte_to_level_filter(level_byte: u8) -> Result<log::LevelFilter> {
         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 {
+    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);
-            true
-        } else {
-            warn!("read error: no such key");
-            false
-        }
+                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.cfg.write(&key, value)
+            .map(|()| debug!("write success"))
+            .map_err(|err| error!("failed to write: {:?}", err))
     }
 
-        self.clear_data();
-        status.is_ok()
-    }
-
-    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))
     }
 }