By checking for usb subsystem and the identifying property we use in queryDeviceInterface we can significantly reduce the number of wrapper items created.
Details
- Reviewers
davidedmundson bruns - Group Reviewers
Frameworks - Maniphest Tasks
- T10958: Faster Startup
- Commits
- R245:079b2bc91cb2: [UDev Backend] Narrow device queried for
R245:fe1c9c02e09f: [UDev Backend] Narrow device queried for
Previously hotplugengine took 46ms to initialize on plasmashell startup here, now it's down to 2ms.
My phone still shows up as Camera when connected via PTP rather than MTP.
Needs D21555 for cameras to show up properly on application start
Diff Detail
- Repository
- R245 Solid
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
libgphoto2.rules lists the following devices (USB VID:PID):
- 1403:0001 Sitronix Digital Photo Frame
which apparently shows up as a generic mass storage (sd[a-z]), but actually is something completely different, see https://github.com/gphoto/libgphoto2/tree/master/camlibs/st2205
- 1908:xxxx Appotech based Photo frame
https://github.com/gphoto/libgphoto2/tree/master/camlibs/ax203
- 04b0:xxxx Nikon Coolpix, pre PTP era (~2002)
sd[a-z] is subsystem "block"
sg[0-9] is susbsystem "scsi_generic"
none of the ID_GPHOTO2==1 devices is a tty or dvb device
src/solid/devices/backends/shared/udevqtclient.h | ||
---|---|---|
53 | This definitely needs some documentation
i.e. in the camera case, (usb || dvb || tty) && (ID_GPHOTO2==1) |
Okay, so we can just filter for "usb" then since we don't monitor "block" and "scsi_generic"
This definitely needs some documentation
Agreed
I think that is completely sufficient, yes.
In all the other places, the !QVariant::toString().isEmpty() and QVariant::toInt() == 1 should be replaced by just QVariant::isValid()
src/solid/devices/backends/shared/udevqtclient.cpp | ||
---|---|---|
223 | Nitpick - the existing code uses toLatin1(), and for the ASCII subset it doesn't matter, but I am quite sure properties and values are UTF8 in general. I have no strong proof, but the udev changelog has:
|
src/solid/devices/backends/shared/udevqtclient.cpp | ||
---|---|---|
223 | This is just copied from the other code. I can fixup all of them separately later if you want. |
src/solid/devices/backends/shared/udevqtclient.cpp | ||
---|---|---|
223 | Yes, thats completely fine for me - I was just looking through the code and picking up everything that should be fixed. |
I just reverted it as it indeed causes problems with initial device lookup. Not just in plasmashell but also in Dolphin. Since Frameworks tagging is tomrrow, better leave it for now and investigate thorougly next week :)
For some reason here my udev_enumerate_add_match_property("ID_MEDIA_PLAYER", null) doesn't work. If I check for "1" it works but null doesn't find any devices. Since the systemd code uses fnmatch, we can probably just pass * to it to match anything.
nullptr only matches when the udev property also has no value (if (!value && !value_dev) return true;). "*" should work, yes.
The question now is, should we also differnentiate here, i.e. map QVariant() to nullptr, and use "*" for wildcards?
Previously, you used QVariant() as wildcard, now it is "*", which matches the udev syntax.