Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[client] Add support of UPower to detect battery status on linux #3425

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AenBleidd
Copy link
Member

Signed-off-by: Vitalii Koshura [email protected]

@KeithMyers
Copy link

Thanks so much Vitalii for getting this new on-battery detect structure put into the module.

@AenBleidd
Copy link
Member Author

@BOINC/committer, could anyone review this PR, please?

@TheAspens
Copy link
Member

I was taking a look at this and I ran watch "upower -d | grep on-battery" while I unplugged my laptop. I waited over a minute and the on-battery indicator never changed from "no". Here is what it showed:

[knreed@oc4182665077 ~]$ upower -d
Device: /org/freedesktop/UPower/devices/line_power_AC
  native-path:          AC
  power supply:         yes
  updated:              Tue 28 Jan 2020 07:31:04 AM CST (7027 seconds ago)
  has history:          no
  has statistics:       no
  line-power
    warning-level:       none
    online:              yes
    icon-name:          'ac-adapter-symbolic'

Device: /org/freedesktop/UPower/devices/battery_BAT0
  native-path:          BAT0
  vendor:               LGC
  model:                01AV495
  serial:               304
  power supply:         yes
  updated:              Tue 28 Jan 2020 09:27:02 AM CST (69 seconds ago)
  has history:          yes
  has statistics:       yes
  battery
    present:             yes
    rechargeable:        yes
    state:               discharging
    warning-level:       none
    energy:              59.43 Wh
    energy-empty:        0 Wh
    energy-full:         61.71 Wh
    energy-full-design:  90.06 Wh
    energy-rate:         0 W
    voltage:             12.469 V
    percentage:          96%
    capacity:            68.521%
    technology:          lithium-polymer
    icon-name:          'battery-full-symbolic'

Device: /org/freedesktop/UPower/devices/DisplayDevice
  power supply:         yes
  updated:              Tue 28 Jan 2020 09:27:02 AM CST (69 seconds ago)
  has history:          no
  has statistics:       no
  battery
    present:             yes
    state:               discharging
    warning-level:       none
    energy:              59.43 Wh
    energy-full:         61.71 Wh
    energy-rate:         0 W
    percentage:          96%
    icon-name:          'battery-full-symbolic'

Daemon:
  daemon-version:  0.99.7
  on-battery:      no
  lid-is-closed:   no
  lid-is-present:  yes
  critical-action: PowerOff

As a result, BOINC never stopped running. However with the previous version of the code the client stopped immediately. I'm running

[knreed@oc4182665077 ~]$ cat /etc/redhat-release 
Red Hat Enterprise Linux Workstation release 7.7 (Maipo)

I'm not sure why upower isn't updating the on-battery state since it recognized that the batteries were discharging.

@AenBleidd
Copy link
Member Author

There is an open issue for this: https://gitlab.freedesktop.org/upower/upower/issues/22
Sometimes it takes too long to update the status. I don't know why. As a solution I can put this detection method last.

@AenBleidd AenBleidd force-pushed the implement_upower_detect_for_pr branch from 8b0de9a to fc7432a Compare January 28, 2020 16:11
@KeithMyers
Copy link

Sounds like no one has tested the upower detect on a UPS battery backed system. Which was the whole impetus for asking for the new detect method.

Exactly what do I need to do to test this for you. I assume I need to pull in the new revised module and compile the client. How do I get the new module?

@AenBleidd
Copy link
Member Author

@KeithMyers, if you feel confident working with command line:

git checkout -b AenBleidd-implement_upower_detect_for_pr master
git pull https://github.com/AenBleidd/boinc.git implement_upower_detect_for_pr

Then just build it

@KeithMyers
Copy link

I was trying to read the code and I wonder if a better choice for the detected status would be "discharging" instead of "on-battery".

if (strstr(upower_out, "on-battery:"))

Or does the code read the daemon output of on-battery state? yes||no?

Daemon:
  daemon-version:  0.99.7
  on-battery:      no
  lid-is-closed:   no
  lid-is-present:  no
  critical-action: PowerOff

Trying to understand what the upower docs mean. So what does 'b' represent?

The "OnBattery" property
'OnBattery' read 'b'

@AenBleidd
Copy link
Member Author

@KeithMyers, I'm not sure that detecting 'discharging' string is a good idea. 'on-battery' should be either 'yes' or 'no', and we actually read this boolean state. If daemon for some reason doesn't change correctly this status, shame on it. But if we add 'discharging' as a criteria of search, then we will have next matrix:

# Discharging On-Battery Result (Use battery)
1 Yes Yes Yes
2 Yes No Yes?
3 No Yes Yes?
4 No No No

And even if case 2 I can accept as 'Yes', so what about case 3? I definitely can't reproduce this

@KeithMyers
Copy link

Don't know. I don't understand programming syntax. I can read it is about all I can do. Comprehension is beyond my skills presently.

I have your branch. Now just have to compile it into a new client for testing. Will get to it tomorrow. Then I can pull the plug on my UPS while watching upower state and see if the Manager drops the compute load.

@truboxl
Copy link
Contributor

truboxl commented May 7, 2020

@AenBleidd
Copy link
Member Author

@truboxl, which dependency?

@truboxl
Copy link
Contributor

truboxl commented May 14, 2020

@AenBleidd Sorry for the long delay, I am thinking upower.service

@davidpanderson
Copy link
Contributor

A couple of suggestions:

  1. read_upower_status() should return bool (whether we're on batteries) rather than std::string
  2. Now is a good time to change the logic of HOST_INFO::host_is_running_on_batteries()
    so that if none of the methods is available, it doesn't test them all again on each call.

@AenBleidd
Copy link
Member Author

@davidpanderson, thanks for the review. I'll make necessary changes asap

@AenBleidd AenBleidd marked this pull request as draft January 24, 2021 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

5 participants