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.

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 LEDs devices #268

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions rootfs/usr/share/inputplumber/devices/50-rog_ally.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ source_devices:
x: [1, 0, 0]
y: [0, -1, 0]
z: [0, 0, -1]
- group: led
led:
name: ASUS ROG Ally Config
Comment on lines +56 to +58
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also add these to the rootfs/usr/share/inputplumber/schema/composite_device_v1.json schema file.

Also, is "ASUS ROG Ally Config" the actual name returned by the driver?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's the name luke gave it


# Optional configuration for the composite device
options:
Expand Down
71 changes: 59 additions & 12 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ pub struct SourceDevice {
pub evdev: Option<Evdev>,
pub hidraw: Option<Hidraw>,
pub iio: Option<IIO>,
pub led: Option<Led>,
pub udev: Option<Udev>,
pub unique: Option<bool>,
pub blocked: Option<bool>,
Expand Down Expand Up @@ -362,6 +363,24 @@ pub struct IIO {
pub mount_matrix: Option<MountMatrix>,
}

#[derive(Debug, Deserialize, Clone, PartialEq)]
#[serde(rename_all = "snake_case")]
#[allow(clippy::upper_case_acronyms)]
pub struct Led {
pub id: Option<String>,
pub name: Option<String>,
pub led_fixed_color: Option<LedFixedColor>,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really see a case where we would ever specify a device start up with a specific color as this is entirely user preference. A simple dbus interface should be sufficient to expose and test this feature.

}

#[derive(Debug, Deserialize, Clone, PartialEq)]
#[serde(rename_all = "snake_case")]
#[allow(clippy::upper_case_acronyms)]
pub struct LedFixedColor {
pub r: u8,
pub g: u8,
pub b: u8,
}

#[derive(Debug, Deserialize, Clone, PartialEq)]
#[serde(rename_all = "snake_case")]
#[allow(clippy::upper_case_acronyms)]
Expand Down Expand Up @@ -460,6 +479,15 @@ impl CompositeDeviceConfig {
}
}
}
"leds" => {
for config in self.source_devices.iter() {
if let Some(led_config) = config.led.as_ref() {
if self.has_matching_led(udevice, led_config) {
return Some(config.clone());
}
}
}
}
_ => (),
};
None
Expand Down Expand Up @@ -578,7 +606,6 @@ impl CompositeDeviceConfig {
/// Returns true if a given hidraw device is within a list of hidraw configs.
pub fn has_matching_hidraw(&self, device: &UdevDevice, hidraw_config: &Hidraw) -> bool {
log::trace!("Checking hidraw config '{:?}'", hidraw_config,);
let hidraw_config = hidraw_config.clone();

// TODO: Switch either evdev of hidraw configs to use the same type. Legacy version had i16
// for hidraw and string for evdev.
Expand Down Expand Up @@ -606,7 +633,7 @@ impl CompositeDeviceConfig {
}
}

if let Some(name) = hidraw_config.name {
if let Some(name) = hidraw_config.name.as_ref() {
let dname = device.name();
log::trace!("Checking name: {name} against {dname}");
if !glob_match(name.as_str(), dname.as_str()) {
Expand All @@ -620,17 +647,39 @@ impl CompositeDeviceConfig {
/// Returns true if a given iio device is within a list of iio configs.
pub fn has_matching_iio(&self, device: &UdevDevice, iio_config: &IIO) -> bool {
log::trace!("Checking iio config: {:?} against {:?}", iio_config, device);
let iio_config = iio_config.clone();

if let Some(id) = iio_config.id {
if let Some(id) = iio_config.id.as_ref() {
let dsyspath = device.syspath();
log::trace!("Checking id: {id} against {dsyspath}");
if !glob_match(id.as_str(), dsyspath.as_str()) {
return false;
}
}

if let Some(name) = iio_config.name.as_ref() {
let dname = device.name();
log::trace!("Checking name: {name} against {dname}");
if !glob_match(name.as_str(), dname.as_str()) {
return false;
}
}

true
}

/// Returns true if a given iio device is within a list of iio configs.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Returns true if a given iio device is within a list of iio configs.
/// Returns true if a given led device is within a list of led configs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again? Something has gone wrong since I fixed this already

pub fn has_matching_led(&self, device: &UdevDevice, led_config: &Led) -> bool {
log::trace!("Checking led config: {:?} against {:?}", led_config, device);

if let Some(id) = led_config.id.as_ref() {
let dsyspath = device.syspath();
log::trace!("Checking id: {id} against {dsyspath}");
if !glob_match(id.as_str(), dsyspath.as_str()) {
return false;
}
}

if let Some(name) = iio_config.name {
if let Some(name) = led_config.name.as_ref() {
let dname = device.name();
log::trace!("Checking name: {name} against {dname}");
if !glob_match(name.as_str(), dname.as_str()) {
Expand All @@ -650,41 +699,39 @@ impl CompositeDeviceConfig {
device
);

let evdev_config = evdev_config.clone();

if let Some(name) = evdev_config.name {
if let Some(name) = evdev_config.name.as_ref() {
let dname = device.name();
log::trace!("Checking name: {name} against {dname}");
if !glob_match(name.as_str(), dname.as_str()) {
return false;
}
}

if let Some(phys_path) = evdev_config.phys_path {
if let Some(phys_path) = evdev_config.phys_path.as_ref() {
let dphys_path = device.phys();
log::trace!("Checking phys_path: {phys_path} against {dphys_path}");
if !glob_match(phys_path.as_str(), dphys_path.as_str()) {
return false;
}
}

if let Some(handler) = evdev_config.handler {
if let Some(handler) = evdev_config.handler.as_ref() {
let handle = device.sysname();
log::trace!("Checking handler: {handler} against {handle}");
if !glob_match(handler.as_str(), handle.as_str()) {
return false;
}
}

if let Some(vendor_id) = evdev_config.vendor_id {
if let Some(vendor_id) = evdev_config.vendor_id.as_ref() {
let id_vendor = format!("{:04x}", device.id_vendor());
log::trace!("Checking vendor ID: {vendor_id} against {id_vendor}");
if !glob_match(vendor_id.as_str(), id_vendor.as_str()) {
return false;
}
}

if let Some(product_id) = evdev_config.product_id {
if let Some(product_id) = evdev_config.product_id.as_ref() {
let id_product = format!("{:04x}", device.id_product());
log::trace!("Checking product ID: {product_id} against {id_product}");
if !glob_match(product_id.as_str(), id_product.as_str()) {
Expand Down
47 changes: 47 additions & 0 deletions src/dbus/interface/source/led.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use crate::input::source::iio::get_dbus_path;
use crate::udev::device::UdevDevice;
use std::error::Error;
use zbus::{fdo, Connection};
use zbus_macros::interface;

/// DBusInterface exposing information about a led device
pub struct SourceLedInterface {
device: UdevDevice,
}

impl SourceLedInterface {
pub fn new(device: UdevDevice) -> SourceLedInterface {
SourceLedInterface { device }
}
/// Creates a new instance of the source led interface on DBus. Returns
/// a structure with information about the source device.
pub async fn listen_on_dbus(
conn: Connection,
device: UdevDevice,
) -> Result<(), Box<dyn Error>> {
let iface = SourceLedInterface::new(device);
let Ok(id) = iface.id() else {
return Ok(());
};
let path = get_dbus_path(id);
tokio::task::spawn(async move {
log::debug!("Starting dbus interface: {path}");
let result = conn.object_server().at(path.clone(), iface).await;
if let Err(e) = result {
log::debug!("Failed to start dbus interface {path}: {e:?}");
} else {
log::debug!("Started dbus interface: {path}");
}
});
Ok(())
}
}

#[interface(name = "org.shadowblip.Input.Source.LEDDevice")]
impl SourceLedInterface {
/// Returns the human readable name of the device (e.g. XBox 360 Pad)
#[zbus(property)]
fn id(&self) -> fdo::Result<String> {
Ok(self.device.sysname())
}
}
Comment on lines +40 to +47
Copy link
Contributor

Choose a reason for hiding this comment

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

We could do this in a follow-up PR, but we should probably make an interface to allow users to change LED colors using DBus.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. Since I could not figure out how to make colors change via dualsense emulation and dbus I just implemented the basic to have leds change colors once.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also add a dbus bool property to permit or block source devices from adjusting the RGB color in the follow on PR. That will ensure targets like the DS5 don't change the RGB if the user prefers they not be changed.

1 change: 1 addition & 0 deletions src/dbus/interface/source/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pub mod evdev;
pub mod hidraw;
pub mod iio_imu;
pub mod led;
pub mod udev;
15 changes: 14 additions & 1 deletion src/input/composite_device/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ use crate::{
Event,
},
output_event::UinputOutputEvent,
source::{evdev::EventDevice, hidraw::HidRawDevice, iio::IioDevice, SourceDevice},
source::{
evdev::EventDevice, hidraw::HidRawDevice, iio::IioDevice, led::LedDevice, SourceDevice,
},
},
udev::{device::UdevDevice, hide_device, unhide_device},
};
Expand Down Expand Up @@ -1423,6 +1425,17 @@ impl CompositeDevice {
let device = IioDevice::new(device, self.client(), config)?;
SourceDevice::Iio(device)
}
"leds" => {
// Get any defined config for the IIO device
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Get any defined config for the IIO device
// Get any defined config for the LED device

let config = if let Some(device_config) = self.config.get_matching_device(&device) {
device_config.led
} else {
None
};

log::debug!("Adding source device: {:?}", device.name());
SourceDevice::Led(LedDevice::new(device, self.client(), config)?)
}
_ => {
return Err(format!(
"Unspported subsystem: {subsystem}, unable to add source device {}",
Expand Down
34 changes: 34 additions & 0 deletions src/input/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use crate::dbus::interface::manager::ManagerInterface;
use crate::dbus::interface::source::evdev::SourceEventDeviceInterface;
use crate::dbus::interface::source::hidraw::SourceHIDRawInterface;
use crate::dbus::interface::source::iio_imu::SourceIioImuInterface;
use crate::dbus::interface::source::led::SourceLedInterface;
use crate::dbus::interface::source::udev::SourceUdevDeviceInterface;
use crate::dmi::data::DMIData;
use crate::dmi::get_cpu_info;
Expand All @@ -37,6 +38,7 @@ use crate::input::composite_device::CompositeDevice;
use crate::input::source::evdev;
use crate::input::source::hidraw;
use crate::input::source::iio;
use crate::input::source::led;
use crate::input::target::TargetDevice;
use crate::input::target::TargetDeviceTypeId;
use crate::udev;
Expand Down Expand Up @@ -1188,6 +1190,35 @@ impl Manager {
log::debug!("Finished adding event device {id}");
}

"leds" => {
log::debug!("LED device added: {} ({})", device.name(), device.sysname());

// Create a DBus interface for the event device
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Create a DBus interface for the event device
// Create a DBus interface for the LED device

let conn = self.dbus.clone();
log::debug!("Attempting to listen on dbus for {dev_path} | {sysname}");
task::spawn(async move {
let result = SourceLedInterface::listen_on_dbus(conn, dev).await;
if let Err(e) = result {
log::error!("Error creating source evdev dbus interface: {e:?}");
}
log::debug!("Finished adding source device on dbus");
});
// Add the device as a source device
let path = led::get_dbus_path(sys_name.clone());
self.source_device_dbus_paths.insert(id.clone(), path);
// Check to see if the device is virtual
if device.is_virtual() {
log::debug!("{} is virtual, skipping consideration.", dev_path);
return Ok(());
} else {
log::trace!("Real device: {}", dev_path);
}
// Signal that a source device was added
log::debug!("Spawing task to add source device: {id}");
self.on_source_device_added(id.clone(), device).await?;
log::debug!("Finished adding event device {id}");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
log::debug!("Finished adding event device {id}");
log::debug!("Finished adding LED device {id}");

}

_ => {
return Err(format!("Device subsystem not supported: {subsystem:?}").into());
}
Expand Down Expand Up @@ -1447,6 +1478,9 @@ impl Manager {
let iio_devices = udev::discover_devices("iio")?;
let iio_devices = iio_devices.into_iter().map(|dev| dev.into()).collect();
Manager::discover_devices(cmd_tx, iio_devices).await?;
let led_devices = udev::discover_devices("leds")?;
let led_devices = led_devices.into_iter().map(|dev| dev.into()).collect();
Manager::discover_devices(&cmd_tx, led_devices).await?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Manager::discover_devices(&cmd_tx, led_devices).await?;
Manager::discover_devices(cmd_tx, led_devices).await?;


Ok(())
}
Expand Down
Loading
Loading