Implement LEDs devices #268

NeroReflex · 2025-01-26T16:17:24Z

This is an initial implementation of LEDs devices: as of now only a static color (defined in the configuration) can be set when the device is acquired.

Implement Led configuration support with a fixed color. Cleanup wasteful clone() calls in various lookup methods.

Enable acquisition of LED devices and setting of a fixed color if specified in the configuration.

pastaq · 2025-01-28T09:18:26Z

Does this replace #246 ?

NeroReflex · 2025-01-28T22:15:20Z

Does this replace #246 ?

Mostly. That other PR still has an improvement (that has a problem) that will be a future PR after this PR knce I will have fixed it.

pastaq · 2025-01-29T02:42:23Z

I would say close that one and make that PR independently. We don't need to track it as open since we won't be merging it.

ShadowApex · 2025-01-29T01:29:36Z

src/config/mod.rs

+        true
+    }
+
+    /// Returns true if a given iio device is within a list of iio configs.


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.

Again? Something has gone wrong since I fixed this already

ShadowApex · 2025-01-29T01:32:45Z

src/input/source/led.rs

+}
+
+impl LedDevice {
+    /// Create a new [IioDevice] associated with the given device and


Suggested change

/// Create a new [IioDevice] associated with the given device and

/// Create a new [LedDevice] associated with the given device and

ShadowApex · 2025-01-29T01:34:51Z

src/input/source/led/multicolor_chassis.rs

+impl SourceOutputDevice for MultiColorChassis {
+    fn write_event(
+        &mut self,
+        event: crate::input::output_event::OutputEvent,
+    ) -> Result<(), crate::input::source::OutputError> {
+        //log::trace!("Received output event: {event:?}");
+        let _ = event;
+        Ok(())
+    }
+
+    fn upload_effect(
+        &mut self,
+        effect: evdev::FFEffectData,
+    ) -> Result<i16, crate::input::source::OutputError> {
+        //log::trace!("Received upload effect: {effect:?}");
+        let _ = effect;
+        Ok(-1)
+    }
+
+    fn update_effect(
+        &mut self,
+        effect_id: i16,
+        effect: evdev::FFEffectData,
+    ) -> Result<(), crate::input::source::OutputError> {
+        //log::trace!("Received update effect: {effect_id:?} {effect:?}");
+        let _ = effect;
+        let _ = effect_id;
+        Ok(())
+    }
+
+    fn erase_effect(&mut self, effect_id: i16) -> Result<(), crate::input::source::OutputError> {
+        //log::trace!("Received erase effect: {effect_id:?}");
+        let _ = effect_id;
+        Ok(())
+    }
+
+    fn stop(&mut self) -> Result<(), crate::input::source::OutputError> {
+        Ok(())
+    }
+}


You can omit these methods if they are not implemented since a default implementation already exists, but I presume we'll want to handle output events; notably DualSense output events that change LED color.

Suggested change

impl SourceOutputDevice for MultiColorChassis {

fn write_event(

&mut self,

event: crate::input::output_event::OutputEvent,

) -> Result<(), crate::input::source::OutputError> {

//log::trace!("Received output event: {event:?}");

let _ = event;

Ok(())

}

fn upload_effect(

&mut self,

effect: evdev::FFEffectData,

) -> Result<i16, crate::input::source::OutputError> {

//log::trace!("Received upload effect: {effect:?}");

let _ = effect;

Ok(-1)

}

fn update_effect(

&mut self,

effect_id: i16,

effect: evdev::FFEffectData,

) -> Result<(), crate::input::source::OutputError> {

//log::trace!("Received update effect: {effect_id:?} {effect:?}");

let _ = effect;

let _ = effect_id;

Ok(())

}

fn erase_effect(&mut self, effect_id: i16) -> Result<(), crate::input::source::OutputError> {

//log::trace!("Received erase effect: {effect_id:?}");

let _ = effect_id;

Ok(())

}

fn stop(&mut self) -> Result<(), crate::input::source::OutputError> {

Ok(())

}

}

impl SourceOutputDevice for MultiColorChassis {}

ShadowApex · 2025-01-29T02:15:33Z

rootfs/usr/share/inputplumber/devices/50-rog_ally.yaml

+  - group: led
+    led:
+      name: ASUS ROG Ally Config


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?

Yes, that's the name luke gave it

ShadowApex · 2025-01-29T02:20:22Z

src/dbus/interface/source/led.rs

+#[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())
+    }
+}


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.

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.

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.

