Logging, better error handling

.idea/dictionaries/luke.xml

@@ -2,7 +2,9 @@
   <dictionary name="luke">
     <words>
       <w>backlight</w>
+      <w>dbus</w>
       <w>hotkey</w>
+      <w>rogcore</w>
     </words>
   </dictionary>
 </component>

Cargo.lock

@@ -15,6 +15,17 @@ dependencies = [
  "memchr",
 ]
 
+[[package]]
+name = "atty"
+version = "0.2.14"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d9b39be18770d11421cdb1b9947a45dd3f37e93092cbf377614828a319d5fee8"
+dependencies = [
+ "hermit-abi",
+ "libc",
+ "winapi",
+]
+
 [[package]]
 name = "bit-set"
 version = "0.5.1"
@@ -61,6 +72,19 @@ dependencies = [
  "libdbus-sys",
 ]
 
+[[package]]
+name = "env_logger"
+version = "0.7.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "44533bbbb3bb3c1fa17d9f2e4e38bbbaf8396ba82193c4cb1b6445d711445d36"
+dependencies = [
+ "atty",
+ "humantime",
+ "log",
+ "regex",
+ "termcolor",
+]
+
 [[package]]
 name = "filetime"
 version = "0.2.9"
@@ -93,6 +117,30 @@ dependencies = [
  "syn",
 ]
 
+[[package]]
+name = "hermit-abi"
+version = "0.1.11"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8a0d737e0f947a1864e93d33fdef4af8445a00d1ed8dc0c8ddb73139ea6abf15"
+dependencies = [
+ "libc",
+]
+
+[[package]]
+name = "humantime"
+version = "1.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "df004cfca50ef23c36850aaaa59ad52cc70d0e90243c3c7737a4dd32dc7a3c4f"
+dependencies = [
+ "quick-error",
+]
+
+[[package]]
+name = "lazy_static"
+version = "1.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646"
+
 [[package]]
 name = "libc"
 version = "0.2.69"
@@ -134,6 +182,15 @@ dependencies = [
  "vcpkg",
 ]
 
+[[package]]
+name = "log"
+version = "0.4.8"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "14b6052be84e6b71ab17edffc2eeabf5c2c3ae1fdb464aae35ac50c67a44e1f7"
+dependencies = [
+ "cfg-if",
+]
+
 [[package]]
 name = "memchr"
 version = "2.3.3"
@@ -161,6 +218,12 @@ dependencies = [
  "unicode-xid",
 ]
 
+[[package]]
+name = "quick-error"
+version = "1.2.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0"
+
 [[package]]
 name = "quote"
 version = "1.0.3"
@@ -176,6 +239,24 @@ version = "0.1.56"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "2439c63f3f6139d1b57529d16bc3b8bb855230c8efcc5d3a896c8bea7c3b1e84"
 
+[[package]]
+name = "regex"
+version = "1.3.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a6020f034922e3194c711b82a627453881bc4682166cabb07134a10c26ba7692"
+dependencies = [
+ "aho-corasick",
+ "memchr",
+ "regex-syntax",
+ "thread_local",
+]
+
+[[package]]
+name = "regex-syntax"
+version = "0.6.17"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7fe5bd57d1d7414c6b5ed48563a2c855d995ff777729dcd91c369ec7fea395ae"
+
 [[package]]
 name = "rle-decode-fast"
 version = "1.0.1"
