Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
215 changes: 173 additions & 42 deletions asusd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@ pub mod aura_types;
pub mod error;

use std::future::Future;
use std::time::Duration;

use dmi_id::DMIID;
use futures_util::stream::StreamExt;
use log::{debug, info, warn};
use log::{debug, error, info, warn};
use logind_zbus::manager::ManagerProxy;
use tokio::time::sleep;
use zbus::object_server::{Interface, SignalEmitter};
use zbus::proxy::CacheProperties;
use zbus::zvariant::ObjectPath;
Expand Down Expand Up @@ -237,66 +235,199 @@ pub trait CtrlTask {
.await
.expect("Controller could not create dbus connection");

let manager = ManagerProxy::builder(&connection)
.cache_properties(CacheProperties::No)
let logind_manager = ManagerProxy::builder(&connection)
.cache_properties(CacheProperties::Lazily)
.build()
.await
.expect("Controller could not create ManagerProxy");
Comment on lines 236 to 242

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System daemons running as systemd root services must avoid .expect() and .unwrap() panics during D-Bus setup. If D-Bus is initializing asynchronously during early system boot, asusd should handle D-Bus connection errors gracefully rather than crashing the daemon process.


let manager1 = manager.clone();
tokio::spawn(async move {
if let Ok(mut notif) = manager1.receive_prepare_for_shutdown().await {
while let Some(event) = notif.next().await {
// blocks thread :|
if let Ok(args) = event.args() {
debug!("Doing on_prepare_for_shutdown({})", args.start);
on_prepare_for_shutdown(args.start).await;
tokio::spawn({
let logind_manager = logind_manager.clone();
async move {
if let Ok(mut notif) = logind_manager.receive_prepare_for_shutdown().await {
while let Some(event) = notif.next().await {
// blocks thread :|
if let Ok(args) = event.args() {
debug!("Doing on_prepare_for_shutdown({})", args.start);
on_prepare_for_shutdown(args.start).await;
}
}
}
}
});

let manager2 = manager.clone();
tokio::spawn(async move {
if let Ok(mut notif) = manager2.receive_prepare_for_sleep().await {
while let Some(event) = notif.next().await {
// blocks thread :|
if let Ok(args) = event.args() {
debug!("Doing on_prepare_for_sleep({})", args.start);
on_prepare_for_sleep(args.start).await;
tokio::spawn({
let logind_manager = logind_manager.clone();
async move {
if let Ok(mut notif) = logind_manager.receive_prepare_for_sleep().await {
while let Some(event) = notif.next().await {
// blocks thread :|
if let Ok(args) = event.args() {
debug!("Doing on_prepare_for_sleep({})", args.start);
on_prepare_for_sleep(args.start).await;
}
}
}
}
});

let manager3 = manager.clone();
tokio::spawn(async move {
let mut last_power = manager3.on_external_power().await.unwrap_or_default();

loop {
if let Ok(next) = manager3.on_external_power().await {
if next != last_power {
last_power = next;
on_external_power_change(next).await;
tokio::spawn({
let logind_manager = logind_manager.clone();
async move {
// 1. Initial Lid State Fetch & Apply at Daemon Startup
let mut last_lid = match logind_manager.lid_closed().await {
Ok(closed) => {
debug!("Initial lid state on startup: {}", closed);
on_lid_change(closed).await;
closed
}
Err(e) => {
warn!("Failed to read initial lid state from logind: {}", e);
false
}
};

// 2. Subscribe to D-Bus Property Change Stream
let mut stream = logind_manager.receive_lid_closed_changed().await;

// 3. Process Signals with Event Deduplication
while let Some(change) = stream.next().await {
if let Ok(lid_closed) = change.get().await {
if lid_closed != last_lid {
last_lid = lid_closed;
debug!("Lid state changed: {}", lid_closed);
on_lid_change(lid_closed).await;
}
}
}
sleep(Duration::from_secs(2)).await;
Comment thread
omuhr marked this conversation as resolved.
}
});

tokio::spawn(async move {
let mut last_lid = manager.lid_closed().await.unwrap_or_default();
// need to loop on these as they don't emit signals

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On several Linux distributions, kernel configurations, and older systemd releases, systemd-logind does not consistently broadcast PropertiesChanged signals for OnExternalPower or LidClosed on the system D-Bus. OnExternalPower is populated via udev/sysfs power_supply rules, but logind's D-Bus interface vtable does not guarantee emits-change-signal="true" for all properties across all systemd versions. Relying solely on stream.next().await without fallback verification or udev event integration will cause asusd to stall indefinitely on systems where logind property signals do not fire.

loop {
if let Ok(next) = manager.lid_closed().await {
if next != last_lid {
last_lid = next;
on_lid_change(next).await;
// External power supply monitoring
if let Some((external_power_supply_sysname, initial_power_supply_state)) =
std::fs::read_dir("/sys/class/power_supply")
.ok()
.and_then(|dir| {
// Look up the external power supply sysname (e.g. "ACAD") by finding one
// with the type "mains"
dir.flatten().find_map(|entry| {
let type_path = entry.path().canonicalize().ok()?.join("type");
let supply_type = std::fs::read_to_string(type_path).ok()?;
supply_type
.trim()
.eq_ignore_ascii_case("mains")
.then(|| entry.file_name().to_string_lossy().to_string())
})
})
.and_then(|sysname| {
// Look up the initial state of the external power supply
let path = std::path::PathBuf::from("/sys/class/power_supply")
.join(&sysname)
.join("online");
let state = std::fs::read_to_string(&path)
.map_err(|e| {
error!(
"Could not read external power supply state from {path:?}: {e}"
)
})
.ok()
.map(|s| s.trim() != "0")?;
Some((sysname, state))
})
{
Comment on lines +322 to +337

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Do not disable monitoring when only the initial online read fails.

The second and_then returns None if reading online fails. The whole if let Some(...) block is then skipped, so the udev monitor never starts and no external-power change is ever reported for the rest of the daemon lifetime. A transient read error should not disable event monitoring permanently.

Separate supply discovery from the initial state read. Start the monitor whenever a mains supply is found, and treat an unreadable initial state as unknown.

🛠️ Proposed restructure
-            if let Some((external_power_supply_sysname, initial_power_supply_state)) =
+            if let Some(external_power_supply_sysname) =
                 std::fs::read_dir("/sys/class/power_supply")
                     .ok()
                     .and_then(|dir| {
                         // Look up the external power supply sysname (e.g. "ACAD") by finding one
                         // with the type "mains"
                         dir.flatten().find_map(|entry| {
                             let type_path = entry.path().canonicalize().ok()?.join("type");
                             let supply_type = std::fs::read_to_string(type_path).ok()?;
                             supply_type
                                 .trim()
                                 .eq_ignore_ascii_case("mains")
                                 .then(|| entry.file_name().to_string_lossy().to_string())
                         })
                     })
-                    .and_then(|sysname| {
-                        // Look up the initial state of the external power supply
-                        let path = std::path::PathBuf::from("/sys/class/power_supply")
-                            .join(&sysname)
-                            .join("online");
-                        let state = std::fs::read_to_string(&path)
-                            .map_err(|e| {
-                                error!(
-                                    "Could not read external power supply state from {path:?}: {e}"
-                                )
-                            })
-                            .ok()
-                            .map(|s| s.trim() != "0")?;
-                        Some((sysname, state))
-                    })
             {
-                debug!(
-                    "External power supply plugged in on startup: {}",
-                    initial_power_supply_state
-                );
-                on_external_power_change(initial_power_supply_state).await;
+                let online_path = std::path::PathBuf::from("/sys/class/power_supply")
+                    .join(&external_power_supply_sysname)
+                    .join("online");
+                let mut initial_power_supply_state = match std::fs::read_to_string(&online_path) {
+                    Ok(s) => Some(s.trim() != "0"),
+                    Err(e) => {
+                        error!("Could not read external power supply state from {online_path:?}: {e}");
+                        None
+                    }
+                };
+                if let Some(state) = initial_power_supply_state {
+                    debug!("External power supply plugged in on startup: {}", state);
+                    on_external_power_change(state).await;
+                }

Then track last_power_supply_state as Option<bool> in the monitor thread.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@asusd/src/lib.rs` around lines 322 - 337, Update the external power-supply
initialization around the `and_then` chain to keep a discovered mains supply
even when reading its initial `online` file fails; log the read error and
represent the initial state as unknown rather than returning `None`. Start the
udev monitor whenever supply discovery succeeds, and change
`last_power_supply_state` in the monitor thread to `Option<bool>` so unknown
initial state is handled without suppressing future change events.

debug!(
"External power supply plugged in on startup: {}",
initial_power_supply_state
);
on_external_power_change(initial_power_supply_state).await;

let handle = tokio::runtime::Handle::current();
std::thread::spawn(move || {
Comment on lines +344 to +345

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Locate the tokio runtime construction and the tokio feature set for asusd.
fd -t f 'Cargo.toml' | xargs rg -n -C3 'tokio'

# Find the runtime entry point / macro attributes.
rg -nP -C5 '#\[tokio::main[^\]]*\]|Builder::new_(multi_thread|current_thread)|Runtime::new\(\)' --type=rust

Repository: OpenGamingCollective/asusctl

Length of output: 2306


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- workspace Tokio features ---'
sed -n '58,78p' Cargo.toml
printf '%s\n' '--- asusd entry points and runtime setup ---'
rg -nP -C8 '#\[tokio::main[^\]]*\]|Builder::new_(multi_thread|current_thread)|Runtime::new\(\)|fn main' asusd --glob '*.rs'
printf '%s\n' '--- target call sites ---'
sed -n '320,435p' asusd/src/lib.rs
printf '%s\n' '--- monitor and callback implementations ---'
rg -n -C8 'poll\.poll|on_external_power_change|external_power|AniMe|hid|usb' asusd/src --glob '*.rs'

Repository: OpenGamingCollective/asusctl

Length of output: 50385


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- create_sys_event_tasks and caller ---'
sed -n '190,345p' asusd/src/lib.rs
printf '%s\n' '--- external-power callback wiring ---'
rg -n -C10 'create_sys_event_tasks|set_power_states|set_power|external_power' asusd/src --glob '*.rs'
printf '%s\n' '--- AniMe power callback methods ---'
rg -n -C12 'set_power_states|power_state|external_power|on_external' asusd/src/aura_anime --glob '*.rs'
printf '%s\n' '--- AniMe implementation outline and relevant body ---'
ast-grep outline asusd/src/aura_anime/trait_impls.rs

Repository: OpenGamingCollective/asusctl

Length of output: 44013


🏁 Script executed:

#!/bin/bash
set -eu

python3 - <<'PY'
from pathlib import Path
import re

workspace = Path("Cargo.toml").read_text()
daemon = Path("asusd/src/daemon.rs").read_text()
lib = Path("asusd/src/lib.rs").read_text()
anime = Path("asusd/src/aura_anime/trait_impls.rs").read_text()

features = re.search(r'tokio\s*=\s*\{.*?features\s*=\s*\[(.*?)\]', workspace, re.S)
assert features and '"rt-multi-thread"' in features.group(1), "workspace lacks rt-multi-thread"
assert re.search(r'#\[tokio::main\]\s*async\s+fn\s+main', daemon), "asusd does not use the default Tokio main macro"
assert "handle.block_on(on_external_power_change" in lib, "target block_on call not found"

callback = re.search(
    r'move \|power_plugged\|.*?async move \{(.*?)\n\s*\},',
    anime,
    re.S,
)
assert callback, "AniMe external-power callback not found"
assert ".write_bytes(" in callback.group(1), "callback has no device write"
assert ".await" in callback.group(1), "callback has no awaited operation"

print("runtime_flavor=multi_thread (workspace feature + default #[tokio::main])")
print("foreign_thread_block_on=present")
print("anime_external_power_callback=async device writes present")
PY

Repository: OpenGamingCollective/asusctl

Length of output: 329


Move external-power handling off the udev monitor thread.

asusd uses Tokio's multi-thread runtime, so Handle::block_on from this thread is valid. However, the call blocks until the callback completes. Slow AniMe USB writes can delay poll.poll, allowing the udev socket queue to overflow and lose power-state changes. Send state changes through a channel to a Tokio task instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@asusd/src/lib.rs` around lines 344 - 345, Update the external-power handling
around tokio::runtime::Handle::current and the spawned udev monitor thread so it
sends power-state changes through a channel instead of synchronously waiting for
the callback. Add a Tokio task that receives channel messages and performs the
potentially slow AniMe USB callback, keeping the udev polling loop non-blocking
and preserving each power-state change.

'external_power_monitor_thread: {
let mut power_supply_monitor = match udev::MonitorBuilder::new()
.and_then(|m| m.match_subsystem("power_supply"))
.and_then(|m| m.listen())
{
Ok(m) => m,
Err(e) => {
error!("Could not create udev power supply monitor: {e}");
break 'external_power_monitor_thread;
}
};

let mut poll = match mio::Poll::new() {
Ok(p) => p,
Err(e) => {
error!("Could not create mio Poll: {e}");
break 'external_power_monitor_thread;
}
};

if let Err(e) = poll.registry().register(
&mut power_supply_monitor,
mio::Token(0),
mio::Interest::READABLE,
) {
error!("Could not register power supply monitor with mio: {e}");
break 'external_power_monitor_thread;
}

let mut events = mio::Events::with_capacity(8);

let mut last_power_supply_state = initial_power_supply_state;
loop {
match poll.poll(&mut events, None) {
Ok(_) => {}
Err(e) if e.kind() == std::io::ErrorKind::Interrupted => continue,
Err(e) => {
error!("Power supply monitor poll error: {e}");
break 'external_power_monitor_thread;
}
}

for event in power_supply_monitor.iter() {
if event.event_type() != udev::EventType::Change {
continue;
}
if event.device().sysname().to_string_lossy()
!= external_power_supply_sysname
{
continue;
}

let Some(current_power_supply_state) = event
.device()
.property_value("POWER_SUPPLY_ONLINE")
.map(|v| v != "0")
else {
warn!(
"Power supply change event for external power supply \
missing POWER_SUPPLY_ONLINE property, skipping..."
);
continue;
};

if current_power_supply_state != last_power_supply_state {
last_power_supply_state = current_power_supply_state;
debug!(
"External power supply state changed: {}",
current_power_supply_state
);
handle.block_on(on_external_power_change(
current_power_supply_state,
));
}
}
}
Comment on lines +388 to 421

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Re-read online from sysfs instead of relying only on the uevent property.

The handler skips any change event that does not carry POWER_SUPPLY_ONLINE. The kernel does not guarantee that every change uevent for a power supply includes the full property set. When the property is missing, the daemon logs a warning and keeps the previous state, so a real transition can be lost permanently.

Fall back to reading /sys/class/power_supply/<sysname>/online when the property is absent.

🛠️ Proposed fallback
-                                let Some(current_power_supply_state) = event
-                                    .device()
-                                    .property_value("POWER_SUPPLY_ONLINE")
-                                    .map(|v| v != "0")
-                                else {
-                                    warn!(
-                                        "Power supply change event for external power supply \
-                                         missing POWER_SUPPLY_ONLINE property, skipping..."
-                                    );
-                                    continue;
-                                };
+                                let device = event.device();
+                                let current_power_supply_state = match device
+                                    .property_value("POWER_SUPPLY_ONLINE")
+                                    .map(|v| v != "0")
+                                {
+                                    Some(state) => state,
+                                    None => {
+                                        let path = std::path::PathBuf::from(
+                                            "/sys/class/power_supply",
+                                        )
+                                        .join(&external_power_supply_sysname)
+                                        .join("online");
+                                        match std::fs::read_to_string(&path) {
+                                            Ok(s) => s.trim() != "0",
+                                            Err(e) => {
+                                                warn!(
+                                                    "Power supply change event missing \
+                                                     POWER_SUPPLY_ONLINE and {path:?} is \
+                                                     unreadable: {e}"
+                                                );
+                                                continue;
+                                            }
+                                        }
+                                    }
+                                };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for event in power_supply_monitor.iter() {
if event.event_type() != udev::EventType::Change {
continue;
}
if event.device().sysname().to_string_lossy()
!= external_power_supply_sysname
{
continue;
}
let Some(current_power_supply_state) = event
.device()
.property_value("POWER_SUPPLY_ONLINE")
.map(|v| v != "0")
else {
warn!(
"Power supply change event for external power supply \
missing POWER_SUPPLY_ONLINE property, skipping..."
);
continue;
};
if current_power_supply_state != last_power_supply_state {
last_power_supply_state = current_power_supply_state;
debug!(
"External power supply state changed: {}",
current_power_supply_state
);
handle.block_on(on_external_power_change(
current_power_supply_state,
));
}
}
}
for event in power_supply_monitor.iter() {
if event.event_type() != udev::EventType::Change {
continue;
}
if event.device().sysname().to_string_lossy()
!= external_power_supply_sysname
{
continue;
}
let device = event.device();
let current_power_supply_state = match device
.property_value("POWER_SUPPLY_ONLINE")
.map(|v| v != "0")
{
Some(state) => state,
None => {
let path = std::path::PathBuf::from(
"/sys/class/power_supply",
)
.join(&external_power_supply_sysname)
.join("online");
match std::fs::read_to_string(&path) {
Ok(s) => s.trim() != "0",
Err(e) => {
warn!(
"Power supply change event missing \
POWER_SUPPLY_ONLINE and {path:?} is \
unreadable: {e}"
);
continue;
}
}
}
};
if current_power_supply_state != last_power_supply_state {
last_power_supply_state = current_power_supply_state;
debug!(
"External power supply state changed: {}",
current_power_supply_state
);
handle.block_on(on_external_power_change(
current_power_supply_state,
));
}
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@asusd/src/lib.rs` around lines 388 - 421, Update the change-event handling
around power_supply_monitor.iter() to fall back to reading the external power
supply’s sysfs online value when property_value("POWER_SUPPLY_ONLINE") is
absent. Use the matched device sysname to read
/sys/class/power_supply/<sysname>/online, parse its value into the same boolean
state, and only warn and skip when both sources are unavailable or invalid;
preserve the existing state-change comparison and on_external_power_change call.

}
sleep(Duration::from_secs(2)).await;
}
});
error!(
"External power supply monitor exited unexpectedly, changes will no \
longer be detected."
);
});
} else {
warn!("External power supply monitoring unavailable");
}
Comment on lines +428 to +430

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Raise the log level and name the failure cause.

If no mains supply is found, external-power tracking is silently disabled for the whole daemon lifetime. Power profiles, fan curves, and dGPU power states then never react to AC changes. This is a functional loss, not a routine condition.

Use error! and include the reason, for example that /sys/class/power_supply was unreadable or that no entry has type == "Mains".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@asusd/src/lib.rs` around lines 428 - 430, Update the external power supply
monitoring fallback in the surrounding initialization flow to use error-level
logging instead of warn-level logging, and include the specific failure cause:
either /sys/class/power_supply was unreadable or no power-supply entry had type
"Mains".

}
}
}
Expand Down