From 9d8104be402dcf45afef8c7a1a39b067265c70c1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 29 Apr 2024 18:52:11 +0200 Subject: [PATCH 1/4] a lot of bugfixes --- src/config.rs | 50 ++++++-- src/events.rs | 2 +- src/handlers/camera.rs | 262 +++++++++++++++++++---------------------- src/pus/action.rs | 65 ++++++++-- src/pus/event.rs | 2 +- src/pus/hk.rs | 2 +- src/pus/mod.rs | 39 +++--- src/pus/mode.rs | 2 +- src/pus/scheduler.rs | 6 +- src/pus/test.rs | 2 +- 10 files changed, 252 insertions(+), 180 deletions(-) diff --git a/src/config.rs b/src/config.rs index 4641073..5fcf541 100644 --- a/src/config.rs +++ b/src/config.rs @@ -2,6 +2,7 @@ use lazy_static::lazy_static; use num_enum::{IntoPrimitive, TryFromPrimitive}; use once_cell::sync::OnceCell; use satrs::events::{EventU32TypedSev, SeverityInfo}; +use satrs::res_code::ResultU16; use satrs::spacepackets::PacketId; use satrs_mib::res_code::ResultU16Info; use satrs_mib::resultcode; @@ -40,11 +41,13 @@ pub enum CustomPusServiceId { #[derive(Debug)] pub enum GroupId { - Tmtc = 0, - Hk = 1, - Mode = 2, - Action = 3, - Controller = 4, + Generic = 0, + Tmtc = 1, + Hk = 2, + Mode = 3, + Action = 4, + Controller = 5, + Camera = 6, } pub const TEST_EVENT: EventU32TypedSev = @@ -187,9 +190,11 @@ pub mod cfg_file { } } +#[resultcode] +pub const GENERIC_FAILED: ResultU16 = ResultU16::new(GroupId::Generic as u8, 1); + pub mod tmtc_err { use super::*; - use satrs::res_code::ResultU16; #[resultcode] pub const INVALID_PUS_SERVICE: ResultU16 = ResultU16::new(GroupId::Tmtc as u8, 0); @@ -225,7 +230,6 @@ pub mod tmtc_err { pub mod action_err { use super::*; - use satrs::res_code::ResultU16; #[resultcode] pub const INVALID_ACTION_ID: ResultU16 = ResultU16::new(GroupId::Action as u8, 0); @@ -235,7 +239,6 @@ pub mod action_err { pub mod hk_err { use super::*; - use satrs::res_code::ResultU16; #[resultcode] pub const TARGET_ID_MISSING: ResultU16 = ResultU16::new(GroupId::Hk as u8, 0); @@ -256,7 +259,6 @@ pub mod hk_err { pub mod mode_err { use super::*; - use satrs::res_code::ResultU16; #[resultcode] pub const WRONG_MODE: ResultU16 = ResultU16::new(GroupId::Mode as u8, 0); @@ -264,7 +266,6 @@ pub mod mode_err { pub mod ctrl_err { use super::*; - use satrs::res_code::ResultU16; #[resultcode] pub const INVALID_CMD_FORMAT: ResultU16 = ResultU16::new(GroupId::Controller as u8, 0); @@ -290,6 +291,35 @@ pub mod ctrl_err { ]; } +pub mod cam_error { + use super::*; + use thiserror::Error; + + #[derive(Debug, Error)] + pub enum CameraError { + #[error("Error taking image: {0}")] + TakeImageError(String), + #[error("error listing image files: {0}")] + ListFileError(String), + #[error("IO error: {0}")] + IoError(#[from] std::io::Error), + } + + #[resultcode] + pub const TAKE_IMAGE_ERROR: ResultU16 = ResultU16::new(GroupId::Camera as u8, 0); + #[resultcode] + pub const NO_DATA: ResultU16 = ResultU16::new(GroupId::Camera as u8, 1); + #[resultcode] + pub const ACTION_REQ_VARIANT_NOT_IMPL: ResultU16 = ResultU16::new(GroupId::Camera as u8, 2); + #[resultcode] + pub const DESERIALIZE_ERROR: ResultU16 = ResultU16::new(GroupId::Camera as u8, 3); + // TODO: Probably could be in a dedicated modules for these returnvalues. + #[resultcode] + pub const LIST_FILE_ERROR: ResultU16 = ResultU16::new(GroupId::Camera as u8, 4); + #[resultcode] + pub const IO_ERROR: ResultU16 = ResultU16::new(GroupId::Camera as u8, 5); +} + pub mod pool { use satrs::pool::{StaticMemoryPool, StaticPoolConfig}; diff --git a/src/events.rs b/src/events.rs index 67a33c9..b341b33 100644 --- a/src/events.rs +++ b/src/events.rs @@ -174,7 +174,7 @@ impl EventHandler { let mut event_manager = EventManagerWithBoundedMpsc::new(event_rx); let pus_event_handler = PusEventHandler::new( tm_sender, - create_verification_reporter(PUS_EVENT_MANAGEMENT.id(), PUS_EVENT_MANAGEMENT.apid), + create_verification_reporter(PUS_EVENT_MANAGEMENT.id(), PUS_EVENT_MANAGEMENT.apid, 16), &mut event_manager, event_request_rx, ); diff --git a/src/handlers/camera.rs b/src/handlers/camera.rs index a3c3481..db21f9c 100644 --- a/src/handlers/camera.rs +++ b/src/handlers/camera.rs @@ -27,22 +27,22 @@ use crate::pus::action::send_data_reply; use crate::requests::CompositeRequest; use derive_new::new; -use log::{debug, info}; +use log::info; use num_enum::TryFromPrimitive; +use ops_sat_rs::config::cam_error::{self, CameraError}; +use ops_sat_rs::config::GENERIC_FAILED; use ops_sat_rs::TimeStampHelper; use satrs::action::{ActionRequest, ActionRequestVariant}; use satrs::hk::HkRequest; +use satrs::params::Params; use satrs::pus::action::{ActionReplyPus, ActionReplyVariant}; -use satrs::pus::EcssTmtcError; use satrs::request::{GenericMessage, MessageMetadata, UniqueApidTargetId}; +use satrs::res_code::ResultU16; use satrs::tmtc::PacketAsVec; use serde::{Deserialize, Serialize}; -use std::fmt; -use std::fmt::Formatter; use std::process::{Command, Output}; use std::sync::mpsc; -// const IMS_TESTAPP: &str = "scripts/ims100_testapp"; const IMS_TESTAPP: &str = "ims100_testapp"; const DEFAULT_SINGLE_CAM_PARAMS: CameraPictureParameters = CameraPictureParameters { @@ -112,58 +112,6 @@ pub struct CameraPictureParameters { pub W: u32, // wait time between pictures in ms, max: 40000 } -#[derive(Debug)] -#[allow(dead_code)] -pub enum CameraError { - TakeImageError, - NoDataSent, - VariantNotImplemented, - DeserializeError, - ListFileError, - IoError(std::io::Error), - EcssTmtcError(EcssTmtcError), -} - -impl From for CameraError { - fn from(value: std::io::Error) -> Self { - Self::IoError(value) - } -} - -impl From for CameraError { - fn from(value: EcssTmtcError) -> Self { - Self::EcssTmtcError(value) - } -} - -impl fmt::Display for CameraError { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - match self { - CameraError::TakeImageError => { - write!(f, "Error taking image.") - } - CameraError::NoDataSent => { - write!(f, "No data sent.") - } - CameraError::VariantNotImplemented => { - write!(f, "Request variant not implemented.") - } - CameraError::DeserializeError => { - write!(f, "Unable to deserialize parameters.") - } - CameraError::ListFileError => { - write!(f, "Error listing image files.") - } - CameraError::IoError(io_error) => { - write!(f, "{}", io_error) - } - CameraError::EcssTmtcError(ecss_tmtc_error) => { - write!(f, "{}", ecss_tmtc_error) - } - } - } -} - #[derive(Debug)] pub struct Ims100BatchHandler { id: UniqueApidTargetId, @@ -205,11 +153,7 @@ impl Ims100BatchHandler { self.handle_hk_request(&msg.requestor_info, hk_request); } CompositeRequest::Action(action_request) => { - if let Err(e) = - self.handle_action_request(&msg.requestor_info, action_request) - { - log::warn!("camera action request IO error: {e}"); - } + self.handle_action_request(&msg.requestor_info, action_request); } }, Err(e) => match e { @@ -235,53 +179,124 @@ impl Ims100BatchHandler { &mut self, requestor_info: &MessageMetadata, action_request: &ActionRequest, - ) -> Result<(), CameraError> { + ) { let param = match ActionId::try_from(action_request.action_id).expect("Invalid action id") { ActionId::DefaultSingle => DEFAULT_SINGLE_CAM_PARAMS, ActionId::BalancedSingle => BALANCED_SINGLE_CAM_PARAMS, ActionId::DefaultSingleFlatSat => DEFAULT_SINGLE_FLATSAT_CAM_PARAMS, ActionId::BalancedSingleFlatSat => BALANCED_SINGLE_FLATSAT_CAM_PARAMS, ActionId::CustomParameters => match &action_request.variant { - ActionRequestVariant::NoData => return Err(CameraError::NoDataSent), - ActionRequestVariant::StoreData(_) => { - // let param = serde_json::from_slice() - // TODO implement non dynamic version - return Err(CameraError::VariantNotImplemented); + ActionRequestVariant::NoData => { + self.send_completion_failure( + requestor_info, + action_request, + cam_error::NO_DATA, + None, + ); + return; } ActionRequestVariant::VecData(data) => { let param: serde_json::Result = serde_json::from_slice(data.as_slice()); match param { Ok(param) => param, - Err(_) => { - return Err(CameraError::DeserializeError); + Err(e) => { + self.send_completion_failure( + requestor_info, + action_request, + cam_error::DESERIALIZE_ERROR, + Some(e.to_string().into()), + ); + return; } } } - _ => return Err(CameraError::VariantNotImplemented), + _ => { + self.send_completion_failure( + requestor_info, + action_request, + cam_error::ACTION_REQ_VARIANT_NOT_IMPL, + None, + ); + return; + } }, }; - let output = self.take_picture(param)?; - info!("Sending action reply!"); - send_data_reply(self.id, output.stdout, &self.stamp_helper, &self.tm_tx)?; - self.action_reply_tx - .send(GenericMessage::new( - *requestor_info, - ActionReplyPus::new(action_request.action_id, ActionReplyVariant::Completed), - )) - .unwrap(); - Ok(()) + match self.take_picture(¶m) { + Ok(ref output) => { + self.send_completion_success(requestor_info, action_request); + if let Err(e) = + send_data_reply(self.id, &output.stdout, &self.stamp_helper, &self.tm_tx) + { + log::error!("sending data reply unexpectedly failed: {e}"); + } + } + Err(e) => match e { + CameraError::TakeImageError(ref err_str) => { + self.send_completion_failure( + requestor_info, + action_request, + cam_error::TAKE_IMAGE_ERROR, + Some(err_str.to_string().into()), + ); + } + CameraError::IoError(ref e) => { + self.send_completion_failure( + requestor_info, + action_request, + cam_error::IO_ERROR, + Some(e.to_string().into()), + ); + } + _ => { + log::warn!("unexpected error: {:?}", e); + self.send_completion_failure( + requestor_info, + action_request, + GENERIC_FAILED, + None, + ); + } + }, + } } - pub fn take_picture(&mut self, param: CameraPictureParameters) -> Result { - info!("Taking image!"); + pub fn send_completion_success(&self, requestor: &MessageMetadata, action_req: &ActionRequest) { + let result = self.action_reply_tx.send(GenericMessage::new_action_reply( + *requestor, + action_req.action_id, + ActionReplyVariant::Completed, + )); + if result.is_err() { + log::error!("sending action reply failed"); + } + } + + pub fn send_completion_failure( + &self, + requestor: &MessageMetadata, + action_req: &ActionRequest, + error_code: ResultU16, + params: Option, + ) { + let result = self.action_reply_tx.send(GenericMessage::new_action_reply( + *requestor, + action_req.action_id, + ActionReplyVariant::CompletionFailed { error_code, params }, + )); + if result.is_err() { + log::error!("sending action reply failed"); + } + } + + pub fn take_picture(&mut self, param: &CameraPictureParameters) -> Result { let mut cmd = Command::new(IMS_TESTAPP); cmd.arg("-R") - .arg(¶m.R.to_string()) + .arg(param.R.to_string()) .arg("-G") - .arg(¶m.G.to_string()) + .arg(param.G.to_string()) .arg("-B") - .arg(¶m.B.to_string()) + .arg(param.B.to_string()) .arg("-c") .arg("/dev/cam_tty") .arg("-m") @@ -289,18 +304,27 @@ impl Ims100BatchHandler { .arg("-v") .arg("0") .arg("-n") - .arg(¶m.N.to_string()); + .arg(param.N.to_string()); if param.P { cmd.arg("-p"); } cmd.arg("-e") - .arg(¶m.E.to_string()) + .arg(param.E.to_string()) .arg("-w") - .arg(¶m.W.to_string()); + .arg(param.W.to_string()); + info!("taking image with command: {cmd:?}"); let output = cmd.output()?; - debug!("Imager Output: {}", String::from_utf8_lossy(&output.stdout)); - + info!("imager cmd status: {}", &output.status); + info!("imager output: {}", String::from_utf8_lossy(&output.stdout)); + let mut error_string = String::new(); + if !output.stderr.is_empty() { + error_string = String::from_utf8_lossy(&output.stderr).to_string(); + log::warn!("imager error: {}", error_string); + } + if !output.status.success() { + return Err(CameraError::TakeImageError(error_string.to_string())); + } Ok(output) } @@ -313,49 +337,11 @@ impl Ims100BatchHandler { let files: Vec = output_str.lines().map(|s| s.to_string()).collect(); Ok(files) } else { - Err(CameraError::ListFileError) + Err(CameraError::ListFileError( + String::from_utf8_lossy(&output.stderr).to_string(), + )) } } - - #[allow(clippy::too_many_arguments)] - #[allow(dead_code)] - pub fn take_picture_from_str( - &mut self, - R: &str, - G: &str, - B: &str, - N: &str, - P: &str, - E: &str, - W: &str, - ) -> Result<(), CameraError> { - let mut cmd = Command::new("ims100_testapp"); - cmd.arg("-R") - .arg(R) - .arg("-G") - .arg(G) - .arg("-B") - .arg(B) - .arg("-c") - .arg("/dev/cam_tty") - .arg("-m") - .arg("/dev/cam_sd") - .arg("-v") - .arg("0") - .arg("-n") - .arg(N) - .arg(P) - .arg("-e") - .arg(E) - .arg("-w") - .arg(W); - - let output = cmd.output()?; - - debug!("{}", String::from_utf8_lossy(&output.stdout)); - - Ok(()) - } } #[cfg(test)] @@ -372,6 +358,7 @@ mod tests { use satrs::tmtc::PacketAsVec; use std::sync::mpsc; + #[allow(dead_code)] struct Ims1000Testbench { pub handler: Ims100BatchHandler, pub composite_req_tx: mpsc::Sender>, @@ -393,13 +380,7 @@ mod tests { time_helper, ); Ims1000Testbench { - handler: Ims100BatchHandler::new( - CAMERA_HANDLER, - composite_request_rx, - tm_tx, - action_reply_tx, - time_helper, - ), + handler: cam_handler, composite_req_tx: composite_request_tx, tm_receiver: tm_rx, action_reply_rx, @@ -412,7 +393,7 @@ mod tests { let mut testbench = Ims1000Testbench::default(); testbench .handler - .take_picture(DEFAULT_SINGLE_FLATSAT_CAM_PARAMS) + .take_picture(&DEFAULT_SINGLE_FLATSAT_CAM_PARAMS) .unwrap(); } @@ -436,7 +417,6 @@ mod tests { testbench .handler .handle_action_request(&MessageMetadata::new(1, 1), &req) - .unwrap(); } #[test] diff --git a/src/pus/action.rs b/src/pus/action.rs index 4101e4d..fda6d03 100644 --- a/src/pus/action.rs +++ b/src/pus/action.rs @@ -1,4 +1,4 @@ -use log::{debug, error, warn}; +use log::{error, warn}; use ops_sat_rs::config::components::PUS_ACTION_SERVICE; use ops_sat_rs::config::tmtc_err; use ops_sat_rs::TimeStampHelper; @@ -35,13 +35,13 @@ use super::{ pub const DATA_REPLY: u8 = 130; pub struct ActionReplyHandler { - fail_data_buf: [u8; 128], + fail_data_buf: [u8; 2048], } impl Default for ActionReplyHandler { fn default() -> Self { Self { - fail_data_buf: [0; 128], + fail_data_buf: [0; 2048], } } } @@ -74,8 +74,57 @@ impl PusReplyHandler for ActionReplyH ActionReplyVariant::CompletionFailed { error_code, params } => { let mut fail_data_len = 0; if let Some(params) = params { - fail_data_len = params.write_to_be_bytes(&mut self.fail_data_buf)?; + match params { + satrs::params::Params::Heapless(heapless_param) => { + // TODO: This should be part of the framework. + match heapless_param { + satrs::params::ParamsHeapless::Raw(raw) => { + // TODO: size check. + let _ = raw.write_to_be_bytes( + &mut self.fail_data_buf[0..raw.written_len()], + ); + } + satrs::params::ParamsHeapless::EcssEnum(ecss_enum) => { + // TODO: size check. + let _ = ecss_enum.write_to_be_bytes( + &mut self.fail_data_buf[0..ecss_enum.written_len()], + ); + } + } + } + satrs::params::Params::Store(_) => { + log::warn!("can not process store parameters") + } + satrs::params::Params::Vec(vec) => { + // Truncate the string for now. + fail_data_len = vec.len(); + if vec.len() > self.fail_data_buf.len() { + log::warn!( + "action reply vec too large, truncating to {} bytes", + self.fail_data_buf.len() + ); + } + self.fail_data_buf[0..fail_data_len] + .copy_from_slice(&vec[0..fail_data_len]); + } + satrs::params::Params::String(str) => { + fail_data_len = str.len(); + // Truncate the string for now. + if str.len() > self.fail_data_buf.len() { + fail_data_len = self.fail_data_buf.len(); + log::warn!( + "action reply string too large, truncating to {} bytes", + self.fail_data_buf.len() + ); + } + self.fail_data_buf[0..fail_data_len] + .copy_from_slice(&str.as_bytes()[0..fail_data_len]); + log::warn!("received string param with len {}", str.len()); + } + _ => todo!(), + } } + log::warn!("completion failure with fail data len: {}", fail_data_len); verification_handler.completion_failure( tm_sender, verif_token, @@ -209,7 +258,7 @@ pub fn create_action_service( PUS_ACTION_SERVICE.id(), pus_action_rx, tm_funnel_tx, - create_verification_reporter(PUS_ACTION_SERVICE.id(), PUS_ACTION_SERVICE.apid), + create_verification_reporter(PUS_ACTION_SERVICE.id(), PUS_ACTION_SERVICE.apid, 2048), EcssTcInVecConverter::default(), ), ActionRequestConverter::default(), @@ -277,7 +326,7 @@ impl TargetedPusService for ActionServiceWrapper { pub fn send_data_reply( apid_target: UniqueApidTargetId, - reply_data: Vec, + reply_data: &Vec, stamp_helper: &TimeStampHelper, tm_sender: &TmSender, ) -> Result<(), EcssTmtcError> { @@ -287,8 +336,8 @@ pub fn send_data_reply( data.extend(apid_target.apid.to_be_bytes()); data.extend(apid_target.unique_id.to_be_bytes()); data.extend(reply_data); - debug!( - "{}", + log::trace!( + "PUS action reply: {}", String::from_utf8(data.clone()[6..].to_vec()).expect("Error decoding data reply.") ); let data_reply_tm = PusTmCreator::new(sp_header, sec_header, &data, true); diff --git a/src/pus/event.rs b/src/pus/event.rs index 8700823..4bab09a 100644 --- a/src/pus/event.rs +++ b/src/pus/event.rs @@ -22,7 +22,7 @@ pub fn create_event_service( PUS_EVENT_MANAGEMENT.id(), pus_event_rx, tm_funnel_tx, - create_verification_reporter(PUS_EVENT_MANAGEMENT.id(), PUS_EVENT_MANAGEMENT.apid), + create_verification_reporter(PUS_EVENT_MANAGEMENT.id(), PUS_EVENT_MANAGEMENT.apid, 16), EcssTcInVecConverter::default(), ), event_request_tx, diff --git a/src/pus/hk.rs b/src/pus/hk.rs index 1f156a3..84e6b37 100644 --- a/src/pus/hk.rs +++ b/src/pus/hk.rs @@ -241,7 +241,7 @@ pub fn create_hk_service( PUS_HK_SERVICE.id(), pus_hk_rx, tm_funnel_tx, - create_verification_reporter(PUS_HK_SERVICE.id(), PUS_HK_SERVICE.apid), + create_verification_reporter(PUS_HK_SERVICE.id(), PUS_HK_SERVICE.apid, 16), EcssTcInVecConverter::default(), ), HkRequestConverter::default(), diff --git a/src/pus/mod.rs b/src/pus/mod.rs index b8fc9c7..5c85223 100644 --- a/src/pus/mod.rs +++ b/src/pus/mod.rs @@ -37,8 +37,12 @@ pub enum HandlingStatus { HandledOne, } -pub fn create_verification_reporter(owner_id: ComponentId, apid: Apid) -> VerificationReporter { - let verif_cfg = VerificationReporterCfg::new(apid, 1, 2, 8).unwrap(); +pub fn create_verification_reporter( + owner_id: ComponentId, + apid: Apid, + max_fail_data_len: usize, +) -> VerificationReporter { + let verif_cfg = VerificationReporterCfg::new(apid, 1, 2, max_fail_data_len).unwrap(); // Every software component which needs to generate verification telemetry, gets a cloned // verification reporter. VerificationReporter::new(owner_id, &verif_cfg) @@ -70,6 +74,7 @@ impl PusTcDistributor { verif_reporter: create_verification_reporter( PUS_ROUTING_SERVICE.id(), PUS_ROUTING_SERVICE.apid, + 16, ), pus_router, stamp_helper: TimeStampHelper::default(), @@ -406,20 +411,24 @@ where return Ok(()); } let active_request = active_req_opt.unwrap(); - let request_finished = self - .reply_handler - .handle_reply( - reply, - active_request, - &self.service_helper.common.tm_sender, - &self.service_helper.common.verif_reporter, - time_stamp, - ) - .unwrap_or(false); - if request_finished { - self.active_request_map.remove(reply.request_id()); + match self.reply_handler.handle_reply( + reply, + active_request, + &self.service_helper.common.tm_sender, + &self.service_helper.common.verif_reporter, + time_stamp, + ) { + Ok(finished) => { + if finished { + self.active_request_map.remove(reply.request_id()); + } + Ok(()) + } + Err(e) => { + self.active_request_map.remove(reply.request_id()); + Err(e) + } } - Ok(()) } pub fn check_for_request_timeouts(&mut self) { diff --git a/src/pus/mode.rs b/src/pus/mode.rs index b6fb569..2007d1f 100644 --- a/src/pus/mode.rs +++ b/src/pus/mode.rs @@ -212,7 +212,7 @@ pub fn create_mode_service( PUS_MODE_SERVICE.id(), pus_action_rx, tm_funnel_tx, - create_verification_reporter(PUS_MODE_SERVICE.id(), PUS_MODE_SERVICE.apid), + create_verification_reporter(PUS_MODE_SERVICE.id(), PUS_MODE_SERVICE.apid, 16), EcssTcInVecConverter::default(), ), ModeRequestConverter::default(), diff --git a/src/pus/scheduler.rs b/src/pus/scheduler.rs index 08cd738..5e696a9 100644 --- a/src/pus/scheduler.rs +++ b/src/pus/scheduler.rs @@ -139,7 +139,11 @@ pub fn create_scheduler_service( PUS_SCHEDULER_SERVICE.id(), pus_sched_rx, tm_funnel_tx, - create_verification_reporter(PUS_SCHEDULER_SERVICE.id(), PUS_SCHEDULER_SERVICE.apid), + create_verification_reporter( + PUS_SCHEDULER_SERVICE.id(), + PUS_SCHEDULER_SERVICE.apid, + 16, + ), EcssTcInVecConverter::default(), ), scheduler, diff --git a/src/pus/test.rs b/src/pus/test.rs index f10696c..31695c2 100644 --- a/src/pus/test.rs +++ b/src/pus/test.rs @@ -27,7 +27,7 @@ pub fn create_test_service( PUS_TEST_SERVICE.id(), pus_test_rx, tm_funnel_tx, - create_verification_reporter(PUS_TEST_SERVICE.id(), PUS_TEST_SERVICE.apid), + create_verification_reporter(PUS_TEST_SERVICE.id(), PUS_TEST_SERVICE.apid, 16), EcssTcInVecConverter::default(), )); TestCustomServiceWrapper { -- 2.43.0 From 3173b18cebe289ae86ccd7f987179485a7f7a6ae Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 29 Apr 2024 23:46:21 +0200 Subject: [PATCH 2/4] updated sat-rs --- Cargo.lock | 48 ++++++++++----------- Cargo.toml | 4 +- src/events.rs | 13 +++--- src/pus/action.rs | 107 +++++++++++++++------------------------------- src/pus/mod.rs | 17 +++----- 5 files changed, 71 insertions(+), 118 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a90bdfb..bfbcfa3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -172,8 +172,7 @@ dependencies = [ [[package]] name = "cobs" version = "0.2.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67ba02a97a2bd10f4b59b25c7973101c79642302776489e030cd13cdab09ed15" +source = "git+https://github.com/robamu/cobs.rs.git?branch=all_features#c70a7f30fd00a7cbdb7666dec12b437977385d40" [[package]] name = "colorchoice" @@ -313,9 +312,9 @@ dependencies = [ [[package]] name = "fastrand" -version = "2.0.2" +version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "658bd65b1cf4c852a3cc96f18a8ce7b5640f6b703f905c7d74532294c2a63984" +checksum = "9fc0510504f03c51ada170672ac806f1f105a88aa97a5281117e1ddc3368e51a" [[package]] name = "fern" @@ -417,9 +416,9 @@ dependencies = [ [[package]] name = "hashbrown" -version = "0.14.3" +version = "0.14.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "290f1a1d9242c78d09ce40a5e87e7554ee637af1351968159f4952f028f75604" +checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" dependencies = [ "ahash", "allocator-api2", @@ -513,9 +512,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" -version = "0.2.153" +version = "0.2.154" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c198f91728a82281a64e1f4f9eeb25d82cb32a5de251c6bd1b5154d63a8e7bd" +checksum = "ae743338b92ff9146ce83992f766a31066a91a8c84a45e0e9f21e7cf6de6d346" [[package]] name = "linux-raw-sys" @@ -655,15 +654,15 @@ dependencies = [ [[package]] name = "parking_lot_core" -version = "0.9.9" +version = "0.9.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4c42a9226546d68acdd9c0a280d17ce19bfe27a46bf68784e4066115788d008e" +checksum = "1e401f977ab385c9e4e3ab30627d6f26d00e2c73eef317493c4ec6d468726cf8" dependencies = [ "cfg-if", "libc", "redox_syscall", "smallvec 1.13.2", - "windows-targets 0.48.5", + "windows-targets 0.52.5", ] [[package]] @@ -713,11 +712,11 @@ dependencies = [ [[package]] name = "redox_syscall" -version = "0.4.1" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4722d768eff46b75989dd134e5c353f0d6296e5aaa3132e776cbdb56be7731aa" +checksum = "469052894dcb553421e483e4209ee581a45100d31b4018de03e5a7ad86374a7e" dependencies = [ - "bitflags 1.3.2", + "bitflags 2.5.0", ] [[package]] @@ -777,8 +776,7 @@ checksum = "e86697c916019a8588c99b5fac3cead74ec0b4b819707a682fd4d23fa0ce1ba1" [[package]] name = "satrs" version = "0.2.0-rc.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2adc1d9369e3f7e21dabb3181e36c914d1a3f68f4900207a2baa129c2fd5baba" +source = "git+https://egit.irs.uni-stuttgart.de/rust/sat-rs.git?branch=rework-params-a-bit#2cc7f03a05eecd7e3f9d33d73651c1ecc230aa6a" dependencies = [ "bus", "cobs", @@ -836,9 +834,9 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.198" +version = "1.0.199" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9846a40c979031340571da2545a4e5b7c4163bdae79b301d5f86d03979451fcc" +checksum = "0c9f6e76df036c77cd94996771fb40db98187f096dd0b9af39c6c6e452ba966a" dependencies = [ "serde_derive", ] @@ -856,9 +854,9 @@ dependencies = [ [[package]] name = "serde_derive" -version = "1.0.198" +version = "1.0.199" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e88edab869b01783ba905e7d0153f9fc1a6505a96e4ad3018011eedb838566d9" +checksum = "11bd257a6541e141e42ca6d24ae26f7714887b47e89aa739099104c7e4d3b7fc" dependencies = [ "proc-macro2", "quote", @@ -911,9 +909,9 @@ checksum = "3c5e1a9a646d36c3599cd173a41282daf47c44583ad367b8e6837255952e5c67" [[package]] name = "socket2" -version = "0.5.6" +version = "0.5.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05ffd9c0a93b7543e062e759284fcf5f5e3b098501104bfbdde4d404db792871" +checksum = "ce305eb0b4296696835b71df73eb912e0f1ffd2556a501fcede6e0c50349191c" dependencies = [ "libc", "windows-sys 0.52.0", @@ -1053,7 +1051,7 @@ dependencies = [ "serde", "serde_spanned", "toml_datetime", - "winnow 0.6.6", + "winnow 0.6.7", ] [[package]] @@ -1333,9 +1331,9 @@ dependencies = [ [[package]] name = "winnow" -version = "0.6.6" +version = "0.6.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0c976aaaa0e1f90dbb21e9587cdaf1d9679a1cde8875c0d6bd83ab96a208352" +checksum = "14b9415ee827af173ebb3f15f9083df5a122eb93572ec28741fb153356ea2578" dependencies = [ "memchr", ] diff --git a/Cargo.toml b/Cargo.toml index 3b8f17a..adeb082 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,8 +25,8 @@ once_cell = "1.19" [dependencies.satrs] version = "0.2.0-rc.5" -# git = "https://egit.irs.uni-stuttgart.de/rust/sat-rs.git" -# branch = "main" +git = "https://egit.irs.uni-stuttgart.de/rust/sat-rs.git" +branch = "rework-params-a-bit" features = ["test_util"] [dependencies.satrs-mib] diff --git a/src/events.rs b/src/events.rs index b341b33..44ca289 100644 --- a/src/events.rs +++ b/src/events.rs @@ -3,7 +3,6 @@ use std::sync::mpsc::{self}; use crate::pus::create_verification_reporter; use ops_sat_rs::config::components::PUS_EVENT_MANAGEMENT; use satrs::event_man::{EventMessageU32, EventRoutingError}; -use satrs::params::WritableToBeBytes; use satrs::pus::event::EventTmHookProvider; use satrs::pus::verification::VerificationReporter; use satrs::request::UniqueApidTargetId; @@ -42,6 +41,7 @@ pub struct PusEventHandler { tm_sender: mpsc::Sender, time_provider: CdsTime, timestamp: [u8; 7], + small_params_buf: [u8; 64], verif_handler: VerificationReporter, } @@ -82,6 +82,7 @@ impl PusEventHandler { pus_event_man_rx, time_provider: CdsTime::new_with_u16_days(0, 0), timestamp: [0; 7], + small_params_buf: [0; 64], verif_handler, tm_sender, } @@ -132,19 +133,17 @@ impl PusEventHandler { // Perform the generation of PUS event packets match self.pus_event_man_rx.try_recv() { Ok(event_msg) => { - update_time(&mut self.time_provider, &mut self.timestamp); - let param_vec = event_msg.params().map_or(Vec::new(), |param| { - param.to_vec().expect("failed to convert params to vec") - }); // We use the TM modification hook to set the sender APID for each event. self.pus_event_tm_creator.reporter.tm_hook.next_apid = UniqueApidTargetId::from(event_msg.sender_id()).apid; + update_time(&mut self.time_provider, &mut self.timestamp); self.pus_event_tm_creator - .generate_pus_event_tm_generic( + .generate_pus_event_tm_generic_with_generic_params( &self.tm_sender, &self.timestamp, event_msg.event(), - Some(¶m_vec), + &mut self.small_params_buf, + event_msg.params(), ) .expect("Sending TM as event failed"); } diff --git a/src/pus/action.rs b/src/pus/action.rs index fda6d03..845bcfe 100644 --- a/src/pus/action.rs +++ b/src/pus/action.rs @@ -3,12 +3,12 @@ use ops_sat_rs::config::components::PUS_ACTION_SERVICE; use ops_sat_rs::config::tmtc_err; use ops_sat_rs::TimeStampHelper; use satrs::action::{ActionRequest, ActionRequestVariant}; -use satrs::params::WritableToBeBytes; use satrs::pus::action::{ ActionReplyPus, ActionReplyVariant, ActivePusActionRequestStd, DefaultActiveActionRequestMap, }; use satrs::pus::verification::{ - FailParams, FailParamsWithStep, TcStateAccepted, TcStateStarted, VerificationReporter, + handle_completion_failure_with_generic_params, handle_step_failure_with_generic_params, + FailParamHelper, FailParams, TcStateAccepted, TcStateStarted, VerificationReporter, VerificationReportingProvider, VerificationToken, }; use satrs::pus::{ @@ -35,13 +35,13 @@ use super::{ pub const DATA_REPLY: u8 = 130; pub struct ActionReplyHandler { - fail_data_buf: [u8; 2048], + fail_data_buf: [u8; 128], } impl Default for ActionReplyHandler { fn default() -> Self { Self { - fail_data_buf: [0; 2048], + fail_data_buf: [0; 128], } } } @@ -64,7 +64,7 @@ impl PusReplyHandler for ActionReplyH active_request: &ActivePusActionRequestStd, tm_sender: &(impl EcssTmSender + ?Sized), verification_handler: &impl VerificationReportingProvider, - time_stamp: &[u8], + timestamp: &[u8], ) -> Result { let verif_token: VerificationToken = active_request .token() @@ -72,64 +72,23 @@ impl PusReplyHandler for ActionReplyH .expect("invalid token state"); let remove_entry = match &reply.message.variant { ActionReplyVariant::CompletionFailed { error_code, params } => { - let mut fail_data_len = 0; - if let Some(params) = params { - match params { - satrs::params::Params::Heapless(heapless_param) => { - // TODO: This should be part of the framework. - match heapless_param { - satrs::params::ParamsHeapless::Raw(raw) => { - // TODO: size check. - let _ = raw.write_to_be_bytes( - &mut self.fail_data_buf[0..raw.written_len()], - ); - } - satrs::params::ParamsHeapless::EcssEnum(ecss_enum) => { - // TODO: size check. - let _ = ecss_enum.write_to_be_bytes( - &mut self.fail_data_buf[0..ecss_enum.written_len()], - ); - } - } - } - satrs::params::Params::Store(_) => { - log::warn!("can not process store parameters") - } - satrs::params::Params::Vec(vec) => { - // Truncate the string for now. - fail_data_len = vec.len(); - if vec.len() > self.fail_data_buf.len() { - log::warn!( - "action reply vec too large, truncating to {} bytes", - self.fail_data_buf.len() - ); - } - self.fail_data_buf[0..fail_data_len] - .copy_from_slice(&vec[0..fail_data_len]); - } - satrs::params::Params::String(str) => { - fail_data_len = str.len(); - // Truncate the string for now. - if str.len() > self.fail_data_buf.len() { - fail_data_len = self.fail_data_buf.len(); - log::warn!( - "action reply string too large, truncating to {} bytes", - self.fail_data_buf.len() - ); - } - self.fail_data_buf[0..fail_data_len] - .copy_from_slice(&str.as_bytes()[0..fail_data_len]); - log::warn!("received string param with len {}", str.len()); - } - _ => todo!(), - } - } - log::warn!("completion failure with fail data len: {}", fail_data_len); - verification_handler.completion_failure( + let error_propagated = handle_completion_failure_with_generic_params( tm_sender, verif_token, - FailParams::new(time_stamp, error_code, &self.fail_data_buf[..fail_data_len]), + verification_handler, + FailParamHelper { + error_code, + params: params.as_ref(), + timestamp, + small_data_buf: &mut self.fail_data_buf, + }, )?; + if !error_propagated { + log::warn!( + "error params for completion failure were not propated: {:?}", + params.as_ref() + ); + } true } ActionReplyVariant::StepFailed { @@ -137,31 +96,35 @@ impl PusReplyHandler for ActionReplyH step, params, } => { - let mut fail_data_len = 0; - if let Some(params) = params { - fail_data_len = params.write_to_be_bytes(&mut self.fail_data_buf)?; - } - verification_handler.step_failure( + let error_propagated = handle_step_failure_with_generic_params( tm_sender, verif_token, - FailParamsWithStep::new( - time_stamp, - &EcssEnumU16::new(*step), + verification_handler, + FailParamHelper { error_code, - &self.fail_data_buf[..fail_data_len], - ), + params: params.as_ref(), + timestamp, + small_data_buf: &mut self.fail_data_buf, + }, + &EcssEnumU16::new(*step), )?; + if !error_propagated { + log::warn!( + "error params for completion failure were not propated: {:?}", + params.as_ref() + ); + } true } ActionReplyVariant::Completed => { - verification_handler.completion_success(tm_sender, verif_token, time_stamp)?; + verification_handler.completion_success(tm_sender, verif_token, timestamp)?; true } ActionReplyVariant::StepSuccess { step } => { verification_handler.step_success( tm_sender, &verif_token, - time_stamp, + timestamp, EcssEnumU16::new(*step), )?; false diff --git a/src/pus/mod.rs b/src/pus/mod.rs index 5c85223..e34c565 100644 --- a/src/pus/mod.rs +++ b/src/pus/mod.rs @@ -411,24 +411,17 @@ where return Ok(()); } let active_request = active_req_opt.unwrap(); - match self.reply_handler.handle_reply( + let result = self.reply_handler.handle_reply( reply, active_request, &self.service_helper.common.tm_sender, &self.service_helper.common.verif_reporter, time_stamp, - ) { - Ok(finished) => { - if finished { - self.active_request_map.remove(reply.request_id()); - } - Ok(()) - } - Err(e) => { - self.active_request_map.remove(reply.request_id()); - Err(e) - } + ); + if result.is_err() || (result.is_ok() && *result.as_ref().unwrap()) { + self.active_request_map.remove(reply.request_id()); } + result.map(|_| ()) } pub fn check_for_request_timeouts(&mut self) { -- 2.43.0 From 5b1392af4fd6f03ade268c90fc56a04e545f503e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 30 Apr 2024 15:43:13 +0200 Subject: [PATCH 3/4] update cargo.lock --- Cargo.lock | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 18e967e..63cc8de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -144,9 +144,9 @@ checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" [[package]] name = "cc" -version = "1.0.95" +version = "1.0.96" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d32a725bc159af97c3e629873bb9f88fb8cf8a4867175f76dc987815ea07c83b" +checksum = "065a29261d53ba54260972629f9ca6bffa69bac13cd1fed61420f7fa68b9f8bd" [[package]] name = "cfg-if" @@ -776,13 +776,14 @@ checksum = "e86697c916019a8588c99b5fac3cead74ec0b4b819707a682fd4d23fa0ce1ba1" [[package]] name = "satrs" version = "0.2.0-rc.5" -source = "git+https://egit.irs.uni-stuttgart.de/rust/sat-rs.git?branch=main#c7a74a844ce7894a87d6765f0ada4a8b95ecc545" +source = "git+https://egit.irs.uni-stuttgart.de/rust/sat-rs.git?branch=main#29f71c2a571e7492cf5d997d4c11c3f844de83bc" dependencies = [ "bus", "cobs", "crc", "crossbeam-channel", "delegate", + "derive-new", "downcast-rs", "dyn-clone", "hashbrown", -- 2.43.0 From 3bad42204621687462b64e1c9518428fefaa0d4f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 30 Apr 2024 15:54:34 +0200 Subject: [PATCH 4/4] this is stupid --- src/config.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/config.rs b/src/config.rs index 5fcf541..e9c08a2 100644 --- a/src/config.rs +++ b/src/config.rs @@ -225,6 +225,7 @@ pub mod tmtc_err { UNKNOWN_TARGET_ID_EXT, ROUTING_ERROR_EXT, NOT_ENOUGH_APP_DATA_EXT, + REQUEST_TIMEOUT_EXT, ]; } @@ -288,6 +289,9 @@ pub mod ctrl_err { SHELL_CMD_IO_ERROR_EXT, SHELL_CMD_EXECUTION_FAILURE_EXT, SHELL_CMD_INVALID_FORMAT_EXT, + FILESYSTEM_COPY_ERROR_EXT, + IMAGE_NOT_FOUND_FOR_COPY_EXT, + INVALID_LOGFILE_PATH_EXT, ]; } @@ -318,6 +322,15 @@ pub mod cam_error { pub const LIST_FILE_ERROR: ResultU16 = ResultU16::new(GroupId::Camera as u8, 4); #[resultcode] pub const IO_ERROR: ResultU16 = ResultU16::new(GroupId::Camera as u8, 5); + + pub const CAM_ERR_RESULTS: &[ResultU16Info] = &[ + TAKE_IMAGE_ERROR_EXT, + NO_DATA_EXT, + ACTION_REQ_VARIANT_NOT_IMPL_EXT, + DESERIALIZE_ERROR_EXT, + LIST_FILE_ERROR_EXT, + IO_ERROR_EXT, + ]; } pub mod pool { -- 2.43.0