@@ -187,7 +268,9 @@ name = "rog-core"
 version = "0.3.2"
 dependencies = [
  "dbus",
+ "env_logger",
  "gumdrop",
+ "log",
  "rog-lib",
  "serde",
  "serde_derive",
@@ -203,6 +286,7 @@ dependencies = [
  "serde",
  "serde_derive",
  "sysfs-class",
+ "thiserror",
  "toml",
 ]
 
@@ -272,6 +356,44 @@ dependencies = [
  "xattr",
 ]
 
+[[package]]
+name = "termcolor"
+version = "1.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "bb6bfa289a4d7c5766392812c0a1f4c1ba45afa1ad47803c11e1f407d846d75f"
+dependencies = [
+ "winapi-util",
+]
+
+[[package]]
+name = "thiserror"
+version = "1.0.15"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "54b3d3d2ff68104100ab257bb6bb0cb26c901abe4bd4ba15961f3bf867924012"
+dependencies = [
+ "thiserror-impl",
+]
+
+[[package]]
+name = "thiserror-impl"
+version = "1.0.15"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ca972988113b7715266f91250ddb98070d033c62a011fa0fcc57434a649310dd"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
+[[package]]
+name = "thread_local"
+version = "1.0.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d40c6d1b69745a6ec6fb1ca717914848da4b44ae29d9b3080cbee91d72a69b14"
+dependencies = [
+ "lazy_static",
+]
+
 [[package]]
 name = "toml"
 version = "0.5.6"
@@ -309,6 +431,15 @@ version = "0.4.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6"
 
+[[package]]
+name = "winapi-util"
+version = "0.1.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "fa515c5163a99cc82bab70fd3bfdd36d827be85de63737b40fcef2ce084a436e"
+dependencies = [
+ "winapi",
+]
+
 [[package]]
 name = "winapi-x86_64-pc-windows-gnu"
 version = "0.4.0"

data/rog-core.service

@@ -2,6 +2,7 @@
 Description=ROG Core Daemon
 
 [Service]
+Environment="RUST_LOG=info"
 ExecStart=/usr/bin/rog-core -d
 Restart=on-failure
 

rog-core/Cargo.toml

@@ -6,8 +6,9 @@ edition = "2018"
 
 [dependencies]
 gumdrop = "0.8"
-dbus = "0.7.1"
+dbus = "0.7"
 serde = "1.0"
 serde_derive = "1.0"
-
+log = "0.4"
+env_logger = "0.7"
 rog-lib = { path = "../rog-lib" }

rog-core/src/daemon.rs

@@ -3,6 +3,7 @@ use dbus::{
     blocking::Connection,
     tree::{Factory, MethodErr},
 };
+use log::{error, info, warn};
 use rog_lib::core::RogCore;
 use std::error::Error;
 use std::time::Duration;
@@ -15,12 +16,30 @@ pub struct Daemon {
 impl Daemon {
     pub fn new() -> Self {
         Daemon {
-            rogcore: RogCore::new().expect("Could not start RogCore"),
+            rogcore: RogCore::new().map_or_else(
+                |err| {
+                    error!("{}", err);
+                    panic!("{}", err);
+                },
+                |daemon| {
+                    info!("RogCore loaded");
+                    daemon
+                },
+            ),
         }
     }
 
     pub fn start() -> Result<(), Box<dyn Error>> {
-        let mut connection = Connection::new_system().expect("Could not set up dbus system");
+        let mut connection = Connection::new_system().map_or_else(
+            |err| {
+                error!("{}", err);
+                panic!("{}", err);
+            },
+            |dbus| {
+                info!("DBus connected");
+                dbus
+            },
+        );
         connection.request_name(DBUS_IFACE, false, true, false)?;
         let factory = Factory::new_fnmut::<()>();
 
@@ -41,15 +60,18 @@ impl Daemon {
                                 move |m| {
                                     // Reads the args passed to the method
                                     let bytes: Vec<u8> = m.msg.read1()?;
-                                    let s = format!("Wrote {:x?}", bytes);
 
                                     match daemon.borrow_mut().rogcore.aura_set_and_save(&bytes[..])
                                     {
                                         Ok(_) => {
-                                            let mret = m.msg.method_return().append1(s);
+                                            let s = format!("Wrote {:x?}", bytes);
+                                            let mret = m.msg.method_return().append1(&s);
                                             Ok(vec![mret])
                                         }
-                                        Err(err) => Err(MethodErr::failed(&err)),
+                                        Err(err) => {
+                                            warn!("{}", err);
+                                            Err(MethodErr::failed(&err))
+                                        }
                                     }
                                 }
                             })
@@ -65,7 +87,12 @@ impl Daemon {
 
         let mut key_buf = [0u8; 32];
         loop {
-            connection.process(Duration::from_millis(1))?;
+            connection
+                .process(Duration::from_millis(10))
+                .unwrap_or_else(|err| {
+                    error!("{}", err);
+                    false
+                });
             // READ KEYBOARD
             // TODO: this needs to move to a thread, but there is unsafety
             let borrowed_daemon = daemon.borrow();
@@ -78,11 +105,15 @@ impl Daemon {
 
                     if let Some(_count) = read {
                         if key_buf[0] == laptop.hotkey_group_byte() {
-                            laptop.do_hotkey_action(&mut rogcore, key_buf[1]);
+                            laptop
+                                .do_hotkey_action(&mut rogcore, key_buf[1])
+                                .unwrap_or_else(|err| {
+                                    warn!("{}", err);
+                                });
                         }
                     }
                 }
-                Err(err) => println!("{:?}", err),
+                Err(err) => error!("{}", err),
             }
         }
     }

rog-core/src/main.rs

@@ -3,7 +3,9 @@ mod daemon;
 use crate::daemon::*;
 use dbus::Error as DbusError;
 use dbus::{ffidisp::Connection, Message};
+use env_logger::{Builder, Target};
 use gumdrop::Options;
+use log::LevelFilter;
 use rog_lib::{
     cli_options::SetAuraBuiltin,
     core::{LedBrightness, RogCore, LED_MSG_LEN},
@@ -40,6 +42,10 @@ struct LedModeCommand {
 }
 
 fn main() -> Result<(), Box<dyn std::error::Error>> {
+    let mut builder = Builder::from_default_env();
+    builder.target(Target::Stdout);
+    builder.filter(None, LevelFilter::Info).init();
+
     let parsed = CLIStart::parse_args_default_or_exit();
     if parsed.daemon {
         Daemon::start()?;

rog-lib/Cargo.toml

@@ -12,3 +12,4 @@ serde_derive = "1.0"
 toml = "0.5"
 sysfs-class = "0.1.2"
 aho-corasick = "0.7"
+thiserror = "1.0.15"

rog-lib/src/aura.rs

@@ -55,7 +55,7 @@ impl From<SetAuraBuiltin> for [u8; LED_MSG_LEN] {
             SetAuraBuiltin::Cycle(_) => msg[3] = 0x02,
             SetAuraBuiltin::Rainbow(_) => msg[3] = 0x03,
             SetAuraBuiltin::Rain(_) => msg[3] = 0x04,
-            SetAuraBuiltin::Random(_) => msg[3] = 0x05,
+            SetAuraBuiltin::Disco(_) => msg[3] = 0x05,
             SetAuraBuiltin::Highlight(_) => msg[3] = 0x06,
             SetAuraBuiltin::Laser(_) => msg[3] = 0x07,
             SetAuraBuiltin::Ripple(_) => msg[3] = 0x08,
@@ -79,7 +79,7 @@ impl From<SetAuraBuiltin> for [u8; LED_MSG_LEN] {
                 msg[11] = settings.colour2.1;
                 msg[12] = settings.colour2.2;
             }
-            SetAuraBuiltin::Cycle(settings) | SetAuraBuiltin::Random(settings) => {
+            SetAuraBuiltin::Cycle(settings) | SetAuraBuiltin::Disco(settings) => {
                 msg[7] = settings.speed as u8;
             }
             SetAuraBuiltin::Rain(settings)
@@ -172,7 +172,7 @@ impl Default for BuiltInModeBytes {
                 SingleSpeedDirection::default(),
             )),
             rain: <[u8; LED_MSG_LEN]>::from(SetAuraBuiltin::Rain(SingleColourSpeed::default())),
-            random: <[u8; LED_MSG_LEN]>::from(SetAuraBuiltin::Random(SingleSpeed::default())),
+            random: <[u8; LED_MSG_LEN]>::from(SetAuraBuiltin::Disco(SingleSpeed::default())),
             highlight: <[u8; LED_MSG_LEN]>::from(SetAuraBuiltin::Highlight(
                 SingleColourSpeed::default(),
             )),

rog-lib/src/cli_options.rs

@@ -147,7 +147,7 @@ pub enum SetAuraBuiltin {
     #[options(help = "random pattern mimicking raindrops")]
     Rain(SingleColourSpeed),
     #[options(help = "random pattern of three preset colours")]
-    Random(SingleSpeed),
+    Disco(SingleSpeed),
     #[options(help = "pressed keys are highlighted to fade")]
     Highlight(SingleColourSpeed),
     #[options(help = "pressed keys generate horizontal laser")]

rog-lib/src/core.rs

@@ -1,13 +1,12 @@
 use crate::{aura::BuiltInModeByte, config::Config, error::AuraError, laptops::*};
 use aho_corasick::AhoCorasick;
 use gumdrop::Options;
-use rusb::{DeviceHandle, Error};
+use rusb::DeviceHandle;
 use std::cell::{Ref, RefCell};
 use std::process::Command;
 use std::str::FromStr;
 use std::time::Duration;
-use sysfs_class::Brightness;
+use sysfs_class::{Brightness, SysClass};
-use sysfs_class::SysClass;
 
 pub const LED_MSG_LEN: usize = 17;
 static LED_INIT1: [u8; 2] = [0x5d, 0xb9];
@@ -20,33 +19,6 @@ static LED_INIT5: [u8; 6] = [0x5e, 0x05, 0x20, 0x31, 0, 0x08];
 static LED_APPLY: [u8; 17] = [0x5d, 0xb4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
 static LED_SET: [u8; 17] = [0x5d, 0xb5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
 
-#[derive(Debug, Options)]
-pub struct LedBrightness {
-    level: u8,
-}
-impl LedBrightness {
-    pub fn level(&self) -> u8 {
-        self.level
-    }
-}
-impl FromStr for LedBrightness {
-    type Err = AuraError;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let s = s.to_lowercase();
-        match s.as_str() {
-            "off" => Ok(LedBrightness { level: 0x00 }),
-            "low" => Ok(LedBrightness { level: 0x01 }),
-            "med" => Ok(LedBrightness { level: 0x02 }),
-            "high" => Ok(LedBrightness { level: 0x03 }),
-            _ => {
-                println!("Missing required argument, must be one of:\noff,low,med,high\n");
-                Err(AuraError::ParseBrightness)
-            }
-        }
-    }
-}
-
 /// ROG device controller
 ///
 /// For the GX502GW the LED setup sequence looks like:
@@ -65,9 +37,8 @@ pub struct RogCore {
 }
 
 impl RogCore {
-    pub fn new() -> Result<RogCore, Error> {
+    pub fn new() -> Result<RogCore, AuraError> {
-        // TODO: use /sys/class/dmi/id/board_name to detect model
+        let laptop = match_laptop()?;
-        let laptop = LaptopGX502GW::new();
 
         let mut dev_handle = RogCore::get_device(laptop.usb_vendor(), laptop.usb_product())?;
         dev_handle.set_active_configuration(0).unwrap_or(());
@@ -78,7 +49,7 @@ impl RogCore {
         for iface in dev_config.interfaces() {
             for desc in iface.descriptors() {
                 for endpoint in desc.endpoint_descriptors() {
-                    if endpoint.address() == 0x81 {
+                    if endpoint.address() == laptop.led_iface_num() {
                         led_interface_num = desc.interface_number();
                         break;
                     }
@@ -93,7 +64,7 @@ impl RogCore {
             initialised: false,
             led_interface_num,
             config: Config::default().read(),
-            laptop: RefCell::new(Box::new(laptop)),
+            laptop: RefCell::new(laptop),
         })
     }
 
@@ -109,17 +80,20 @@ impl RogCore {
         &mut self.config
     }
 
-    fn get_device(vendor: u16, product: u16) -> Result<DeviceHandle<rusb::GlobalContext>, Error> {
+    fn get_device(
+        vendor: u16,
+        product: u16,
+    ) -> Result<DeviceHandle<rusb::GlobalContext>, AuraError> {
         for device in rusb::devices().unwrap().iter() {
             let device_desc = device.device_descriptor().unwrap();
             if device_desc.vendor_id() == vendor && device_desc.product_id() == product {
-                return device.open();
+                return device.open().map_err(|err| AuraError::from(err));
             }
         }
-        Err(Error::NoDevice)
+        Err(AuraError::from(rusb::Error::NoDevice))
     }
 
-    fn aura_write_messages(&mut self, messages: &[&[u8]]) -> Result<(), Error> {
+    fn aura_write_messages(&mut self, messages: &[&[u8]]) -> Result<(), AuraError> {
         self.handle.claim_interface(self.led_interface_num)?;
         // Declared as a zoomy so that it is hidden
         let write = |message: &[u8]| {
@@ -146,7 +120,7 @@ impl RogCore {
         Ok(())
     }
 
-    pub fn aura_brightness_bytes(brightness: u8) -> Result<[u8; 17], Error> {
+    pub fn aura_brightness_bytes(brightness: u8) -> Result<[u8; 17], AuraError> {
         // TODO: check brightness range
         let mut bright = [0u8; LED_MSG_LEN];
         bright[0] = 0x5a;
@@ -157,7 +131,7 @@ impl RogCore {
         Ok(bright)
     }
 
-    pub fn aura_set_and_save(&mut self, bytes: &[u8]) -> Result<(), Error> {
+    pub fn aura_set_and_save(&mut self, bytes: &[u8]) -> Result<(), AuraError> {
         let mode = BuiltInModeByte::from(bytes[3]);
         if self.laptop().supported_modes().contains(&mode) || bytes[1] == 0xba {
             let messages = [bytes];
@@ -166,23 +140,22 @@ impl RogCore {
             self.config.write();
             return Ok(());
         }
-        Err(Error::NotSupported)
+        Err(AuraError::NotSupported)
     }
 
-    pub fn poll_keyboard(&self, buf: &mut [u8; 32]) -> Result<Option<usize>, Error> {
+    pub fn poll_keyboard(&self, buf: &mut [u8; 32]) -> Result<Option<usize>, AuraError> {
         match self
             .handle
             .read_interrupt(0x83, buf, Duration::from_micros(10))
         {
             Ok(o) => {
-                if buf[0] == 0x5a {
+                if buf[0] == self.laptop.borrow().hotkey_group_byte() {
                     return Ok(Some(o));
                 }
             }
-            Err(err) => match err {
+            Err(err) => {
-                //Error::Timeout => {}
+                return Err(AuraError::from(err));
-                _ => return Err(err),
+            }
-            },
         }
         Ok(None)
     }
@@ -194,17 +167,22 @@ impl RogCore {
             .expect("failed to suspend");
     }
 
-    pub fn toggle_airplane_mode(&self) {
+    pub fn toggle_airplane_mode(&self) -> Result<(), AuraError> {
-        if let Ok(output) = Command::new("rfkill").arg("list").output() {
+        match Command::new("rfkill").arg("list").output() {
-            if output.status.success() {
+            Ok(output) => {
-                let patterns = &["yes"];
+                if output.status.success() {
-                let ac = AhoCorasick::new(patterns);
+                    let patterns = &["yes"];
-                if ac.earliest_find(output.stdout).is_some() {
+                    let ac = AhoCorasick::new(patterns);
-                    Command::new("rfkill").arg("unblock").arg("all").spawn();
+                    if ac.earliest_find(output.stdout).is_some() {
-                } else {
+                        Command::new("rfkill").arg("unblock").arg("all").spawn()?;
-                    Command::new("rfkill").arg("block").arg("all").spawn();
+                    } else {
+                        Command::new("rfkill").arg("block").arg("all").spawn()?;
+                    }
+                    return Ok(());
                 }
+                return Err(AuraError::CommandFailed);
             }
+            Err(err) => Err(AuraError::from(err)),
         }
     }
 }
@@ -248,3 +226,30 @@ impl Backlight {
         }
     }
 }
+
+#[derive(Debug, Options)]
+pub struct LedBrightness {
+    level: u8,
+}
+impl LedBrightness {
+    pub fn level(&self) -> u8 {
+        self.level
+    }
+}
+impl FromStr for LedBrightness {
+    type Err = AuraError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let s = s.to_lowercase();
+        match s.as_str() {
+            "off" => Ok(LedBrightness { level: 0x00 }),
+            "low" => Ok(LedBrightness { level: 0x01 }),
+            "med" => Ok(LedBrightness { level: 0x02 }),
+            "high" => Ok(LedBrightness { level: 0x03 }),
+            _ => {
+                println!("Missing required argument, must be one of:\noff,low,med,high\n");
+                Err(AuraError::ParseBrightness)
+            }
+        }
+    }
+}

rog-lib/src/error.rs

@@ -1,38 +1,24 @@
-use std::error::Error;
+use std::fmt::Debug;
-use std::fmt;
+use thiserror::Error;
-use std::fmt::{Debug, Display};
 
-#[derive(PartialEq)]
+#[derive(Error, Debug)]
 pub enum AuraError {
+    #[error("unable to parse string to colour")]
     ParseColour,
+    #[error("unable to parse string to speed")]
     ParseSpeed,
+    #[error("unable to parse string to direction")]
     ParseDirection,
+    #[error("unable to parse string to brightness")]
     ParseBrightness,
+    #[error("could not poll the keyboard for input")]
     PollKeyboard,
-}
+    #[error("mode not supported")]
-
+    NotSupported,
-impl Debug for AuraError {
+    #[error("USB error")]
-    #[inline]
+    UsbError(#[from] rusb::Error),
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    #[error("IO error")]
-        Display::fmt(self.description(), f)
+    IOError(#[from] std::io::Error),
-    }
+    #[error("external command failed")]
-}
+    CommandFailed,
-
-impl Display for AuraError {
-    #[inline]
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        Display::fmt(self.description(), f)
-    }
-}
-
-impl Error for AuraError {
-    fn description(&self) -> &str {
-        match self {
-            AuraError::ParseColour => "could not parse colour",
-            AuraError::ParseSpeed => "could not parse speed",
-            AuraError::ParseDirection => "could not parse direction",
-            AuraError::ParseBrightness => "could not parse brightness",
-            AuraError::PollKeyboard => "failed to poll keyboard",
-        }
-    }
 }

rog-lib/src/laptops.rs

@@ -1,14 +1,30 @@
 use crate::aura::BuiltInModeByte;
 use crate::core::{Backlight, RogCore};
+use crate::error::AuraError;
 
-// ENV{POWER_SUPPLY_ONLINE}=="0", RUN+="gdbus call
+pub fn match_laptop() -> Result<Box<dyn Laptop>, AuraError> {
-// --session --dest org.gnome.SettingsDaemon.Power
+    let dmi = sysfs_class::DmiId::default();
-// --object-path /org/gnome/SettingsDaemon/Power
+    let board_name = dmi.board_name()?;
-// --method org.freedesktop.DBus.Properties.Set org.gnome.SettingsDaemon.Power.Screen Brightness '<int32 65>'"
+    match board_name.as_str() {
+        // The hell does it have a \n for anyway?
+        "GX502GW\n" => Ok(Box::new(LaptopGX502GW::new())),
+        _ => {
+            panic!("could not match laptop");
+        }
+    }
+}
 
+/// All laptop models should implement this trait
+///
+/// `do_hotkey_action` is passed the byte that a hotkey emits, and is expected to
+/// perform whichever action matches that. For now the only key bytes passed in are
+/// the ones which match `byte[0] == hotkey_group_byte`. On the GX502GW the keyboard
+/// has 3 explicit groups: main, vol+media, and the ones that the Linux kernel doesn't
+/// map.
 pub trait Laptop {
-    fn do_hotkey_action(&self, core: &mut RogCore, key_byte: u8);
+    fn do_hotkey_action(&self, core: &mut RogCore, key_byte: u8) -> Result<(), AuraError>;
     fn hotkey_group_byte(&self) -> u8;
+    fn led_iface_num(&self) -> u8;
     fn supported_modes(&self) -> &[BuiltInModeByte];
     fn usb_vendor(&self) -> u16;
     fn usb_product(&self) -> u16;
@@ -22,9 +38,10 @@ pub struct LaptopGX502GW {
     board_name: &'static str,
     prod_family: &'static str,
     hotkey_group_byte: u8,
-    min_bright: u8,
+    min_led_bright: u8,
-    max_bright: u8,
+    max_led_bright: u8,
-    supported_modes: Vec<BuiltInModeByte>,
+    led_iface_num: u8,
+    supported_modes: [BuiltInModeByte; 12],
     backlight: Backlight,
 }
 
@@ -37,9 +54,10 @@ impl LaptopGX502GW {
             board_name: "GX502GW",
             prod_family: "Zephyrus S",
             hotkey_group_byte: 0x5a,
-            min_bright: 0x00,
+            min_led_bright: 0x00,
-            max_bright: 0x03,
+            max_led_bright: 0x03,
-            supported_modes: vec![
+            led_iface_num: 0x81,
+            supported_modes: [
                 BuiltInModeByte::Stable,
                 BuiltInModeByte::Breathe,
                 BuiltInModeByte::Cycle,
@@ -58,28 +76,28 @@ impl LaptopGX502GW {
     }
 }
 impl Laptop for LaptopGX502GW {
-    fn do_hotkey_action(&self, rogcore: &mut RogCore, key_byte: u8) {
+    fn do_hotkey_action(&self, rogcore: &mut RogCore, key_byte: u8) -> Result<(), AuraError> {
         match GX502GWKeys::from(key_byte) {
             GX502GWKeys::Rog => {
                 println!("ROG!");
             }
             GX502GWKeys::LedBrightUp => {
                 let mut bright = rogcore.config().brightness;
-                if bright < self.max_bright {
+                if bright < self.max_led_bright {
                     bright += 1;
                     rogcore.config_mut().brightness = bright;
                 }
-                let bytes = RogCore::aura_brightness_bytes(bright).unwrap();
+                let bytes = RogCore::aura_brightness_bytes(bright)?;
-                rogcore.aura_set_and_save(&bytes).unwrap();
+                rogcore.aura_set_and_save(&bytes)?;
             }
             GX502GWKeys::LedBrightDown => {
                 let mut bright = rogcore.config().brightness;
-                if bright > self.min_bright {
+                if bright > self.min_led_bright {
                     bright -= 1;
                     rogcore.config_mut().brightness = bright;
                 }
-                let bytes = RogCore::aura_brightness_bytes(bright).unwrap();
+                let bytes = RogCore::aura_brightness_bytes(bright)?;
-                rogcore.aura_set_and_save(&bytes).unwrap();
+                rogcore.aura_set_and_save(&bytes)?;
             }
             GX502GWKeys::AuraNext => {
                 let mut mode = rogcore.config().current_mode[3] + 1;
@@ -90,7 +108,7 @@ impl Laptop for LaptopGX502GW {
                 }
                 rogcore.config_mut().current_mode[3] = mode;
                 if let Some(bytes) = rogcore.config_mut().get_current() {
-                    rogcore.aura_set_and_save(&bytes).unwrap();
+                    rogcore.aura_set_and_save(&bytes)?;
                 }
             }
             GX502GWKeys::AuraPrevious => {
@@ -104,7 +122,7 @@ impl Laptop for LaptopGX502GW {
                 }
                 rogcore.config_mut().current_mode[3] = mode;
                 if let Some(bytes) = rogcore.config_mut().get_current() {
-                    rogcore.aura_set_and_save(&bytes).unwrap();
+                    rogcore.aura_set_and_save(&bytes)?;
                     rogcore.config().write();
                 }
             }
@@ -118,7 +136,7 @@ impl Laptop for LaptopGX502GW {
                 rogcore.suspend();
             }
             GX502GWKeys::AirplaneMode => {
-                rogcore.toggle_airplane_mode();
+                rogcore.toggle_airplane_mode()?;
             }
             _ => {
                 if key_byte != 0 {
@@ -126,6 +144,7 @@ impl Laptop for LaptopGX502GW {
                 }
             }
         }
+        Ok(())
     }
     fn hotkey_group_byte(&self) -> u8 {
         self.hotkey_group_byte
@@ -147,6 +166,10 @@ impl Laptop for LaptopGX502GW {
     fn prod_family(&self) -> &str {
         &self.prod_family
     }
+
+    fn led_iface_num(&self) -> u8 {
+        self.led_iface_num
+    }
 }
 
 pub enum GX502GWKeys {