rust: Add doc comments for osdp module

And while at it, enhance the public API and clean up some methods here and there. Signed-off-by: Siddharth Chandrasekaran <[email protected]>
goToMain · Nov 29, 2023 · 09b178b · 09b178b
1 parent 12459b8
commit 09b178b
Show file tree

Hide file tree

Showing 17 changed files with 973 additions and 418 deletions.
diff --git a/include/osdp.h b/include/osdp.h
@@ -584,7 +584,7 @@ enum osdp_event_cardread_format_e {
  * @param direction Card read direction of PD 0 - Forward; 1 - Backward
  * @param length Length of card data in bytes or bits depending on `format`
  *        (see note).
- * @param data Card data of `length` bytes or bits bits depending on `format`
+ * @param data Card data of `length` bytes or bits depending on `format`
  *        (see note).
  *
  * @note When `format` is set to OSDP_CARD_FMT_RAW_UNSPECIFIED or

diff --git a/osdpctl/src/config.rs b/osdpctl/src/config.rs
@@ -1,8 +1,7 @@
 use configparser::ini::Ini;
 use libosdp::{
     channel::{unix_channel::UnixChannel, OsdpChannel},
-    pdinfo::{OsdpFlag, PdInfo},
-    pdid::PdId,
+    OsdpFlag, PdInfo, PdId,
     pdcap::PdCapability,
 };
 use std::{
@@ -141,13 +140,11 @@ impl CpConfig {
             .iter()
             .map(|d| {
                 let stream = UnixChannel::new(&d.channel)?;
-                Ok(PdInfo::new(
+                Ok(PdInfo::for_cp(
                     &self.name,
                     d.address,
                     9600,
                     d.flags,
-                    PdId::default(),
-                    vec![],
                     OsdpChannel::new::<UnixChannel>(Box::new(stream)),
                     d.key_store.load()?,
                 ))
@@ -171,13 +168,15 @@ pub struct PdConfig {
 
 impl PdConfig {
     pub fn from(config: &Ini) -> Result<Self> {
-        let pd_id = PdId {
-            version: config.getuint("PdId", "Version").unwrap().unwrap() as i32,
-            model: config.getuint("PdId", "Model").unwrap().unwrap() as i32,
-            vendor_code: config.getuint("PdId", "VendorCode").unwrap().unwrap() as u32,
-            serial_number: config.getuint("PdId", "SerialNumber").unwrap().unwrap() as u32,
-            firmware_version: config.getuint("PdId", "FirmwareVersion").unwrap().unwrap() as u32,
-        };
+        // TODO: Fix this
+        let pd_id = PdId::from_number(1);
+        // let pd_id = PdId {
+        //     version: config.getuint("PdId", "Version").unwrap().unwrap() as i32,
+        //     model: config.getuint("PdId", "Model").unwrap().unwrap() as i32,
+        //     vendor_code: config.getuint("PdId", "VendorCode").unwrap().unwrap() as u32,
+        //     serial_number: config.getuint("PdId", "SerialNumber").unwrap().unwrap() as u32,
+        //     firmware_version: config.getuint("PdId", "FirmwareVersion").unwrap().unwrap() as u32,
+        // };
         let mut flags = OsdpFlag::empty();
         if let Some(val) = config.get("default", "Flags") {
             let fl: Vec<&str> = val.split('|').collect();
@@ -216,7 +215,7 @@ impl PdConfig {
 
     pub fn pd_info(&self) -> Result<PdInfo> {
         let stream = UnixChannel::new(&self.channel)?;
-        Ok(PdInfo::new(
+        Ok(PdInfo::for_pd(
             &self.name,
             self.address,
             9600,

diff --git a/rust/Cargo.toml b/rust/Cargo.toml
@@ -23,6 +23,7 @@ env_logger = "0.10.0"
 lazy_static = "1.4.0"
 log = "0.4.20"
 multiqueue = "0.3.2"
+once_cell = "1.18.0"
 serde = { version = "1.0.192", features = ["derive"] }
 serde_with = "3.4.0"
 thiserror = "1.0.50"
diff --git a/rust/examples/cp.rs b/rust/examples/cp.rs
@@ -5,9 +5,8 @@ use std::{
 };
 use libosdp::{
     cp::ControlPanel,
-    pdinfo::{PdInfo, OsdpFlag},
+    PdInfo, OsdpFlag, PdId, OsdpError,
     channel::{OsdpChannel, unix_channel::UnixChannel},
-    error::OsdpError, pdid::PdId,
 };
 
 fn main() -> Result<(), OsdpError> {
@@ -18,18 +17,10 @@ fn main() -> Result<(), OsdpError> {
         .init();
     let stream = UnixChannel::connect("conn-1")?;
     let pd_info = vec![
-        PdInfo::new(
+        PdInfo::for_cp(
             "PD 101", 101,
             115200,
             OsdpFlag::EnforceSecure,
-            PdId {
-                version: 0x01,
-                model: 0x02,
-                vendor_code: 0x03,
-                serial_number: 0x04,
-                firmware_version: 0x05,
-            },
-            vec![],
             OsdpChannel::new::<UnixChannel>(Box::new(stream)),
             [
                 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,

diff --git a/rust/examples/pd.rs b/rust/examples/pd.rs
@@ -5,11 +5,9 @@ use std::{
 };
 use libosdp::{
     pd::PeripheralDevice,
-    pdinfo::{PdInfo, OsdpFlag},
+    PdInfo, OsdpFlag, OsdpError, PdId,
     channel::{OsdpChannel, unix_channel::UnixChannel},
-    error::OsdpError,
-    pdid::PdId,
-    pdcap::{PdCapability, PdCapEntry},
+    pdcap::{PdCapability, PdCapEntity},
 };
 
 fn main() -> Result<(), OsdpError> {
@@ -19,20 +17,14 @@ fn main() -> Result<(), OsdpError> {
         .format_timestamp(None)
         .init();
     let stream = UnixChannel::new("conn-1")?;
-    let pd_info =  PdInfo::new(
+    let pd_info =  PdInfo::for_pd(
         "PD 101",
         101,
         115200,
         OsdpFlag::EnforceSecure,
-        PdId {
-            version: 0x01,
-            model: 0x02,
-            vendor_code: 0x03,
-            serial_number: 0x04,
-            firmware_version: 0x05,
-        },
+        PdId::from_number(101),
         vec![
-            PdCapability::CommunicationSecurity(PdCapEntry::new(1, 1)),
+            PdCapability::CommunicationSecurity(PdCapEntity::new(1, 1)),
         ],
         OsdpChannel::new::<UnixChannel>(Box::new(stream)),
         [

diff --git a/rust/src/channel/mod.rs b/rust/src/channel/mod.rs
@@ -1,3 +1,16 @@
+//! The OSDP specification defines that communication between OSDP devices
+//! happen over an RS-485 connection. For testing and development purpose this
+//! can be limiting so LibOSDP defines a notion called "Channel" which is a
+//! software representation (abstraction) of the physical transport medium.
+//!
+//! Since RS-485 is stream based protocol, we can think of it to be  something
+//! that we can read from and write to (which in turn is Read and Write traits
+//! in rust). This allows us to run OSDP devices over many IPC schemes such as
+//! Unix socket and message queues.
+//!
+//! This module provides a way to define an OSDP channel and export it to
+//! LibOSDP.
+
 pub mod unix_channel;
 
 use crate::osdp_sys;
@@ -58,19 +71,32 @@ unsafe extern "C" fn raw_flush(data: *mut c_void) {
     let _ = stream.flush();
 }
 
-pub trait Channel: Read + Write + Sync + Send {
+/// The Channel trait acts as an interface for all channel implementors. See
+/// module description for the definition of a "channel" in LibOSDP.
+pub trait Channel: Read + Write + Send + Sync {
+    /// Since OSDP channels can be multi-drop (more than one PD can talk to a
+    /// CP on the same channel) and LibOSDP supports mixing multi-drop
+    /// connections among PDs it needs a way to identify each unique channel by
+    /// a channel ID. Implementors of this trait must also provide a method
+    /// which returns a unique i32 per channel.
     fn get_id(&self) -> i32;
 }
 
+/// A wrapper to hold anything that implements the Channel trait. It is used to
+/// export the inner value to LibOSDP in a format that it expects it to be in.
 pub struct OsdpChannel {
     stream: Arc<Mutex<Box<dyn Channel>>>,
 }
 
 impl OsdpChannel {
+    /// Create an instance of OsdpChannel for a given object that implements
+    /// the Channel Trait.
     pub fn new<T: Channel>(stream: Box<dyn Channel>) -> OsdpChannel {
         Self { stream: Arc::new(Mutex::new(stream)) }
     }
 
+    /// For internal use; in as_struct() of [`crate::PdInfo`]. This methods
+    /// exports the channel to LibOSDP as a C struct.
     pub fn as_struct(&self) -> osdp_sys::osdp_channel {
         let stream = self.stream.clone();
         let id = stream.lock().unwrap().get_id();
@@ -85,7 +111,7 @@ impl OsdpChannel {
     }
 }
 
-pub fn str_to_channel_id(key: &str) -> i32 {
+fn str_to_channel_id(key: &str) -> i32 {
     let mut hasher = DefaultHasher::new();
     key.hash(&mut hasher);
     let mut id = hasher.finish();

diff --git a/rust/src/channel/unix_channel.rs b/rust/src/channel/unix_channel.rs
@@ -1,3 +1,5 @@
+//! OSDP unix channel
+
 use super::Channel;
 use std::{
     io::{Read, Write},
@@ -6,22 +8,25 @@ use std::{
     str::FromStr,
 };
 
-type Result<T> = std::result::Result<T, crate::error::OsdpError>;
+type Result<T> = std::result::Result<T, crate::OsdpError>;
 
+/// A reference OSDP channel implementation for unix domain socket.
 #[derive(Debug)]
 pub struct UnixChannel {
     id: i32,
     stream: UnixStream,
 }
 
 impl UnixChannel {
+    /// Connect to a channel identified by `name`.
     pub fn connect(name: &str) -> Result<Self> {
         let path = format!("/tmp/osdp-{name}");
         let id = super::str_to_channel_id(&path);
         let stream = UnixStream::connect(&path)?;
         Ok(Self { id, stream })
     }
 
+    /// Listen on a channel identified by `name`.
     pub fn new(name: &str) -> Result<Self> {
         let path = format!("/tmp/osdp-{name}");
         let id = super::str_to_channel_id(&path);