Re: Review Request 130123: Fix the skip disabled backlight device

2017-05-09 Thread AceLan Kao 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130123/
---

(Updated May 10, 2017, 1:58 a.m.)


Review request for Solid.


Changes
---

Changes V2:
   1. Declare isRawBacklightEnabled() as a static function
   2. Using QFile constructor to pass the file path
   3. Declare "QByteArray buffer" when it's really needed below
   4. Remove "bool result", and just return true/false directly
   5. Adding curly brackets to if statement


Repository: powerdevil


Description
---

Only raw backlight interface have the "enabled" file under "device"
directory.
So the commit
   5c0d35c skip the disabled backlight device
affects all types of backlight interfaces is wrong, it will drop out all
other type of backlight interfaces except raw tyep backlight.

To fix this, we just need to check the enabled file for raw backlight
interfaces only.

Signed-off-by: AceLan Kao 


Diffs (updated)
-

  daemon/backends/upower/backlighthelper.h 
1382dd09938840371dbf34312fe2e8093abfbe10 
  daemon/backends/upower/backlighthelper.cpp 
e0eb6c461a7aac50533e833953977d46a7c8e4f3 

Diff: https://git.reviewboard.kde.org/r/130123/diff/


Testing
---


Thanks,

AceLan Kao

Re: Review Request 130123: Fix the skip disabled backlight device

2017-05-09 Thread Lamarque Souza 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130123/#review103208
---




daemon/backends/upower/backlighthelper.h (line 58)


static bool isRawBacklightEnabled(const QString );



daemon/backends/upower/backlighthelper.cpp (line 68)


You can pass the file path directly to the constructor:

QFile file(BACKLIGHT_SYSFS_PATH + interface + "/device/enabled");



daemon/backends/upower/backlighthelper.cpp (line 69)


Remove this line and declare this variable when it is really needed below.



daemon/backends/upower/backlighthelper.cpp (line 70)


This variable is not needed.



daemon/backends/upower/backlighthelper.cpp (line 72)


This line can be removed after the change above.



daemon/backends/upower/backlighthelper.cpp (line 74)


return false;



daemon/backends/upower/backlighthelper.cpp (line 77)


QByteBarry buffer = file.readLine().trimmed();



daemon/backends/upower/backlighthelper.cpp (line 79)


return true.

QFile's destructor already closes the file.



daemon/backends/upower/backlighthelper.cpp (line 83)


return false.



daemon/backends/upower/backlighthelper.cpp (line 110)


Code style: add a { at the and of this line and a } after the line below.


- Lamarque Souza


On May 9, 2017, 6:49 a.m., AceLan Kao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130123/
> ---
> 
> (Updated May 9, 2017, 6:49 a.m.)
> 
> 
> Review request for Solid.
> 
> 
> Repository: powerdevil
> 
> 
> Description
> ---
> 
> Only raw backlight interface have the "enabled" file under "device"
> directory.
> So the commit
>5c0d35c skip the disabled backlight device
> affects all types of backlight interfaces is wrong, it will drop out all
> other type of backlight interfaces except raw tyep backlight.
> 
> To fix this, we just need to check the enabled file for raw backlight
> interfaces only.
> 
> Signed-off-by: AceLan Kao 
> 
> 
> Diffs
> -
> 
>   daemon/backends/upower/backlighthelper.h 
> 1382dd09938840371dbf34312fe2e8093abfbe10 
>   daemon/backends/upower/backlighthelper.cpp 
> e0eb6c461a7aac50533e833953977d46a7c8e4f3 
> 
> Diff: https://git.reviewboard.kde.org/r/130123/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> AceLan Kao
> 
>

Review Request 130123: Fix the skip disabled backlight device

2017-05-09 Thread AceLan Kao 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130123/
---

Review request for Solid.


Repository: powerdevil


Description
---

Only raw backlight interface have the "enabled" file under "device"
directory.
So the commit
   5c0d35c skip the disabled backlight device
affects all types of backlight interfaces is wrong, it will drop out all
other type of backlight interfaces except raw tyep backlight.

To fix this, we just need to check the enabled file for raw backlight
interfaces only.

Signed-off-by: AceLan Kao 


Diffs
-

  daemon/backends/upower/backlighthelper.h 
1382dd09938840371dbf34312fe2e8093abfbe10 
  daemon/backends/upower/backlighthelper.cpp 
e0eb6c461a7aac50533e833953977d46a7c8e4f3 

Diff: https://git.reviewboard.kde.org/r/130123/diff/


Testing
---


Thanks,

AceLan Kao