From 7b878b13d6738a0612abbda74dfb3ae5db22fded Mon Sep 17 00:00:00 2001 From: Florian Guggi Date: Wed, 27 Dec 2023 14:28:06 +0100 Subject: [PATCH] improve test coverage --- src/communication/cep.rs | 61 ++++++++++++++++++------------------- src/communication/mod.rs | 66 ++++++++++++++++++++++++++++++---------- 2 files changed, 80 insertions(+), 47 deletions(-) diff --git a/src/communication/cep.rs b/src/communication/cep.rs index 55e23b4..3e0c69c 100644 --- a/src/communication/cep.rs +++ b/src/communication/cep.rs @@ -21,7 +21,7 @@ pub enum CEPPacketHeader { } impl CEPPacket { - pub const MAXIMUM_DATA_LENGTH: usize = 32768; + pub const MAXIMUM_DATA_LENGTH: usize = 11000; pub const MAXIMUM_PACKET_LENGTH: usize = 7 + Self::MAXIMUM_DATA_LENGTH; const CRC: Crc = Crc::::new(&CRC_32_MPEG_2); @@ -71,7 +71,7 @@ impl CEPPacket { reader.read_exact(&mut header_buffer)?; let header = CEPPacketHeader::from_repr(header_buffer[0] as usize) - .ok_or(CEPParseError::WrongLength)?; + .ok_or(CEPParseError::InvalidLength)?; let packet = match header { CEPPacketHeader::Ack => CEPPacket::Ack, CEPPacketHeader::Nack => CEPPacket::Nack, @@ -82,6 +82,10 @@ impl CEPPacket { reader.read_exact(&mut length_buffer)?; let length = u16::from_le_bytes(length_buffer); + if length as usize > Self::MAXIMUM_DATA_LENGTH { + return Err(CEPParseError::InvalidLength); + } + let mut data_buffer = vec![0; length as usize]; reader.read_exact(&mut data_buffer)?; @@ -118,7 +122,7 @@ impl From<&CEPPacket> for Vec { #[derive(Debug, strum::Display)] pub enum CEPParseError { - WrongLength, + InvalidLength, InvalidHeader, InvalidCRC, Io(std::io::Error), @@ -135,34 +139,8 @@ impl From for CEPParseError { impl TryFrom> for CEPPacket { type Error = CEPParseError; - fn try_from(mut value: Vec) -> Result { - let header_byte = value.first().ok_or(CEPParseError::WrongLength)?; - let header = CEPPacketHeader::from_repr(*header_byte as usize) - .ok_or(CEPParseError::InvalidHeader)?; - - let packet = match header { - CEPPacketHeader::Ack => CEPPacket::Ack, - CEPPacketHeader::Nack => CEPPacket::Nack, - CEPPacketHeader::Stop => CEPPacket::Stop, - CEPPacketHeader::Eof => CEPPacket::Eof, - CEPPacketHeader::Data => { - let length_bytes = value.get(1..3).ok_or(CEPParseError::WrongLength)?; - let length = u16::from_le_bytes(length_bytes.try_into().unwrap()) as usize; - value.drain(0..3); - - let crc_bytes = value.drain(length..length + 4); - let crc = u32::from_le_bytes(crc_bytes.as_slice().try_into().unwrap()); - drop(crc_bytes); - - if !CEPPacket::crc_is_valid(&value, crc) { - return Err(CEPParseError::InvalidCRC); - } - - CEPPacket::Data(value) - } - }; - - Ok(packet) + fn try_from(value: Vec) -> Result { + Self::try_from_read(&mut std::io::Cursor::new(value)) } } @@ -176,8 +154,29 @@ mod tests { #[test_case(vec![0x59], CEPPacket::Eof)] #[test_case(vec![0xB4], CEPPacket::Stop)] #[test_case(vec![0x8B, 0, 0, 0xff, 0xff, 0xff, 0xff], CEPPacket::Data(vec![]); "empty Data packet")] + #[test_case(vec![0x8B, 4, 0, 0x0a, 0x0b, 0x05, 0x73, 0x52, 0x27, 0x92, 0xf4], CEPPacket::Data(vec![0x0a, 0x0b, 0x05, 0x73]); "filled data packet")] fn packet_is_parsed_and_serialized_correctly(vec: Vec, packet: CEPPacket) { assert_eq!(&packet.clone().serialize(), &vec); assert_eq!(CEPPacket::try_from(vec).unwrap(), packet); } + + #[test] + fn invalid_crc_is_rejected() { + assert!( + matches!( + CEPPacket::try_from(vec![0x8B, 4, 0, 0x0a, 0x0b, 0x05, 0x74, 0x52, 0x27, 0x92, 0xf4]), + Err(CEPParseError::InvalidCRC) + ) + ) + } + + #[test] + fn invalid_length_is_rejected() { + assert!( + matches!( + CEPPacket::try_from(vec![0x8B, 0xff, 0xff]), + Err(CEPParseError::InvalidLength) + ) + ) + } } diff --git a/src/communication/mod.rs b/src/communication/mod.rs index 096df87..f85c060 100644 --- a/src/communication/mod.rs +++ b/src/communication/mod.rs @@ -20,10 +20,11 @@ pub trait CommunicationHandle: Read + Write { fn send_packet(&mut self, packet: &CEPPacket) -> ComResult<()> { let bytes = Vec::from(packet); - self.write_all(&bytes)?; - if matches!(packet, CEPPacket::Data(_)) { - for _ in 0..Self::DATA_PACKET_RETRIES { + for _ in 0..Self::DATA_PACKET_RETRIES { + self.write_all(&bytes)?; + + if matches!(packet, CEPPacket::Data(_)) { let response = self.receive_packet()?; match response { CEPPacket::Ack => return Ok(()), @@ -33,11 +34,9 @@ pub trait CommunicationHandle: Read + Write { return Err(CommunicationError::PacketInvalidError); } } - - self.write_all(&bytes)?; + } else { + return Ok(()); } - } else { - return Ok(()); } log::error!("No ACK after {} retries, giving up", Self::DATA_PACKET_RETRIES); @@ -249,10 +248,8 @@ mod tests { com.send_packet(&CEPPacket::Data(vec![1, 2, 3])).unwrap(); - let mut expected = CEPPacket::Data(vec![1, 2, 3]).serialize(); - expected.extend(CEPPacket::Data(vec![1, 2, 3]).serialize()); - expected.extend(CEPPacket::Data(vec![1, 2, 3]).serialize()); - assert_eq!(com.written_data, expected); + assert_eq!(com.written_data, CEPPacket::Data(vec![1, 2, 3]).serialize().repeat(3)); + assert!(com.data_to_read.is_empty()); } #[test] @@ -262,11 +259,48 @@ mod tests { com.data_to_read.append(&mut CEPPacket::Nack.serialize()); } - assert!( - matches!( - com.send_packet(&CEPPacket::Data(vec![1, 2, 3])), - Err(CommunicationError::PacketInvalidError) - ) + assert!(matches!( + com.send_packet(&CEPPacket::Data(vec![1, 2, 3])), + Err(CommunicationError::PacketInvalidError) + )); + assert!(com.data_to_read.is_empty()); + assert_eq!( + com.written_data, + CEPPacket::Data(vec![1, 2, 3]).serialize().repeat(TestComHandle::DATA_PACKET_RETRIES) ); } + + #[test] + fn multi_packet_is_sent_correctly() { + let mut com = TestComHandle::default(); + + let data = vec![123u8; 2 * CEPPacket::MAXIMUM_DATA_LENGTH + 50]; + let chunks = data.chunks(CEPPacket::MAXIMUM_DATA_LENGTH); + com.data_to_read = CEPPacket::Ack.serialize().repeat(chunks.len() + 1); + + com.send_multi_packet(&data).unwrap(); + + assert!(com.data_to_read.is_empty()); + for c in chunks { + assert_eq!(com.written_data.drain(0..c.len()+7).as_slice(), CEPPacket::Data(c.to_vec()).serialize()); + } + assert_eq!(com.written_data, CEPPacket::Eof.serialize()); + } + + #[test] + fn multi_packet_is_received_correctly() { + let mut com = TestComHandle::default(); + + let data = vec![123u8; 2 * CEPPacket::MAXIMUM_DATA_LENGTH + 50]; + let chunks = data.chunks(CEPPacket::MAXIMUM_DATA_LENGTH); + for c in chunks.clone() { + com.data_to_read.append(&mut CEPPacket::Data(c.to_vec()).serialize()); + } + com.data_to_read.append(&mut CEPPacket::Eof.serialize()); + + assert_eq!(com.receive_multi_packet(|| false).unwrap(), data); + assert!(com.data_to_read.is_empty()); + assert_eq!(com.written_data, CEPPacket::Ack.serialize().repeat(chunks.len() + 1)) + } + }