Code cleanup

src/core.rs

@@ -12,7 +12,7 @@ use rusb::DeviceHandle;
 use std::error::Error;
 use std::fs::OpenOptions;
 use std::io::{Read, Write};
-use std::marker::PhantomData;
+use std::marker::{PhantomData, PhantomPinned};
 use std::path::Path;
 use std::process::Command;
 use std::ptr::NonNull;
@@ -45,6 +45,7 @@ static FAN_TYPE_2_PATH: &str = "/sys/devices/platform/asus-nb-wmi/fan_boost_mode
 pub(crate) struct RogCore {
     handle: DeviceHandle<rusb::GlobalContext>,
     virt_keys: VirtKeys,
+    _pin: PhantomPinned,
 }
 
 impl RogCore {
@@ -79,6 +80,7 @@ impl RogCore {
         Ok(RogCore {
             handle: dev_handle,
             virt_keys: VirtKeys::new(),
+            _pin: PhantomPinned,
         })
     }
 
@@ -181,13 +183,6 @@ impl RogCore {
         Ok(())
     }
 
-    pub(crate) fn get_raw_device_handle(&mut self) -> NonNull<DeviceHandle<rusb::GlobalContext>> {
-        // Breaking every damn lifetime guarantee rust gives us
-        unsafe {
-            NonNull::new_unchecked(&mut self.handle as *mut DeviceHandle<rusb::GlobalContext>)
-        }
-    }
-
     /// A direct call to systemd to suspend the PC.
     ///
     /// This avoids desktop environments being required to handle it
@@ -237,13 +232,16 @@ impl RogCore {
             }
         }
     }
+
+    pub(crate) fn get_raw_device_handle(&mut self) -> NonNull<DeviceHandle<rusb::GlobalContext>> {
+        // Breaking every damn lifetime guarantee rust gives us
+        unsafe {
+            NonNull::new_unchecked(&mut self.handle as *mut DeviceHandle<rusb::GlobalContext>)
+        }
+    }
 }
 
-/// UNSAFE: Must live as long as RogCore
-///
-/// Because we're holding a pointer to something that *may* go out of scope while the
-/// pointer is held. We're relying on access to struct to be behind a Mutex, and for behaviour
-/// that may cause invalididated pointer to cause the program to panic rather than continue.
+/// Lifetime is tied to `DeviceHandle` from `RogCore`
 pub(crate) struct KeyboardReader<'d, C: 'd>
 where
     C: rusb::UsbContext,
@@ -316,7 +314,6 @@ where
 
 /// UNSAFE
 unsafe impl<'d, C> Send for LedWriter<'d, C> where C: rusb::UsbContext {}
-unsafe impl<'d, C> Sync for LedWriter<'d, C> where C: rusb::UsbContext {}
 
 impl<'d, C> LedWriter<'d, C>
 where

src/daemon.rs

@@ -31,22 +31,20 @@ pub async fn start_daemon() -> Result<(), Box<dyn Error>> {
     let laptop = match_laptop();
     let mut config = Config::default().read();
 
-    let mut rogcore = Box::pin(
-        RogCore::new(
-            laptop.usb_vendor(),
-            laptop.usb_product(),
-            laptop.led_endpoint(),
-        )
-        .map_or_else(
-            |err| {
-                error!("{}", err);
-                panic!("{}", err);
-            },
-            |daemon| {
-                info!("RogCore loaded");
-                daemon
-            },
-        ),
+    let mut rogcore = RogCore::new(
+        laptop.usb_vendor(),
+        laptop.usb_product(),
+        laptop.led_endpoint(),
+    )
+    .map_or_else(
+        |err| {
+            error!("{}", err);
+            panic!("{}", err);
+        },
+        |daemon| {
+            info!("RogCore loaded");
+            daemon
+        },
     );
     // Reload settings
     rogcore.reload(&mut config).await?;
@@ -76,31 +74,27 @@ pub async fn start_daemon() -> Result<(), Box<dyn Error>> {
 
     // Keyboard reader goes in separate task because we want a high interrupt timeout
     // and don't want that to hold up other tasks, or miss keystrokes
-    {
-        let keyboard_reader = KeyboardReader::new(
-            rogcore.get_raw_device_handle(),
-            laptop.key_endpoint(),
-            laptop.key_filter().to_owned(),
-        );
-        // This is *not* safe
-        let led_writer = led_writer.clone();
-        let config = config.clone();
-        // start the keyboard reader and laptop-action loop
-        tokio::spawn(async move {
-            loop {
-                let data = unsafe { keyboard_reader.poll_keyboard().await };
-                if let Some(bytes) = data {
-                    match laptop.run(&mut rogcore, &led_writer, &config, bytes).await {
-                        Ok(_) => {}
-                        Err(err) => {
-                            error!("{:?}", err);
-                            panic!("Force crash for systemd to restart service")
-                        }
-                    }
-                }
+    let keyboard_reader = KeyboardReader::new(
+        rogcore.get_raw_device_handle(),
+        laptop.key_endpoint(),
+        laptop.key_filter().to_owned(),
+    );
+
+    let led_writer1 = led_writer.clone();
+    let config1 = config.clone();
+    // start the keyboard reader and laptop-action loop
+    tokio::task::spawn(async move {
+        loop {
+            let data = keyboard_reader.poll_keyboard().await;
+            if let Some(bytes) = data {
+                laptop
+                    .run(&mut rogcore, &led_writer1, &config1, bytes)
+                    .await
+                    .map_err(|err| warn!("{:?}", err))
+                    .unwrap();
             }
-        });
-    }
+        }
+    });
 
     // start the LED writer loop
     let mut time_mark = Instant::now();
@@ -122,6 +116,7 @@ pub async fn start_daemon() -> Result<(), Box<dyn Error>> {
         }
 
         // Write a colour block
+        let led_writer = led_writer.clone();
         if let Ok(mut lock) = effect.try_lock() {
             // Spawn a writer
             if let Some(stuff) = lock.take() {