Open dan9er opened 2 years ago
Related to #45
Add detection into iptsd (more code, goes against UNIX philosophy)
I wouldn't say that would be a problem. I was thinking of doing that because I got a tiny bit annoyed after forgetting to type /dev/hidraw1
when developing. We see modern Vulkan video games getting a list of GPUs then decide which GPU to use, and iptsd reading the list of hidraw devices then choosing one automatically would be similar.
We might want/need to do that anyways. At the moment, we're just loading based on drivers. The spi_hid
driver used on the Qualcomm and AMD devices (although it doesn't work yet on the latter) is quite generic. So who's to say that in the future there won't be any other devices provided via it. Technically even ITHC and IPTS could provide other HID devices that IPTSd shouldn't load on (although we can safely say that the latter won't ever). Finally, there also are some issues when loading that we might want to handle better: https://github.com/linux-surface/linux-surface/issues/1048.
However, there are things that the udev/systemd combination handles quite nicely: Devices not being present yet/waiting for devices to become ready, or devices going away. Support for multiple devices (in case we ever want to support the Surface Duo or something like it) by just spawning multiple IPTSd instances. All that is basically mostly for free and should be considered when doing something like that.
This is not much use for development, but for other service managers this can already be done, create a wrapper script that calls exec /usr/bin/iptsd $(iptsd-find-hidraw)
.
The problem with this approach is that if we put the detection code into iptsd, then the service manager can not use the name anymore. We cant tell the service to wait until the device is available, so it would have to launch at a point where the device is guaranteed to exist.
It also wouldn't solve the other problem that the current setup is solving: That the device name can change during suspend. Finding the device in iptsd wouldn't be enough, we would also need to re-find the device once it goes away.
We might want/need to do that anyways. At the moment, we're just loading based on drivers. The
spi_hid
driver used on the Qualcomm and AMD devices (although it doesn't work yet on the latter) is quite generic. So who's to say that in the future there won't be any other devices provided via it.
I think the best way to solve that problem would be to match by the device ID in udev, or to have a program that checks whether the report descriptor has the correct usage types, like quo suggested.
We could have a hybrid approach where iptsd would only autodetect the device itself if no parameters were passed.
device name can change during suspend
Why is this happening? Linux has had USB persist for 17 years https://www.kernel.org/doc/html/v4.13/driver-api/usb/persist.html . It's there even though replacing a USB drive with one of the same model causes corruption. Our ipts kernel module could add some similar logic, or another kernel module could provide another device that acts as a multiplexer for the other ones.
The problem with this approach is that if we put the detection code into iptsd, then the service manager can not use the name anymore. We cant tell the service to wait until the device is available, so it would have to launch at a point where the device is guaranteed to exist.
I was thinking more along the lines of IPTSd constantly monitoring HID device changes (i.e. devices being added or removed). That should also be able to deal with suspend. But the thing is that this would have to handle all of those cases properly, and likely also a couple of edge-cases.
The benefit is that we could reduce some duplication (you only need a IPTSd config and not additional udev rules) and might have a bit more control over when the daemon starts (and with that we can maybe improve the hid-multitouch/hid-generic situation).
So I'd be open to a prototype doing that, but as I've said, lots of things to consider that udev/systemd essentially give us for free without all the headaches of some custom handler.
Why is this happening?
The HID device of IPTS gets removed when suspending, re-added when resuming. Numbers are non-deterministic, so it might be /dev/hidraw1
before suspend, /dev/hidraw2
after.
Edit: It's probably possible to keep the HID device around in the driver. Not sure if it's really necessary though. I'd argue that if we want this kind of device handling inside IPTSd, it should be able to detect devices being removed and added. Even if it's just to make it more robust.
Edit 2: Keeping the HID device around for the IPTS would be pretty annoying to implement. The MEI client bus removes all devices during suspend. And IPTS is a MEI client... So doing this properly would require us to change the MEI driver to not remove all client devices. Which would probably require changes in all the other MEI client drivers.
I adapted this issue from a conversion I had with @StollD on the Matrix room (#linux-surface-support:matrix.org). I am not familiar with the internals of the project and I apologize for any inaccuracies.
Why
Even though systemd is widespread, there is a large group of users/distros that don't use it, for many reasons. (This group includes myself and my daily driver, Devuan.)
For these people, there should be some SysV-style service files as a fallback.
How
Ideally this would be a simple porting job, but there are complications:
https://github.com/linux-surface/iptsd/blob/bcb12c6901a7c784845ab4ccaa7aef389daabf6c/README.md?plain=1#L12
udev is not an issue, as eudev exists. Non-systemd distros use it as a drop-in replacement. We don't even need to touch the udev dependency declaration (eg. on Devuan, the
eudev
package "provides"udev
).The real roadblock is in dealing with the touchscreen's unpredictable device name. It depends on the amount of existing
hidraw
devices and the kernel config. It can even change when waking from suspend, since the touchscreen driver reloads. So, iptsd starts/stops when the driver loads/unloads, and each time it starts it must be passed the device name.Currently, we do this with systemd's service file parameters (eg.
iptsd@/dev/hidraw1
). To remove the dependency on systemd, we need to use another method. Here are some ideas:IPTSD_DEVICE
enviroment variableThese all come with their own annoying issues.