From d3baaa081db266a8ff5854bbb8445da077905619 Mon Sep 17 00:00:00 2001
From: Daniel Trnka <daniel.trnka@gmail.com>
Date: Mon, 7 Oct 2024 22:27:34 +0200
Subject: [PATCH] Pid::new returns Result instead of asserting in case of an
 error

The Pid::new() function can be used, for example, with untrusted
PID received from master without causing a panic.

This patch may introduce a breaking change, but it can be addressed
with a simple fix by checking the Result.
---
 CHANGELOG.md  |  4 ++++
 src/frame.rs  | 15 ++++++++-------
 src/master.rs | 19 +++++++++++--------
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index eeaa4f1..a9e89df 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 This project follows [semantic versioning](https://semver.org/).
 
+## [Unreleased]
+ * breaking: `Pid::new` returns `Result` instead of asserting in case of an error.
+   ([#37](https://github.com/Sensirion/lin-bus-rs/pull/37))
+
 ## [0.4.0] (2021-12-16)
 
  * changed: `PID::new`, `PID::from_id`, `PCI::new_sf` and `PCI::get_type` are
diff --git a/src/frame.rs b/src/frame.rs
index 46c156f..b92e65e 100644
--- a/src/frame.rs
+++ b/src/frame.rs
@@ -13,11 +13,13 @@ pub struct PID(u8);
 
 impl PID {
     /// Creates a new PID object with given PID
-    pub const fn new(pid: u8) -> PID {
-        // check that the given PID has valid parity bits
+    pub const fn new(pid: u8) -> Result<PID, &'static str> {
         let correct_pid = PID::from_id(pid & 0b0011_1111);
-        assert!(correct_pid.0 == pid, "Invalid PID");
-        correct_pid
+        if correct_pid.0 == pid {
+            Ok(correct_pid)
+        } else {
+            Err("Invalid parity bits")
+        }
     }
 
     /// Calculate the PID from an ID.
@@ -414,14 +416,13 @@ mod tests {
         ];
 
         for d in &test_data {
-            assert_eq!(d.0, d.1.get());
+            assert_eq!(d.0, d.1.unwrap().get());
         }
     }
 
     #[test]
-    #[should_panic]
     fn test_invalid_pid_new() {
-        PID::new(0x07);
+        assert_eq!(Err("Invalid parity bits"), PID::new(0x07));
     }
 
     #[test]
diff --git a/src/master.rs b/src/master.rs
index 51275d1..ca35404 100644
--- a/src/master.rs
+++ b/src/master.rs
@@ -63,10 +63,10 @@ mod tests {
     #[test]
     fn test_frame_from_data() {
         let test_data = [FrameTestData {
-            pid: PID::new(0xDD),
+            pid: PID::new(0xDD).unwrap(),
             data: &[0x01],
             frame: Frame {
-                pid: PID::new(0xDD),
+                pid: PID::new(0xDD).unwrap(),
                 buffer: [0x01, 0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00],
                 data_length: 1,
             },
@@ -84,22 +84,25 @@ mod tests {
     fn test_data_decode() {
         let test_data = [
             (
-                Frame::from_data(PID::new(80), &[254, 251, 239, 255]),
+                Frame::from_data(PID::new(80).unwrap(), &[254, 251, 239, 255]),
                 [1022, 1022, 2046],
             ),
             (
-                Frame::from_data(PID::new(80), &[3, 12, 240, 182]),
+                Frame::from_data(PID::new(80).unwrap(), &[3, 12, 240, 182]),
                 [3, 3, 879],
             ),
             (
-                Frame::from_data(PID::new(80), &[3, 12, 0, 183]),
+                Frame::from_data(PID::new(80).unwrap(), &[3, 12, 0, 183]),
                 [3, 3, 880],
             ),
             (
-                Frame::from_data(PID::new(80), &[2, 12, 240, 182]),
+                Frame::from_data(PID::new(80).unwrap(), &[2, 12, 240, 182]),
                 [2, 3, 879],
             ),
-            (Frame::from_data(PID::new(80), &[2, 8, 0, 183]), [2, 2, 880]),
+            (
+                Frame::from_data(PID::new(80).unwrap(), &[2, 8, 0, 183]),
+                [2, 2, 880],
+            ),
         ];
 
         for d in &test_data {
@@ -111,7 +114,7 @@ mod tests {
 
     #[test]
     fn test_data_decode_all_bits() {
-        let frame = Frame::from_data(PID::new(80), &[0x55, 0xDD]);
+        let frame = Frame::from_data(PID::new(80).unwrap(), &[0x55, 0xDD]);
         assert_eq!(frame.decode::<u16>(0, 16), 0xdd55);
     }
 }