search

KevinTsui1234 / tint2

Automatically exported from code.google.com/p/tint2
GNU General Public License v2.0
1 stars 0 forks source link

Battery monitor fails to work properly after suspend using UPower. #367

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago 
What steps will reproduce the problem?
1. Using OpenBox and oblogout script
2. Oblogout doesn't use pm-suspend. It uses freedesktop.org UPower
3. Suspend the computer, when revived, the monitor states 0%

What is the expected output? What do you see instead?
If I kill and restart tint2, it shows the battery state correctly.

What version of the product are you using? On what operating system?
tint2 version: 0.11-4
Archlinux and Openbox
Please provide any additional information below.

In the oblogout.conf file, this is the code for suspend:

suspend = dbus-send --system --print-reply --dest="org.freedesktop.UPower" 
/org/freedesktop/UPower org.freedesktop.UPower.Suspend

Original issue reported on code.google.com by dast...@gmail.com on 12 Oct 2011 at 10:17

GoogleCodeExporter commented 8 years ago 
I'm seeing this behaviour, but my suspend command is simply `echo -n "mem" > 
/sys/power/state` in an acpi script.

(Linux kernel 3.1.4-gentoo. tint2 0.11 built from Gentoo portage.)

I can confirm that restarting tint2 restores correct battery status.

Original comment by joss-goo...@pseudonymity.net on 30 Dec 2011 at 10:14

GoogleCodeExporter commented 8 years ago 
same here, running arch linux x86_64, kernel 3.1.8-1-ARCH, suspend is done by 
pm-suspend

Original comment by goo...@deaktualisierung.org on 23 Jan 2012 at 9:08

GoogleCodeExporter commented 8 years ago 
How's about this issue status ? I'm using Arch, kernel 3.6.3 x86_64, systemd 
sleep.target and tint2 has the same issue. I have to restart tint2 to receive 
the battery notification.

Original comment by vdchu...@gmail.com on 26 Oct 2012 at 11:05

GoogleCodeExporter commented 8 years ago 
Arch Linux, suspend with systemd, have same issue

Original comment by keryasco...@gmail.com on 18 Jan 2014 at 7:42

GoogleCodeExporter commented 8 years ago 
Debian Jessie, suspend with:

echo -n mem > /sys/power/state

Have the exact same issue (and restarting tint2 solves it)

Original comment by jmcclell...@gmail.com on 30 Jan 2014 at 2:09

GoogleCodeExporter commented 8 years ago

Original comment by mrovi9...@gmail.com on 2 Feb 2015 at 12:24

GoogleCodeExporter commented 8 years ago 
Hi, thanks for the report.

We have never managed to reproduce this issue. If anyone is still interested in 
fixing this problem, could you please let us know if it is still present?

Original comment by mrovi9...@gmail.com on 4 Feb 2015 at 10:44

GoogleCodeExporter commented 8 years ago 
I am also using debian jessie; I can confirm it is present in tint2 version 
0.11+svn20121014.

Original comment by Loser...@gmail.com on 24 Mar 2015 at 4:43

GoogleCodeExporter commented 8 years ago 
Thanks.

I have just tried it on a Debian LiveCD (from here: 
http://live.debian.net/cdimage/release/7.6/amd64/iso-hybrid/ with Linux 3.2.0) 
and the battery monitor works perfectly fine after suspend, regardless of 
whether the system is plugged or not :(

Does it happen every time you suspend?

Do you need to suspend the laptop for a long time to see it happen?

Original comment by mrovi9...@gmail.com on 24 Mar 2015 at 6:39

GoogleCodeExporter commented 8 years ago 
I don't want to shift blame, but the battery status is handled by ACPI in the 
kernel, and in the past I have seen really bad and strange behavior in that 
area.

The tint2 code that monitors the battery is very simple, it just reads 
periodically 3 integers from 3 files and displays a text. There really isn't 
much that could be broken there...

Original comment by mrovi9...@gmail.com on 25 Mar 2015 at 6:51

GoogleCodeExporter commented 8 years ago 
It definitely does not happen during every suspend. I'm running tint2 from a 
terminal now; hope catch an interesting message in stderr the next time it 
happens.

Original comment by Loser...@gmail.com on 26 Mar 2015 at 3:15

GoogleCodeExporter commented 8 years ago 
Thanks. My laptop does not have this problem (and I suspend almost every day) 
so only with help from you there is a chance of debugging it.

But if this doesn't work maybe it would make sense to use one of the power 
monitors that run from the system tray (e.g. XFCE has xfce4-power-manager. I'm 
not sure if you can start it from other desktop environments; but there should 
be other such programs. Being specialized in power management, they might 
implement various "quirks" that solve your issue.

Original comment by mrovi9...@gmail.com on 26 Mar 2015 at 3:30

GoogleCodeExporter commented 8 years ago 
For future reference, the laptop that I'm running this on is a Lenovo T420. 

Some observations: I just did a suspend overnight with the AC adapter plugged 
in and unplugged while the laptop was still asleep. After resume it was stuck 
on 0%, Full. cat'ing /sys/class/power_supply showed that the files were still 
being updated. Somehow after another suspend-resume cycle the battery display 
started working again.

I noticed that if the display ever gets stuck, it gets stuck at 0%, with 
"battery_low_cmd_send = 1" seeming to trigger the "battery low" notification. 
Is it possible that some transient reading of 0% can stop the display from 
updating? I can't seem to see any execution path in battery.c alone that would 
cause this though.

Original comment by Loser...@gmail.com on 26 Mar 2015 at 6:04

GoogleCodeExporter commented 8 years ago 
Interesting. I will take a look in the weekend.

Original comment by mrovi9...@gmail.com on 26 Mar 2015 at 9:02

GoogleCodeExporter commented 8 years ago 
[deleted comment]
GoogleCodeExporter commented 8 years ago 
I'm not sure if this is a cause of the issue, but one thing I noticed while 
building and running the debug version of tint2 is that on my system there is 
no file at /sys/class/power_supply/current_now. The other _now files that the 
battery applet opens appear to be there.

When running the debug version
    fopen(path_current_now, "r");                                             
will fail and return a NULL file pointer--and since the files get closed 
regardless of whether opening them succeeded (lines 218-221), this causes a 
segfault.
When I'm not running the debug version, either the applet doesn't try to open 
current_now or it fails silently without segfaulting (and somehow the display 
still works). Is the debug version out of date?

Original comment by Loser...@gmail.com on 28 Mar 2015 at 2:18

GoogleCodeExporter commented 8 years ago 
Nevermind; turns out my debug version didn't include the current_now -> 
power_now change. Will keep my eyes peeled for other issues.

It might be useful to add the check for whether files are opened correctly 
anyway in init_battery, let me know if you are willing to accept a patch for 
that.

Original comment by Loser...@gmail.com on 28 Mar 2015 at 2:24

GoogleCodeExporter commented 8 years ago 
Oh wow, this is getting really freaky... I noticed after testing suspend and 
unplugging while the laptop was suspended that I could still get a segfault if 
I started the debug version after resuming. It turns out that on some resumes 
/sys/class/power_supply/current_now is there but power_now is gone! Then if I 
suspend and resume again current_now is gone but power_now is there!

This behavior makes sense as update_battery will skip checking power_now if the 
file isn't there and will simply set the new percentages to their initial 
values (0). It also explains why after another suspend-resume cycle it will 
start working again.

It's not the greatest fix, but I'm currently testing if checking for the old 
"current_now" if "power_now" isn't found works.

Original comment by Loser...@gmail.com on 28 Mar 2015 at 3:08

GoogleCodeExporter commented 8 years ago 
After a lot of lid opening and closing on my laptop I think I've reached a fix 
that works! It turns out that not only is power_now reverted to current_now on 
some resumes, but the files that begin with energy* are also changed to 
charge*. I found the least invasive way to check for this to be right after 
energy_now is opened; if opening fails, we can reinitialize by calling 
battery_init() again which actually already checks whether charge or energy is 
used. The difference is now that we account for a possible change __after__ 
battery_init() has been called for the first time.

A similar thing is done when the default "power_now" fails to open and we try 
to open "current_now" instead. It seems your intuition is correct: this fix is 
really just adding robustness against some really bizarre and strange behavior.

I have attached a patch that includes these fixes; let me know if the changes 
are good or anything that I should modify! There is a debug (stderr) fprintf 
that shows when reinitialization due to the files changing occurs.

Original comment by Loser...@gmail.com on 28 Mar 2015 at 6:29

Attachments:

GoogleCodeExporter commented 8 years ago 
[deleted comment]
GoogleCodeExporter commented 8 years ago 
**update: I realized that r738 has some checking for power_ vs current_ as 
well, so I have been able to simplify the patch.

Here's the new version.

Original comment by Loser...@gmail.com on 28 Mar 2015 at 8:28

Attachments:

GoogleCodeExporter commented 8 years ago 
Thanks! I will try to integrate it this weekend.

Original comment by mrovi9...@gmail.com on 28 Mar 2015 at 9:05

GoogleCodeExporter commented 8 years ago 
I pushed r739 which ended up being a much bigger change than expected (the code 
was really messy and not designed for reloading parameters).

The only way I could test it was by removing my laptop's battery and plugging 
it back in with the AC still connected. This is not covering all the code paths 
but is close enough to your problem.

When you have time, could you please let me know if it works correctly now?

Original comment by mrovi9...@gmail.com on 28 Mar 2015 at 6:25

GoogleCodeExporter commented 8 years ago 
Issue 250 has been merged into this issue.

Original comment by mrovi9...@gmail.com on 28 Mar 2015 at 6:36

GoogleCodeExporter commented 8 years ago 
r739 looks good for the battery issue. I'll see if I notice anything down the 
road.

Original comment by Loser...@gmail.com on 28 Mar 2015 at 8:33

GoogleCodeExporter commented 8 years ago 
I noticed that with the new changes error-checking isn't performed before the 
low battery warning is raised, which can lead to a false alarm when resuming 
from suspend. 

I've attached a simple one-line fix.

Original comment by eddieyan...@gmail.com on 8 Apr 2015 at 10:27

Attachments:

GoogleCodeExporter commented 8 years ago 
Good point.

Actually the notification code does not belong there at all, so I changed the 
logic a bit in r746. If you still have problems kindly let me know.

Original comment by mrovi9...@gmail.com on 9 Apr 2015 at 9:15

GoogleCodeExporter commented 8 years ago

Original comment by mrovi9...@gmail.com on 17 Apr 2015 at 8:34