D11331: add gaming_input devices and others to Battery

2018-03-22 Thread Kai Uwe Broulik 
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 okay.

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

2018-03-22 Thread Kai Uwe Broulik 
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=30195#toc

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11331?vs=30141=30195

REVISION DETAIL
  https://phabricator.kde.org/D11331

AFFECTED FILES
  src/solid/devices/backends/upower/upowerdevice.cpp

To: dollinger, broulik, #plasma
Cc: aleksejshilin, #frameworks, michaelh, ngraham

D11331: add gaming_input devices and others to Battery

2018-03-21 Thread Florian Dollinger 
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=30141

REVISION DETAIL
  https://phabricator.kde.org/D11331

AFFECTED FILES
  src/solid/devices/backends/upower/upowerdevice.cpp

To: dollinger, broulik, #plasma
Cc: aleksejshilin, #frameworks, michaelh, ngraham

D11331: add gaming_input devices and others to Battery

2018-03-21 Thread Kai Uwe Broulik 
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 the 
compiler to complain about not handled `case` statements

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

2018-03-21 Thread Kai Uwe Broulik 
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

2018-03-21 Thread Florian Dollinger 
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

2018-03-19 Thread Kai Uwe Broulik 
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 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
};

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

2018-03-15 Thread Florian Dollinger 
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, KeyboardBattery,
   KeyboardMouseBattery, CameraBattery,
   PhoneBattery, MonitorBattery
 };
  
  But what we need is:
  
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;

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11331?vs=29504=29636

REVISION DETAIL
  https://phabricator.kde.org/D11331

AFFECTED FILES
  src/solid/devices/backends/upower/upowerdevice.cpp

To: dollinger, broulik, #plasma
Cc: aleksejshilin, #frameworks, michaelh, ngraham

D11331: add gaming_input devices and others to Battery

2018-03-15 Thread Алексей Шилин 
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 probably that they are missing in the documentation 
 (which is likely 
a bug).

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

2018-03-15 Thread Kai Uwe Broulik 
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 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

2018-03-15 Thread Kai Uwe Broulik 
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 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 :)

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

2018-03-15 Thread Florian Dollinger 
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: aleksejshilin, #frameworks, michaelh, ngraham

D11331: add gaming_input devices and others to Battery

2018-03-15 Thread Kai Uwe Broulik 
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.org/D11331

To: dollinger, broulik, #plasma
Cc: aleksejshilin, #frameworks, michaelh, ngraham

D11331: add gaming_input devices and others to Battery

2018-03-15 Thread Kai Uwe Broulik 
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?

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

2018-03-14 Thread Florian Dollinger 
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

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

2018-03-14 Thread Florian Dollinger 
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 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`

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

2018-03-14 Thread Florian Dollinger 
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 would expect I think, 
and that is the reason why gaming_input wasn't detected.

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

2018-03-14 Thread Алексей Шилин 
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 the upstream?

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

2018-03-14 Thread Bhushan Shah 
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

2018-03-14 Thread Florian Dollinger 
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/viewtopic.php?f=204=151433=395938#p395938
  
  especially necessary for:
  https://github.com/atar-axis/xpadneo/issues/21

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D11331

AFFECTED FILES
  src/solid/devices/backends/upower/upowerdevice.cpp

To: dollinger
Cc: #frameworks, michaelh, ngraham