Merge pull request 'add first builder API' (#147) from add-tc-builder-api into main

Reviewed-on: #147
2025-09-10 16:38:25 +02:00
parent 3a21daf8de 578be2da8f
commit 89788c1341
3 changed files with 211 additions and 28 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

 ## Changed

+- `PusTcCreator` has its own `service`, `subservice` and `apid` methods and does not require trait
+  imports anymore.
 - CFDP NAK PDU `SegmentRequestIter` is not generic over the file size anymore. Instead, the
  iterator returns pairs of `u64` for both large and normal file size.
 - `PusVersion::VersionNotSupported` contains raw version number instead of `PusVersion` enum now
@@ -33,6 +35,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - ACK field changed from `u8` to `AckFlags` structure.
 - PUS version raw representation is `u4` now.
 - SC time reference status representation is `u4` now.
+- Renamed `ptype` to `packet_type`

 ## Removed

@@ -46,6 +49,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Added `SequenceCounter::increment_mut` and `SequenceCounter::get_and_increment_mut`
 - Implemented `SequenceCounter` for `Atomic` unsigned types and references of them
 - `PusPacket::has_checksum` and `WritablePusPacket::has_checksum`
+- PUS TC builder API, either via `PusTcBuilder::new`, or `PusTcCreator::builder`

 # [v0.15.0] 2025-07-18

--- a/src/ecss/tc.rs
+++ b/src/ecss/tc.rs
@@ -4,8 +4,8 @@
 //! # Examples
 //!
 //! ```rust
-//! use spacepackets::{CcsdsPacket, SpHeader};
-//! use spacepackets::ecss::{PusPacket, WritablePusPacket};
+//! use spacepackets::SpHeader;
+//! use spacepackets::ecss::WritablePusPacket;
 //! use spacepackets::ecss::tc::{PusTcCreator, PusTcReader, PusTcSecondaryHeader, CreatorConfig};
 //! use arbitrary_int::u11;
 //!
@@ -33,6 +33,14 @@
 //! assert_eq!(pus_tc.service(), 17);
 //! assert_eq!(pus_tc.subservice(), 1);
 //! assert_eq!(pus_tc.apid().value(), 0x02);
+//!
+//! // Alternative builder API
+//! let pus_tc_by_builder = PusTcCreator::builder()
+//!     .with_service(17)
+//!     .with_subservice(1)
+//!     .with_apid(u11::new(0x02))
+//!     .build();
+//! assert_eq!(pus_tc_by_builder, pus_tc);
 //! ```
 use crate::crc::{CRC_CCITT_FALSE, CRC_CCITT_FALSE_NO_TABLE};
 pub use crate::ecss::CreatorConfig;
@@ -41,8 +49,8 @@ use crate::ecss::{
    verify_crc16_ccitt_false_from_raw_to_pus_error, PusError, PusPacket, PusVersion,
    WritablePusPacket,
 };
-use crate::SpHeader;
 use crate::{ByteConversionError, CcsdsPacket, PacketType, SequenceFlags, CCSDS_HEADER_LEN};
+use crate::{PacketId, PacketSequenceControl, SpHeader};
 use arbitrary_int::{u11, u14, u3, u4};
 use core::mem::size_of;
 use delegate::delegate;
@@ -314,6 +322,10 @@ impl<'app_data> PusTcCreator<'app_data> {
        Self::new(sp_header, sec_header, &[], packet_config)
    }

+    pub fn builder<'a>() -> PusTcBuilder<'a> {
+        PusTcBuilder::default()
+    }
+
    #[inline]
    pub fn sp_header(&self) -> &SpHeader {
        &self.sp_header
@@ -324,6 +336,21 @@ impl<'app_data> PusTcCreator<'app_data> {
        &mut self.sp_header
    }

+    #[inline]
+    pub fn service(&self) -> u8 {
+        self.sec_header.service
+    }
+
+    #[inline]
+    pub fn subservice(&self) -> u8 {
+        self.sec_header.subservice
+    }
+
+    #[inline]
+    pub fn apid(&self) -> u11 {
+        self.sp_header.packet_id.apid
+    }
+
    #[inline]
    pub fn set_ack_flags(&mut self, ack_flags: AckFlags) {
        self.sec_header.ack_flags = ack_flags;
@@ -681,6 +708,108 @@ impl<'buf> PusTcCreatorWithReservedAppData<'buf> {
    }
 }

+pub struct PusTcBuilder<'a> {
+    sp_header: SpHeader,
+    sec_header: PusTcSecondaryHeader,
+    app_data: &'a [u8],
+    has_checksum: bool,
+}
+
+impl PusTcBuilder<'_> {
+    pub fn new() -> Self {
+        Self {
+            sp_header: SpHeader::new(
+                PacketId::new(PacketType::Tc, true, u11::new(0)),
+                PacketSequenceControl::new(SequenceFlags::Unsegmented, u14::new(0)),
+                0,
+            ),
+            sec_header: PusTcSecondaryHeader::new(0, 0, ACK_ALL, 0),
+            app_data: &[],
+            has_checksum: true,
+        }
+    }
+
+    #[inline]
+    pub fn with_packet_id(mut self, mut packet_id: PacketId) -> Self {
+        packet_id.packet_type = PacketType::Tc;
+        self.sp_header.packet_id = packet_id;
+        self
+    }
+
+    #[inline]
+    pub fn with_packet_sequence_control(mut self, psc: PacketSequenceControl) -> Self {
+        self.sp_header.psc = psc;
+        self
+    }
+
+    #[inline]
+    pub fn with_sequence_count(mut self, seq_count: u14) -> Self {
+        self.sp_header.psc.seq_count = seq_count;
+        self
+    }
+
+    #[inline]
+    pub fn with_service(mut self, service: u8) -> Self {
+        self.sec_header.service = service;
+        self
+    }
+
+    #[inline]
+    pub fn with_subservice(mut self, service: u8) -> Self {
+        self.sec_header.subservice = service;
+        self
+    }
+
+    #[inline]
+    pub fn with_source_id(mut self, source_id: u16) -> Self {
+        self.sec_header.source_id = source_id;
+        self
+    }
+
+    #[inline]
+    pub fn with_ack_flags(mut self, ack_flags: AckFlags) -> Self {
+        self.sec_header.ack_flags = ack_flags;
+        self
+    }
+
+    #[inline]
+    pub fn with_apid(mut self, apid: u11) -> Self {
+        self.sp_header.packet_id.set_apid(apid);
+        self
+    }
+
+    #[inline]
+    pub fn with_checksum(mut self, has_checksum: bool) -> Self {
+        self.has_checksum = has_checksum;
+        self
+    }
+}
+
+impl Default for PusTcBuilder<'_> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl<'a> PusTcBuilder<'a> {
+    pub fn build(self) -> PusTcCreator<'a> {
+        PusTcCreator::new(
+            self.sp_header,
+            self.sec_header,
+            self.app_data,
+            CreatorConfig {
+                has_checksum: self.has_checksum,
+                set_ccsds_len: true,
+            },
+        )
+    }
+
+    pub fn with_app_data(mut self, app_data: &'a [u8]) -> Self {
+        self.app_data = app_data;
+        self
+    }
+}
+
 /// This class can be used to read a PUS TC telecommand from raw memory.
 ///
 /// This class also derives the [serde::Serialize] and [serde::Deserialize] trait if the
