improve test coverage

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<u32> = Crc::<u32>::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<u8> {
 
 #[derive(Debug, strum::Display)]
 pub enum CEPParseError {
-    WrongLength,
+    InvalidLength,
     InvalidHeader,
     InvalidCRC,
     Io(std::io::Error),
@@ -135,34 +139,8 @@ impl From<std::io::Error> for CEPParseError {
 impl TryFrom<Vec<u8>> for CEPPacket {
     type Error = CEPParseError;
 
-    fn try_from(mut value: Vec<u8>) -> Result<Self, Self::Error> {
-        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<u8>) -> Result<Self, Self::Error> {
+        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<u8>, 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)
+            )
+        )
+    }
 }

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))
+    }
+
 }