automatically add new types like gaming_input, as discussed here:
https://forum.kde.org/viewtopic.php?f=204&t=151433&p=395938#p395938
especially necessary for:
https://github.com/atar-axis/xpadneo/issues/21
broulik |
Plasma |
automatically add new types like gaming_input, as discussed here:
https://forum.kde.org/viewtopic.php?f=204&t=151433&p=395938#p395938
especially necessary for:
https://github.com/atar-axis/xpadneo/issues/21
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
src/solid/devices/backends/upower/upowerdevice.cpp | ||
---|---|---|
98 | So you're changing from 'whitelist' to 'blacklist' behavior here. What will happen when another UPower device type is added by the upstream? |
src/solid/devices/backends/upower/upowerdevice.cpp | ||
---|---|---|
98 | Well then it is added automatically, which is what you would expect I think, and that is the reason why gaming_input wasn't detected. |
src/solid/devices/backends/upower/upowerdevice.cpp | ||
---|---|---|
98 | But we can also do it the following way, personally I don't care to be honest: return (uptype == 2 || uptype == 3 || uptype == 5 || uptype == 6 || uptype == 7 || uptype == 8 || uptype == 12); While 12 is UP_DEVICE_KIND_GAMING_INPUT |
I would still prefer a whitelist instead of a balcklist. Recently my mouse has started showing up twice, who knows what else might happen with newer release.
Also, can we use the defines/enums from upower there rather than adding seemingly random numbers?
But then, using headers probably relies on recent upower versions being present, so numbers is fine, but please change it to be a whitelist, then this can surely go in. Thanks for taking care of this!
Okay :) But what's about the other entries, like UP_DEVICE_KIND_TABLET, UP_DEVICE_KIND_MEDIA_PLAYER?
Is there a reason why they are excluded?
Is there a reason why they are excluded?
I don't think there's a particular reason, but there's no type enum in it either, so I suppose HAL didn't have it and it wasns't adjusted for when UPower came around. I also don't think they're as /common, gamepads are much more common. Let's first fix gamepads fully (ie. this patch, then we need a new enum in Solid::Battery and the powermanagement dataengine in Plasma adjusted as well as a new battery icon in the theme etc etc) and then we can look at what else makes sense :)
If you can update your patch to the whitelist you suggested, this could go in.
If you want you can also look at adding a new type enum (separate to this patch I would say), I could guide you through if you want, or just tell me that I should do it
The reason is probably that they are missing in the documentation (which is likely a bug).
I updated the diff, but unfortunately I'm not sure where to add that enum you @broulik mentioned since there is already one in frontend\battery.h:
enum BatteryType { UnknownBattery, PdaBattery, UpsBattery, PrimaryBattery, MouseBattery, KeyboardBattery, KeyboardMouseBattery, CameraBattery, PhoneBattery, MonitorBattery };
But what we need is this ordering:
typedef enum { UP_DEVICE_KIND_UNKNOWN, UP_DEVICE_KIND_LINE_POWER, UP_DEVICE_KIND_BATTERY, UP_DEVICE_KIND_UPS, UP_DEVICE_KIND_MONITOR, UP_DEVICE_KIND_MOUSE, UP_DEVICE_KIND_KEYBOARD, UP_DEVICE_KIND_PDA, UP_DEVICE_KIND_PHONE, UP_DEVICE_KIND_MEDIA_PLAYER, UP_DEVICE_KIND_TABLET, UP_DEVICE_KIND_COMPUTER, UP_DEVICE_KIND_GAMING_INPUT, UP_DEVICE_KIND_LAST } UpDeviceKind;
I think you can add an enum to upower.h and use it instead of the defines. I thought we already linked against UPower and could just use its header but we don't and I don't want to change that to avoid a dependency increase.
enum UPowerBatteryType { UPowerBatteryTypeBattery, UPowerBatteryTypeUps, UPowerBatteryTypeMouse, ... UPowerBatteryTypeGamingInput };
src/solid/devices/backends/upower/upowerdevice.cpp | ||
---|---|---|
93 | Sorry, just spotted that you removed the default case, this will cause the compiler to complain about not handled case statements |
Thanks a lot for yout patience and sorry it had to go through that many revisions.
I took the liberty of altering it to check each value explicitly, so when a new one is added it isn't forgotten to evaluate whether it makes sense to support or not, I hope that's okay.