Cargo.lock

@@ -483,7 +483,7 @@ checksum = "212d0f5754cb6769937f4501cc0e67f4f4483c8d2c3e1e922ee9edbe4ab4c7c0"
 
 [[package]]
 name = "disco_accuracy_over_life"
-version = "5.0.0-alpha.1"
+version = "5.0.0"
 dependencies = [
  "async-std",
  "chrono",
@@ -499,6 +499,7 @@ dependencies = [
  "rppal",
  "rust-ini 0.19.0",
  "serialport",
+ "signal-hook",
  "time 0.2.27",
 ]
 
@@ -1251,6 +1252,25 @@ dependencies = [
  "digest",
 ]
 
+[[package]]
+name = "signal-hook"
+version = "0.3.17"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8621587d4798caf8eb44879d42e56b9a93ea5dcd315a6487c357130095b62801"
+dependencies = [
+ "libc",
+ "signal-hook-registry",
+]
+
+[[package]]
+name = "signal-hook-registry"
+version = "1.4.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d8229b473baa5980ac72ef434c4415e70c4b5e71b423043adb4ba059f89c99a1"
+dependencies = [
+ "libc",
+]
+
 [[package]]
 name = "slab"
 version = "0.4.8"

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "disco_accuracy_over_life"
-version = "5.0.0-alpha.1"
+version = "5.0.0"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
@@ -20,6 +20,7 @@ clap = { version = "4.3.2", features = ["derive"] }
 futures = "0.3.28"
 async-std = "1.12.0"
 glob = "0.3.1"
+signal-hook = "0.3.17"
 
 [dev-dependencies]
 time = "0.2.23"

src/gpio_facade.rs

@@ -6,48 +6,99 @@ use std::result::Result;
 use futures::executor;
 use std::fmt;
 
+//10ms delay
 const POLL_DELAY:Duration = Duration::from_millis(10);
+
+//address of the motor enable pin
 const MOTOR_ENABLE_ADDR:u8 = 22;
+
+//address of the motor direction pin
 const MOTOR_DIRECTION_ADDR:u8 = 27;
+
+//Address of the air cylinder pin; used to press the button on the disco
 const PISTON_ADDR:u8 = 25;
+
+//address of the physical run switch on the fixture
+//Used to interrupt fixture movement
 const RUN_SWITCH_ADDR:u8 = 10;
+
+//Raspberry Pi GPIO is a little finicky. 
+//Active-high pins can occasionally drift high.
+//Active-low portion of the switches are used as checks on the active-high pins.
+//------
+//address of Upper limit switch 
 const UPPER_LIMIT_ADDR:u8 = 23;
+
+//Address of upper limit switch 
 const UPPER_NC_LIMIT_ADDR:u8 = 5;
+
+//address of lower limit switch 
 const LOWER_LIMIT_ADDR:u8 = 24;
+
+//Address of lower limit switch 
 const LOWER_NC_LIMIT_ADDR:u8 = 6;
 
+//Boolean used to store whether the fixture is safe to move.
+//This is stored in a RwLock to ensure that it is modifiable across threads
 static MOVE_LOCK:RwLock<bool> = RwLock::new(true);
 
+//Fixture struct definition
 pub struct Fixture{
     gpio_api:Gpio,
+    //Motor Direction: 
+    //  Low = fixture travels down
+    //  High = fixture travels up
     motor_direction:Option<OutputPin>,
+    //Motor Enable: 
+    //  Low = fixture doesn't travel
+    //  High = fixture travels
     motor_enable: Option<OutputPin>,
+    //Piston enable:
+    //  Low = Piston retracted; button is not pressed
+    //  high = piston extended; button is pressed
     piston_enable: Option<OutputPin>,
+    //Upper Limit switch [active high]:
+    //  Low = Upper limit switch is triggered
+    //  high = upper limit switch has not been triggered yet
     upper_limit: Option<InputPin>,
+    //Upper Limit switch [active low]:
+    //  Low = upper limit switch has not been triggered yet
+    //  high = Upper limit switch is triggered
     upper_nc_limit: Option<InputPin>,
+    //Lower Limit switch [active high]:
+    //  Low = Lower limit switch is triggered
+    //  high = Lower limit switch has not been triggered yet
     lower_limit: Option<InputPin>,
+    //Lower Limit switch [active low]:
+    //  Low = Lower limit switch has not been triggered yet
+    //  high = Lower limit switch is triggered
     lower_nc_limit: Option<InputPin>,
 }
 
+//Possible fixture movement directions
 pub enum Direction{Up,Down}
 
+//Reset arm on close
 impl Drop for Fixture{
     fn drop(&mut self) {
         self.reset_arm();
     }
 }
 
+//Custom error for initialisation
 #[derive(Debug,Clone)]
 pub struct FixtureInitError;
 
+//to_string for the above error
 impl fmt::Display for FixtureInitError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "Invalid fixture PWM value")
     }
 }
 
