Refactor code to reduce LedWriter to single thread

- Fixes race conditions (mostly) between awaits
- Fixes possible deadlock of mode changes when effect is running
- Fixes sending kill signal to effect clients

aura/examples/per-key-effect-2.rs

@@ -1,7 +1,7 @@
 use rog_aura::{AuraDbusWriter, Key, KeyColourArray};
 
 fn main() -> Result<(), Box<dyn std::error::Error>> {
-    let writer = AuraDbusWriter::new()?;
+    let mut writer = AuraDbusWriter::new()?;
 
     let mut per_key_led = Vec::new();
     let mut key_colours = KeyColourArray::new();

aura/src/aura_dbus.rs

@@ -1,20 +1,26 @@
 use super::*;
-use dbus::{ffidisp::Connection, Message};
+use dbus::blocking::BlockingSender;
+use dbus::channel::Sender;
+use dbus::{blocking::Connection, Message};
 use std::error::Error;
+use std::sync::{Arc, Mutex};
 use std::{thread, time::Duration};
 
 /// Simplified way to write a effect block
 pub struct AuraDbusWriter {
-    connection: Connection,
+    connection: Box<Connection>,
     block_time: u64,
+    stop: Arc<Mutex<bool>>,
 }
 
 impl AuraDbusWriter {
     #[inline]
     pub fn new() -> Result<Self, Box<dyn Error>> {
+        let connection = Connection::new_system()?;
         Ok(AuraDbusWriter {
-            connection: Connection::new_system()?,
+            connection: Box::new(connection),
             block_time: 10,
+            stop: Arc::new(Mutex::new(false)),
         })
     }
 
@@ -22,7 +28,23 @@ impl AuraDbusWriter {
     /// the keyboard LED EC in the correct mode
     #[inline]
     pub fn init_effect(&self) -> Result<(), Box<dyn Error>> {
-        let msg = Message::new_method_call(DBUS_NAME, DBUS_PATH, DBUS_IFACE, "ledmessage")?
+        let match_rule = dbus::message::MatchRule::new_signal(DBUS_IFACE, "LedCancelEffect");
+        let stopper = self.stop.clone();
+        self.connection
+            .add_match(match_rule, move |_: (), _, msg| {
+                println!("GOT {:?}", msg);
+                if let Ok(stop) = msg.read1::<bool>() {
+                    if stop {
+                        if let Ok(mut lock) = stopper.lock() {
+                            println!("SHOULD STOP");
+                            *lock = true;
+                        }
+                    }
+                }
+                true
+            })?;
+
+        let msg = Message::new_method_call(DBUS_NAME, DBUS_PATH, DBUS_IFACE, "LedWriteBytes")?
             .append1(KeyColourArray::get_init_msg());
         self.connection.send(msg).unwrap();
         Ok(())
@@ -34,11 +56,13 @@ impl AuraDbusWriter {
     /// be written to the keyboard EC. This should not be async.
     #[inline]
     pub fn write_colour_block(
-        &self,
+        &mut self,
         key_colour_array: &KeyColourArray,
     ) -> Result<(), Box<dyn Error>> {
+        self.connection.process(Duration::from_micros(300))?;
+
         let group = key_colour_array.get();
-        let msg = Message::new_method_call(DBUS_NAME, DBUS_PATH, DBUS_IFACE, "ledeffect")?
+        let msg = Message::new_method_call(DBUS_NAME, DBUS_PATH, DBUS_IFACE, "LedWriteEffect")?
             .append1(&group[0].to_vec())
             .append1(&group[1].to_vec())
             .append1(&group[2].to_vec())
@@ -51,14 +75,21 @@ impl AuraDbusWriter {
             .append1(&group[9].to_vec());
         self.connection.send(msg).unwrap();
         thread::sleep(Duration::from_millis(self.block_time));
+        if let Ok(lock) = self.stop.try_lock() {
+            if *lock {
+                panic!("Stopping!");
+            }
+        }
         Ok(())
     }
 
     #[inline]
     pub fn write_bytes(&self, bytes: &[u8]) -> Result<String, Box<dyn std::error::Error>> {
-        let msg = Message::new_method_call(DBUS_NAME, DBUS_PATH, DBUS_IFACE, "ledmessage")?
+        let msg = Message::new_method_call(DBUS_NAME, DBUS_PATH, DBUS_IFACE, "LedWriteBytes")?
             .append1(bytes.to_vec());
-        let r = self.connection.send_with_reply_and_block(msg, 5000)?;
+        let r = self
+            .connection
+            .send_with_reply_and_block(msg, Duration::from_millis(5000))?;
         if let Some(reply) = r.get1::<&str>() {
             return Ok(reply.to_owned());
         }

aura/src/builtins.rs

@@ -44,7 +44,7 @@ impl BuiltInModeBytes {
     }
 
     #[inline]
-    pub fn get_field_from(&mut self, byte: u8) -> Option<&[u8]> {
+    pub fn get_field_from(&self, byte: u8) -> Option<&[u8]> {
         let bytes = match BuiltInModeByte::from(byte) {
             BuiltInModeByte::Single => &self.stable,
             BuiltInModeByte::Breathing => &self.breathe,

rog-core/Cargo.toml

@@ -29,7 +29,7 @@ env_logger = "^0.7.1"
 # async
 dbus = { version = "^0.8.2", features = ["futures"] }
 dbus-tokio = "^0.5.1"
-tokio = { version = "0.2.4", features = ["rt-threaded", "macros", "sync"] }
+tokio = { version = "0.2.4", features = ["rt-threaded", "sync"] }
 
 # serialisation
 serde = "1.0"

rog-core/src/core.rs

@@ -300,6 +300,16 @@ where
     }
 }
 
+pub enum AuraCommand {
+    BrightInc,
+    BrightDec,
+    BuiltinNext,
+    BuiltinPrev,
+    WriteBytes(Vec<u8>),
+    WriteEffect(Vec<Vec<u8>>),
+    ReloadLast,
+}
+
 /// UNSAFE: Must live as long as RogCore
 ///
 /// Because we're holding a pointer to something that *may* go out of scope while the
@@ -310,6 +320,8 @@ where
     C: rusb::UsbContext,
 {
     handle: NonNull<DeviceHandle<C>>,
+    bright_min_max: (u8, u8),
+    supported_modes: Vec<BuiltInModeByte>,
     led_endpoint: u8,
     initialised: bool,
     _phantom: PhantomData<&'d DeviceHandle<C>>,
@@ -323,61 +335,125 @@ impl<'d, C> LedWriter<'d, C>
 where
     C: rusb::UsbContext,
 {
-    pub fn new(device_handle: NonNull<DeviceHandle<C>>, led_endpoint: u8) -> Self {
+    pub fn new(
+        device_handle: NonNull<DeviceHandle<C>>,
+        led_endpoint: u8,
+        bright_min_max: (u8, u8),
+        supported_modes: Vec<BuiltInModeByte>,
+    ) -> Self {
         LedWriter {
             handle: device_handle,
             led_endpoint,
+            bright_min_max,
+            supported_modes,
             initialised: false,
             _phantom: PhantomData,
         }
     }
 
+    pub async fn do_command(
+        &mut self,
+        command: AuraCommand,
+        config: &mut Config,
+    ) -> Result<(), AuraError> {
+        if !self.initialised {
+            self.write_bytes(&LED_INIT1).await?;
+            self.write_bytes(LED_INIT2.as_bytes()).await?;
+            self.write_bytes(&LED_INIT3).await?;
+            self.write_bytes(LED_INIT4.as_bytes()).await?;
+            self.write_bytes(&LED_INIT5).await?;
+            self.initialised = true;
+        }
+
+        match command {
+            AuraCommand::BrightInc => {
+                let mut bright = config.brightness;
+                if bright < self.bright_min_max.1 {
+                    bright += 1;
+                    config.brightness = bright;
+                    let bytes = aura_brightness_bytes(bright);
+                    self.set_and_save(&bytes, config).await?;
+                    info!("Increased LED brightness to {:#?}", bright);
+                }
+            }
+            AuraCommand::BrightDec => {
+                let mut bright = config.brightness;
+                if bright > self.bright_min_max.0 {
+                    bright -= 1;
+                    config.brightness = bright;
+                    let bytes = aura_brightness_bytes(bright);
+                    self.set_and_save(&bytes, config).await?;
+                    info!("Decreased LED brightness to {:#?}", bright);
+                }
+            }
+            AuraCommand::BuiltinNext => {
+                // TODO: different path for multi-zone (byte 2 controlled, non-zero)
+                let mode_curr = config.current_mode[3];
+                let idx = self
+                    .supported_modes
+                    .binary_search(&mode_curr.into())
+                    .unwrap();
+                let idx_next = if idx < self.supported_modes.len() - 1 {
+                    idx + 1
+                } else {
+                    0
+                };
+                self.set_builtin(config, idx_next).await?;
+            }
+            AuraCommand::BuiltinPrev => {
+                // TODO: different path for multi-zone (byte 2 controlled, non-zero)
+                let mode_curr = config.current_mode[3];
+                let idx = self
+                    .supported_modes
+                    .binary_search(&mode_curr.into())
+                    .unwrap();
+                let idx_next = if idx > 0 {
+                    idx - 1
+                } else {
+                    self.supported_modes.len() - 1
+                };
+                self.set_builtin(config, idx_next).await?;
+            }
+            AuraCommand::WriteBytes(bytes) => self.write_bytes(&bytes).await?,
+            AuraCommand::WriteEffect(effect) => self.write_effect(effect).await?,
+            AuraCommand::ReloadLast => self.reload_last_builtin(&config).await?,
+        }
+        Ok(())
+    }
+
     /// Should only be used if the bytes you are writing are verified correct
-    pub async fn aura_write(&mut self, message: &[u8]) -> Result<(), AuraError> {
+    async fn write_bytes(&mut self, message: &[u8]) -> Result<(), AuraError> {
         match unsafe { self.handle.as_ref() }.write_interrupt(
             self.led_endpoint,
             message,
-            Duration::from_millis(1),
+            Duration::from_millis(2),
         ) {
             Ok(_) => {}
             Err(err) => match err {
                 rusb::Error::Timeout => {}
-                _ => error!("Failed to read keyboard interrupt: {:?}", err),
+                _ => error!("Failed to write to led interrupt: {:?}", err),
             },
         }
         Ok(())
     }
 
-    async fn aura_write_messages(&mut self, messages: &[&[u8]]) -> Result<(), AuraError> {
-        if !self.initialised {
-            self.aura_write(&LED_INIT1).await?;
-            self.aura_write(LED_INIT2.as_bytes()).await?;
-            self.aura_write(&LED_INIT3).await?;
-            self.aura_write(LED_INIT4.as_bytes()).await?;
-            self.aura_write(&LED_INIT5).await?;
-            self.initialised = true;
-        }
-
+    async fn write_array_of_bytes(&mut self, messages: &[&[u8]]) -> Result<(), AuraError> {
         for message in messages {
-            self.aura_write(*message).await?;
-            self.aura_write(&LED_SET).await?;
+            self.write_bytes(*message).await?;
+            self.write_bytes(&LED_SET).await?;
         }
         // Changes won't persist unless apply is set
-        self.aura_write(&LED_APPLY).await?;
+        self.write_bytes(&LED_APPLY).await?;
         Ok(())
     }
 
     /// Write an effect block
     ///
     /// `aura_effect_init` must be called any effect routine, and called only once.
-    pub async fn async_write_effect(
-        &self,
-        endpoint: u8,
-        effect: Vec<Vec<u8>>,
-    ) -> Result<(), AuraError> {
+    async fn write_effect(&self, effect: Vec<Vec<u8>>) -> Result<(), AuraError> {
         for row in effect.iter() {
             match unsafe { self.handle.as_ref() }.write_interrupt(
-                endpoint,
+                self.led_endpoint,
                 row,
                 Duration::from_millis(1),
             ) {
@@ -392,19 +468,15 @@ where
     }
 
     /// Used to set a builtin mode and save the settings for it
-    pub async fn aura_set_and_save(
-        &mut self,
-        supported_modes: &[BuiltInModeByte],
-        bytes: &[u8],
-        config: &mut Config,
-    ) -> Result<(), AuraError> {
+    async fn set_and_save(&mut self, bytes: &[u8], config: &mut Config) -> Result<(), AuraError> {
         let mode = BuiltInModeByte::from(bytes[3]);
+        // safety pass-through of possible effect write
         if bytes[1] == 0xbc {
-            self.aura_write(bytes).await?;
+            self.write_bytes(bytes).await?;
             return Ok(());
-        } else if supported_modes.contains(&mode) || bytes[1] == 0xba {
+        } else if self.supported_modes.contains(&mode) || bytes[1] == 0xba {
             let messages = [bytes];
-            self.aura_write_messages(&messages).await?;
+            self.write_array_of_bytes(&messages).await?;
             config.set_field_from(bytes);
             config.write();
             return Ok(());
@@ -413,93 +485,31 @@ where
         Err(AuraError::NotSupported)
     }
 
-    pub async fn aura_bright_inc(
-        &mut self,
-        supported_modes: &[BuiltInModeByte],
-        max_bright: u8,
-        config: &mut Config,
-    ) -> Result<(), AuraError> {
-        let mut bright = config.brightness;
-        if bright < max_bright {
-            bright += 1;
-            config.brightness = bright;
-            let bytes = aura_brightness_bytes(bright);
-            self.aura_set_and_save(supported_modes, &bytes, config)
-                .await?;
-            info!("Increased LED brightness to {:#?}", bright);
-        }
-        Ok(())
-    }
-
-    pub async fn aura_bright_dec(
-        &mut self,
-        supported_modes: &[BuiltInModeByte],
-        min_bright: u8,
-        config: &mut Config,
-    ) -> Result<(), AuraError> {
-        let mut bright = config.brightness;
-        if bright > min_bright {
-            bright -= 1;
-            config.brightness = bright;
-            let bytes = aura_brightness_bytes(bright);
-            self.aura_set_and_save(supported_modes, &bytes, config)
-                .await?;
-            info!("Decreased LED brightness to {:#?}", bright);
-        }
+    /// Used to set a builtin mode and save the settings for it
+    async fn reload_last_builtin(&mut self, config: &Config) -> Result<(), AuraError> {
+        let mode_curr = config.current_mode[3];
+        let mode = config
+            .builtin_modes
+            .get_field_from(mode_curr)
+            .unwrap()
+            .to_owned();
+        self.write_bytes(&mode).await?;
+        info!("Reloaded last built-in mode");
         Ok(())
     }
 
     /// Select next Aura effect
     ///
     /// If the current effect is the last one then the effect selected wraps around to the first.
-    pub async fn aura_mode_next(
-        &mut self,
-        supported_modes: &[BuiltInModeByte],
-        config: &mut Config,
-    ) -> Result<(), AuraError> {
-        // TODO: different path for multi-zone (byte 2 controlled, non-zero)
-        let mode_curr = config.current_mode[3];
-        let idx = supported_modes.binary_search(&mode_curr.into()).unwrap();
-        let idx_next = if idx < supported_modes.len() - 1 {
-            idx + 1
-        } else {
-            0
-        };
+    async fn set_builtin(&mut self, config: &mut Config, index: usize) -> Result<(), AuraError> {
         let mode_next = config
             .builtin_modes
-            .get_field_from(supported_modes[idx_next].into())
+            .get_field_from(self.supported_modes[index].into())
             .unwrap()
             .to_owned();
-        self.aura_set_and_save(supported_modes, &mode_next, config)
-            .await?;
-        info!("Switched LED mode to {:#?}", supported_modes[idx_next]);
-        Ok(())
-    }
-
-    /// Select previous Aura effect
-    ///
-    /// If the current effect is the first one then the effect selected wraps around to the last.
-    pub async fn aura_mode_prev(
-        &mut self,
-        supported_modes: &[BuiltInModeByte],
-        config: &mut Config,
-    ) -> Result<(), AuraError> {
-        // TODO: different path for multi-zone (byte 2 controlled, non-zero)
-        let mode_curr = config.current_mode[3];
-        let idx = supported_modes.binary_search(&mode_curr.into()).unwrap();
-        let idx_next = if idx > 0 {
-            idx - 1
-        } else {
-            supported_modes.len() - 1
-        };
-        let mode_next = config
-            .builtin_modes
-            .get_field_from(supported_modes[idx_next].into())
-            .unwrap()
-            .to_owned();
-        self.aura_set_and_save(supported_modes, &mode_next, config)
-            .await?;
-        info!("Switched LED mode to {:#?}", supported_modes[idx_next]);
+        println!("{:X?}", &mode_next);
+        self.set_and_save(&mode_next, config).await?;
+        info!("Switched LED mode to {:#?}", self.supported_modes[index]);
         Ok(())
     }
 }

rog-core/src/daemon.rs

@@ -1,13 +1,14 @@
 use crate::{config::Config, core::*, laptops::match_laptop};
 use dbus::{
+    channel::Sender,
     nonblock::Process,
-    tree::{Factory, MTSync, Method, MethodErr, Tree},
+    tree::{Factory, MTSync, Method, MethodErr, Signal, Tree},
 };
 use dbus_tokio::connection;
 use log::{error, info, warn};
-use rog_aura::{BuiltInModeByte, DBUS_IFACE, DBUS_PATH};
+use rog_aura::{DBUS_IFACE, DBUS_PATH};
 use std::error::Error;
-use std::sync::Arc;
+use std::sync::{mpsc, Arc};
 use std::time::{Duration, Instant};
 use tokio::sync::Mutex;
 
@@ -46,19 +47,17 @@ pub async fn start_daemon() -> Result<(), Box<dyn Error>> {
 
     // Reload settings
     rogcore.reload(&mut config).await?;
-    let mut led_writer = LedWriter::new(rogcore.get_raw_device_handle(), laptop.led_endpoint());
-    {
-        let mode_curr = config.current_mode[3];
-        let mode = config
-            .builtin_modes
-            .get_field_from(BuiltInModeByte::from(mode_curr).into())
-            .unwrap()
-            .to_owned();
-        led_writer.aura_write(&mode).await?;
-    }
+    let mut led_writer = LedWriter::new(
+        rogcore.get_raw_device_handle(),
+        laptop.led_endpoint(),
+        (laptop.min_led_bright(), laptop.max_led_bright()),
+        laptop.supported_modes().to_owned(),
+    );
+    led_writer
+        .do_command(AuraCommand::ReloadLast, &mut config)
+        .await?;
 
     // Set up the mutexes
-    let led_writer = Arc::new(Mutex::new(led_writer));
     let config = Arc::new(Mutex::new(config));
     let (resource, connection) = connection::new_system_sync()?;
     tokio::spawn(async {
@@ -70,13 +69,12 @@ pub async fn start_daemon() -> Result<(), Box<dyn Error>> {
         .request_name(DBUS_IFACE, false, true, true)
         .await?;
 
-    let (tree, input, effect) = dbus_create_tree();
+    let (aura_command_send, aura_command_recv) = mpsc::sync_channel::<AuraCommand>(1);
+
+    let (tree, input, effect, effect_cancel_signal) = dbus_create_tree();
     // We add the tree to the connection so that incoming method calls will be handled.
     tree.start_receive_send(&*connection);
 
-    let supported = Vec::from(laptop.supported_modes());
-    let led_endpoint = laptop.led_endpoint();
-
     // 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(
@@ -85,15 +83,15 @@ pub async fn start_daemon() -> Result<(), Box<dyn Error>> {
         laptop.key_filter().to_owned(),
     );
 
-    let led_writer1 = led_writer.clone();
     let config1 = config.clone();
     // start the keyboard reader and laptop-action loop
     let key_read_handle = tokio::spawn(async move {
         loop {
+            let acs = aura_command_send.clone();
             let data = keyboard_reader.poll_keyboard().await;
             if let Some(bytes) = data {
                 laptop
-                    .run(&mut rogcore, &led_writer1, &config1, bytes)
+                    .run(&mut rogcore, &config1, bytes, acs)
                     .await
                     .map_err(|err| warn!("{:?}", err))
                     .unwrap();
@@ -107,32 +105,60 @@ pub async fn start_daemon() -> Result<(), Box<dyn Error>> {
         loop {
             connection.process_all();
 
-            let led_writer = led_writer.clone();
-            if let Ok(mut lock) = input.try_lock() {
-                if let Some(bytes) = lock.take() {
-                    let mut led_writer = led_writer.lock().await;
-                    let mut config = config.lock().await;
-                    led_writer
-                        .aura_set_and_save(&supported, &bytes, &mut config)
-                        .await
-                        .map_err(|err| warn!("{:?}", err))
-                        .unwrap();
-                    time_mark = Instant::now();
-                }
-            }
+            let res = aura_command_recv.recv_timeout(Duration::from_micros(50));
+            if let Ok(command) = res {
+                let mut config = config.lock().await;
+                led_writer
+                    .do_command(command, &mut config)
+                    .await
+                    .map_err(|err| warn!("{:?}", err))
+                    .unwrap();
 
-            // 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() {
-                    let led_writer = led_writer.lock().await;
-                    led_writer
-                        .async_write_effect(led_endpoint, stuff)
-                        .await
-                        .map_err(|err| warn!("{:?}", err))
-                        .unwrap();
-                    time_mark = Instant::now();
+                connection
+                    .send(
+                        effect_cancel_signal
+                            .msg(&DBUS_PATH.into(), &DBUS_IFACE.into())
+                            .append1(true),
+                    )
+                    .unwrap();
+                // Clear any possible queued effect
+                let mut effect = effect.lock().await;
+                *effect = None;
+                time_mark = Instant::now();
+            } else {
+                if let Ok(mut lock) = input.try_lock() {
+                    if let Some(bytes) = lock.take() {
+                        if bytes.len() > 8 {
+                            let mut config = config.lock().await;
+                            led_writer
+                                .do_command(AuraCommand::WriteBytes(bytes), &mut config)
+                                .await
+                                .map_err(|err| warn!("{:?}", err))
+                                .unwrap();
+                            // Also cancel any effect client
+                            connection
+                                .send(
+                                    effect_cancel_signal.msg(&DBUS_PATH.into(), &DBUS_IFACE.into()),
+                                )
+                                .unwrap();
+                            time_mark = Instant::now();
+                        }
+                    }
+                }
+                // Write a colour block
+                if let Ok(mut effect_lock) = effect.try_lock() {
+                    // Spawn a writer
+                    if let Some(stuff) = effect_lock.take() {
+                        if stuff.len() == 10 {
+                            let mut config = config.lock().await;
+                            led_writer
+                                .do_command(AuraCommand::WriteEffect(stuff), &mut config)
+                                .await
+                                .map_err(|err| warn!("{:?}", err))
+                                .unwrap();
+                            time_mark = Instant::now();
+                        }
+                    }
                 }
             }
 
@@ -158,7 +184,7 @@ fn dbus_create_ledmsg_method(msg: LedMsgType) -> Method<MTSync, ()> {
     let factory = Factory::new_sync::<()>();
     factory
         // method for ledmessage
-        .method("ledmessage", (), {
+        .method("LedWriteBytes", (), {
             move |m| {
                 let bytes: Vec<u8> = m.msg.read1()?;
                 if let Ok(mut lock) = msg.try_lock() {
@@ -181,7 +207,7 @@ fn dbus_create_ledeffect_method(effect: EffectType) -> Method<MTSync, ()> {
     let factory = Factory::new_sync::<()>();
     factory
         // method for ledmessage
-        .method("ledeffect", (), {
+        .method("LedWriteEffect", (), {
             move |m| {
                 if let Ok(mut lock) = effect.try_lock() {
                     let mut iter = m.msg.iter_init();
@@ -219,18 +245,20 @@ fn dbus_create_ledeffect_method(effect: EffectType) -> Method<MTSync, ()> {
         .inarg::<Vec<u8>, _>("bytearray")
 }
 
-fn dbus_create_tree() -> (Tree<MTSync, ()>, LedMsgType, EffectType) {
-    let input: LedMsgType = Arc::new(Mutex::new(None));
-    let effect: EffectType = Arc::new(Mutex::new(None));
+fn dbus_create_tree() -> (Tree<MTSync, ()>, LedMsgType, EffectType, Arc<Signal<()>>) {
+    let input_bytes: LedMsgType = Arc::new(Mutex::new(None));
+    let input_effect: EffectType = Arc::new(Mutex::new(None));
 
     let factory = Factory::new_sync::<()>();
+    let effect_cancel_sig = Arc::new(factory.signal("LedCancelEffect", ()));
     let tree = factory.tree(()).add(
         factory.object_path(DBUS_PATH, ()).add(
             factory
                 .interface(DBUS_IFACE, ())
-                .add_m(dbus_create_ledmsg_method(input.clone()))
-                .add_m(dbus_create_ledeffect_method(effect.clone())),
+                .add_m(dbus_create_ledmsg_method(input_bytes.clone()))
+                .add_m(dbus_create_ledeffect_method(input_effect.clone()))
+                .add_s(effect_cancel_sig.clone()),
         ),
     );
-    (tree, input, effect)
+    (tree, input_bytes, input_effect, effect_cancel_sig)
 }

rog-core/src/laptops.rs

@@ -1,5 +1,5 @@
 use crate::config::Config;
-use crate::core::{LedWriter, RogCore};
+use crate::core::{AuraCommand, RogCore};
 use rog_aura::{error::AuraError, BuiltInModeByte};
 //use keycode::{KeyMap, KeyMappingId, KeyState, KeyboardState};
 use crate::virt_device::ConsumerKeys;
@@ -78,33 +78,37 @@ pub(super) struct LaptopBase {
     //backlight: Backlight,
 }
 
+use std::sync::mpsc;
 use tokio::sync::Mutex;
 
 impl LaptopBase {
     /// Pass in LedWriter as Mutex so it is only locked when required
-    pub(super) async fn run<'a, C>(
+    pub(super) async fn run(
         &self,
         rogcore: &mut RogCore,
-        led_writer: &Mutex<LedWriter<'a, C>>,
         config: &Mutex<Config>,
         key_buf: [u8; 32],
-    ) -> Result<(), AuraError>
-    where
-        C: rusb::UsbContext,
-    {
+        aura_command: mpsc::SyncSender<AuraCommand>,
+    ) -> Result<(), AuraError> {
         match self.usb_product {
             0x1869 | 0x1866 => {
-                self.gx502_runner(rogcore, led_writer, config, key_buf)
+                self.gx502_runner(rogcore, config, key_buf, aura_command)
                     .await
             }
             0x1854 => {
-                self.gl753_runner(rogcore, led_writer, config, key_buf)
+                self.gl753_runner(rogcore, config, key_buf, aura_command)
                     .await
             }
             _ => panic!("No runner available for this device"),
         }
     }
 
+    pub(super) fn min_led_bright(&self) -> u8 {
+        self.min_led_bright
+    }
+    pub(super) fn max_led_bright(&self) -> u8 {
+        self.max_led_bright
+    }
     pub(super) fn led_endpoint(&self) -> u8 {
         self.led_endpoint
     }
@@ -125,47 +129,25 @@ impl LaptopBase {
     }
 
     // 0x1866, per-key LEDs, media-keys split from vendor specific
-    async fn gx502_runner<'a, C>(
+    async fn gx502_runner(
         &self,
         rogcore: &mut RogCore,
-        led_writer: &Mutex<LedWriter<'a, C>>,
         config: &Mutex<Config>,
         key_buf: [u8; 32],
-    ) -> Result<(), AuraError>
-    where
-        C: rusb::UsbContext,
-    {
-        let max_led_bright = self.max_led_bright;
-        let min_led_bright = self.min_led_bright;
-        let supported_modes = self.supported_modes.to_owned();
+        aura_command: mpsc::SyncSender<AuraCommand>,
+    ) -> Result<(), AuraError> {
         match GX502Keys::from(key_buf[1]) {
             GX502Keys::LedBrightUp => {
-                let mut led_writer = led_writer.lock().await;
-                let mut config = config.lock().await;
-                led_writer
-                    .aura_bright_inc(&supported_modes, max_led_bright, &mut config)
-                    .await?;
+                aura_command.send(AuraCommand::BrightInc).unwrap();
             }
             GX502Keys::LedBrightDown => {
-                let mut led_writer = led_writer.lock().await;
-                let mut config = config.lock().await;
-                led_writer
-                    .aura_bright_dec(&supported_modes, min_led_bright, &mut config)
-                    .await?;
+                aura_command.send(AuraCommand::BrightDec).unwrap();
             }
             GX502Keys::AuraNext => {
-                let mut led_writer = led_writer.lock().await;
-                let mut config = config.lock().await;
-                led_writer
-                    .aura_mode_next(&supported_modes, &mut config)
-                    .await?;
+                aura_command.send(AuraCommand::BuiltinNext).unwrap();
             }
             GX502Keys::AuraPrevious => {
-                let mut led_writer = led_writer.lock().await;
-                let mut config = config.lock().await;
-                led_writer
-                    .aura_mode_prev(&supported_modes, &mut config)
-                    .await?;
+                aura_command.send(AuraCommand::BuiltinPrev).unwrap();
             }
             GX502Keys::ScreenBrightUp => {
                 rogcore.virt_keys().press(ConsumerKeys::BacklightInc.into())
@@ -210,33 +192,19 @@ impl LaptopBase {
     }
 
     // GL753VE == 0x1854, 4 zone keyboard
-    async fn gl753_runner<'a, C>(
+    async fn gl753_runner(
         &self,
         rogcore: &mut RogCore,
-        led_writer: &Mutex<LedWriter<'a, C>>,
-        config: &Mutex<Config>,
+        _config: &Mutex<Config>,
         key_buf: [u8; 32],
-    ) -> Result<(), AuraError>
-    where
-        C: rusb::UsbContext,
-    {
-        let max_led_bright = self.max_led_bright;
-        let min_led_bright = self.min_led_bright;
-        let supported_modes = self.supported_modes.to_owned();
+        aura_command: mpsc::SyncSender<AuraCommand>,
+    ) -> Result<(), AuraError> {
         match GL753Keys::from(key_buf[1]) {
             GL753Keys::LedBrightUp => {
-                let mut led_writer = led_writer.lock().await;
-                let mut config = config.lock().await;
-                led_writer
-                    .aura_bright_inc(&supported_modes, max_led_bright, &mut config)
-                    .await?;
+                aura_command.send(AuraCommand::BrightInc).unwrap();
             }
             GL753Keys::LedBrightDown => {
-                let mut led_writer = led_writer.lock().await;
-                let mut config = config.lock().await;
-                led_writer
-                    .aura_bright_dec(&supported_modes, min_led_bright, &mut config)
-                    .await?;
+                aura_command.send(AuraCommand::BrightDec).unwrap();
             }
             GL753Keys::ScreenBrightUp => {
                 rogcore.virt_keys().press(ConsumerKeys::BacklightInc.into())

rog-core/src/main.rs

@@ -40,9 +40,12 @@ struct LedModeCommand {
 #[tokio::main]
 pub async fn main() -> Result<(), Box<dyn std::error::Error>> {
     let mut builder = Builder::new();
-    builder.target(Target::Stdout);
-    builder.format_timestamp(None);
-    builder.filter(None, LevelFilter::Info).init();
+    builder
+        .target(Target::Stdout)
+        .format_module_path(false)
+        .format_timestamp(None)
+        .filter(None, LevelFilter::Info)
+        .init();
 
     let parsed = CLIStart::parse_args_default_or_exit();
     if parsed.daemon {