D11331: add gaming_input devices and others to Battery

broulik added a comment. 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

D11331: add gaming_input devices and others to Battery

This revision was automatically updated to reflect the committed changes. Closed by commit R245:12dc9354e598: add gaming_input devices and others to Battery (authored by dollinger, committed by broulik). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D11331?vs=30141&id=30195#toc REPOSITOR

D11331: add gaming_input devices and others to Battery

dollinger updated this revision to Diff 30141. dollinger added a comment. Hopefully correct now :D REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11331?vs=29636&id=30141 REVISION DETAIL https://phabricator.kde.org/D11331 AFFECTED FILES src/solid/devices

D11331: add gaming_input devices and others to Battery

broulik added inline comments. INLINE COMMENTS > upowerdevice.cpp:93 > -return (uptype == 2 || uptype == 3 || uptype == 5 || uptype == 6 || > uptype == 7 || uptype == 8); > -default: > -return false; Sorry, just spotted that you removed the `default` case, this will cause th

D11331: add gaming_input devices and others to Battery

broulik accepted this revision. This revision is now accepted and ready to land. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D11331 To: dollinger, broulik, #plasma Cc: aleksejshilin, #frameworks, michaelh, ngraham

D11331: add gaming_input devices and others to Battery

dollinger added a comment. see D11553 REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D11331 To: dollinger, broulik, #plasma Cc: aleksejshilin, #frameworks, michaelh, ngraham

D11331: add gaming_input devices and others to Battery

broulik added a comment. In D11331#226852 , @dollinger wrote: > 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`: I think you can add an e

D11331: add gaming_input devices and others to Battery

dollinger updated this revision to Diff 29636. dollinger added a comment. I'm not sure where to add that enum you mentioned since there is already one in `frontend\battery.h`: enum BatteryType { UnknownBattery, PdaBattery, UpsBattery, PrimaryBattery, MouseBattery,

D11331: add gaming_input devices and others to Battery

aleksejshilin added a comment. In D11331#226428 , @dollinger wrote: > 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? The reason is p

D11331: add gaming_input devices and others to Battery

broulik added a comment. 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 REPOSITORY R245

D11331: add gaming_input devices and others to Battery

broulik added a comment. > 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 m

D11331: add gaming_input devices and others to Battery

dollinger added a comment. 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? REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D11331 To: dollinger, broulik, #plasma Cc: al

D11331: add gaming_input devices and others to Battery

broulik added a comment. 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! REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde

D11331: add gaming_input devices and others to Battery

broulik added a comment. 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? REPOSITO

D11331: add gaming_input devices and others to Battery

dollinger added a comment. Btw, this "new" type (`UP_DEVICE_KIND_GAMING_INPUT`) was added half an year ago ;) https://cgit.freedesktop.org/upower/tree/libupower-glib/up-types.h?h=UPOWER_0_99_5 **vs** https://cgit.freedesktop.org/upower/tree/libupower-glib/up-types.h?h=UPOWER_0_99_6 RE

D11331: add gaming_input devices and others to Battery

dollinger added inline comments. INLINE COMMENTS > dollinger wrote in 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. But we can also do it the following way, personally I don't care t

D11331: add gaming_input devices and others to Battery

dollinger added inline comments. INLINE COMMENTS > aleksejshilin wrote in 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? Well then it is added automatically, which is what you w

D11331: add gaming_input devices and others to Battery

aleksejshilin added inline comments. INLINE COMMENTS > upowerdevice.cpp:98 > + */ > +return (uptype != 0 && uptype != 1 && uptype != 4); > default: So you're changing from 'whitelist' to 'blacklist' behavior here. What will happen when another UPower device type is added by

D11331: add gaming_input devices and others to Battery

bshah added reviewers: broulik, Plasma. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D11331 To: dollinger, broulik, #plasma Cc: #frameworks, michaelh, ngraham

D11331: add gaming_input devices and others to Battery

dollinger created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. dollinger requested review of this revision. REVISION SUMMARY automatically add new types like gaming_input, as discussed here: https://forum.kde.org/view