refactor!: do some refactoring

src/lib.rs

@@ -57,14 +57,20 @@ const CRSF_HEADER_LEN: usize = 2;
 pub enum Error {
     #[snafu(display("No sync byte was found in the given buffer"))]
     NoSyncByte,
-    #[snafu(display("Unknown type: {typ:#04x}, see PacketType enum for valid types"))]
-    UnknownType { typ: u8 },
-    #[snafu(display("Invalid length: {len}, should be between 2 and 62"))]
+    #[snafu(display("Invalid type {typ:#04x}, see PacketType enum"))]
+    InvalidType { typ: u8 },
+    #[snafu(display("Unimplemented type {typ:?}, should be implemented ASAP"))]
+    UnimplementedType { typ: PacketType },
+    #[snafu(display("Packet of type {typ:?} is not extended, see PacketType enum"))]
+    PacketNotExtended { typ: PacketType },
+    #[snafu(display("Invalid length {len}, should be between 2 and 62"))]
     InvalidLength { len: u8 },
+    #[snafu(display("Invalid address {addr:#04x}, see PacketAddress enum"))]
+    InvalidAddress { addr: u8 },
+    #[snafu(display("Packet has invalid payload data"))]
+    InvalidPayload,
     #[snafu(display("Crc checksum mismatch: expected {exp:#04x}, got {act:#04x}"))]
     CrcMismatch { exp: u8, act: u8 },
-    #[snafu(display("A general buffer error relating to the parser occured"))]
+    #[snafu(display("General buffer error"))]
     BufferError,
-    #[snafu(display("Invalid payload data, could not parse packet"))]
-    InvalidPayload,
 }

src/packet/mod.rs

@@ -67,89 +67,59 @@ impl RawPacket {
         &self.buf[..self.len.min(CRSF_MAX_LEN)]
     }
 
