-
Notifications
You must be signed in to change notification settings - Fork 74
refactor: Not meant for direct merge (Read note) #211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 13 commits
67d4c21
3c9c86e
80d14e9
2783795
8145713
8fc17e6
9096d32
4775aab
1698a63
c753305
b64488d
3ea1209
e1ed5b2
a895088
9535953
b4c9513
f79bcd0
a7923bb
c589640
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| use argh::FromArgs; | ||
| use rog_anime::usb::{AnimAwake, AnimBooting, AnimShutdown, AnimSleeping}; | ||
| use log::warn; | ||
| use rog_anime::usb::{get_anime_type, AnimAwake, AnimBooting, AnimShutdown, AnimSleeping}; | ||
| use rog_anime::AnimeType; | ||
|
|
||
| #[derive(FromArgs, Debug)] | ||
|
|
@@ -149,3 +150,227 @@ pub struct AnimeGifDiagonal { | |
| )] | ||
| pub loops: u32, | ||
| } | ||
|
|
||
| pub fn handle_anime(cmd: &AnimeCommand) -> Result<(), Box<dyn std::error::Error>> { | ||
| if cmd.command.is_none() | ||
| && cmd.enable_display.is_none() | ||
| && cmd.enable_powersave_anim.is_none() | ||
| && cmd.brightness.is_none() | ||
| && cmd.off_when_lid_closed.is_none() | ||
| && cmd.off_when_suspended.is_none() | ||
| && cmd.off_when_unplugged.is_none() | ||
| && cmd.off_with_his_head.is_none() | ||
| && !cmd.clear | ||
| { | ||
| warn!("Missing arg or command; run 'asusctl anime --help' for usage"); | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let animes = crate::platform_cli::find_iface_blocking::< | ||
| rog_dbus::zbus_anime::AnimeProxyBlocking, | ||
| >("xyz.ljones.Anime")?; | ||
|
|
||
| for proxy in animes { | ||
| if let Some(enable) = cmd.enable_display { | ||
| proxy.set_enable_display(enable)?; | ||
| } | ||
| if let Some(enable) = cmd.enable_powersave_anim { | ||
| proxy.set_builtins_enabled(enable)?; | ||
| } | ||
| if let Some(bright) = cmd.brightness { | ||
| proxy.set_brightness(bright)?; | ||
| } | ||
| if let Some(enable) = cmd.off_when_lid_closed { | ||
| proxy.set_off_when_lid_closed(enable)?; | ||
| } | ||
| if let Some(enable) = cmd.off_when_suspended { | ||
| proxy.set_off_when_suspended(enable)?; | ||
| } | ||
| if let Some(enable) = cmd.off_when_unplugged { | ||
| proxy.set_off_when_unplugged(enable)?; | ||
| } | ||
|
|
||
| let mut anime_type = get_anime_type(); | ||
| if let AnimeType::Unsupported = anime_type { | ||
| if let Some(model) = cmd.override_type { | ||
| anime_type = model; | ||
| } | ||
| } | ||
|
|
||
| if cmd.clear { | ||
| let data = vec![255u8; anime_type.data_length()]; | ||
| let tmp = rog_anime::AnimeDataBuffer::from_vec(anime_type, data)?; | ||
| proxy.write(tmp)?; | ||
| } | ||
|
Comment on lines
+153
to
+203
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Find any proxy setter/property for off_with_his_head and its usages
rg -nP -C3 'off_with_his_head' --type=rustRepository: OpenGamingCollective/asusctl Length of output: 166 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Locate the CLI definition and any proxy API around the relevant option/property.
rg -n -C 3 'off_with_his_head|off-with-his-head|off_with_his' asusctl/src rog_dbus -g '*.rs'
# Show the anime CLI handling area with line numbers for context.
sed -n '140,230p' asusctl/src/anime_cli.rsRepository: OpenGamingCollective/asusctl Length of output: 1190 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Find anime proxy definitions and all mentions of the option/property.
fd -a 'anime' .
rg -n -C 3 'AnimeProxyBlocking|off_with_his_head|off-with-his-head|set_off_with_his_head|builtins_enabled|enable_display|off_when_lid_closed|off_when_suspended|off_when_unplugged' . -g '*.rs'
# Show candidate files if found.
fd -a 'anime.*\.rs$|anime.*\.toml$|anime.*\.json$|anime.*\.yaml$' .Repository: OpenGamingCollective/asusctl Length of output: 50384 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Search for anime proxy definitions and any setter/property related to the option.
fd -a 'anime' .
rg -n -C 3 'AnimeProxyBlocking|off_with_his_head|off-with-his-head|set_off_with_his_head|builtins_enabled|enable_display|off_when_lid_closed|off_when_suspended|off_when_unplugged' . -g '*.rs'
# Show any matching files that look relevant.
fd -a 'anime.*\.rs$|anime.*\.toml$|anime.*\.json$|anime.*\.yaml$' .Repository: OpenGamingCollective/asusctl Length of output: 50384 Wire or remove 🤖 Prompt for AI Agents |
||
|
|
||
| if let Some(action) = cmd.command.as_ref() { | ||
| match action { | ||
| AnimeActions::Image(image) => { | ||
| if image.path.is_empty() { | ||
| warn!("Missing arg or command; run 'asusctl anime image --help' for usage"); | ||
| return Ok(()); | ||
| } | ||
| verify_brightness(image.bright)?; | ||
|
|
||
| let matrix = rog_anime::AnimeImage::from_png( | ||
| std::path::Path::new(&image.path), | ||
| image.scale, | ||
| image.angle, | ||
| rog_anime::Vec2::new(image.x_pos, image.y_pos), | ||
| image.bright, | ||
| anime_type, | ||
| )?; | ||
|
|
||
| proxy.write(<rog_anime::AnimeDataBuffer>::try_from(&matrix)?)?; | ||
| } | ||
| AnimeActions::PixelImage(image) => { | ||
| if image.path.is_empty() { | ||
| warn!("Missing arg or command; run 'asusctl anime pixel-image --help' for usage"); | ||
| return Ok(()); | ||
| } | ||
| verify_brightness(image.bright)?; | ||
|
|
||
| let matrix = rog_anime::AnimeDiagonal::from_png( | ||
| std::path::Path::new(&image.path), | ||
| None, | ||
| image.bright, | ||
| anime_type, | ||
| )?; | ||
|
|
||
| proxy.write(matrix.into_data_buffer(anime_type)?)?; | ||
| } | ||
| AnimeActions::Gif(gif) => { | ||
| if gif.path.is_empty() { | ||
| warn!("Missing arg or command; run 'asusctl anime gif --help' for usage"); | ||
| return Ok(()); | ||
| } | ||
| verify_brightness(gif.bright)?; | ||
|
|
||
| let matrix = rog_anime::AnimeGif::from_gif( | ||
| std::path::Path::new(&gif.path), | ||
| gif.scale, | ||
| gif.angle, | ||
| rog_anime::Vec2::new(gif.x_pos, gif.y_pos), | ||
| rog_anime::AnimTime::Count(1), | ||
| gif.bright, | ||
| anime_type, | ||
| )?; | ||
|
|
||
| play_gif_animation(&proxy, &matrix, gif.loops)?; | ||
| } | ||
| AnimeActions::PixelGif(gif) => { | ||
| if gif.path.is_empty() { | ||
| warn!("Missing arg or command; run 'asusctl anime pixel-gif --help' for usage"); | ||
| return Ok(()); | ||
| } | ||
| verify_brightness(gif.bright)?; | ||
|
|
||
| let matrix = rog_anime::AnimeGif::from_diagonal_gif( | ||
| std::path::Path::new(&gif.path), | ||
| rog_anime::AnimTime::Count(1), | ||
| gif.bright, | ||
| anime_type, | ||
| )?; | ||
|
|
||
| play_gif_animation(&proxy, &matrix, gif.loops)?; | ||
| } | ||
| AnimeActions::SetBuiltins(builtins) => { | ||
| if builtins.set.is_none() { | ||
| warn!("Missing arg; run 'asusctl anime set-builtins --help' for usage"); | ||
| return Ok(()); | ||
| } | ||
|
|
||
| proxy.set_builtin_animations(rog_anime::Animations { | ||
| boot: builtins.boot, | ||
| awake: builtins.awake, | ||
| sleep: builtins.sleep, | ||
| shutdown: builtins.shutdown, | ||
| })?; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Helper to determine the playback iteration strategy. | ||
| /// Returns `None` for infinite loops (`loops == 0`), or `Some(count)` for finite playback. | ||
| fn compute_loop_plan(loops: u32) -> Option<u32> { | ||
| if loops == 0 { | ||
| None | ||
| } else { | ||
| Some(loops) | ||
| } | ||
| } | ||
|
|
||
| /// Play GIF animation frames. `loops == 0` means infinite playback until interrupted. | ||
| fn play_gif_animation( | ||
| proxy: &rog_dbus::zbus_anime::AnimeProxyBlocking, | ||
| matrix: &rog_anime::AnimeGif, | ||
| loops: u32, | ||
| ) -> Result<(), Box<dyn std::error::Error>> { | ||
| let mut remaining = compute_loop_plan(loops); | ||
|
|
||
| loop { | ||
| for frame in matrix.frames() { | ||
| proxy.write(frame.frame().clone())?; | ||
| std::thread::sleep(frame.delay()); | ||
| } | ||
| match remaining { | ||
| None => continue, | ||
| Some(1) => break, | ||
| Some(ref mut count) => *count -= 1, | ||
| } | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn verify_brightness(brightness: f32) -> Result<(), Box<dyn std::error::Error>> { | ||
| if !(0.0..=1.0).contains(&brightness) { | ||
| return Err(format!( | ||
| "Brightness must be between 0.0 and 1.0 (inclusive), was {brightness}" | ||
| ) | ||
| .into()); | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn test_verify_brightness_valid() { | ||
| assert!(verify_brightness(0.0).is_ok()); | ||
| assert!(verify_brightness(0.5).is_ok()); | ||
| assert!(verify_brightness(1.0).is_ok()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_verify_brightness_invalid() { | ||
| assert!(verify_brightness(-0.1).is_err()); | ||
| assert!(verify_brightness(1.1).is_err()); | ||
| assert!(verify_brightness(f32::NAN).is_err()); | ||
| assert!(verify_brightness(f32::INFINITY).is_err()); | ||
| assert!(verify_brightness(f32::NEG_INFINITY).is_err()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_loop_iteration_count() { | ||
| assert_eq!( | ||
| compute_loop_plan(0), | ||
| None, | ||
| "0 loops must plan for infinite playback" | ||
| ); | ||
| assert_eq!( | ||
| compute_loop_plan(1), | ||
| Some(1), | ||
| "1 loop must plan for 1 iteration" | ||
| ); | ||
| assert_eq!( | ||
| compute_loop_plan(5), | ||
| Some(5), | ||
| "5 loops must plan for 5 iterations" | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,5 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use argh::FromArgs; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use log::warn; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use rog_platform::platform::PlatformProfile; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use rog_profiles::fan_curve_set::CurveData; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use rog_profiles::FanCurvePU; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -42,3 +43,71 @@ pub struct FanCurveCommand { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub data: Option<CurveData>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const REQ_MOD_PROFILE_MSG: &str = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "--enable-fan-curves, --enable-fan-curve, --fan, and --data options require --mod-profile"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub fn handle_fan_curve( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| conn: &zbus::blocking::Connection, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmd: &FanCurveCommand, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Result<(), Box<dyn std::error::Error>> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let fan_proxy = rog_dbus::zbus_fan_curves::FanCurvesProxyBlocking::new(conn).map_err(|e| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| warn!("Fan curves unavailable: {e}"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rog_profiles::error::ProfileError::NotSupported | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| })?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if !cmd.get_enabled && !cmd.default && cmd.mod_profile.is_none() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| warn!("Missing arg or command; run 'asusctl fan-curve --help' for usage"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Ok(()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (cmd.enable_fan_curves.is_some() || cmd.fan.is_some() || cmd.data.is_some()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| && cmd.mod_profile.is_none() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| warn!("{REQ_MOD_PROFILE_MSG}"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Ok(()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let plat_proxy = rog_dbus::zbus_platform::PlatformProxyBlocking::new(conn)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if cmd.get_enabled { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let profile = plat_proxy.platform_profile()?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let curves = fan_proxy.fan_curve_data(profile)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for curve in curves.iter() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!("{}", String::from(curve)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if cmd.default { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let active = plat_proxy.platform_profile()?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fan_proxy.set_curves_to_defaults(active)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if let Some(profile) = cmd.mod_profile { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if cmd.enable_fan_curves.is_none() && cmd.data.is_none() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let data = fan_proxy.fan_curve_data(profile)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let ron = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ron::ser::to_string_pretty(&data, ron::ser::PrettyConfig::new().depth_limit(4))?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!("\nFan curves for {:?}\n\n{}", profile, ron); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if let Some(enabled) = cmd.enable_fan_curves { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fan_proxy.set_fan_curves_enabled(profile, enabled)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+46
to
+99
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value Move proxy construction after argument validation.
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if let Some(enabled) = cmd.enable_fan_curve { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if let Some(fan) = cmd.fan { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fan_proxy.set_profile_fan_curve_enabled(profile, fan, enabled)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| warn!("{REQ_MOD_PROFILE_MSG}"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if let Some(mut curve) = cmd.data.clone() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let fan = cmd.fan.unwrap_or_default(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| curve.set_fan(fan); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fan_proxy.set_fan_curve(profile, curve)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+100
to
+115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Wrong warning text, and a missing At line 101 Separately, line 106 falls back to 🐛 Proposed fix if let Some(enabled) = cmd.enable_fan_curve {
if let Some(fan) = cmd.fan {
fan_proxy.set_profile_fan_curve_enabled(profile, fan, enabled)?;
} else {
- warn!("{REQ_MOD_PROFILE_MSG}");
+ warn!("--enable-fan-curve requires --fan <cpu/gpu/mid>");
}
}
if let Some(mut curve) = cmd.data.clone() {
- let fan = cmd.fan.unwrap_or_default();
+ let fan = cmd.fan.unwrap_or_else(|| {
+ let fan = FanCurvePU::default();
+ warn!("--fan not specified, applying curve to {fan:?}");
+ fan
+ });
curve.set_fan(fan);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Ok(()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Warn when the anime type stays
Unsupported.If
get_anime_type()returnsUnsupportedand no--override-typeis given, the code proceeds with a guesseddata_length(), so writes fail later with an opaque buffer-length error. A warning pointing at--override-typehere would make the failure actionable. Also, this is loop-invariant and can be hoisted above thefor proxyloop.🤖 Prompt for AI Agents