From 9d2f26d83957c6f9745bab5aceb2bcd0c6a7835c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 3 Apr 2024 18:21:37 +0200 Subject: [PATCH] unify CCSDS API as well --- CHANGELOG.md | 8 +++ src/ecss/tc.rs | 14 ++--- src/ecss/tm.rs | 10 ++-- src/lib.rs | 142 ++++++++++++++++++++++++++++--------------------- 4 files changed, 103 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b87bd34..a0c0eae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/). # [unreleased] +## Changed + +- Renamed `PacketId` and `PacketSequenceCtrl` `new` method to `new_checked` and former + `new_const` method to `new`. +- Renamed `tc`, `tm`, `tc_unseg` and `tm_unseg` variants for `PacketId` and `SpHeader` + to `new_for_tc_checked`, `new_for_tm_checked`, `new_for_unseg_tc_checked` and + `new_for_unseg_tm_checked`. + # [v0.11.0-rc.1] 2024-04-03 Major API changes for the time API. If you are using the time API, it is strongly recommended diff --git a/src/ecss/tc.rs b/src/ecss/tc.rs index a945837..da42b53 100644 --- a/src/ecss/tc.rs +++ b/src/ecss/tc.rs @@ -568,18 +568,18 @@ mod tests { use postcard::{from_bytes, to_allocvec}; fn base_ping_tc_full_ctor() -> PusTcCreator<'static> { - let mut sph = SpHeader::tc_unseg(0x02, 0x34, 0).unwrap(); + let mut sph = SpHeader::new_for_unseg_tc_checked(0x02, 0x34, 0).unwrap(); let tc_header = PusTcSecondaryHeader::new_simple(17, 1); PusTcCreator::new_no_app_data(&mut sph, tc_header, true) } fn base_ping_tc_simple_ctor() -> PusTcCreator<'static> { - let mut sph = SpHeader::tc_unseg(0x02, 0x34, 0).unwrap(); + let mut sph = SpHeader::new_for_unseg_tc_checked(0x02, 0x34, 0).unwrap(); PusTcCreator::new_simple(&mut sph, 17, 1, &[], true) } fn base_ping_tc_simple_ctor_with_app_data(app_data: &'static [u8]) -> PusTcCreator<'static> { - let mut sph = SpHeader::tc_unseg(0x02, 0x34, 0).unwrap(); + let mut sph = SpHeader::new_for_unseg_tc_checked(0x02, 0x34, 0).unwrap(); PusTcCreator::new_simple(&mut sph, 17, 1, app_data, true) } @@ -636,7 +636,7 @@ mod tests { #[test] fn test_update_func() { - let mut sph = SpHeader::tc_unseg(0x02, 0x34, 0).unwrap(); + let mut sph = SpHeader::new_for_unseg_tc_checked(0x02, 0x34, 0).unwrap(); let mut tc = PusTcCreator::new_simple(&mut sph, 17, 1, &[], false); assert_eq!(tc.data_len(), 0); tc.update_ccsds_data_len(); @@ -809,7 +809,8 @@ mod tests { if !has_user_data { assert!(tc.user_data().is_empty()); } - let mut comp_header = SpHeader::tc_unseg(0x02, 0x34, exp_full_len as u16 - 7).unwrap(); + let mut comp_header = + SpHeader::new_for_unseg_tc_checked(0x02, 0x34, exp_full_len as u16 - 7).unwrap(); comp_header.set_sec_header_flag(); assert_eq!(*tc.sp_header(), comp_header); } @@ -820,7 +821,8 @@ mod tests { assert!(tc.user_data().is_empty()); } assert_eq!(tc.len_packed(), exp_full_len); - let mut comp_header = SpHeader::tc_unseg(0x02, 0x34, exp_full_len as u16 - 7).unwrap(); + let mut comp_header = + SpHeader::new_for_unseg_tc_checked(0x02, 0x34, exp_full_len as u16 - 7).unwrap(); comp_header.set_sec_header_flag(); assert_eq!(*tc.sp_header(), comp_header); } diff --git a/src/ecss/tm.rs b/src/ecss/tm.rs index a244387..352414c 100644 --- a/src/ecss/tm.rs +++ b/src/ecss/tm.rs @@ -788,18 +788,18 @@ mod tests { const DUMMY_DATA: &[u8] = &[0, 1, 2]; fn base_ping_reply_full_ctor(timestamp: &[u8]) -> PusTmCreator { - let mut sph = SpHeader::tm_unseg(0x123, 0x234, 0).unwrap(); + let mut sph = SpHeader::new_for_unseg_tm_checked(0x123, 0x234, 0).unwrap(); let tm_header = PusTmSecondaryHeader::new_simple(17, 2, timestamp); PusTmCreator::new_no_source_data(&mut sph, tm_header, true) } fn ping_reply_with_data(timestamp: &[u8]) -> PusTmCreator { - let mut sph = SpHeader::tm_unseg(0x123, 0x234, 0).unwrap(); + let mut sph = SpHeader::new_for_unseg_tm_checked(0x123, 0x234, 0).unwrap(); let tm_header = PusTmSecondaryHeader::new_simple(17, 2, timestamp); PusTmCreator::new(&mut sph, tm_header, DUMMY_DATA, true) } fn base_hk_reply<'a, 'b>(timestamp: &'a [u8], src_data: &'b [u8]) -> PusTmCreator<'a, 'b> { - let mut sph = SpHeader::tm_unseg(0x123, 0x234, 0).unwrap(); + let mut sph = SpHeader::new_for_unseg_tm_checked(0x123, 0x234, 0).unwrap(); let tc_header = PusTmSecondaryHeader::new_simple(3, 5, timestamp); PusTmCreator::new(&mut sph, tc_header, src_data, true) } @@ -816,7 +816,7 @@ mod tests { } #[test] fn test_basic_simple_api() { - let mut sph = SpHeader::tm_unseg(0x123, 0x234, 0).unwrap(); + let mut sph = SpHeader::new_for_unseg_tm_checked(0x123, 0x234, 0).unwrap(); let time_provider = CdsTime::new_with_u16_days(0, 0); let mut stamp_buf: [u8; 8] = [0; 8]; let pus_tm = @@ -917,7 +917,7 @@ mod tests { #[test] fn test_manual_field_update() { - let mut sph = SpHeader::tm_unseg(0x123, 0x234, 0).unwrap(); + let mut sph = SpHeader::new_for_unseg_tm_checked(0x123, 0x234, 0).unwrap(); let tc_header = PusTmSecondaryHeader::new_simple(17, 2, dummy_timestamp()); let mut tm = PusTmCreator::new_no_source_data(&mut sph, tc_header, false); tm.calc_crc_on_serialization = false; diff --git a/src/lib.rs b/src/lib.rs index 29d9d7d..9549113 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -242,23 +242,29 @@ impl Default for PacketId { } impl PacketId { - pub const fn const_tc(sec_header: bool, apid: u16) -> Self { - Self::const_new(PacketType::Tc, sec_header, apid) - } - - pub const fn const_tm(sec_header: bool, apid: u16) -> Self { - Self::const_new(PacketType::Tm, sec_header, apid) - } - - pub fn tc(sec_header: bool, apid: u16) -> Option { + /// This constructor will panic if the passed APID exceeds [MAX_APID]. + /// Use the checked constructor variants to avoid panics. + pub const fn new_for_tc(sec_header: bool, apid: u16) -> Self { Self::new(PacketType::Tc, sec_header, apid) } - pub fn tm(sec_header: bool, apid: u16) -> Option { + /// This constructor will panic if the passed APID exceeds [MAX_APID]. + /// Use the checked constructor variants to avoid panics. + pub const fn new_for_tm(sec_header: bool, apid: u16) -> Self { Self::new(PacketType::Tm, sec_header, apid) } - pub const fn const_new(ptype: PacketType, sec_header: bool, apid: u16) -> Self { + pub fn new_for_tc_checked(sec_header: bool, apid: u16) -> Option { + Self::new_checked(PacketType::Tc, sec_header, apid) + } + + pub fn new_for_tm_checked(sec_header: bool, apid: u16) -> Option { + Self::new_checked(PacketType::Tm, sec_header, apid) + } + + /// This constructor will panic if the passed APID exceeds [MAX_APID]. + /// Use the checked variants to avoid panics. + pub const fn new(ptype: PacketType, sec_header: bool, apid: u16) -> Self { if apid > MAX_APID { panic!("APID too large"); } @@ -269,11 +275,11 @@ impl PacketId { } } - pub fn new(ptype: PacketType, sec_header_flag: bool, apid: u16) -> Option { + pub fn new_checked(ptype: PacketType, sec_header_flag: bool, apid: u16) -> Option { if apid > MAX_APID { return None; } - Some(PacketId::const_new(ptype, sec_header_flag, apid)) + Some(PacketId::new(ptype, sec_header_flag, apid)) } /// Set a new Application Process ID (APID). If the passed number is invalid, the APID will @@ -318,9 +324,9 @@ pub struct PacketSequenceCtrl { } impl PacketSequenceCtrl { - /// const variant of [PacketSequenceCtrl::new], but panics if the sequence count exceeds - /// [MAX_SEQ_COUNT]. - pub const fn const_new(seq_flags: SequenceFlags, seq_count: u16) -> PacketSequenceCtrl { + /// This constructor panics if the sequence count exceeds [MAX_SEQ_COUNT]. + /// Use [Self::new_checked] to avoid panics. + pub const fn new(seq_flags: SequenceFlags, seq_count: u16) -> PacketSequenceCtrl { if seq_count > MAX_SEQ_COUNT { panic!("Sequence count too large"); } @@ -331,11 +337,11 @@ impl PacketSequenceCtrl { } /// Returns [None] if the passed sequence count exceeds [MAX_SEQ_COUNT]. - pub fn new(seq_flags: SequenceFlags, seq_count: u16) -> Option { + pub fn new_checked(seq_flags: SequenceFlags, seq_count: u16) -> Option { if seq_count > MAX_SEQ_COUNT { return None; } - Some(PacketSequenceCtrl::const_new(seq_flags, seq_count)) + Some(PacketSequenceCtrl::new(seq_flags, seq_count)) } pub fn raw(&self) -> u16 { ((self.seq_flags as u16) << 14) | self.seq_count @@ -506,9 +512,11 @@ impl SpHeader { } } - /// const variant of the [SpHeader::new_fron_single_fields] function. Panics if the passed - /// APID exceeds [MAX_APID] or the passed packet sequence count exceeds [MAX_SEQ_COUNT]. - const fn const_new_from_single_fields( + /// This constructor panics if the passed APID exceeds [MAX_APID] or the passed packet sequence + /// count exceeds [MAX_SEQ_COUNT]. + /// + /// The checked constructor variants can be used to avoid panics. + const fn new_from_fields( ptype: PacketType, sec_header: bool, apid: u16, @@ -523,17 +531,19 @@ impl SpHeader { panic!("APID is too large"); } Self { - psc: PacketSequenceCtrl::const_new(seq_flags, seq_count), - packet_id: PacketId::const_new(ptype, sec_header, apid), + psc: PacketSequenceCtrl::new(seq_flags, seq_count), + packet_id: PacketId::new(ptype, sec_header, apid), data_len, version: 0, } } /// Create a new Space Packet Header instance which can be used to create generic - /// Space Packets. This will return [None] if the APID or sequence count argument + /// Space Packets. + /// + /// This will return [None] if the APID or sequence count argument /// exceed [MAX_APID] or [MAX_SEQ_COUNT] respectively. The version field is set to 0b000. - pub fn new_from_single_fields( + pub fn new_from_fields_checked( ptype: PacketType, sec_header: bool, apid: u16, @@ -544,31 +554,41 @@ impl SpHeader { if seq_count > MAX_SEQ_COUNT || apid > MAX_APID { return None; } - Some(SpHeader::const_new_from_single_fields( + Some(SpHeader::new_from_fields( ptype, sec_header, apid, seq_flags, seq_count, data_len, )) } /// Helper function for telemetry space packet headers. The packet type field will be /// set accordingly. The secondary header flag field is set to false. - pub fn tm(apid: u16, seq_flags: SequenceFlags, seq_count: u16, data_len: u16) -> Option { - Self::new_from_single_fields(PacketType::Tm, false, apid, seq_flags, seq_count, data_len) + pub fn new_for_tm( + apid: u16, + seq_flags: SequenceFlags, + seq_count: u16, + data_len: u16, + ) -> Option { + Self::new_from_fields_checked(PacketType::Tm, false, apid, seq_flags, seq_count, data_len) } /// Helper function for telemetry space packet headers. The packet type field will be /// set accordingly. The secondary header flag field is set to false. - pub fn tc(apid: u16, seq_flags: SequenceFlags, seq_count: u16, data_len: u16) -> Option { - Self::new_from_single_fields(PacketType::Tc, false, apid, seq_flags, seq_count, data_len) + pub fn new_for_tc_checked( + apid: u16, + seq_flags: SequenceFlags, + seq_count: u16, + data_len: u16, + ) -> Option { + Self::new_from_fields_checked(PacketType::Tc, false, apid, seq_flags, seq_count, data_len) } - /// Variant of [SpHeader::tm] which sets the sequence flag field to [SequenceFlags::Unsegmented] - pub fn tm_unseg(apid: u16, seq_count: u16, data_len: u16) -> Option { - Self::tm(apid, SequenceFlags::Unsegmented, seq_count, data_len) + /// Variant of [SpHeader::new_for_tm] which sets the sequence flag field to [SequenceFlags::Unsegmented] + pub fn new_for_unseg_tm_checked(apid: u16, seq_count: u16, data_len: u16) -> Option { + Self::new_for_tm(apid, SequenceFlags::Unsegmented, seq_count, data_len) } - /// Variant of [SpHeader::tc] which sets the sequence flag field to [SequenceFlags::Unsegmented] - pub fn tc_unseg(apid: u16, seq_count: u16, data_len: u16) -> Option { - Self::tc(apid, SequenceFlags::Unsegmented, seq_count, data_len) + /// Variant of [SpHeader::new_for_tc] which sets the sequence flag field to [SequenceFlags::Unsegmented] + pub fn new_for_unseg_tc_checked(apid: u16, seq_count: u16, data_len: u16) -> Option { + Self::new_for_tc_checked(apid, SequenceFlags::Unsegmented, seq_count, data_len) } //noinspection RsTraitImplementation @@ -778,12 +798,12 @@ pub(crate) mod tests { use serde::{de::DeserializeOwned, Serialize}; const CONST_SP: SpHeader = SpHeader::new( - PacketId::const_tc(true, 0x36), - PacketSequenceCtrl::const_new(SequenceFlags::ContinuationSegment, 0x88), + PacketId::new_for_tc(true, 0x36), + PacketSequenceCtrl::new(SequenceFlags::ContinuationSegment, 0x88), 0x90, ); - const PACKET_ID_TM: PacketId = PacketId::const_tm(true, 0x22); + const PACKET_ID_TM: PacketId = PacketId::new_for_tm(true, 0x22); #[cfg(feature = "serde")] pub(crate) fn generic_serde_test( @@ -795,11 +815,12 @@ pub(crate) mod tests { } #[test] + #[allow(clippy::assertions_on_constants)] fn verify_const_packet_id() { assert_eq!(PACKET_ID_TM.apid(), 0x22); assert!(PACKET_ID_TM.sec_header_flag); assert_eq!(PACKET_ID_TM.ptype, PacketType::Tm); - let const_tc_id = PacketId::const_tc(true, 0x23); + let const_tc_id = PacketId::new_for_tc(true, 0x23); assert_eq!(const_tc_id.ptype, PacketType::Tc); } @@ -813,27 +834,27 @@ pub(crate) mod tests { #[test] fn test_packet_id_ctors() { - let packet_id = PacketId::new(PacketType::Tc, true, 0x1ff); + let packet_id = PacketId::new_checked(PacketType::Tc, true, 0x1ff); assert!(packet_id.is_some()); let packet_id = packet_id.unwrap(); assert_eq!(packet_id.apid(), 0x1ff); assert_eq!(packet_id.ptype, PacketType::Tc); assert!(packet_id.sec_header_flag); - let packet_id_tc = PacketId::tc(true, 0x1ff); + let packet_id_tc = PacketId::new_for_tc_checked(true, 0x1ff); assert!(packet_id_tc.is_some()); let packet_id_tc = packet_id_tc.unwrap(); assert_eq!(packet_id_tc, packet_id); - let packet_id_tm = PacketId::tm(true, 0x2ff); + let packet_id_tm = PacketId::new_for_tm_checked(true, 0x2ff); assert!(packet_id_tm.is_some()); let packet_id_tm = packet_id_tm.unwrap(); - assert_eq!(packet_id_tm.sec_header_flag, true); + assert!(packet_id_tm.sec_header_flag); assert_eq!(packet_id_tm.ptype, PacketType::Tm); assert_eq!(packet_id_tm.apid, 0x2ff); } #[test] fn verify_const_sp_header() { - assert_eq!(CONST_SP.sec_header_flag(), true); + assert!(CONST_SP.sec_header_flag()); assert_eq!(CONST_SP.apid(), 0x36); assert_eq!( CONST_SP.sequence_flags(), @@ -874,7 +895,7 @@ pub(crate) mod tests { #[test] fn test_packet_id() { let packet_id = - PacketId::new(PacketType::Tm, false, 0x42).expect("Packet ID creation failed"); + PacketId::new_checked(PacketType::Tm, false, 0x42).expect("Packet ID creation failed"); assert_eq!(packet_id.raw(), 0x0042); let packet_id_from_raw = PacketId::from(packet_id.raw()); assert_eq!( @@ -882,26 +903,26 @@ pub(crate) mod tests { PacketType::Tm ); assert_eq!(packet_id_from_raw, packet_id); - let packet_id_from_new = PacketId::new(PacketType::Tm, false, 0x42).unwrap(); + let packet_id_from_new = PacketId::new_checked(PacketType::Tm, false, 0x42).unwrap(); assert_eq!(packet_id_from_new, packet_id); } #[test] fn test_invalid_packet_id() { - let packet_id_invalid = PacketId::new(PacketType::Tc, true, 0xFFFF); + let packet_id_invalid = PacketId::new_checked(PacketType::Tc, true, 0xFFFF); assert!(packet_id_invalid.is_none()); } #[test] fn test_invalid_apid_setter() { let mut packet_id = - PacketId::new(PacketType::Tm, false, 0x42).expect("Packet ID creation failed"); + PacketId::new_checked(PacketType::Tm, false, 0x42).expect("Packet ID creation failed"); assert!(!packet_id.set_apid(0xffff)); } #[test] fn test_invalid_seq_count() { - let mut psc = PacketSequenceCtrl::new(SequenceFlags::ContinuationSegment, 77) + let mut psc = PacketSequenceCtrl::new_checked(SequenceFlags::ContinuationSegment, 77) .expect("PSC creation failed"); assert_eq!(psc.seq_count(), 77); assert!(!psc.set_seq_count(0xffff)); @@ -909,7 +930,7 @@ pub(crate) mod tests { #[test] fn test_packet_seq_ctrl() { - let mut psc = PacketSequenceCtrl::new(SequenceFlags::ContinuationSegment, 77) + let mut psc = PacketSequenceCtrl::new_checked(SequenceFlags::ContinuationSegment, 77) .expect("PSC creation failed"); assert_eq!(psc.raw(), 77); let psc_from_raw = PacketSequenceCtrl::from(psc.raw()); @@ -918,9 +939,10 @@ pub(crate) mod tests { assert!(!psc.set_seq_count(2u16.pow(15))); assert_eq!(psc.raw(), 77); - let psc_invalid = PacketSequenceCtrl::new(SequenceFlags::FirstSegment, 0xFFFF); + let psc_invalid = PacketSequenceCtrl::new_checked(SequenceFlags::FirstSegment, 0xFFFF); assert!(psc_invalid.is_none()); - let psc_from_new = PacketSequenceCtrl::new(SequenceFlags::ContinuationSegment, 77).unwrap(); + let psc_from_new = + PacketSequenceCtrl::new_checked(SequenceFlags::ContinuationSegment, 77).unwrap(); assert_eq!(psc_from_new, psc); } @@ -981,7 +1003,7 @@ pub(crate) mod tests { #[test] fn test_setters() { - let sp_header = SpHeader::tc(0x42, SequenceFlags::Unsegmented, 25, 0); + let sp_header = SpHeader::new_for_tc_checked(0x42, SequenceFlags::Unsegmented, 25, 0); assert!(sp_header.is_some()); let mut sp_header = sp_header.unwrap(); sp_header.set_apid(0x12); @@ -999,7 +1021,7 @@ pub(crate) mod tests { #[test] fn test_tc_ctor() { - let sp_header = SpHeader::tc(0x42, SequenceFlags::Unsegmented, 25, 0); + let sp_header = SpHeader::new_for_tc_checked(0x42, SequenceFlags::Unsegmented, 25, 0); assert!(sp_header.is_some()); let sp_header = sp_header.unwrap(); verify_sp_fields(PacketType::Tc, &sp_header); @@ -1007,7 +1029,7 @@ pub(crate) mod tests { #[test] fn test_tc_ctor_unseg() { - let sp_header = SpHeader::tc_unseg(0x42, 25, 0); + let sp_header = SpHeader::new_for_unseg_tc_checked(0x42, 25, 0); assert!(sp_header.is_some()); let sp_header = sp_header.unwrap(); verify_sp_fields(PacketType::Tc, &sp_header); @@ -1015,7 +1037,7 @@ pub(crate) mod tests { #[test] fn test_tm_ctor() { - let sp_header = SpHeader::tm(0x42, SequenceFlags::Unsegmented, 25, 0); + let sp_header = SpHeader::new_for_tm(0x42, SequenceFlags::Unsegmented, 25, 0); assert!(sp_header.is_some()); let sp_header = sp_header.unwrap(); verify_sp_fields(PacketType::Tm, &sp_header); @@ -1023,7 +1045,7 @@ pub(crate) mod tests { #[test] fn test_tm_ctor_unseg() { - let sp_header = SpHeader::tm_unseg(0x42, 25, 0); + let sp_header = SpHeader::new_for_unseg_tm_checked(0x42, 25, 0); assert!(sp_header.is_some()); let sp_header = sp_header.unwrap(); verify_sp_fields(PacketType::Tm, &sp_header); @@ -1041,8 +1063,8 @@ pub(crate) mod tests { fn test_zc_sph() { use zerocopy::AsBytes; - let sp_header = - SpHeader::tc_unseg(0x7FF, pow(2, 14) - 1, 0).expect("Error creating SP header"); + let sp_header = SpHeader::new_for_unseg_tc_checked(0x7FF, pow(2, 14) - 1, 0) + .expect("Error creating SP header"); assert_eq!(sp_header.ptype(), PacketType::Tc); assert_eq!(sp_header.apid(), 0x7FF); assert_eq!(sp_header.data_len(), 0);