complete peakcan driver (#28)

* complete peakcan driver

builds, driver works, no functional test (no pcan available)

* fix clippy suggestion

* other clippy suggestions

* rustfmt

---------

Co-authored-by: Jannes Brands <[email protected]>

Cargo.toml

@@ -8,9 +8,9 @@ keywords = ["agriculture", "can", "canbus", "isobus", "j1939", "agritech", "smar
 
 [dependencies]
 rand = "0.8.5"
-pcan-basic = { version = "1.0.2", optional = true }
-socketcan = { version = "2.0.0", optional = true }
 tracing = { version = "0.1.37", optional = true }
+socketcan = { version = "2.0.0", optional = true }
+pcan-basic = { version = "1.0.2", optional = true }
 
 [features]
 default = []
@@ -20,12 +20,12 @@ socketcan = ["dep:socketcan"]
 tracing = ["dep:tracing"]
 # Peak driver
 peak = ["dep:pcan-basic"]
-
 [dev-dependencies]
 clap = { version = "4.3.19", features = ["derive"] }
 ctrlc = "3.4.0"
+# TODO: Add optional tracing to the main library
+tracing = "0.1.37"
 tracing-subscriber = "0.3.17"
-
 [[example]]
 name = "forward"
-required-features = ["tracing"]
+required-features = ["tracing"]

examples/forward.rs

@@ -81,7 +81,7 @@ fn create_driver(iface: &str, driver: CanDriver) -> Box<dyn Driver> {
         CanDriver::Pcan => {
             let bus = parse_usb_bus(iface).unwrap();
             let baud = ag_iso_stack::driver::Baudrate::Baud250K;
-            Box::new(PeakDriver::new(bus, baud))
+            Box::new(PeakDriver::new_usb(bus, baud))
         }
         #[allow(unreachable_patterns)]
         _ => unreachable!(),
@@ -98,6 +98,8 @@ fn main() {
         .map_err(|_err| eprintln!("Unable to set global default subscriber"))
         .unwrap();
 
+    tracing::info!("AgIsoStack-rs example starts...");
+
     tracing::info!(
         "Forwarding CAN traffic from {} to {}",
         opts.input_interface,

src/driver/address.rs

@@ -5,7 +5,11 @@
 pub struct Address(pub u8);
 
 impl Address {
+    /// Address representing broadcasts for destination specific PGNs
     pub const GLOBAL: Address = Address(0xFF);
+    /// Alias for the global address
+    pub const BROADCAST: Address = Address(0xFF);
+    /// The null address is used by ECUs without an address such as during address claiming
     pub const NULL: Address = Address(0xFE);
 }
 

src/driver/can_id.rs

@@ -49,6 +49,14 @@ pub enum Type {
     Extended = 0x1,
 }
 
+#[derive(Debug, Clone)]
+pub struct EncodingError {
+    pub priority: Priority,
+    pub parameter_group_number: Pgn,
+    pub source_address: Address,
+    pub destination_address: Address,
+}
+
 #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)]
 #[repr(transparent)]
 pub struct CanId(u32);
@@ -71,6 +79,63 @@ impl CanId {
         Self(raw)
     }
 
+    /// Encodes a new extended ID using the discrete parts of an identifier
+    pub fn try_encode(
+        parameter_group_number: Pgn,
+        source_address: Address,
+        destination_address: Address,
+        priority: Priority,
+    ) -> Result<CanId, EncodingError> {
+        if destination_address != Address::GLOBAL && parameter_group_number.is_broadcast() {
+            return Err(EncodingError {
+                priority,
+                parameter_group_number,
+                source_address,
+                destination_address,
+            });
+        }
+        Ok(unsafe {
+            CanId::encode_unchecked(
+                parameter_group_number,
+                source_address,
+                destination_address,
+                priority,
+            )
+        })
+    }
+
+    /// Encodes a new extended ID using the discrete parts of an identifier but won't validate
+    /// your combination of PGN and destination address.
+    ///
+    /// # Safety
+    /// Calling this without validating your PGN and destination address combination may result in your PGN field
+    /// getting trashed. Specifically, the risk is when you are using a broadcast PGN but supply a non-0xFF
+    /// destination address.
+    pub unsafe fn encode_unchecked(
+        parameter_group_number: Pgn,
+        source_address: Address,
+        destination_address: Address,
+        priority: Priority,
+    ) -> CanId {
+        let mut raw_id: u32 = 0;
+
+        raw_id |= (priority as u32 & 0x07) << 26;
+        raw_id |= source_address.0 as u32;
+
+        if Address::GLOBAL == destination_address {
+            if (parameter_group_number.raw() & 0xF000) >= 0xF000 {
+                raw_id |= (parameter_group_number.raw() & 0x3FFFF) << 8;
+            } else {
+                raw_id |= (destination_address.0 as u32) << 8;
+                raw_id |= (parameter_group_number.raw() & 0x3FF00) << 8;
+            }
+        } else if (parameter_group_number.raw() & 0xF000) < 0xF000 {
+            raw_id |= (destination_address.0 as u32) << 8;
+            raw_id |= (parameter_group_number.raw() & 0x3FF00) << 8;
+        }
+        CanId::new(raw_id & CAN_EFF_MASK, Type::Extended)
+    }
+
     /// Get the raw value of the CAN ID
     #[inline]
     pub fn raw(&self) -> u32 {
@@ -188,4 +253,45 @@ mod tests {
         let can_id = CanId::new(0x18EEFF1C, Type::Extended);
         assert_eq!(can_id.pgn(), Pgn::from_raw(0x0EE00));
     }
+
+    #[test]
+    fn test_encode() {
+        let encode_result = CanId::try_encode(
+            Pgn::from_raw(0x00EF00),
+            Address(0x81),
+            Address(0xF9),
+            Priority::Six,
+        );
+        let can_id = encode_result.expect("EF00 Message was not encodable");
+        assert_eq!(can_id.pgn(), Pgn::from_raw(0xEF00));
+        assert_eq!(can_id.destination_address(), Address(0xF9));
+        assert_eq!(can_id.source_address(), Address(0x81));
+        assert_eq!(can_id.priority(), Priority::Six);
+
+        let encode_result = CanId::try_encode(
+            Pgn::from_raw(0x00FF40),
+            Address(0x81),
+            Address(0xFF),
+            Priority::Six,
+        );
+        let can_id = encode_result.expect("FF40 Message was not encodable");
+        assert_eq!(can_id.pgn(), Pgn::from_raw(0xFF40));
+        assert_eq!(can_id.destination_address(), Address(0xFF));
+        assert_eq!(can_id.source_address(), Address(0x81));
+        assert_eq!(can_id.priority(), Priority::Six);
+
+        let encode_result = CanId::try_encode(
+            Pgn::from_raw(0x00FF40),
+            Address(0x81),
+            Address(0x0F),
+            Priority::Six,
+        );
+        assert!(matches!(encode_result.is_err(), true));
+
+        let error_contents: EncodingError = encode_result.unwrap_err();
+        assert_eq!(error_contents.priority, Priority::Six);
+        assert_eq!(error_contents.source_address, Address(0x81));
+        assert_eq!(error_contents.destination_address, Address(0x0F));
+        assert_eq!(error_contents.parameter_group_number, Pgn::from_raw(0xFF40));
+    }
 }

src/driver/driver.rs

@@ -6,10 +6,6 @@ use crate::driver::Frame;
 pub enum DriverOpenError {
     /// The driver failed to open with filesystem semantics
     IoError(std::io::Error),
-    // TODO: Here and throughout. I don't love the pcan errors. They're not real std::error::Error
-    // types, and it's not obvious what they mean. Maybe we should re-think this error design?
-    #[cfg(feature = "peak")]
-    PeakError(pcan_basic::error::PcanError),
 }
 
 impl std::fmt::Display for DriverOpenError {
@@ -21,10 +17,6 @@ impl std::error::Error for DriverOpenError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match &self {
             DriverOpenError::IoError(e) => Some(e),
-            // PcanError doesn't implement the Error trait
-            // DriverOpenError::PeakError(e) => Some(e),
-            #[allow(unreachable_patterns)]
-            _ => None,
         }
     }
 }
@@ -57,8 +49,6 @@ pub enum DriverReadError {
     ErrorFrame(),
     /// The driver failed to read with filesystem semantics
     IoError(std::io::Error),
-    #[cfg(feature = "peak")]
-    PeakError(pcan_basic::error::PcanError),
 }
 
 impl std::fmt::Display for DriverReadError {
@@ -89,8 +79,6 @@ pub enum DriverWriteError {
     BusError(),
     /// Some fault with filesystem semantics
     IoError(std::io::Error),
-    #[cfg(feature = "peak")]
-    PeakError(pcan_basic::error::PcanError),
 }
 
 impl std::fmt::Display for DriverWriteError {

src/driver/mod.rs

@@ -12,19 +12,22 @@ mod driver;
 mod frame;
 mod pgn;
 
-#[cfg(feature = "peak")]
-mod peak;
-
 #[cfg(feature = "socketcan")]
 mod socketcan;
 
+#[cfg(feature = "peak")]
+mod peak;
+
 pub use address::Address;
 pub use can_id::{CanId, Priority, Type};
 pub use driver::{Driver, DriverCloseError, DriverOpenError, DriverReadError, DriverWriteError};
 pub use frame::{Channel, Frame};
 pub use pgn::Pgn;
 
-#[cfg(feature = "peak")]
-pub use self::peak::{Baudrate, PeakDriver};
 #[cfg(feature = "socketcan")]
 pub use self::socketcan::SocketcanDriver;
+
+#[cfg(feature = "peak")]
+pub use self::peak::Baudrate;
+#[cfg(feature = "peak")]
+pub use self::peak::PeakDriver;