+//Future Improvement: modify implementation to allow for multiple fixtures simultaneously
 impl Fixture{
-    //modify implementation to allow for multiple fixtures simultaneously
+    //Fixture Constructor
     pub fn new() -> Result<Self,FixtureInitError>{
         let possible_gpio = Gpio::new();
         if let Ok(gpio) = possible_gpio{
@@ -62,6 +113,8 @@ impl Fixture{
                 lower_nc_limit:None
             };
 
+            //Output [control] pin ceation; initialise all as low [off]
+            //-------------
             match output.gpio_api.get(MOTOR_ENABLE_ADDR){
                 Ok(pin) =>{
                     output.motor_enable = Some(pin.into_output_low());
@@ -89,6 +142,9 @@ impl Fixture{
                     return Err(FixtureInitError)
                 }
             }
+
+            //Input [sense] pin creation; initialise all as active-high
+            //-------------
             match output.gpio_api.get(UPPER_LIMIT_ADDR){
                 Ok(pin) =>{
                     output.upper_limit = Some(pin.into_input_pulldown());
@@ -126,9 +182,12 @@ impl Fixture{
                 }
             }
 
+            //Initialise run switch as sense pin as active-high
             match output.gpio_api.get(RUN_SWITCH_ADDR){
                 Ok(run_pin) =>{
+                    //Use ethe switch as an asynchronous interrupt
                     _ = run_pin.into_input_pulldown().set_async_interrupt(Trigger::Both, |switch_state|{
+                        //Block fixture movement when the run switch is low
                         let mut move_allowed = executor::block_on(MOVE_LOCK.write());
                         match switch_state {
                             Level::Low=> {
@@ -147,44 +206,61 @@ impl Fixture{
             }
             log::info!("GPIO initialised successfully! Finding fixture travel distance.");
 
+            //If GPIO is completely initialised properly, reset the arm to the top of 
+            //the fixture, then test the fixture's range of motion, before returning the fixture
+            //object
             output.reset_arm();
             output.goto_limit(Direction::Down);
             output.goto_limit(Direction::Up);
             return Ok(output);
 
-        }
+        } // end if Ok(gpio) = possible_gpio
         else { 
+            //Most errors can be resolved by running the software as root
             log::error!("Gpio could not be opened! Did you run with 'sudo'? ");
             return Err(FixtureInitError) 
         }
     }
 
+    //Function to reset the arm
+    //returns how many polls it took to reset [polled at 10ms intervals]
+    //returns 0 if fixture could not be triggered
+    //Note: Polling can probably be removed at this time
     fn reset_arm(&mut self) -> u16{
         log::debug!("Resetting arm...");
         if let (Some(upper_limit),Some(upper_nc_limit),          Some(motor_direction_pin),      Some(motor_enable_pin)) = 
                (self.upper_limit.as_mut(),self.upper_nc_limit.as_mut(), self.motor_direction.as_mut(), self.motor_enable.as_mut()){
             log::trace!("Upper limit: {}",upper_limit.is_high());
             log::trace!("Upper NC limit: {}",upper_nc_limit.is_high());
+            //If the fixture believes it is at the top of the fixture, send the fixture down
+            //briefly to re-confirm it is at the top
             if upper_limit.is_high(){
                 {
+                    //Wait until run switch says its safe to go
                     while !*executor::block_on(MOVE_LOCK.read()) {log::trace!("blocking!");}
-                    motor_direction_pin.set_low();//
+                    motor_direction_pin.set_low();
                     motor_enable_pin.set_high()
                 }
+                //Stop the motor once its traveled for 0.5s
                 thread::sleep(Duration::from_millis(500));
                 motor_enable_pin.set_low()
             }
+            //Once its safe, start travelling up
             while !*executor::block_on(MOVE_LOCK.read()){log::trace!("blocking!");}
-            motor_direction_pin.set_high();//
+            motor_direction_pin.set_high();
             motor_enable_pin.set_high();
             let mut counter = 0;
+            //Every 10ms, check if the fixture is done travelling
             while upper_limit.is_low() && upper_nc_limit.is_high() {
+                //If the run switch is flipped during movement, immediately pause
                 while !*executor::block_on(MOVE_LOCK.read()){
                     motor_enable_pin.set_low();
                 }
+                //Recover once the run switch is reset
                 if *executor::block_on(MOVE_LOCK.read()) && motor_enable_pin.is_set_low(){
                     motor_enable_pin.set_high();
                 }
+                //This probably shouldn't be logging
                 if upper_limit.is_high() && upper_nc_limit.is_low() { 
                     log::trace!("Breaking early!");
                     break; 
@@ -192,23 +268,31 @@ impl Fixture{
                 counter += 1;
                 thread::sleep(POLL_DELAY);
             }
+            //Stop moving once the fixture is back at the highest point; return the number of
+            //polls
             motor_enable_pin.set_low();
             return counter;
         };
         return 0;
     }
 
+    //Go to either the top or bottom of the fixure's mmovement
+    //Returns `bool` of whether the movement was successful
     pub fn goto_limit(&mut self, direction:Direction) -> bool{
         let ref mut limit_sense:InputPin;
         let ref mut limit_nc_sense:InputPin;
+        //Movement is the same idea in either direction; only difference is which limit senses
+        //we're listening to
         match direction{
             Direction::Down => {
                 log::trace!("Sending fixture down...");
                 if let (Some(obj),Some(obj2),Some(motor_direction_pin)) = (self.lower_limit.as_mut(),self.lower_nc_limit.as_mut(),self.motor_direction.as_mut()) {
                     limit_sense = obj;
                     limit_nc_sense = obj2;
-                    motor_direction_pin.set_low();//
+                    motor_direction_pin.set_low();
                 }
+                //If the fixture's GPIO pins haven't been initialised yet, or they can't be
+                //accessed, obviously the fixture won't go anywhere; early return
                 else { return false; }
             },
             Direction::Up => {
@@ -216,14 +300,19 @@ impl Fixture{
                 if let (Some(obj),Some(obj2),Some(motor_direction_pin)) = (self.upper_limit.as_mut(),self.upper_nc_limit.as_mut(),self.motor_direction.as_mut()) {
                     limit_sense = obj;
                     limit_nc_sense = obj2;
-                    motor_direction_pin.set_high();//
+                    motor_direction_pin.set_high();
                 }
+                //If the fixture's GPIO pins haven't been initialised yet, or they can't be
+                //accessed, obviously the fixture won't go anywhere; early return
                 else { return false; }
             }
         }
 
+        //If we're already at the limit switch, no reason to break the fixture. Technically, a
+        //successful fixture movement, return true
         if limit_sense.is_high() && limit_nc_sense.is_low(){ log::debug!("Fixture already at proper limit switch!"); return true; }
 
+        //Move the fixture until its at the proper limit switch
         if let Some(motor_enable_pin) = self.motor_enable.as_mut(){
             while !*executor::block_on(MOVE_LOCK.read()){}
             motor_enable_pin.set_high();
@@ -239,13 +328,17 @@ impl Fixture{
             motor_enable_pin.set_low();
         }
 
+        //LEGACY CHECK: This is covered by the active-low pin, can be safely removed
         if limit_sense.is_low(){
             log::warn!("Fixture did not complete travel! Inspect fixture if this warning shows consistently.");
         }
 
+        //Should probably be changed to be the same as the premature successful movement's return
+        //status
         return limit_sense.is_high();
     }
 
+    //Extend the piston for 0.25s
     pub fn push_button(&mut self){
         if let Some(piston_enable) = self.piston_enable.as_mut(){
             while !*executor::block_on(MOVE_LOCK.read()){}

src/main.rs

@@ -1,15 +1,16 @@
 mod gpio_facade;
 mod output_facade;
 mod serial;
-use std::{fs,path::Path,io::{Write,stdin,stdout}, thread::{self, JoinHandle}};
+use std::{fs,path::Path,io::{Write,stdin,stdout}, thread::{self, JoinHandle}, sync::{Arc, atomic::AtomicBool}};
 use chrono::{DateTime,Local};
 use gpio_facade::{Fixture,Direction};
 use glob::glob;
+use signal_hook;
 use clap::Parser;
 use crate::{serial::TTY, output_facade::{OutputFile, TestState}};
 
 
-const VERSION:&str = "5.0.0-alpha.1";
+const VERSION:&str = "5.0.1";
 const DEFAULT_ITERATIONS:u64 = 10;
 
 
@@ -29,13 +30,29 @@ struct Args{
     iterations:Option<u64>
 
 }
+
 fn main() {
+    //Listen for kernel-level signals to exit. These are sent by keyboard shortcuts:
+    let terminate = Arc::new(AtomicBool::new(false));
+    //There is not a keyboard shortcut for SIGTERM, generally sent by a task manager like htop
+    //or btop
+    _ = signal_hook::flag::register(signal_hook::consts::SIGTERM, Arc::clone(&terminate));
+    //SIGINT = Ctrl+c
+    _ = signal_hook::flag::register(signal_hook::consts::SIGINT, Arc::clone(&terminate));
+    //SIGQUIT = Ctrl+\
+    _ = signal_hook::flag::register(signal_hook::consts::SIGQUIT, Arc::clone(&terminate));
+
+    //Import command-line arguments
     let args = Args::parse();
+
     setup_logs(&args.debug);
+
+    //Repot version of software to user and log file
     log::info!("Rust OCR version {}",VERSION);
 
     //Initialise fixture
     let mut fixture:Option<Fixture> = None;
+    //Keep trying until the fixture inits properly, or the user overrides
     loop {
         match Fixture::new() {
             Ok(fixture_object) => { 
@@ -54,10 +71,14 @@ fn main() {
         }
     }
 
-    #[allow(unreachable_code)]
-    loop{
+    //As long as the user doesn't kill the process, continue to loop here
+    while !terminate.load(std::sync::atomic::Ordering::Relaxed)
+    {
+
+        log::info!("Finding devices connected to debug cables....");
         let mut available_ttys:Vec<Box<Path>> = Vec::new();
 
+        //Try to detect serial devices by proper symlinks
         for entry in glob("/dev/serial/*").expect("Failed to read glob pattern"){
             match entry{
                 Ok(real_path) =>{
@@ -82,6 +103,7 @@ fn main() {
                 }
             }
         }
+        //If no symlinks exist, also check USB serial devices
         if available_ttys.is_empty(){
             for entry in glob::glob("/dev/ttyUSB*").expect("Unable to read glob"){
                 match entry{
@@ -95,17 +117,21 @@ fn main() {
             }
         }
 
+        //We're talking to the disco over serial; if we can't open a serial connection, we can't
+        //talk to the device. End the program here.
         if available_ttys.is_empty(){
             log::error!("No serial devices detected! Please ensure all connections.");
             return;
         }
 
+        //We now have a list of possible TTY device locations. Try to open them, each in their
+        //own thread
         let mut possible_devices: Vec<Option<TTY>> = Vec::new();
         let mut tty_test_threads: Vec<JoinHandle<Option<TTY>>> = Vec::new();
         for tty in available_ttys.into_iter(){
             tty_test_threads.push( thread::spawn( move|| {
                 match TTY::new(&tty.to_string_lossy()){
-                    Some(mut port) => {
+                    Some(port) => {
                         log::info!("Found device {}!",port.get_serial());
                         Some(port)
                     }
@@ -114,15 +140,18 @@ fn main() {
             }));
         }
 
+        //Get the possible TTYs from the above threads
         for thread in tty_test_threads{
             let output = thread.join().unwrap_or_else(|x|{log::trace!("{:?}",x); None});
             possible_devices.push(output);
         }
+
+        //Filter possible TTYs down to real ones; check that their serials are set
         let mut serials_set:bool = true;
         let mut devices:Vec<TTY> = Vec::new();
         let mut device_names:Vec<String> = Vec::new();
         for possible_device in possible_devices.into_iter(){
-            if let Some(mut device) = possible_device{
+            if let Some(device) = possible_device{
                 if device.get_serial().eq("unknown"){
                     serials_set = false;
                 }
@@ -131,51 +160,86 @@ fn main() {
             }
         }
 
+        //Create a new output file and storage of the current state of the test
         let mut out_file: OutputFile = OutputFile::new(device_names.clone());
         let state: TestState = TestState::new(device_names);
 
+        //Tell the user how many devices we have
         log::info!("--------------------------------------");
         log::info!("Number of devices detected: {}",devices.len());
         log::info!("--------------------------------------\n\n");
 
+        //If we need to set the serials manually for the device....
+        //well this will require a udev rule. 
+        //Check https://git.blizzard.systems/blizzardfinnegan/javaOCR for more information on
+        //udev rules.
         for _device in devices.iter_mut(){
             if !serials_set || args.manual{
               todo!();
             }
         }
 
+        //If the user set an iteration count in the CLI, then just use that, don't prompt
         let iteration_count:u64;
         if let Some(count) = args.iterations{
             iteration_count = count;
         }
+        //If the user didn't set an iteration count, prommpt for it
         else{
-            print!("How many times would you like to test the devices attached to the fixture? ");
+            print!("How many times would you like to test the devices attached to the fixture?  Enter '0' to quit: \n> ");
             _ = stdout().flush();
             let mut user_input:String = String::new();
             stdin().read_line(&mut user_input).expect("Did not input a valid number.");
+            //Minor string parsing/cleanup
+            //********
             if let Some('\n') = user_input.chars().next_back() {
                 user_input.pop();
             };
             if let Some('\r') = user_input.chars().next_back() {
                 user_input.pop();
             };
+            //********
 
-            iteration_count = user_input.parse().unwrap_or(DEFAULT_ITERATIONS);
+            //Convert the string to an integer; fallback to default iteration count of 10
+            match user_input.parse::<u64>(){
+                Err(_) => {
+                    iteration_count = DEFAULT_ITERATIONS; 
+                },
+                Ok(parsed_user_input) => {
+                    if parsed_user_input == 0 { break; }
+                    else { iteration_count = parsed_user_input; }
+                }
+            }
         }
 
-        for iter in 0..iteration_count{
-            log::info!("Starting iteration {} of {}...",iter+1, iteration_count);
-            if let Some(ref mut real_fixture) = fixture{
-                real_fixture.goto_limit(Direction::Up);
-                real_fixture.goto_limit(Direction::Down);
-                real_fixture.push_button();
+        //Assuming we haven't gotten a kill signal yet from the kernel, keep going
+        if !terminate.load(std::sync::atomic::Ordering::Relaxed){
+            for iter in 0..iteration_count{
+                log::info!("Starting iteration {} of {}...",iter+1, iteration_count);
+                if let Some(ref mut real_fixture) = fixture{
+                    if terminate.load(std::sync::atomic::Ordering::Relaxed) { break; }
+                    real_fixture.goto_limit(Direction::Up);
+                    if terminate.load(std::sync::atomic::Ordering::Relaxed) { break; }
+                    real_fixture.goto_limit(Direction::Down);
+                    if terminate.load(std::sync::atomic::Ordering::Relaxed) { break; }
+                    real_fixture.push_button();
+                    if terminate.load(std::sync::atomic::Ordering::Relaxed) { break; }
+                }
+                //Get the temperature from the device; if its a bad value, default to f32::MAX
+                //Save out to file
                 for ref mut device in devices.iter_mut(){
                     state.add_iteration(device.get_serial().to_string(), device.get_temp().unwrap_or(f32::MAX));
                 }
                 out_file.write_values(&state, None, None);
+
+                //Check again for the kill signal from kernel
+                if terminate.load(std::sync::atomic::Ordering::Relaxed) { break; }
             }
         }
-        break;
+    }
+    //Before exiting, reset the fixture arm
+    if let Some(ref mut real_fixture) = fixture{
+        real_fixture.goto_limit(Direction::Up);
     }
 }
 
@@ -193,6 +257,7 @@ fn setup_logs(debug:&bool) {
             ))
         })
         .chain({
+            //Write verbose logs to log file
             let mut file_logger = fern::Dispatch::new();
             let date_format = chrono_now.format("%Y-%m-%d_%H.%M").to_string();
             let local_log_file = fern::log_file(format!("logs/{}.log",date_format)).unwrap();
@@ -205,6 +270,7 @@ fn setup_logs(debug:&bool) {
             file_logger.chain(local_log_file)
         })
         .chain({
+            //Use higher level logging as wrapper for print to user
             let mut stdout_logger = fern::Dispatch::new();
             if *debug {
                 stdout_logger = stdout_logger.level(log::LevelFilter::Trace);

src/output_facade.rs

@@ -1,69 +1,98 @@
 use std::{collections::{HashMap,BTreeMap}, sync::Mutex};
 use ini::Ini;
 use chrono::Local;
+
+//According to IEEE-754, floats can contain NaN, which is weird because:
+//NaN != 0, (NaN < 0) == false, (NaN > 0) == false
+//This means that Rust by default does not order floats.
+//This crate fixes this shortcoming.
 use ordered_float::OrderedFloat;
 
+//Text file headers
 const ITERATION_COUNT:&str="iterations completed this run";
 const PASS_COUNT:&str="passing iterations";
+const PASS_PERCENT:&str="Pass %";
+
 const DEFAULT_LOWER:f32=35.8;
 const DEFAULT_UPPER:f32=36.2;
+
 pub struct OutputFile{
     file:Ini,
     filename:String,
 }
 
 pub struct TestState{
+    //Mutex: Only modifiable by one thread at a time
+    //HashMap: Key = String (device name), Value = TreeMap
+    //  TreeMap: Key = float (measured value), Value = integer (how many times we've seen it)
+    //                  TreeMaps require keys to be ordered, thus OrderedFloat
     data_map: Mutex<HashMap<String,BTreeMap<OrderedFloat<f32>,u64>>>
 }
 
 impl TestState{
+    //TestState Constructor.
     pub fn new(device_names:Vec<String>) -> Self{
         let output = Self{
             data_map:Mutex::new(HashMap::new())
         };
+        //initialise hashmap with the input device names
         device_names.iter()
-            .for_each(|device| _ = output.data_map.lock().unwrap().insert(device.trim().trim_end_matches("\0").to_string(),BTreeMap::new()));
+            .for_each(|device| 
+                _ = output.data_map.lock().unwrap().insert(
+                    device.trim().trim_end_matches("\0").to_string(), BTreeMap::new()
+                )
+            );
         return output;
     }
 
     pub fn add_iteration(&self,device_name:String, value:f32){
         let name = device_name.trim().trim_end_matches("\0");
         let mut all_data = self.data_map.lock().unwrap();
+        //if the device passed in doesn't exist yet in the HashMap, make it
         if !all_data.contains_key(&name.to_string()){
             all_data.insert(name.to_string(),BTreeMap::new());
         }
         //Device object should be created at this point, unwrap is safe
         let device_data = all_data.get_mut(&name.to_string()).unwrap();
+        //If this value has never been found before, then create it as the first iteration
         if !device_data.contains_key(&value.into()){
             device_data.insert(OrderedFloat(value), 1);
         }
-        //Value object should be created at this point, unwrap is safe
+        //If the value has been seen before, just increment the counter
         else{
+            //Value object should be created at this point, unwrap is safe
             let stored_value = device_data.get_mut(&value.into()).unwrap();
             *stored_value += 1;
         }
     }
 
     pub fn get_data(&self) -> HashMap<String,BTreeMap<OrderedFloat<f32>,u64>>{
+        //Dump a copy of the hashmap
         self.data_map.lock().unwrap().clone()
     }
 }
 
 impl OutputFile{
+    //OutputFile Constructor
     pub fn new(device_names:Vec<String>) -> Self{
         let mut config = Ini::new();
+        //Init the Ini config with all devices
         for name in device_names{
             config.with_section(Some(name.trim().trim_end_matches("\0")))
                   .set(ITERATION_COUNT,0.to_string())
                   .set(PASS_COUNT,0.to_string());
         };
         let mut filename = String::from("output/");
+        //Create a windows-safe filename with the format:
+        //YYYY-MM-DD.HH_MM.txt
         filename.push_str(&Local::now().to_rfc3339());
         filename.truncate(23);
         filename = filename.replace(":","_");
         filename = filename.replace("T",".");
         filename.push_str(".txt");
+        //Save a "blank" formatted file
         _ = config.write_to_file(&filename);
+        //Return the created and initialised OutputFile
         Self{
             file:config,
             filename,
@@ -73,6 +102,7 @@ impl OutputFile{
     pub fn write_values(&mut self, current_state:&TestState,upper_bound:Option<f32>,lower_bound:Option<f32>){
         let local_upper:f32;
         let local_lower:f32;
+        //if the user didn't specify bounds, fallback to defaults
         match upper_bound{
             None => local_upper = DEFAULT_UPPER,
             Some(forced_upper) => local_upper = forced_upper,
@@ -81,27 +111,47 @@ impl OutputFile{
             None => local_lower = DEFAULT_LOWER,
             Some(forced_lower) => local_lower = forced_lower,
         };
+
+        //get the hashmap from the current state
         let data_map = current_state.get_data();
+        
+        //For each device...
         data_map.iter().for_each(|(device,value_map)|{
+            //Get all values read from the device, and sort it
             let mut value_list:Vec<&OrderedFloat<f32>> = value_map.keys().collect();
             value_list.sort();
+
             let mut iteration_count = 0;
             let mut sum = 0;
             let mut pass_iteration_count = 0;
             let saved_data = &mut self.file;
+
+            //For each value read from the device...
             value_map.iter().for_each(|(value,count)|{
+                //Keep track of the total iterations, and the pass count
                 iteration_count += count;
                 if value.into_inner() > local_lower && value.into_inner() < local_upper{
                     pass_iteration_count += count;
                 }
+
+                //LEGACY CODE: sum is no longer being used
                 sum += (value.into_inner() * *count as f32) as u128;
+
+                //Add this value to the ini object
                 saved_data.with_section(Some(&(device.to_owned() + " read value counts").to_string())).set(&value.to_string(),&count.to_string());
             });
 
+            //Calculate pass percent
+            let pass_percent= (iteration_count - (iteration_count - pass_iteration_count)) / iteration_count;
+
+            //Add Pass percent, pass iteration count, and iteration count to ini object
             saved_data.with_section(Some(device))
+                .set(PASS_PERCENT, pass_percent.to_string())
                 .set(ITERATION_COUNT, iteration_count.to_string())
                 .set(PASS_COUNT,pass_iteration_count.to_string());
         });
+
+        //Flush ini object to text file
         _ = self.file.write_to_file(self.filename.clone());
     }
 }

src/serial.rs

@@ -12,10 +12,12 @@ const SERIAL_TIMEOUT: std::time::Duration = Duration::from_millis(50);
 ///----------------------
 
 //Request currently shown temp from device 
-const REQUEST_TEMP:   &[u8; 26]= b"\x17\x01\x0c\x00\x00\x00\x1a\x01\x19\x00\x03\x0b\x00\x00\x00\x00\x07\x00\x00\x00\x00\x00\xe9\x32\x94\xfe";
+const REQUEST_TEMP:   &[u8; 26]= 
+b"\x17\x01\x0c\x00\x00\x00\x1a\x01\x19\x00\x03\x0b\x00\x00\x00\x00\x07\x00\x00\x00\x00\x00\xe9\x32\x94\xfe";
 
 //Request a device's serial
-const REQUEST_SERIAL: &[u8; 26]= b"\x17\x01\x0c\x00\x00\x00\x1a\x01\x19\x00\x18\x0b\x00\x00\x00\x00\x07\x00\x00\x00\x00\x00\x71\xe8\x80\x3e";
+const REQUEST_SERIAL: &[u8; 26]= 
+b"\x17\x01\x0c\x00\x00\x00\x1a\x01\x19\x00\x18\x0b\x00\x00\x00\x00\x07\x00\x00\x00\x00\x00\x71\xe8\x80\x3e";
 
 pub struct TTY{
     tty: Box<dyn SerialPort>,
@@ -25,6 +27,7 @@ impl std::fmt::Debug for TTY{
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result{
         let absolute_location = self.tty.name();
         let relative_location:String;
+        //Trim down absolute location to just the relevant portion
         match absolute_location{
             Some(abs_location_string) => {
                 let sectioned_abs_location = abs_location_string.rsplit_once('/');
@@ -43,23 +46,30 @@ impl std::fmt::Debug for TTY{
 }
 
 impl TTY{
+    //TTY constructor
     pub fn new(serial_location:&str) -> Option<Self>{
+        //Initialise serialport with baudrate, timeout, and try to open the device
         let possible_tty = serialport::new(serial_location,BAUD_RATE).timeout(SERIAL_TIMEOUT).open();
+        //If the serialport is real, try to get the serialnumber of the device when initialising the
+        //device
         if let Ok(mut tty) = possible_tty{
             if tty.write_all(REQUEST_SERIAL).is_ok(){
+                //force-write the command to the serialport
                 _ = tty.flush();
+
+                //Read back the response
                 let mut reader = BufReader::new(&mut tty);
                 let mut read_buffer: Vec<u8> = Vec::new();
                 _ = reader.read_to_end(&mut read_buffer);
+
+                //Parse the readbuffer, return the TTY object with a set serialnumber
                 let serial:String = TTY::parse_serial_response(read_buffer);
                 Some(TTY{tty,serial})
             }
-            else{
-                None
-            }
-        } else{
-            None
-        }
+            //If writing to the TTY fails, error out
+            else{ None }
+        //If opening the TTY fails, error out
+        } else{ None }
     }
 
     fn parse_serial_response(read_buffer:Vec<u8>) -> String{
@@ -72,7 +82,12 @@ impl TTY{
             let mut buffer_index:usize = 0;
             //The preamble is weird, and is only 3 bytes long. Putting it in a u32, and setting the
             //first octet to 0
-            let preamble = u32::from_be_bytes([0x00,buffer[buffer_index],buffer[buffer_index + 1],buffer[buffer_index + 2]]);
+            let preamble = u32::from_be_bytes([
+                0x00,
+                buffer[buffer_index],
+                buffer[buffer_index + 1],
+                buffer[buffer_index + 2]
+            ]);
             buffer_index += 3;
             //Predefined WACP Preamble
             if preamble != 0x17010c {
@@ -91,7 +106,8 @@ impl TTY{
             let msg_class_id = TTY::u32_from_bytes(&buffer, &mut buffer_index);
             //Expected message class: Temperature Response [assumed]
             if msg_class_id != 0x00180f00 {
-                log::error!("Unknown message response class: {}. Expected: 1576704. See WACP documentation.",msg_class_id);
+                log::error!("Unknown message response class: {}. Expected: 1576704. See WACP documentation.",
+                                                        msg_class_id);
             }
 
             let msg_size = TTY::u32_from_bytes(&buffer, &mut buffer_index);
@@ -105,7 +121,8 @@ impl TTY{
             //Encryption bytes are not implemented as of now, and this code does not know how to
             //interpret encrypted or compressed data.
             if encrypted != 0x0{
-                log::error!("Message potentially encrypted! Consult documentation concerning bitmask {}!",encrypted);
+                log::error!("Message potentially encrypted! Consult documentation concerning bitmask {}!",
+                                encrypted);
             }
 
             //Bytes counted in packet length but not in msg length: 19
@@ -161,12 +178,17 @@ impl TTY{
         return "Invalid device!".to_string();
     }
 
-    pub fn get_serial(&mut self) -> &str { &self.serial }
+    pub fn get_serial(&self) -> &str { &self.serial }
 
     pub fn get_temp(&mut self) -> Option<f32> {
+        //Send command for getting temp from the screen
         let output = self.tty.write_all(REQUEST_TEMP).is_ok();
         _ = self.tty.flush();
+
+        //Wait for a response
         std::thread::sleep(SERIAL_TIMEOUT);
+
+        //If you actually wrote safely...
         if output{
             let mut reader = BufReader::new(&mut self.tty);
             let mut read_buffer: Vec<u8> = Vec::new();
@@ -392,14 +414,17 @@ impl TTY{
 
                 //The value the Disco reports is in Kelvin. Convert to Celsius for easier comparison
                 //with bounds.
-                log::info!("Temp from device {}: {}, self.serial, temp-273.15);
+                log::info!("Temp from device {}: {}", self.serial, temp-273.15);
                 return Some(temp - 273.15);
             }
+            //If you didn't get a response back from the device, error out, mark logs
+            //accordingly
             else {
                 log::trace!("Read an empty string from device {:?}. Possible read error.", self);
                 return None;
             };
         };
+        //If you didn't write successfully, error out
         return None;
     }