ShadowApex · 2025-01-29T02:26:24Z

src/input/source/led/multicolor_chassis.rs

+        let multi_index_strings = contents
+            .split_whitespace()
+            .into_iter()
+            .map(|str| String::from(str))
+            .collect::<Vec<String>>();


To make clippy happy:

Suggested change

let multi_index_strings = contents

.split_whitespace()

.into_iter()

.map(|str| String::from(str))

.collect::<Vec<String>>();

let multi_index_strings = contents

.split_whitespace()

.map(String::from)

.collect::<Vec<String>>();

ShadowApex · 2025-01-29T02:27:09Z

src/input/source/led/multicolor_chassis.rs

+enum IndexType {
+    RGB,
+}


To make clippy happy:

Suggested change

enum IndexType {

RGB,

}

enum IndexType {

Rgb,

}

Oh no my eyes! Noooo not this.

We can also do an #allow here to make clippy ignore it since it's an acronym

ShadowApex · 2025-01-29T02:38:32Z

src/input/source/led/multicolor_chassis.rs

+pub struct MultiColorChassis {
+    multi_intensity_path: PathBuf,
+    multi_index_map: Vec<IndexType>,
+    fixed_color: Option<LedFixedColor>,


Do we ever use fixed_color outside initialization? Maybe we can remove this and just set the color in the constructor?

If we want to set it back after a resume or keep track of it I left it just for good measure, but yeah not very usefult.

Are we not storing the last setting otherwise? We could call this current_color and report it in dbus.

ShadowApex · 2025-01-29T02:45:58Z

src/input/source/led/multicolor_chassis.rs

+        let contents = (self
+            .multi_index_map
+            .iter()
+            .zip(contents.split_whitespace().into_iter())
+            .map(|(index_type, index_value)| match index_type {
+                IndexType::RGB => {
+                    (((r as u32) << 16u32) | ((g as u32) << 8u32) | ((b as u32) << 0u32))
+                        .to_string()
+                }
+                _ => String::from(index_value),
+            })
+            .collect::<Vec<String>>())
+        .join(" ");
+        Ok(
+            std::fs::write(self.multi_intensity_path.as_path(), contents)
+                .map_err(|err| Box::new(MultiColorChassisError::MultiIntensityUpdateError(err)))?,
+        )


To fix clippy warnings:

Suggested change

let contents = (self

.multi_index_map

.iter()

.zip(contents.split_whitespace().into_iter())

.map(|(index_type, index_value)| match index_type {

IndexType::RGB => {

(((r as u32) << 16u32) | ((g as u32) << 8u32) | ((b as u32) << 0u32))

.to_string()

}

_ => String::from(index_value),

})

.collect::<Vec<String>>())

.join(" ");

Ok(

std::fs::write(self.multi_intensity_path.as_path(), contents)

.map_err(|err| Box::new(MultiColorChassisError::MultiIntensityUpdateError(err)))?,

)

let values = self

.multi_index_map

.iter()

.zip(contents.split_whitespace())

.map(|(index_type, _index_value)| match index_type {

IndexType::Rgb => {

(((r as u32) << 16u32) | ((g as u32) << 8u32) | (b as u32)).to_string()

}

})

.collect::<Vec<String>>();

let contents = values.join(" ");

std::fs::write(self.multi_intensity_path.as_path(), contents)

.map_err(|err| Box::new(MultiColorChassisError::MultiIntensityUpdateError(err)))?;

Ok(())

ShadowApex · 2025-01-29T02:51:22Z

src/input/source/led/multicolor_chassis.rs

+    fn write_event(
+        &mut self,
+        event: crate::input::output_event::OutputEvent,
+    ) -> Result<(), crate::input::source::OutputError> {
+        //log::trace!("Received output event: {event:?}");
+        let _ = event;
+        Ok(())
+    }


We should probably handle output events so LEDs can be controlled. Or we can handle this in a follow-up PR.

Suggested change

fn write_event(

&mut self,

event: crate::input::output_event::OutputEvent,

) -> Result<(), crate::input::source::OutputError> {

//log::trace!("Received output event: {event:?}");

let _ = event;

Ok(())

}

fn write_event(&mut self, event: OutputEvent) -> Result<(), crate::input::source::OutputError> {

//log::trace!("Received output event: {event:?}");

match event {

OutputEvent::DualSense(report) => {

if !report.allow_led_color {

return Ok(());

}

// Handle DualSense LED events...

}

_ => {

return Ok(());

}

}

Ok(())

}

I was unable to figure out why so I left it alone for this PR, but I will look into getting it in a follow-up commit in this PR

pastaq · 2025-02-04T02:47:19Z

src/config/mod.rs

+pub struct Led {
+    pub id: Option<String>,
+    pub name: Option<String>,
+    pub led_fixed_color: Option<LedFixedColor>,


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.

pastaq · 2025-02-04T02:49:37Z

src/dbus/interface/source/led.rs

+#[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())
+    }
+}


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.