@@ -917,6 +1046,23 @@ mod tests {
        PusTcCreator::new_simple(sph, 17, 1, &[], CreatorConfig::default())
    }

+    fn base_ping_tc_with_builder(alt_api: bool) -> PusTcCreator<'static> {
+        if alt_api {
+            return PusTcCreator::builder()
+                .with_service(17)
+                .with_subservice(1)
+                .with_apid(u11::new(0x02))
+                .with_sequence_count(u14::new(0x34))
+                .build();
+        }
+        PusTcBuilder::new()
+            .with_service(17)
+            .with_subservice(1)
+            .with_apid(u11::new(0x02))
+            .with_sequence_count(u14::new(0x34))
+            .build()
+    }
+
    fn base_ping_tc_simple_ctor_no_checksum() -> PusTcCreator<'static> {
        let sph = SpHeader::new_for_unseg_tc(u11::new(0x02), u14::new(0x34), 0);
        PusTcCreator::new_simple(
@@ -1467,4 +1613,36 @@ mod tests {
        let output_converted_back: PusTcCreator = from_bytes(&output).unwrap();
        assert_eq!(output_converted_back, pus_tc);
    }
+
+    #[test]
+    fn test_builder() {
+        assert_eq!(base_ping_tc_with_builder(false), base_ping_tc_simple_ctor());
+    }
+
+    #[test]
+    fn test_builder_2() {
+        assert_eq!(base_ping_tc_with_builder(true), base_ping_tc_simple_ctor());
+    }
+
+    #[test]
+    fn test_builder_3() {
+        let tc = PusTcBuilder::new()
+            .with_packet_id(PacketId::new_for_tc(true, u11::new(0x02)))
+            .with_packet_sequence_control(PacketSequenceControl::new(
+                SequenceFlags::Unsegmented,
+                u14::new(0x34),
+            ))
+            .with_service(17)
+            .with_subservice(1)
+            .with_ack_flags(AckFlags::new_with_raw_value(u4::new(0b1010)))
+            .with_source_id(0x2f2f)
+            .with_checksum(false)
+            .build();
+        assert_eq!(tc.seq_count().value(), 0x34);
+        assert_eq!(tc.sequence_flags(), SequenceFlags::Unsegmented);
+        assert_eq!(tc.apid().value(), 0x02);
+        assert_eq!(tc.packet_type(), PacketType::Tc);
+        assert_eq!(tc.service(), 17);
+        assert_eq!(tc.subservice(), 1);
+    }
 }
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -158,7 +158,7 @@ pub enum SequenceFlags {
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct PacketId {
-    pub ptype: PacketType,
+    pub packet_type: PacketType,
    pub sec_header_flag: bool,
    pub apid: u11,
 }
@@ -195,7 +195,7 @@ impl Default for PacketId {
    #[inline]
    fn default() -> Self {
        PacketId {
-            ptype: PacketType::Tm,
+            packet_type: PacketType::Tm,
            sec_header_flag: false,
            apid: u11::new(0),
        }
@@ -222,7 +222,7 @@ impl PacketId {
    #[inline]
    pub const fn new(ptype: PacketType, sec_header: bool, apid: u11) -> Self {
        PacketId {
-            ptype,
+            packet_type: ptype,
            sec_header_flag: sec_header,
            apid,
        }
@@ -243,14 +243,16 @@ impl PacketId {

    #[inline]
    pub const fn raw(&self) -> u16 {
-        ((self.ptype as u16) << 12) | ((self.sec_header_flag as u16) << 11) | self.apid.value()
+        ((self.packet_type as u16) << 12)
+            | ((self.sec_header_flag as u16) << 11)
+            | self.apid.value()
    }
 }

 impl From<u16> for PacketId {
    fn from(raw_id: u16) -> Self {
        PacketId {
-            ptype: PacketType::try_from(((raw_id >> 12) & 0b1) as u8).unwrap(),
+            packet_type: PacketType::try_from(((raw_id >> 12) & 0b1) as u8).unwrap(),
            sec_header_flag: ((raw_id >> 11) & 0b1) != 0,
            apid: u11::new(raw_id & 0x7FF),
        }
@@ -335,21 +337,20 @@ pub trait CcsdsPacket {
        self.psc().raw()
    }

-    /// Retrieve Packet Type (TM: 0, TC: 1).
    #[inline]
-    fn ptype(&self) -> PacketType {
+    fn packet_type(&self) -> PacketType {
        // This call should never fail because only 0 and 1 can be passed to the try_from call
-        self.packet_id().ptype
+        self.packet_id().packet_type
    }

    #[inline]
    fn is_tm(&self) -> bool {
-        self.ptype() == PacketType::Tm
+        self.packet_type() == PacketType::Tm
    }

    #[inline]
    fn is_tc(&self) -> bool {
-        self.ptype() == PacketType::Tc
+        self.packet_type() == PacketType::Tc
    }

    /// Retrieve the secondary header flag. Returns true if a secondary header is present
@@ -542,7 +543,7 @@ impl SpHeader {

    #[inline]
    pub fn set_packet_type(&mut self, packet_type: PacketType) {
-        self.packet_id.ptype = packet_type;
+        self.packet_id.packet_type = packet_type;
    }

    /// Create a struct from a raw slice where the fields have network endianness (big).
@@ -757,15 +758,15 @@ pub(crate) mod tests {
    fn verify_const_packet_id() {
        assert_eq!(PACKET_ID_TM.apid().value(), 0x22);
        assert!(PACKET_ID_TM.sec_header_flag);
-        assert_eq!(PACKET_ID_TM.ptype, PacketType::Tm);
+        assert_eq!(PACKET_ID_TM.packet_type, PacketType::Tm);
        let const_tc_id = PacketId::new_for_tc(true, u11::new(0x23));
-        assert_eq!(const_tc_id.ptype, PacketType::Tc);
+        assert_eq!(const_tc_id.packet_type, PacketType::Tc);
    }

    #[test]
    fn test_default_packet_id() {
        let id_default = PacketId::default();
-        assert_eq!(id_default.ptype, PacketType::Tm);
+        assert_eq!(id_default.packet_type, PacketType::Tm);
        assert_eq!(id_default.apid.value(), 0x000);
        assert!(!id_default.sec_header_flag);
    }
@@ -774,13 +775,13 @@ pub(crate) mod tests {
    fn test_packet_id_ctors() {
        let packet_id = PacketId::new(PacketType::Tc, true, u11::new(0x1ff));
        assert_eq!(packet_id.apid().value(), 0x1ff);
-        assert_eq!(packet_id.ptype, PacketType::Tc);
+        assert_eq!(packet_id.packet_type, PacketType::Tc);
        assert!(packet_id.sec_header_flag);
        let packet_id_tc = PacketId::new_for_tc(true, u11::new(0x1ff));
        assert_eq!(packet_id_tc, packet_id);
        let packet_id_tm = PacketId::new_for_tm(true, u11::new(0x2ff));
        assert!(packet_id_tm.sec_header_flag);
-        assert_eq!(packet_id_tm.ptype, PacketType::Tm);
+        assert_eq!(packet_id_tm.packet_type, PacketType::Tm);
        assert_eq!(packet_id_tm.apid, u11::new(0x2ff));
    }

@@ -853,7 +854,7 @@ pub(crate) mod tests {
        assert_eq!(sp_header.ccsds_version().value(), 0b000);
        assert!(sp_header.is_tc());
        assert!(!sp_header.sec_header_flag());
-        assert_eq!(sp_header.ptype(), PacketType::Tc);
+        assert_eq!(sp_header.packet_type(), PacketType::Tc);
        assert_eq!(sp_header.seq_count().value(), 12);
        assert_eq!(sp_header.apid().value(), 0x42);
        assert_eq!(sp_header.sequence_flags(), SequenceFlags::Unsegmented);
@@ -862,7 +863,7 @@ pub(crate) mod tests {
        let sp_header: SpHeader = from_bytes(&output).unwrap();
        assert_eq!(sp_header.version.value(), 0b000);
        assert!(!sp_header.packet_id.sec_header_flag);
-        assert_eq!(sp_header.ptype(), PacketType::Tc);
+        assert_eq!(sp_header.packet_type(), PacketType::Tc);
        assert_eq!(sp_header.seq_count().value(), 12);
        assert_eq!(sp_header.apid().value(), 0x42);
        assert_eq!(sp_header.sequence_flags(), SequenceFlags::Unsegmented);
@@ -875,7 +876,7 @@ pub(crate) mod tests {
        assert_eq!(sp_header.ccsds_version().value(), 0b000);
        assert!(sp_header.is_tm());
        assert!(!sp_header.sec_header_flag());
-        assert_eq!(sp_header.ptype(), PacketType::Tm);
+        assert_eq!(sp_header.packet_type(), PacketType::Tm);
        assert_eq!(sp_header.seq_count().value(), 22);
        assert_eq!(sp_header.apid().value(), 0x07);
        assert_eq!(sp_header.sequence_flags(), SequenceFlags::Unsegmented);
@@ -890,7 +891,7 @@ pub(crate) mod tests {
            0,
            None,
        );
-        assert_eq!(from_comp_fields.ptype(), PacketType::Tc);
+        assert_eq!(from_comp_fields.packet_type(), PacketType::Tc);
        assert_eq!(from_comp_fields.apid().value(), 0x42);
        assert!(from_comp_fields.sec_header_flag());
        assert_eq!(
@@ -911,9 +912,9 @@ pub(crate) mod tests {
        assert!(sp_header.sec_header_flag());
        sp_header.clear_sec_header_flag();
        assert!(!sp_header.sec_header_flag());
-        assert_eq!(sp_header.ptype(), PacketType::Tc);
+        assert_eq!(sp_header.packet_type(), PacketType::Tc);
        sp_header.set_packet_type(PacketType::Tm);
-        assert_eq!(sp_header.ptype(), PacketType::Tm);
+        assert_eq!(sp_header.packet_type(), PacketType::Tm);
        sp_header.set_seq_count(u14::new(0x45));
        assert_eq!(sp_header.seq_count().as_u16(), 0x45);
    }
@@ -958,7 +959,7 @@ pub(crate) mod tests {
    }

    fn verify_sp_fields(ptype: PacketType, sp_header: &SpHeader) {
-        assert_eq!(sp_header.ptype(), ptype);
+        assert_eq!(sp_header.packet_type(), ptype);
        assert_eq!(sp_header.sequence_flags(), SequenceFlags::Unsegmented);
        assert_eq!(sp_header.apid().value(), 0x42);
        assert_eq!(sp_header.seq_count(), u14::new(25));
@@ -970,7 +971,7 @@ pub(crate) mod tests {
        use zerocopy::IntoBytes;

        let sp_header = SpHeader::new_for_unseg_tc(u11::MAX, u14::MAX, 0);
-        assert_eq!(sp_header.ptype(), PacketType::Tc);
+        assert_eq!(sp_header.packet_type(), PacketType::Tc);
        assert_eq!(sp_header.apid().value(), 0x7FF);
        assert_eq!(sp_header.data_len(), 0);
        assert_eq!(sp_header.ccsds_version().value(), 0b000);
@@ -1013,7 +1014,7 @@ pub(crate) mod tests {
        assert_eq!(sp_header.ccsds_version().value(), 0b000);
        assert_eq!(sp_header.packet_id_raw(), 0x17FF);
        assert_eq!(sp_header.apid().value(), 0x7FF);
-        assert_eq!(sp_header.ptype(), PacketType::Tc);
+        assert_eq!(sp_header.packet_type(), PacketType::Tc);
        assert_eq!(sp_header.data_len(), 0);
    }