Skip to content
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.

Sign up for GitHub

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

Implement LED source output device #246

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Implement LED source output device #246

Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
cd3dbdd
feat(VSCode IDE): add debug button
NeroReflex Dec 28, 2024
57271e6
improv(manager): reorganize code and reduce dependency of UdevDevice:…
NeroReflex Dec 30, 2024
53edb30
feat(VSCode IDE): specify the language as being rust for remote debug…
NeroReflex Dec 30, 2024
005be00
improv(composite_device): better error reporting
NeroReflex Dec 30, 2024
48772bd
fix(udev::Device): bufix for number being u32 and code cleanup
NeroReflex Dec 30, 2024
7135d06
improv(udev::Device): don't use udevadm to inspect devices
NeroReflex Dec 30, 2024
bd6c100
manager: check for devnode() validity for device types that actually …
NeroReflex Dec 30, 2024
3e21daa
feat(config) implement LED configuration
NeroReflex Dec 30, 2024
006df63
feat(LED): LEDs initial implementation
NeroReflex Dec 31, 2024
0fc2c59
feat(LED): implement leds as a source output device
NeroReflex Dec 31, 2024
b6f0803
fix(manager): avoid cycling twice configurations
NeroReflex Dec 31, 2024
27f33ec
improv(CompositeDevice): make the code more readable
NeroReflex Dec 31, 2024
54db7ae
improv(main) do not lose the reference to the ctrl+C signal watcher
NeroReflex Dec 31, 2024
368cf3e
improv(LED): add configuration for the ASUS ROG Ally device
NeroReflex Dec 31, 2024
45d1cdf
improv(LED): increase code readability
NeroReflex Dec 31, 2024
45eb711
improv(code): cargo fmt
NeroReflex Dec 31, 2024
79a2bc6
fix(MultiColorChassis): solve a warning about an unused field
NeroReflex Dec 31, 2024
bbee9b3
improv(code): increase readability by replacing if let() with match
NeroReflex Dec 31, 2024
c65effe
fix(dbus) make dbus path parth of a CompositeDevice at construction
NeroReflex Dec 31, 2024
7d9f0cc
fix(manager): avoid borrowing syntax from typescript
NeroReflex Dec 31, 2024
0b79670
improv(code): cargo fmt
NeroReflex Dec 31, 2024
ac25809
fix(comments): apply suggestions from the PR feedback
NeroReflex Dec 31, 2024
00efbb9
fix(LED): fix inverted b and r components
NeroReflex Dec 31, 2024
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
126 changes: 46 additions & 80 deletions src/udev/mod.rs
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an interface that predates our use of udev-rs and is being depreciated over time. I don't know if there is value in updating it vice removing it and using the newer udev methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an interface that predates our use of udev-rs and is being depreciated over time. I don't know if there is value in updating it vice removing it and using the newer udev methods.

I think there is value in that: if it is kept as small as possible it will reflect the bare minimum needed for the software to run and allows for future ways of discovering devices.

Copy link
Contributor

Choose a reason for hiding this comment

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

You really want to not use Command::new("udevadm") in these things unless there is no other way. I would suggest full removal

Copy link
Contributor Author

@NeroReflex NeroReflex Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You really want to not use Command::new("udevadm") in these things unless there is no other way. I would suggest full removal

See your answer here: #246 (comment)

