diff --git a/Cargo.lock b/Cargo.lock index fe2bda56..809812a3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1022,6 +1022,7 @@ version = "1.3.3" dependencies = [ "gif", "glam", + "log", "pix", "png_pong", "serde", diff --git a/asusctl/src/cli_opts.rs b/asusctl/src/cli_opts.rs index 3be33db7..13585ff6 100644 --- a/asusctl/src/cli_opts.rs +++ b/asusctl/src/cli_opts.rs @@ -59,7 +59,10 @@ pub struct LedModeCommand { help = "set the keyboard LED suspend animation to enabled while the device is suspended" )] pub sleep_enable: Option, - #[options(meta = "", help = "set the full keyboard LEDs (keys and side) to enabled")] + #[options( + meta = "", + help = "set the full keyboard LEDs (keys and side) to enabled" + )] pub all_leds_enable: Option, #[options(meta = "", help = "set the keyboard keys LEDs to enabled")] pub keys_leds_enable: Option, diff --git a/asusctl/src/main.rs b/asusctl/src/main.rs index fff588fe..fe0313c5 100644 --- a/asusctl/src/main.rs +++ b/asusctl/src/main.rs @@ -365,8 +365,7 @@ fn handle_led_mode( return true; } } - if supported.multizone_led_mode && command.trim().starts_with("multi") - { + if supported.multizone_led_mode && command.trim().starts_with("multi") { return true; } false diff --git a/daemon-user/src/ctrl_anime.rs b/daemon-user/src/ctrl_anime.rs index c81781ed..8e5daa1c 100644 --- a/daemon-user/src/ctrl_anime.rs +++ b/daemon-user/src/ctrl_anime.rs @@ -91,12 +91,16 @@ impl<'a> CtrlAnimeInner<'static> { for action in self.sequences.iter() { match action { ActionData::Animation(frames) => { - rog_anime::run_animation(frames, self.do_early_return.clone(), &|output| { + rog_anime::run_animation(frames, &|output| { + if self.do_early_return.load(Ordering::Acquire) { + return Ok(true); // Do safe exit + } self.client .proxies() .anime() .write(output) .map_err(|e| AnimeError::Dbus(format!("{}", e))) + .map(|_| false) })?; } ActionData::Image(image) => { diff --git a/daemon/src/ctrl_anime/mod.rs b/daemon/src/ctrl_anime/mod.rs index 14a34be6..8fc85077 100644 --- a/daemon/src/ctrl_anime/mod.rs +++ b/daemon/src/ctrl_anime/mod.rs @@ -141,6 +141,7 @@ impl CtrlAnime { warn!("AniMe system actions was empty"); return; } + // Loop rules: // - Lock the mutex **only when required**. That is, the lock must be held for the shortest duration possible. // - An AtomicBool used for thread exit should be checked in every loop, including nested @@ -150,55 +151,54 @@ impl CtrlAnime { std::thread::Builder::new() .name("AniMe system thread start".into()) .spawn(move || { - info!("AniMe system thread started"); + info!("AniMe new system thread started"); // Getting copies of these Atomics is done *in* the thread to ensure // we don't block other threads/main let thread_exit; let thread_running; - // First two loops are to ensure we *do* aquire a lock on the mutex - // The reason the loop is required is because the USB writes can block - // for up to 10ms. We can't fail to get the atomics. loop { if let Ok(lock) = inner.try_lock() { thread_exit = lock.thread_exit.clone(); thread_running = lock.thread_running.clone(); - // Make any running loop exit first - thread_exit.store(true, Ordering::SeqCst); break; } } - - loop { - // wait for other threads to set not running so we know they exited - if !thread_running.load(Ordering::SeqCst) { - info!("AniMe forced a thread to exit"); - break; - } + // First two loops are to ensure we *do* aquire a lock on the mutex + // The reason the loop is required is because the USB writes can block + // for up to 10ms. We can't fail to get the atomics. + while thread_running.load(Ordering::SeqCst) { + // Make any running loop exit first + thread_exit.store(true, Ordering::SeqCst); + break; } + info!("AniMe no previous system thread running (now)"); thread_exit.store(false, Ordering::SeqCst); - thread_running.store(true, Ordering::SeqCst); 'main: loop { + thread_running.store(true, Ordering::SeqCst); for action in actions.iter() { if thread_exit.load(Ordering::SeqCst) { break 'main; } match action { ActionData::Animation(frames) => { - if let Err(err) = rog_anime::run_animation( - frames, - thread_exit.clone(), - &|frame| { - inner - .try_lock() - .map(|lock| lock.write_data_buffer(frame)) - .map_err(|err| { - warn!("rog_anime::run_animation:callback {}", err); - AnimeError::NoFrames - }) - }, - ) { + if let Err(err) = rog_anime::run_animation(frames, &|frame| { + if thread_exit.load(Ordering::Acquire) { + info!("rog-anime: frame-loop was asked to exit"); + return Ok(true); // Do safe exit + } + inner + .try_lock() + .map(|lock| { + lock.write_data_buffer(frame); + false // Don't exit yet + }) + .map_err(|err| { + warn!("rog_anime::run_animation:callback {}", err); + AnimeError::NoFrames + }) + }) { warn!("rog_anime::run_animation:Animation {}", err); break 'main; }; diff --git a/daemon/src/ctrl_aura/config.rs b/daemon/src/ctrl_aura/config.rs index 6f277bd4..a562b552 100644 --- a/daemon/src/ctrl_aura/config.rs +++ b/daemon/src/ctrl_aura/config.rs @@ -83,7 +83,6 @@ impl AuraConfigV407 { } } - #[derive(Deserialize, Serialize)] pub struct AuraConfig { pub brightness: LedBrightness, @@ -94,7 +93,7 @@ pub struct AuraConfig { pub sleep_anim_enabled: bool, pub all_leds_enabled: bool, pub keys_leds_enabled: bool, - pub side_leds_enabled: bool + pub side_leds_enabled: bool, } impl Default for AuraConfig { diff --git a/daemon/src/ctrl_aura/controller.rs b/daemon/src/ctrl_aura/controller.rs index 92f4161a..6698d1f8 100644 --- a/daemon/src/ctrl_aura/controller.rs +++ b/daemon/src/ctrl_aura/controller.rs @@ -9,10 +9,9 @@ use crate::{ use async_trait::async_trait; use log::{error, info, warn}; use logind_zbus::manager::ManagerProxy; +use rog_aura::usb::leds_message; use rog_aura::{ - usb::{ - LED_APPLY, LED_SET - }, + usb::{LED_APPLY, LED_SET}, AuraEffect, LedBrightness, LED_MSG_LEN, }; use rog_supported::LedSupportedFunctions; @@ -23,14 +22,11 @@ use std::path::Path; use std::sync::Arc; use std::sync::Mutex; use zbus::Connection; -use rog_aura::usb::leds_message; use crate::GetSupported; use super::config::AuraConfig; - - impl GetSupported for CtrlKbdLed { type A = LedSupportedFunctions; @@ -281,20 +277,19 @@ impl CtrlKbdLed { self.set_brightness(self.config.brightness) } - - /// Set combination state for boot animation/sleep animation/all leds/keys leds/side leds LED active pub(super) fn set_power_states(&self, config: &AuraConfig) -> Result<(), RogError> { - - let bytes = leds_message(config.boot_anim_enabled, - config.sleep_anim_enabled, - config.all_leds_enabled, - config.keys_leds_enabled, - config.side_leds_enabled); + let bytes = leds_message( + config.boot_anim_enabled, + config.sleep_anim_enabled, + config.all_leds_enabled, + config.keys_leds_enabled, + config.side_leds_enabled, + ); // Quite ugly, must be a more idiomatic way to do let message = [ - 0x5d, 0xbd, 0x01, bytes[0], bytes[1], bytes[2], 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 + 0x5d, 0xbd, 0x01, bytes[0], bytes[1], bytes[2], 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ]; self.write_bytes(&message)?; diff --git a/daemon/src/ctrl_aura/zbus.rs b/daemon/src/ctrl_aura/zbus.rs index 48d96b87..30c20ea3 100644 --- a/daemon/src/ctrl_aura/zbus.rs +++ b/daemon/src/ctrl_aura/zbus.rs @@ -38,15 +38,15 @@ impl CtrlKbdLedZbus { ctrl.config.write(); ctrl.set_power_states(&ctrl.config) - .map_err(|err| warn!("{}", err)) - .ok(); + .map_err(|err| warn!("{}", err)) + .ok(); states = Some(LedPowerStates { boot_anim: ctrl.config.boot_anim_enabled, sleep_anim: ctrl.config.sleep_anim_enabled, all_leds: ctrl.config.all_leds_enabled, keys_leds: ctrl.config.keys_leds_enabled, - side_leds: ctrl.config.side_leds_enabled + side_leds: ctrl.config.side_leds_enabled, }); } // Need to pull state out like this due to MutexGuard @@ -69,15 +69,15 @@ impl CtrlKbdLedZbus { ctrl.config.write(); ctrl.set_power_states(&ctrl.config) - .map_err(|err| warn!("{}", err)) - .ok(); + .map_err(|err| warn!("{}", err)) + .ok(); states = Some(LedPowerStates { boot_anim: ctrl.config.boot_anim_enabled, sleep_anim: ctrl.config.sleep_anim_enabled, all_leds: ctrl.config.all_leds_enabled, keys_leds: ctrl.config.keys_leds_enabled, - side_leds: ctrl.config.side_leds_enabled + side_leds: ctrl.config.side_leds_enabled, }); } if let Some(states) = states { @@ -109,7 +109,7 @@ impl CtrlKbdLedZbus { sleep_anim: ctrl.config.sleep_anim_enabled, all_leds: ctrl.config.all_leds_enabled, keys_leds: ctrl.config.keys_leds_enabled, - side_leds: ctrl.config.side_leds_enabled + side_leds: ctrl.config.side_leds_enabled, }); } // Need to pull state out like this due to MutexGuard @@ -140,7 +140,7 @@ impl CtrlKbdLedZbus { sleep_anim: ctrl.config.sleep_anim_enabled, all_leds: ctrl.config.all_leds_enabled, keys_leds: ctrl.config.keys_leds_enabled, - side_leds: ctrl.config.side_leds_enabled + side_leds: ctrl.config.side_leds_enabled, }); } // Need to pull state out like this due to MutexGuard @@ -171,7 +171,7 @@ impl CtrlKbdLedZbus { sleep_anim: ctrl.config.sleep_anim_enabled, all_leds: ctrl.config.all_leds_enabled, keys_leds: ctrl.config.keys_leds_enabled, - side_leds: ctrl.config.side_leds_enabled + side_leds: ctrl.config.side_leds_enabled, }); } // Need to pull state out like this due to MutexGuard diff --git a/rog-anime/Cargo.toml b/rog-anime/Cargo.toml index 7e7d0f55..2d410ee6 100644 --- a/rog-anime/Cargo.toml +++ b/rog-anime/Cargo.toml @@ -20,6 +20,7 @@ dbus = ["zvariant"] png_pong = "^0.8.0" pix = "0.13" gif = "^0.11.2" +log = "*" serde = "^1.0" serde_derive = "^1.0" diff --git a/rog-anime/src/data.rs b/rog-anime/src/data.rs index d745462f..9029afd7 100644 --- a/rog-anime/src/data.rs +++ b/rog-anime/src/data.rs @@ -1,12 +1,9 @@ use std::{ - sync::{ - atomic::{AtomicBool, Ordering}, - Arc, - }, thread::sleep, time::{Duration, Instant}, }; +use log::info; use serde_derive::{Deserialize, Serialize}; #[cfg(feature = "dbus")] use zvariant::Type; @@ -92,10 +89,11 @@ impl From for AnimePacketType { } /// This runs the animations as a blocking loop by using the `callback` to write data +/// +/// If `callback` is `Ok(true)` then `run_animation` will exit the animation loop early. pub fn run_animation( frames: &AnimeGif, - do_early_return: Arc, - callback: &dyn Fn(AnimeDataBuffer) -> Result<(), AnimeError>, + callback: &dyn Fn(AnimeDataBuffer) -> Result, ) -> Result<(), AnimeError> { let mut count = 0; let start = Instant::now(); @@ -140,9 +138,6 @@ pub fn run_animation( 'animation: loop { for frame in frames.frames() { let frame_start = Instant::now(); - if do_early_return.load(Ordering::SeqCst) { - return Ok(()); - } let mut output = frame.frame().clone(); if let AnimTime::Fade(_) = frames.duration() { @@ -164,12 +159,14 @@ pub fn run_animation( } } - callback(output).ok(); + if matches!(callback(output), Ok(true)) { + info!("rog-anime: frame-loop callback asked to exit early"); + return Ok(()); + } if timed && Instant::now().duration_since(start) > run_time { break 'animation; } - sleep(frame.delay()); } if let AnimTime::Count(times) = frames.duration() { diff --git a/rog-aura/src/usb.rs b/rog-aura/src/usb.rs index 9c9a8273..10de6c37 100644 --- a/rog-aura/src/usb.rs +++ b/rog-aura/src/usb.rs @@ -1,6 +1,6 @@ +use crate::usb::LedCfgState::{Off, On}; use std::convert::TryFrom; use std::ops::{BitAnd, BitOr}; -use crate::usb::LedCfgState::{Off, On}; pub const LED_INIT1: [u8; 2] = [0x5d, 0xb9]; pub const LED_INIT2: &str = "]ASUS Tech.Inc."; // ] == 0x5d @@ -12,12 +12,12 @@ pub const LED_INIT5: [u8; 6] = [0x5e, 0x05, 0x20, 0x31, 0, 0x08]; pub const LED_APPLY: [u8; 17] = [0x5d, 0xb4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]; pub const LED_SET: [u8; 17] = [0x5d, 0xb5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]; -pub const BOOT_MASK:i32 = 0xc31309; -pub const SLEEP_MASK:i32 = 0x300904; -pub const ALL_LEDS_MASK:i32 = 0x000002; -pub const KBD_LEDS_MASK:i32 = 0x080000; -pub const SIDE_LEDS_MASK:i32 = 0x040500; -pub const LEDS_STATE_MASK:i32 = ALL_LEDS_MASK | KBD_LEDS_MASK | SIDE_LEDS_MASK; +pub const BOOT_MASK: i32 = 0xc31309; +pub const SLEEP_MASK: i32 = 0x300904; +pub const ALL_LEDS_MASK: i32 = 0x000002; +pub const KBD_LEDS_MASK: i32 = 0x080000; +pub const SIDE_LEDS_MASK: i32 = 0x040500; +pub const LEDS_STATE_MASK: i32 = ALL_LEDS_MASK | KBD_LEDS_MASK | SIDE_LEDS_MASK; /// Writes out the correct byte string for brightness pub const fn aura_brightness_bytes(brightness: u8) -> [u8; 17] { @@ -29,7 +29,7 @@ pub const fn aura_brightness_bytes(brightness: u8) -> [u8; 17] { #[derive(Clone, Copy)] pub enum LedCfgState { On = 0xffffff, - Off = 0x0 + Off = 0x0, } impl From for LedCfgState { @@ -37,7 +37,7 @@ impl From for LedCfgState { match state { 0xffffff => On, 0x0 => Off, - _ => Off + _ => Off, } } } @@ -46,19 +46,19 @@ impl From for LedCfgState { fn from(state: bool) -> Self { match state { true => On, - false => Off + false => Off, } } } -impl TryFrom <[u8; 3]> for LedCfgState { +impl TryFrom<[u8; 3]> for LedCfgState { type Error = &'static str; fn try_from(value: [u8; 3]) -> Result { match value { [0xff, 0xff, 0xff] => Ok(On), [0, 0, 0] => Ok(Off), - _ => Err("Unconvertible value") + _ => Err("Unconvertible value"), } } } @@ -67,14 +67,14 @@ impl BitAnd for i32 { type Output = i32; fn bitand(self, rhs: LedCfgState) -> i32 { - return self & rhs as i32 + return self & rhs as i32; } } impl BitOr for i32 { type Output = i32; fn bitor(self, rhs: LedCfgState) -> Self::Output { - return self | rhs as i32 + return self | rhs as i32; } } @@ -94,17 +94,38 @@ impl BitAnd for LedCfgState { } } -pub fn leds_message (boot_state: bool, sleep_state: bool, all_leds_state: bool, kbd_leds_state: bool, side_leds_state: bool) -> [u8; 3] { - let raw_message = _leds_message(boot_state.into(), sleep_state.into(), all_leds_state.into(), kbd_leds_state.into(), side_leds_state.into()); +pub fn leds_message( + boot_state: bool, + sleep_state: bool, + all_leds_state: bool, + kbd_leds_state: bool, + side_leds_state: bool, +) -> [u8; 3] { + let raw_message = _leds_message( + boot_state.into(), + sleep_state.into(), + all_leds_state.into(), + kbd_leds_state.into(), + side_leds_state.into(), + ); let [_, lows @ ..] = i32::to_be_bytes(raw_message); return lows; } -fn _leds_message (boot_state: LedCfgState, sleep_state: LedCfgState, all_leds_state: LedCfgState, kbd_leds_state: LedCfgState, side_leds_state: LedCfgState) -> i32 { - +fn _leds_message( + boot_state: LedCfgState, + sleep_state: LedCfgState, + all_leds_state: LedCfgState, + kbd_leds_state: LedCfgState, + side_leds_state: LedCfgState, +) -> i32 { let full_leds_state = match all_leds_state { - On => (ALL_LEDS_MASK & all_leds_state) | (KBD_LEDS_MASK & kbd_leds_state) | (SIDE_LEDS_MASK & side_leds_state), + On => { + (ALL_LEDS_MASK & all_leds_state) + | (KBD_LEDS_MASK & kbd_leds_state) + | (SIDE_LEDS_MASK & side_leds_state) + } Off => 0x0100 & side_leds_state, }; @@ -112,6 +133,6 @@ fn _leds_message (boot_state: LedCfgState, sleep_state: LedCfgState, all_leds_st return match (all_leds_state | kbd_leds_state | side_leds_state).into() { On => boot_xor_sleep ^ ((boot_xor_sleep ^ full_leds_state) & LEDS_STATE_MASK), - _ => boot_xor_sleep - } + _ => boot_xor_sleep, + }; }