bugfix and improvements for CCSDS SP decoder #167

Merged
muellerr merged 1 commits from ccsds-decoder-bugfix into main 2024-04-22 10:23:12 +02:00
3 changed files with 80 additions and 44 deletions

View File

@ -8,6 +8,20 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
# [unreleased] # [unreleased]
# [v0.2.0-rc.4] 2024-04-20
## Changed
- The `parse_for_ccsds_space_packets` method now expects a non-mutable slice and does not copy
broken tail packets anymore. It also does not expect a mutable `next_write_idx` argument anymore.
Instead, a `ParseResult` structure is returned which contains the `packets_found` and an
optional `incomplete_tail_start` value.
## Fixed
- `parse_for_ccsds_space_packets` did not detect CCSDS space packets at the buffer end with the
smallest possible size of 7 bytes.
# [v0.2.0-rc.3] 2024-04-17 # [v0.2.0-rc.3] 2024-04-17
docs-rs hotfix 2 docs-rs hotfix 2

View File

@ -21,6 +21,13 @@ pub trait SpacePacketValidator {
fn validate(&self, sp_header: &SpHeader, raw_buf: &[u8]) -> SpValidity; fn validate(&self, sp_header: &SpHeader, raw_buf: &[u8]) -> SpValidity;
} }
#[derive(Default, Debug, PartialEq, Eq)]
pub struct ParseResult {
pub packets_found: u32,
/// If an incomplete space packet was found, its start index is indicated by this value.
pub incomplete_tail_start: Option<usize>,
}
/// This function parses a given buffer for tightly packed CCSDS space packets. It uses the /// This function parses a given buffer for tightly packed CCSDS space packets. It uses the
/// [spacepackets::SpHeader] of the CCSDS packets and a user provided [SpacePacketValidator] /// [spacepackets::SpHeader] of the CCSDS packets and a user provided [SpacePacketValidator]
/// to check whether a received space packet is relevant for processing. /// to check whether a received space packet is relevant for processing.
@ -41,35 +48,29 @@ pub trait SpacePacketValidator {
/// 3. [SpValidity::Skip]: The parser skips the packet using the packet length determined from the /// 3. [SpValidity::Skip]: The parser skips the packet using the packet length determined from the
/// space packet header. /// space packet header.
pub fn parse_buffer_for_ccsds_space_packets<SendError>( pub fn parse_buffer_for_ccsds_space_packets<SendError>(
buf: &mut [u8], buf: &[u8],
packet_validator: &(impl SpacePacketValidator + ?Sized), packet_validator: &(impl SpacePacketValidator + ?Sized),
sender_id: ComponentId, sender_id: ComponentId,
packet_sender: &(impl PacketSenderRaw<Error = SendError> + ?Sized), packet_sender: &(impl PacketSenderRaw<Error = SendError> + ?Sized),
next_write_idx: &mut usize, ) -> Result<ParseResult, SendError> {
) -> Result<u32, SendError> { let mut parse_result = ParseResult::default();
*next_write_idx = 0;
let mut packets_found = 0;
let mut current_idx = 0; let mut current_idx = 0;
let buf_len = buf.len(); let buf_len = buf.len();
loop { loop {
if current_idx + 7 >= buf.len() { if current_idx + 7 > buf.len() {
break; break;
} }
let sp_header = SpHeader::from_be_bytes(&buf[current_idx..]).unwrap().0; let sp_header = SpHeader::from_be_bytes(&buf[current_idx..]).unwrap().0;
// let packet_id = u16::from_be_bytes(buf[current_idx..current_idx + 2].try_into().unwrap());
match packet_validator.validate(&sp_header, &buf[current_idx..]) { match packet_validator.validate(&sp_header, &buf[current_idx..]) {
SpValidity::Valid => { SpValidity::Valid => {
let packet_size = sp_header.total_len(); let packet_size = sp_header.total_len();
if (current_idx + packet_size) <= buf_len { if (current_idx + packet_size) <= buf_len {
packet_sender packet_sender
.send_packet(sender_id, &buf[current_idx..current_idx + packet_size])?; .send_packet(sender_id, &buf[current_idx..current_idx + packet_size])?;
packets_found += 1; parse_result.packets_found += 1;
} else { } else {
// Move packet to start of buffer if applicable. // Move packet to start of buffer if applicable.
if current_idx > 0 { parse_result.incomplete_tail_start = Some(current_idx);
buf.copy_within(current_idx.., 0);
*next_write_idx = buf.len() - current_idx;
}
} }
current_idx += packet_size; current_idx += packet_size;
continue; continue;
@ -83,14 +84,14 @@ pub fn parse_buffer_for_ccsds_space_packets<SendError>(
} }
} }
} }
Ok(packets_found) Ok(parse_result)
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use spacepackets::{ use spacepackets::{
ecss::{tc::PusTcCreator, WritablePusPacket}, ecss::{tc::PusTcCreator, WritablePusPacket},
CcsdsPacket, PacketId, SpHeader, CcsdsPacket, PacketId, PacketSequenceCtrl, PacketType, SequenceFlags, SpHeader,
}; };
use crate::{encoding::tests::TcCacher, ComponentId}; use crate::{encoding::tests::TcCacher, ComponentId};
@ -136,17 +137,15 @@ mod tests {
.write_to_bytes(&mut buffer) .write_to_bytes(&mut buffer)
.expect("writing packet failed"); .expect("writing packet failed");
let tc_cacher = TcCacher::default(); let tc_cacher = TcCacher::default();
let mut next_write_idx = 0;
let parse_result = parse_buffer_for_ccsds_space_packets( let parse_result = parse_buffer_for_ccsds_space_packets(
&mut buffer, &buffer,
&SimpleVerificator::default(), &SimpleVerificator::default(),
PARSER_ID, PARSER_ID,
&tc_cacher, &tc_cacher,
&mut next_write_idx,
); );
assert!(parse_result.is_ok()); assert!(parse_result.is_ok());
let parsed_packets = parse_result.unwrap(); let parse_result = parse_result.unwrap();
assert_eq!(parsed_packets, 1); assert_eq!(parse_result.packets_found, 1);
let mut queue = tc_cacher.tc_queue.borrow_mut(); let mut queue = tc_cacher.tc_queue.borrow_mut();
assert_eq!(queue.len(), 1); assert_eq!(queue.len(), 1);
let packet_with_sender = queue.pop_front().unwrap(); let packet_with_sender = queue.pop_front().unwrap();
@ -167,17 +166,15 @@ mod tests {
.write_to_bytes(&mut buffer[packet_len_ping..]) .write_to_bytes(&mut buffer[packet_len_ping..])
.expect("writing packet failed"); .expect("writing packet failed");
let tc_cacher = TcCacher::default(); let tc_cacher = TcCacher::default();
let mut next_write_idx = 0;
let parse_result = parse_buffer_for_ccsds_space_packets( let parse_result = parse_buffer_for_ccsds_space_packets(
&mut buffer, &buffer,
&SimpleVerificator::default(), &SimpleVerificator::default(),
PARSER_ID, PARSER_ID,
&tc_cacher, &tc_cacher,
&mut next_write_idx,
); );
assert!(parse_result.is_ok()); assert!(parse_result.is_ok());
let parsed_packets = parse_result.unwrap(); let parse_result = parse_result.unwrap();
assert_eq!(parsed_packets, 2); assert_eq!(parse_result.packets_found, 2);
let mut queue = tc_cacher.tc_queue.borrow_mut(); let mut queue = tc_cacher.tc_queue.borrow_mut();
assert_eq!(queue.len(), 2); assert_eq!(queue.len(), 2);
let packet_with_addr = queue.pop_front().unwrap(); let packet_with_addr = queue.pop_front().unwrap();
@ -205,18 +202,16 @@ mod tests {
.write_to_bytes(&mut buffer[packet_len_ping..]) .write_to_bytes(&mut buffer[packet_len_ping..])
.expect("writing packet failed"); .expect("writing packet failed");
let tc_cacher = TcCacher::default(); let tc_cacher = TcCacher::default();
let mut next_write_idx = 0;
let verificator = SimpleVerificator::new_with_second_id(); let verificator = SimpleVerificator::new_with_second_id();
let parse_result = parse_buffer_for_ccsds_space_packets( let parse_result = parse_buffer_for_ccsds_space_packets(
&mut buffer, &buffer,
&verificator, &verificator,
PARSER_ID, PARSER_ID,
&tc_cacher, &tc_cacher,
&mut next_write_idx,
); );
assert!(parse_result.is_ok()); assert!(parse_result.is_ok());
let parsed_packets = parse_result.unwrap(); let parse_result = parse_result.unwrap();
assert_eq!(parsed_packets, 2); assert_eq!(parse_result.packets_found, 2);
let mut queue = tc_cacher.tc_queue.borrow_mut(); let mut queue = tc_cacher.tc_queue.borrow_mut();
assert_eq!(queue.len(), 2); assert_eq!(queue.len(), 2);
let packet_with_addr = queue.pop_front().unwrap(); let packet_with_addr = queue.pop_front().unwrap();
@ -242,24 +237,25 @@ mod tests {
.write_to_bytes(&mut buffer[packet_len_ping..]) .write_to_bytes(&mut buffer[packet_len_ping..])
.expect("writing packet failed"); .expect("writing packet failed");
let tc_cacher = TcCacher::default(); let tc_cacher = TcCacher::default();
let mut next_write_idx = 0;
let verificator = SimpleVerificator::new_with_second_id(); let verificator = SimpleVerificator::new_with_second_id();
let parse_result = parse_buffer_for_ccsds_space_packets( let parse_result = parse_buffer_for_ccsds_space_packets(
&mut buffer[..packet_len_ping + packet_len_action - 4], &buffer[..packet_len_ping + packet_len_action - 4],
&verificator, &verificator,
PARSER_ID, PARSER_ID,
&tc_cacher, &tc_cacher,
&mut next_write_idx,
); );
assert!(parse_result.is_ok()); assert!(parse_result.is_ok());
let parsed_packets = parse_result.unwrap(); let parse_result = parse_result.unwrap();
assert_eq!(parsed_packets, 1); assert_eq!(parse_result.packets_found, 1);
assert!(parse_result.incomplete_tail_start.is_some());
let incomplete_tail_idx = parse_result.incomplete_tail_start.unwrap();
assert_eq!(incomplete_tail_idx, packet_len_ping);
let queue = tc_cacher.tc_queue.borrow(); let queue = tc_cacher.tc_queue.borrow();
assert_eq!(queue.len(), 1); assert_eq!(queue.len(), 1);
// The broken packet was moved to the start, so the next write index should be after the // The broken packet was moved to the start, so the next write index should be after the
// last segment missing 4 bytes. // last segment missing 4 bytes.
assert_eq!(next_write_idx, packet_len_action - 4);
} }
#[test] #[test]
@ -273,19 +269,40 @@ mod tests {
let tc_cacher = TcCacher::default(); let tc_cacher = TcCacher::default();
let verificator = SimpleVerificator::new_with_second_id(); let verificator = SimpleVerificator::new_with_second_id();
let mut next_write_idx = 0;
let parse_result = parse_buffer_for_ccsds_space_packets( let parse_result = parse_buffer_for_ccsds_space_packets(
&mut buffer[..packet_len_ping - 4], &buffer[..packet_len_ping - 4],
&verificator, &verificator,
PARSER_ID, PARSER_ID,
&tc_cacher, &tc_cacher,
&mut next_write_idx,
); );
assert_eq!(next_write_idx, 0);
assert!(parse_result.is_ok()); assert!(parse_result.is_ok());
let parsed_packets = parse_result.unwrap(); let parse_result = parse_result.unwrap();
assert_eq!(parsed_packets, 0); assert_eq!(parse_result.packets_found, 0);
let queue = tc_cacher.tc_queue.borrow(); let queue = tc_cacher.tc_queue.borrow();
assert_eq!(queue.len(), 0); assert_eq!(queue.len(), 0);
} }
#[test]
fn test_smallest_packet() {
let ccsds_header_only = SpHeader::new(
PacketId::new(PacketType::Tc, true, TEST_APID_0),
PacketSequenceCtrl::new(SequenceFlags::Unsegmented, 0),
0,
);
let mut buf: [u8; 7] = [0; 7];
ccsds_header_only
.write_to_be_bytes(&mut buf)
.expect("writing failed");
let verificator = SimpleVerificator::default();
let tc_cacher = TcCacher::default();
let parse_result = parse_buffer_for_ccsds_space_packets(
&buf,
&verificator,
PARSER_ID,
&tc_cacher,
);
assert!(parse_result.is_ok());
let parse_result = parse_result.unwrap();
assert_eq!(parse_result.packets_found, 1);
}
} }

View File

@ -26,14 +26,19 @@ impl<T: SpacePacketValidator, TmError, TcError: 'static> TcpTcParser<TmError, Tc
next_write_idx: &mut usize, next_write_idx: &mut usize,
) -> Result<(), TcpTmtcError<TmError, TcError>> { ) -> Result<(), TcpTmtcError<TmError, TcError>> {
// Reader vec full, need to parse for packets. // Reader vec full, need to parse for packets.
conn_result.num_received_tcs += parse_buffer_for_ccsds_space_packets( let parse_result = parse_buffer_for_ccsds_space_packets(
&mut tc_buffer[..current_write_idx], &tc_buffer[..current_write_idx],
self, self,
sender_id, sender_id,
tc_sender, tc_sender,
next_write_idx,
) )
.map_err(|e| TcpTmtcError::TcError(e))?; .map_err(|e| TcpTmtcError::TcError(e))?;
if let Some(broken_tail_start) = parse_result.incomplete_tail_start {
// Copy broken tail to front of buffer.
tc_buffer.copy_within(broken_tail_start..current_write_idx, 0);
*next_write_idx = current_write_idx - broken_tail_start;
}
conn_result.num_received_tcs += parse_result.packets_found;
Ok(()) Ok(())
} }
} }