pastaq · 2025-02-04T02:51:53Z

src/input/source/led.rs

+/// [LedDevice] represents an input device using the leds subsystem.
+#[derive(Debug)]
+pub enum LedDevice {
+    MultiColorChassis(SourceDriver<MultiColorChassis>),


We might consider changing this class name fro MultiColorChassis to JoystickRing since that's what the new asus driver calls them and I'll be changing ayn/ayaneo-platform drivers to do the same.

pastaq · 2025-02-04T02:53:54Z

src/input/source/led/multicolor_chassis.rs

+pub struct MultiColorChassis {
+    multi_intensity_path: PathBuf,
+    multi_index_map: Vec<IndexType>,
+    fixed_color: Option<LedFixedColor>,


Are we not storing the last setting otherwise? We could call this current_color and report it in dbus.

pastaq · 2025-02-23T22:11:38Z

Continued in #297

NeroReflex added 3 commits January 26, 2025 17:00

feat(config) implement LED configuration

20f1243

Implement Led configuration support with a fixed color. Cleanup wasteful clone() calls in various lookup methods.

chore(LED): add configuration for the ASUS ROG Ally device

ad295a9

feat(LED): LEDs initial implementation

9d76067

Enable acquisition of LED devices and setting of a fixed color if specified in the configuration.

ShadowApex requested changes Jan 29, 2025

View reviewed changes

NeroReflex mentioned this pull request Jan 29, 2025

Implement LED source output device #246

Closed

pastaq reviewed Feb 4, 2025

View reviewed changes

ShadowApex mentioned this pull request Feb 23, 2025

Implement LEDs devices (Updated) #297

Merged

pastaq closed this Feb 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement LEDs devices #268

Implement LEDs devices #268

NeroReflex commented Jan 26, 2025

pastaq commented Jan 28, 2025 •

edited

Loading

NeroReflex commented Jan 28, 2025

pastaq commented Jan 29, 2025 •

edited

Loading

ShadowApex Jan 29, 2025

NeroReflex Jan 29, 2025

ShadowApex Jan 29, 2025

ShadowApex Jan 29, 2025

ShadowApex Jan 29, 2025

NeroReflex Jan 29, 2025

ShadowApex Jan 29, 2025

NeroReflex Jan 29, 2025

pastaq Feb 4, 2025

ShadowApex Jan 29, 2025

ShadowApex Jan 29, 2025

NeroReflex Jan 29, 2025

pastaq Jan 29, 2025

ShadowApex Jan 29, 2025

NeroReflex Jan 29, 2025

pastaq Feb 4, 2025

ShadowApex Jan 29, 2025

ShadowApex Jan 29, 2025

NeroReflex Jan 29, 2025

pastaq Feb 4, 2025

pastaq Feb 4, 2025

pastaq Feb 4, 2025

pastaq Feb 4, 2025

pastaq commented Feb 23, 2025

	/// 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.

	/// Create a new [IioDevice] associated with the given device and
	/// Create a new [LedDevice] associated with the given device and

Implement LEDs devices #268

Implement LEDs devices #268

Conversation

NeroReflex commented Jan 26, 2025

pastaq commented Jan 28, 2025 • edited Loading

NeroReflex commented Jan 28, 2025

pastaq commented Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pastaq commented Feb 23, 2025

pastaq commented Jan 28, 2025 •

edited

Loading

pastaq commented Jan 29, 2025 •

edited

Loading