Skip to content

Refactor code making it easier to add new device types#249

Merged
pastaq merged 4 commits intoShadowBlip:mainfrom
NeroReflex:main_refactor
Jan 22, 2025
Merged

Refactor code making it easier to add new device types#249
pastaq merged 4 commits intoShadowBlip:mainfrom
NeroReflex:main_refactor

Conversation

@NeroReflex
Copy link
Contributor

Since LEDs devices do not have a devnode the hard requirement for source devices of having one is to be removed and the presence check must be postponed to type of devices that actually have one and requires it to work properly.

Refactor manager.rs to remove device specialization where no particular benefits are provided by that specialization.

This PR reduces the overall number of running threads, without changing how the application behaves.

Pending is an infinite loop, this means that if run() terminates the program will be stuck in an endless (unresponsive) loop: remove the usage of pending and replace it with a join on run so that if the program exits due to an error systemd can restart it; this also avoids using an unnecessary task.

Also avoid losing reference to the spawned Ctrl+C task.
…:devnode being populated

For a future implementation of leds it is important checks of devnode being populated are pushed where devnode is acually used: do that while also splitting up a gigantic method that both leaked a reference to a spaned task and was very hard to work with.
…ctually requires it

In order to implement LED(s) handling and potentially other device types that
don't have a node in /dev early checks for devnode() being valid must be dropped.
@NeroReflex NeroReflex force-pushed the main_refactor branch 8 times, most recently from 81ba6f6 to 91d58d9 Compare January 3, 2025 19:50
Copy link
Contributor

@pastaq pastaq left a comment

Choose a reason for hiding this comment

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

Some minor nits and a couple questions. Overall I like the changes, good work. Especially the simplified functions, that has been on my TODO list for a while.

@NeroReflex NeroReflex force-pushed the main_refactor branch 2 times, most recently from 08a0005 to 2927a34 Compare January 4, 2025 13:56
@NeroReflex NeroReflex force-pushed the main_refactor branch 2 times, most recently from d035686 to e5a9f36 Compare January 5, 2025 14:27
@NeroReflex NeroReflex requested a review from ShadowApex January 8, 2025 02:20
Copy link
Contributor

@pastaq pastaq left a comment

Choose a reason for hiding this comment

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

When ManageAllDevices is set to true, Manager fails to create a CompositeDevice for an external controller. We get a false positive for a virtual controller when it should detect as a bluetooth device.

Tested on Xbox Elite Series 2 Controller.

[2025-01-09T00:46:30Z DEBUG inputplumber::input::manager] Xbox Wireless Controller (event21) is virtual, skipping consideration for /devices/virtual/misc/uhid/0005:045E:0B22.002C/input/input125/event21

@ShadowApex
Copy link
Contributor

Testing this change with the Sony DualSense controller, I get two CompositeDevice objects:

└─ /org
  └─ /org/shadowblip
    └─ /org/shadowblip/InputPlumber
      ├─ /org/shadowblip/InputPlumber/CompositeDevice0
      ├─ /org/shadowblip/InputPlumber/CompositeDevice1

One device captures the hidraw interface:

busctl get-property org.shadowblip.InputPlumber /org/shadowblip/InputPlumber/CompositeDevice0 org.shadowblip.Input.CompositeDevice SourceDevicePaths
as 1 "/dev/hidraw8"

And the other captures the evdev nodes:

busctl get-property org.shadowblip.InputPlumber /org/shadowblip/InputPlumber/CompositeDevice1 org.shadowblip.Input.CompositeDevice SourceDevicePaths
as 3 "/dev/input/event24" "/dev/input/event25" "/dev/input/event26"

@NeroReflex
Copy link
Contributor Author

NeroReflex commented Jan 19, 2025

Testing this change with the Sony DualSense controller, I get two CompositeDevice objects:

└─ /org
  └─ /org/shadowblip
    └─ /org/shadowblip/InputPlumber
      ├─ /org/shadowblip/InputPlumber/CompositeDevice0
      ├─ /org/shadowblip/InputPlumber/CompositeDevice1

One device captures the hidraw interface:

busctl get-property org.shadowblip.InputPlumber /org/shadowblip/InputPlumber/CompositeDevice0 org.shadowblip.Input.CompositeDevice SourceDevicePaths
as 1 "/dev/hidraw8"

And the other captures the evdev nodes:

busctl get-property org.shadowblip.InputPlumber /org/shadowblip/InputPlumber/CompositeDevice1 org.shadowblip.Input.CompositeDevice SourceDevicePaths
as 3 "/dev/input/event24" "/dev/input/event25" "/dev/input/event26"

I answer to this mainly for personal record:

After doing this to manage all devices:

busctl set-property org.shadowblip.InputPlumber /org/shadowblip/InputPlumber/Manager org.shadowblip.InputManager ManageAllDevices b 1

I only got two composite devices:

one for ds5 and one for the ally

[gamer@chimeraos ~]$ busctl get-property org.shadowblip.InputPlumber /org/shadowblip/InputPlumber/CompositeDevice0 org.shadowblip.Input.CompositeDevice SourceDevicePaths
as 6 "/dev/hidraw3" "/dev/input/event2" "/dev/input/event6" "/dev/input/event8" "/dev/iio:device0" "/dev/input/event14"
[gamer@chimeraos ~]$ busctl get-property org.shadowblip.InputPlumber /org/shadowblip/InputPlumber/CompositeDevice1 org.shadowblip.Input.CompositeDevice SourceDevicePaths
as 4 "/dev/hidraw1" "/dev/input/event5" "/dev/input/event7" "/dev/input/event9"
[gamer@chimeraos ~]$ busctl get-property org.shadowblip.InputPlumber /org/shadowblip/InputPlumber/CompositeDevice2 org.shadowblip.Input.CompositeDevice SourceDevicePaths
Failed to get property SourceDevicePaths on interface org.shadowblip.Input.CompositeDevice: Unknown object '/org/shadowblip/InputPlumber/CompositeDevice2'

EDIT: bluetooth is the same even with bluetooth. No separated composites are created.

@ShadowApex
Copy link
Contributor

@NeroReflex It seems like I can no longer replicate the duplicate CompositeDevice issue I was seeing before, so ignore my comment 😅

… the manager component

This makes it easier in the future to add new device types suc as led devices.
@pastaq pastaq merged commit b62f3a4 into ShadowBlip:main Jan 22, 2025
1 check passed
@github-actions
Copy link
Contributor

🎉 This PR is included in version 0.41.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants