rust-protocol: Let decode functions return errors for easier debugging.

base-station/software/src/radio.rs

@@ -161,11 +161,14 @@ impl<'a> Radio<'a> {
 
         // Decode the packet.
         let packet = match Packet::decrypt_and_decode(key, payload) {
-            None => {
-                info!("invalid packet from device {:02x}", device_id);
+            Err(e) => {
+                info!(
+                    "invalid packet from device {:02x}, error {:?}",
+                    device_id, e
+                );
                 return Ok(());
             }
-            Some(p) => p,
+            Ok(p) => p,
         };
         // TODO: For all packets except for salt requests, check whether the
         // salt was incremented to prevent replay attacks.

common/rust-protocol/src/lib.rs

@@ -17,12 +17,12 @@ pub enum Packet {
 }
 
 impl Packet {
-    pub fn decrypt_and_decode(key: &[u8], data: &mut [u8]) -> Option<Packet> {
+    pub fn decrypt_and_decode(key: &[u8], data: &mut [u8]) -> Result<Packet, Error> {
         decrypt_cbc(key, data);
         let checksummed = &data[8..];
         let remainder = State::<KERMIT>::calculate(&checksummed);
         if remainder != 0 {
-            return None;
+            return Err(Error::CRC);
         }
         Self::decode(&checksummed[..22])
     }
@@ -38,21 +38,21 @@ impl Packet {
         true
     }
 
-    fn decode(data: &[u8]) -> Option<Packet> {
+    fn decode(data: &[u8]) -> Result<Packet, Error> {
         let type_ = data[0] & 0x1f;
         // count can be at most 7, so we do not need any checks when indexing 8-element arrays
         // below.
         let count = data[0] >> 5;
         match type_ {
-            0 => Some(Self::GetSalt),
-            1 => Some(Self::Salt({
+            0 => Ok(Self::GetSalt),
+            1 => Ok(Self::Salt({
                 // The lowest 8 bit of the salt are the device ID and are filled in by the caller.
                 u64::from_le_bytes(data[1..9].try_into().unwrap()) << 8
             })),
-            2 => Some(Self::Report(Report::decode(count, &data[1..])?)),
-            3 => Some(Self::GetValues(GetValues::decode(count, &data[1..])?)),
-            4 => Some(Self::Values(Values::decode(count, &data[1..])?)),
-            _ => None,
+            2 => Ok(Self::Report(Report::decode(count, &data[1..])?)),
+            3 => Ok(Self::GetValues(GetValues::decode(count, &data[1..])?)),
+            4 => Ok(Self::Values(Values::decode(count, &data[1..])?)),
+            _ => Err(Error::InvalidPacketType),
         }
     }
 
@@ -90,7 +90,7 @@ pub struct Report {
 }
 
 impl Report {
-    fn decode(count: u8, mut data: &[u8]) -> Option<Report> {
+    fn decode(count: u8, mut data: &[u8]) -> Result<Report, Error> {
         let mut report = Self {
             count,
             values: [Value::Invalid; 8],
@@ -98,7 +98,7 @@ impl Report {
         for i in 0..count {
             report.values[i as usize] = Value::decode(&mut data)?;
         }
-        Some(report)
+        Ok(report)
     }
 
     fn encode(&self, mut data: &mut [u8]) -> bool {
@@ -124,9 +124,9 @@ pub struct GetValues {
 }
 
 impl GetValues {
-    fn decode(_count: u8, _data: &[u8]) -> Option<GetValues> {
+    fn decode(_count: u8, _data: &[u8]) -> Result<GetValues, Error> {
         // TODO
-        None
+        Err(Error::InvalidPacketType)
     }
 
     fn encode(&self, mut _data: &[u8]) -> bool {
@@ -143,9 +143,9 @@ pub struct Values {
 }
 
 impl Values {
-    fn decode(_count: u8, _data: &[u8]) -> Option<Values> {
+    fn decode(_count: u8, _data: &[u8]) -> Result<Values, Error> {
         // TODO
-        None
+        Err(Error::InvalidPacketType)
     }
 
     fn encode(&self, mut _data: &[u8]) -> bool {
@@ -173,27 +173,27 @@ pub enum Value {
 }
 
 impl Value {
-    fn decode(data: &mut &[u8]) -> Option<Value> {
+    fn decode(data: &mut &[u8]) -> Result<Value, Error> {
         let type_ = data[0];
         let length = match type_ {
             0 => 9,
             1 => 3,
             2 => 5,
             3 => 3,
-            _ => return None,
+            _ => return Err(Error::InvalidValueType),
         };
         if data.len() < length {
-            return None;
+            return Err(Error::TooShort);
         }
         let result = match type_ {
             0 => Self::Time(u64::from_le_bytes(data[1..9].try_into().unwrap())),
             1 => Self::Temperature(i16::from_le_bytes(data[1..3].try_into().unwrap())),
             2 => Self::Pressure(u32::from_le_bytes(data[1..5].try_into().unwrap())),
             3 => Self::Humidity(u16::from_le_bytes(data[1..3].try_into().unwrap())),
-            _ => return None,
+            _ => return Err(Error::InvalidValueType),
         };
         *data = &data[length..];
-        Some(result)
+        Ok(result)
     }
 
     fn encode(&self, data: &mut [u8]) -> bool {
@@ -263,3 +263,12 @@ fn decrypt_cbc(key: &[u8], data: &mut [u8]) {
         }
     }
 }
+
+#[derive(Debug, Copy, Clone)]
+pub enum Error {
+    CRC,
+    InvalidPacketType,
+    InvalidValueType,
+    InvalidLocation,
+    TooShort,
+}