-    /// Get the payload section of the raw packet
-    pub fn payload(&self) -> Result<&[u8], Error> {
-        match (self.is_extended(), self.as_slice()) {
-            // Skip the [sync], [len], [type], [dst], [src] and [crc] bytes
-            (true, [_, _, _, _, _, payload @ .., _]) => Ok(payload),
-            // Skip the [sync], [len], [type] and [crc] bytes
-            (false, [_, _, _, payload @ .., _]) => Ok(payload),
-            _ => Err(Error::BufferError),
-        }
-    }
-
-    /// Check if the packet is an extended format packet
-    pub fn is_extended(&self) -> bool {
-        // Skip the [sync], [len], [type] and [crc] bytes
-        if let [_, _, ty, ..] = self.as_slice() {
-            ty >= &0x28
-        } else {
-            false
-        }
-    }
-
-    /// Get the source and destination addresses of the packet.
-    /// This is only valid for extended packets, and will
-    /// return an error otherwise
-    pub fn dst_src(&self) -> Result<(PacketAddress, PacketAddress), Error> {
-        if self.is_extended() {
-            if let [_, _, _, dst, src, ..] = self.as_slice() {
-                match (PacketAddress::try_from(*dst), PacketAddress::try_from(*src)) {
-                    (Ok(dst), Ok(src)) => Ok((dst, src)),
-                    _ => Err(Error::InvalidPayload),
-                }
-            } else {
-                Err(Error::BufferError)
-            }
-        } else {
-            // NOTE Not sure what the error here should be
-            Err(Error::UnknownType { typ: 0 })
-        }
-    }
-
     /// Convert the raw packet into a parsed packet
     pub fn to_packet(&self) -> Result<Packet, Error> {
-        let payload = self.payload()?;
-        match PacketType::try_from(self.buf[2]) {
-            Ok(PacketType::RcChannelsPacked) => RcChannelsPacked::decode(payload).map(Packet::RcChannelsPacked),
-            Ok(PacketType::LinkStatistics) => LinkStatistics::decode(payload).map(Packet::LinkStatistics),
-            Ok(typ) if typ.is_extended() => {
-                let (dst, src) = self.dst_src()?;
-
-                match typ {
-                    PacketType::DevicePing => DevicePing::decode(payload).map(ExtendedPacket::DevicePing),
-                    _ => Err(Error::UnknownType { typ: self.buf[2] }),
+        if let [_, _, typ, payload @ .., _] = self.as_slice() {
+            let typ = PacketType::try_from(*typ).map_err(|_| Error::InvalidType { typ: *typ })?;
+            match typ {
+                PacketType::RcChannelsPacked => RcChannelsPacked::decode(payload).map(Packet::RcChannelsPacked),
+                PacketType::LinkStatistics => LinkStatistics::decode(payload).map(Packet::LinkStatistics),
+                typ if typ.is_extended() => {
+                    if let [dst, src, payload @ ..] = payload {
+                        let dst = PacketAddress::try_from(*dst).map_err(|_| Error::InvalidAddress { addr: *dst })?;
+                        let src = PacketAddress::try_from(*src).map_err(|_| Error::InvalidAddress { addr: *src })?;
+                        match typ {
+                            PacketType::DevicePing => DevicePing::decode(payload).map(ExtendedPacket::DevicePing),
+                            _ => Err(Error::UnimplementedType { typ }),
+                        }
+                        .map(|packet| Packet::Extended { src, dst, packet })
+                    } else {
+                        Err(Error::BufferError)
+                    }
                 }
-                .map(|packet| Packet::Extended { src, dst, packet })
+                typ => Err(Error::UnimplementedType { typ }),
             }
-            _ => Err(Error::UnknownType { typ: self.buf[2] }),
+        } else {
+            Err(Error::BufferError)
         }
     }
 }
 
-// TODO: write test for DevicePing and move each test in it's payload file
 #[cfg(test)]
 mod tests {
-    use crate::{LinkStatistics, Payload, RcChannelsPacked, CRSF_SYNC_BYTE};
+    use super::LinkStatistics;
+    use crate::packet::{DevicePing, ExtendedPacket};
+    use crate::{ExtendedPayload, Packet, PacketAddress, Payload, RcChannelsPacked, CRSF_SYNC_BYTE};
 
     #[test]
-    fn test_rc_channels_packet_dump() {
-        let channels: [u16; 16] = [0x7FF; 16];
-        let packet = RcChannelsPacked(channels);
-
-        let raw = packet.to_raw_packet().unwrap();
+    fn test_rc_channels_packed_dump_and_parse() {
+        let orig = RcChannelsPacked([0x7FF; 16]);
 
+        let raw = orig.to_raw_packet().unwrap();
         let mut expected_data: [u8; 26] = [0xff; 26];
         expected_data[0] = CRSF_SYNC_BYTE;
         expected_data[1] = 24;
         expected_data[2] = 0x16;
         expected_data[25] = 143;
-        assert_eq!(&raw.as_slice(), &expected_data)
+        assert_eq!(raw.as_slice(), expected_data.as_slice());
+
+        let parsed = raw.to_packet().unwrap();
+        assert!(matches!(parsed, Packet::RcChannelsPacked(parsed) if parsed == orig));
     }
 
     #[test]
-    fn test_link_statistics_packet_dump() {
-        let packet = LinkStatistics {
+    fn test_link_statistics_dump_and_parse() {
+        let orig = LinkStatistics {
             uplink_rssi_1: 16,
             uplink_rssi_2: 19,
             uplink_link_quality: 99,
@@ -162,9 +132,26 @@ mod tests {
             downlink_snr: -108,
         };
 
-        let raw = packet.to_raw_packet().unwrap();
-
+        let raw = orig.to_raw_packet().unwrap();
         let expected_data = [CRSF_SYNC_BYTE, 12, 0x14, 16, 19, 99, 151, 1, 2, 3, 8, 88, 148, 252];
-        assert_eq!(raw.as_slice(), expected_data.as_slice())
+        assert_eq!(raw.as_slice(), expected_data.as_slice());
+
+        let parsed = raw.to_packet().unwrap();
+        assert!(matches!(parsed, Packet::LinkStatistics(parsed) if parsed == orig));
+    }
+
+    #[test]
+    fn test_device_ping_dump_and_parse() {
+        let orig = DevicePing;
+
+        let raw = orig
+            .to_raw_packet(PacketAddress::Broadcast, PacketAddress::FlightController)
+            .unwrap();
+        // TODO: maybe check against an expected_data
+
+        let parsed = raw.to_packet().unwrap();
+        assert!(
+            matches!(parsed, Packet::Extended { dst: PacketAddress::Broadcast, src: PacketAddress::FlightController, packet: ExtendedPacket::DevicePing(parsed) } if parsed == orig)
+        );
     }
 }

src/packet/payload/device_ping.rs

@@ -1,42 +1,17 @@
 //! DevicePing packet and related functions/implementations
 
+/// DevicePing payload length
+pub const LEN: usize = 0;
+
 /// Represents a DevicePing packet
 #[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
-#[allow(missing_docs)]
 pub struct DevicePing;
 
-pub const LEN: usize = 0;
-
 /// The raw decoder (parser) for the DevicePing packet.
 pub fn raw_decode(_data: &[u8; LEN]) -> DevicePing {
     DevicePing
 }
 
 /// The raw encoder (serializer) for the DevicePing packet.
 pub fn raw_encode(_device_ping: &DevicePing, _data: &mut [u8; LEN]) {}
-
-impl_extended_payload!(DevicePing, LEN);
-
-#[cfg(test)]
-mod tests {
-    use super::DevicePing;
-    use crate::{AnyPayload, ExtendedPayload, PacketAddress};
-
-    #[test]
-    fn test_device_ping_write_and_parse() {
-        let original = DevicePing;
-
-        let raw = original
-            .to_raw_packet(PacketAddress::Broadcast, PacketAddress::FlightController)
-            .unwrap();
-
-        let data = raw.payload().unwrap();
-        assert!(raw.is_extended());
-        assert_eq!(data.len(), 0);
-
-        let parsed = DevicePing::decode(data).unwrap();
-
-        assert_eq!(parsed, DevicePing);
-    }
-}

src/packet/payload/link_statistics.rs

@@ -1,5 +1,8 @@
 //! LinkStatistics packet and related functions/implementations
 
+/// LinkStatistics payload length
+pub const LEN: usize = 10;
+
 /// Represents a LinkStatistics packet
 #[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
@@ -17,8 +20,6 @@ pub struct LinkStatistics {
     pub downlink_snr: i8,
 }
 
-pub const LEN: usize = 10;
-
 /// The raw decoder (parser) for the LinkStatistics packet.
 pub fn raw_decode(data: &[u8; LEN]) -> LinkStatistics {
     LinkStatistics {
@@ -48,55 +49,3 @@ pub fn raw_encode(link_statistics: &LinkStatistics, data: &mut [u8; LEN]) {
     data[8] = link_statistics.downlink_link_quality;
     data[9] = link_statistics.downlink_snr as u8;
 }
-
-impl_payload!(LinkStatistics, LEN);
-
-#[cfg(test)]
-mod tests {
-    use super::LinkStatistics;
-    use crate::{AnyPayload, Payload};
-
-    #[test]
-    fn test_link_statistics_write_and_parse() {
-        let original = LinkStatistics {
-            uplink_rssi_1: 100,
-            uplink_rssi_2: 98,
-            uplink_link_quality: 100,
-            uplink_snr: -65,
-            active_antenna: 0,
-            rf_mode: 1,
-            uplink_tx_power: 2,
-            downlink_rssi: 120,
-            downlink_link_quality: 98,
-            downlink_snr: -68,
-        };
-
-        let raw = original.to_raw_packet().unwrap();
-
-        let data = raw.payload().unwrap();
-
-        assert_eq!(data[0], 100_u8);
-        assert_eq!(data[1], 98_u8);
-        assert_eq!(data[2], 100_u8);
-        assert_eq!(data[3], -(65_i8) as u8);
-        assert_eq!(data[4], 0_u8);
-        assert_eq!(data[5], 1_u8);
-        assert_eq!(data[6], 2_u8);
-        assert_eq!(data[7], 120_u8);
-        assert_eq!(data[8], 98_u8);
-        assert_eq!(data[9], -(68_i8) as u8);
-
-        let parsed = LinkStatistics::decode(data).unwrap();
-
-        assert_eq!(parsed.uplink_rssi_1, 100);
-        assert_eq!(parsed.uplink_rssi_2, 98);
-        assert_eq!(parsed.uplink_link_quality, 100);
-        assert_eq!(parsed.uplink_snr, -65);
-        assert_eq!(parsed.active_antenna, 0);
-        assert_eq!(parsed.rf_mode, 1);
-        assert_eq!(parsed.uplink_tx_power, 2);
-        assert_eq!(parsed.downlink_rssi, 120);
-        assert_eq!(parsed.downlink_link_quality, 98);
-        assert_eq!(parsed.downlink_snr, -68);
-    }
-}

src/packet/payload/mod.rs

@@ -4,9 +4,6 @@
 use crate::crc8::Crc8;
 use crate::{Error, PacketAddress, PacketType, RawPacket, CRSF_MAX_LEN, CRSF_SYNC_BYTE};
 
-#[macro_use]
-mod macros;
-
 pub mod link_statistics;
 pub use link_statistics::LinkStatistics;
 
@@ -149,3 +146,46 @@ pub trait ExtendedPayload: AnyPayload {
         Ok(raw)
     }
 }
+
+macro_rules! impl_any_payload {
+    ($module:ident, $name:ident) => {
+        impl $crate::packet::payload::AnyPayload for $module::$name {
+            const LEN: usize = $module::LEN;
+
+            fn packet_type(&self) -> $crate::packet::typ::PacketType {
+                $crate::packet::typ::PacketType::$name
+            }
+
+            fn decode(buf: &[u8]) -> Result<Self, $crate::Error> {
+                let data: &[u8; $module::LEN] =
+                    $crate::to_array::ref_array_start(buf).ok_or($crate::Error::BufferError)?;
+                Ok($module::raw_decode(data))
+            }
+
+            fn encode<'a>(&self, buf: &'a mut [u8]) -> Result<&'a [u8], $crate::Error> {
+                let data: &mut [u8; $module::LEN] =
+                    $crate::to_array::mut_array_start(buf).ok_or($crate::Error::BufferError)?;
+                $module::raw_encode(self, data);
+                Ok(data)
+            }
+        }
+    };
+}
+
+macro_rules! impl_payload {
+    ($module:ident, $name:ident) => {
+        impl_any_payload!($module, $name);
+        impl $crate::packet::payload::Payload for $module::$name {}
+    };
+}
+
+macro_rules! impl_extended_payload {
+    ($module:ident, $name:ident) => {
+        impl_any_payload!($module, $name);
+        impl $crate::packet::payload::ExtendedPayload for $module::$name {}
+    };
+}
+
+impl_payload!(link_statistics, LinkStatistics);
+impl_payload!(rc_channels_packed, RcChannelsPacked);
+impl_extended_payload!(device_ping, DevicePing);