Other changes are to cope well with the removal of calling udevadm so you three have to decide if that class has to be:

  • fixed and I have to find some other way to implement leds (there might be one that will be more complex, will that be fine?): might be major work for me
  • left bugged (it's rare, who cares lol) and call udevadm and not touch that code at all: remove noise from the PR
  • pick all changes and futher rework that struct to make it relevant again: maybe needless work as UdevDevice is used way more anyway
  • drop that udev::Device structure completely and only use UdevDevice: probably better, but still adds noise so I will do in a separate PR

There might be the option "accept the current state", but it received three comments so I guess it's not liked by neither of the two. I will change accordingly to what you choose

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub mod device_test;
pub mod device;

use std::{error::Error, fs, path::Path};
use std::os::linux::fs::MetadataExt;

use device::AttributeGetter;
use tokio::process::Command;
Expand Down Expand Up @@ -129,89 +130,54 @@ async fn reload_all() -> Result<(), Box<dyn Error>> {
/// Returns device information for the given device path using udevadm.
pub async fn get_device(path: String) -> Result<Device, Box<dyn Error>> {
let mut device = Device::default();
let output = Command::new("udevadm")
.args(["info", path.as_str()])
.output()
.await?;
let output = String::from_utf8(output.stdout)?;

for line in output.split('\n') {
if line.starts_with("P: ") {
let line = line.replace("P: ", "");
device.path = line;
continue;
}
if line.starts_with("M: ") {
let line = line.replace("M: ", "");
device.name = line;
continue;
}
if line.starts_with("R: ") {
let line = line.replace("R: ", "");
let number = line.parse().unwrap_or_default();
device.number = number;
continue;
}
if line.starts_with("U: ") {
let line = line.replace("U: ", "");
device.subsystem = line;
continue;
}
if line.starts_with("T: ") {
let line = line.replace("T: ", "");
device.device_type = line;
continue;
}
if line.starts_with("D: ") {
let line = line.replace("D: ", "");
device.node = line;
continue;
}
if line.starts_with("I: ") {
let line = line.replace("I: ", "");
device.network_index = line;
continue;
}
if line.starts_with("N: ") {
let line = line.replace("N: ", "");
device.node_name = line;
continue;
}
if line.starts_with("L: ") {
let line = line.replace("L: ", "");
let priority = line.parse().unwrap_or_default();
device.symlink_priority = priority;
continue;
}
if line.starts_with("S: ") {
let line = line.replace("S: ", "");
device.symlink.push(line);
continue;
}
if line.starts_with("Q: ") {
let line = line.replace("Q: ", "");
let seq = line.parse().unwrap_or_default();
device.sequence_num = seq;
continue;
}
if line.starts_with("V: ") {
let line = line.replace("V: ", "");
device.driver = line;
continue;
let output = (match path.starts_with("/dev/") {
true => {
let metadata = fs::metadata(path).map_err(|e| Box::new(e))?;
let devtype = match metadata.st_mode() & nix::libc::S_IFMT {
nix::libc::S_IFCHR => Some(udev::DeviceType::Character),
nix::libc::S_IFBLK => Some(udev::DeviceType::Block),
Copy link
Contributor

Choose a reason for hiding this comment

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

It's likely to be preferable to use the rustix crate instead of nix in most cases now.

_ => None,
}.expect("Not a character or block special file");
udev::Device::from_devnum(devtype, metadata.st_rdev())
},
false => {
udev::Device::from_syspath(Path::new(path.as_str()))
}
if line.starts_with("E: ") {
let line = line.replace("E: ", "");
let mut parts = line.splitn(2, '=');
if parts.clone().count() != 2 {
continue;
}
let key = parts.next().unwrap();
let value = parts.last().unwrap();
device.properties.insert(key.to_string(), value.to_string());
continue;
}
}
}).map_err(|e| Box::new(e))?;

device.path = String::from(output.syspath().to_string_lossy());
device.name = String::from(output.name());

if let Some(val) = output.devnum() {
device.number = val
};

if let Some(val) = output.subsystem() {
device.subsystem = String::from(val.to_string_lossy())
};

if let Some(val) = output.devtype() {
device.device_type = String::from(val.to_string_lossy())
};

if let Some(val) = output.devnode() {
device.node = String::from(val.to_string_lossy())
};

if let Some(val) = output.driver() {
device.driver = String::from(val.to_string_lossy())
};

device.properties = output.properties().into_iter().map(|p| (
String::from(p.name().to_string_lossy()),
String::from(p.value().to_string_lossy())
)).collect();

// TODO: L: device.symlink_priority
// TODO: S: device.symlink
// TODO: Q: device.sequence_num
// TODO: I: Network interface index
Ok(device)
}

Expand Down