Skip to content
This repository was archived by the owner on Apr 3, 2020. It is now read-only.

Fix support for differing sysfs naming conventions#3

Open
h3xx wants to merge 3 commits into
denilsonsa:masterfrom
h3xx:acpi-naming-conventions-fix
Open

Fix support for differing sysfs naming conventions#3
h3xx wants to merge 3 commits into
denilsonsa:masterfrom
h3xx:acpi-naming-conventions-fix

Conversation

@h3xx
Copy link
Copy Markdown

@h3xx h3xx commented Feb 13, 2012

This fixes an issue where the battery constantly reads at 0%.

It's because (at least on my system) the naming conventions for the
special files inside the sysfs tree are different from what the script
expects.

The script expects:

  • /sys/class/power_supply/BAT*/energy_full
  • /sys/class/power_supply/BAT*/energy_now
  • /sys/class/power_supply/BAT*/power_now

My sysfs has:

  • /sys/class/power_supply/BAT*/charge_full
  • /sys/class/power_supply/BAT*/charge_now
  • /sys/class/power_supply/BAT*/current_now

Unfortunately I am unable to find the exact commit log where this naming
convention changed, but now the script will support both and everyone
will be happy.

This bug was also observed in kernel 2.6.38.7.

Evidence

$ uname -a

Linux delicious 3.2.2 #1 SMP Fri Jan 27 15:19:49 CST 2012 x86_64 Intel(R) Atom(TM) CPU N455 @ 1.66GHz GenuineIntel GNU/Linux

$ ls -la /sys/class/power_supply/BAT1/

total 0
drwxr-xr-x 3 root root    0 Feb  8 15:20 .
drwxr-xr-x 3 root root    0 Feb  8 15:20 ..
-rw-r--r-- 1 root root 4096 Feb 13 10:44 alarm
-r--r--r-- 1 root root 4096 Feb  8 15:20 charge_full
-r--r--r-- 1 root root 4096 Feb 13 10:44 charge_full_design
-r--r--r-- 1 root root 4096 Feb  8 15:20 charge_now
-r--r--r-- 1 root root 4096 Feb  8 15:20 current_now
-r--r--r-- 1 root root 4096 Feb 13 10:44 cycle_count
lrwxrwxrwx 1 root root    0 Feb 13 10:44 device -> ../../../PNP0C0A:00
-r--r--r-- 1 root root 4096 Feb 13 10:44 manufacturer
-r--r--r-- 1 root root 4096 Feb 13 10:44 model_name
drwxr-xr-x 2 root root    0 Feb 13 10:44 power
-r--r--r-- 1 root root 4096 Feb  8 15:20 present
-r--r--r-- 1 root root 4096 Feb 13 10:44 serial_number
-r--r--r-- 1 root root 4096 Feb  8 15:20 status
lrwxrwxrwx 1 root root    0 Feb  8 16:01 subsystem -> ../../../../../../class/power_supply
-r--r--r-- 1 root root 4096 Feb 13 10:44 technology
-r--r--r-- 1 root root 4096 Feb 13 10:33 type
-rw-r--r-- 1 root root 4096 Feb  8 16:01 uevent
-r--r--r-- 1 root root 4096 Feb 13 10:44 voltage_min_design
-r--r--r-- 1 root root 4096 Feb  8 15:20 voltage_now

This fixes an issue where the battery constantly reads at 0%.

It's because (at least on my system) the naming conventions for the
special files inside the sysfs tree are different from what the script
expects.

The script expects:
 /sys/class/power_supply/BAT*/energy_full
 /sys/class/power_supply/BAT*/energy_now
 /sys/class/power_supply/BAT*/power_now

My sysfs has:
 /sys/class/power_supply/BAT*/charge_full
 /sys/class/power_supply/BAT*/charge_now
 /sys/class/power_supply/BAT*/current_now

Unfortunately I am unable to find the exact commit log where this naming
convention changed, but now the script will support both and everyone
will be happy.

This bug was also observed in kernel 2.6.38.7.

* Evidence *

$ uname -a

Linux delicious 3.2.2 denilsonsa#1 SMP Fri Jan 27 15:19:49 CST 2012 x86_64 Intel(R) Atom(TM) CPU N455   @ 1.66GHz GenuineIntel GNU/Linux

$ ls -la /sys/class/power_supply/BAT1/
total 0
drwxr-xr-x 3 root root    0 Feb  8 15:20 .
drwxr-xr-x 3 root root    0 Feb  8 15:20 ..
-rw-r--r-- 1 root root 4096 Feb 13 10:44 alarm
-r--r--r-- 1 root root 4096 Feb  8 15:20 charge_full
-r--r--r-- 1 root root 4096 Feb 13 10:44 charge_full_design
-r--r--r-- 1 root root 4096 Feb  8 15:20 charge_now
-r--r--r-- 1 root root 4096 Feb  8 15:20 current_now
-r--r--r-- 1 root root 4096 Feb 13 10:44 cycle_count
lrwxrwxrwx 1 root root    0 Feb 13 10:44 device -> ../../../PNP0C0A:00
-r--r--r-- 1 root root 4096 Feb 13 10:44 manufacturer
-r--r--r-- 1 root root 4096 Feb 13 10:44 model_name
drwxr-xr-x 2 root root    0 Feb 13 10:44 power
-r--r--r-- 1 root root 4096 Feb  8 15:20 present
-r--r--r-- 1 root root 4096 Feb 13 10:44 serial_number
-r--r--r-- 1 root root 4096 Feb  8 15:20 status
lrwxrwxrwx 1 root root    0 Feb  8 16:01 subsystem -> ../../../../../../class/power_supply
-r--r--r-- 1 root root 4096 Feb 13 10:44 technology
-r--r--r-- 1 root root 4096 Feb 13 10:33 type
-rw-r--r-- 1 root root 4096 Feb  8 16:01 uevent
-r--r--r-- 1 root root 4096 Feb 13 10:44 voltage_min_design
-r--r--r-- 1 root root 4096 Feb  8 15:20 voltage_now
@denilsonsa
Copy link
Copy Markdown
Owner

Interesting… According to the following two links, current_now is deprecated and should be replaced by power_now.

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7faa144a518c456e2057918f030f50100144ccc6
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=532000

Also, according to the Linux documentation, charge_*, energy_* and capacity_* all measure the same thing, but using different units (µAh, µWh, percent). I guess that, if we change batterymon to support those, we must also change the remaining_seconds calculation, depending on how which sysfs variable gets read.

http://lxr.linux.no/#linux+v3.2.7/Documentation/power/power_supply_class.txt#L56

Do you have any idea if your current_now is measured in (milli)amperes or (milli)watts? It is quite possible it uses the same unit as power_now, but if the name is really accurate, the unit should be different.

Can you improve your patch? Here are my comments:

  • Try to find out the unit used by current_now. Maybe needs to dig into the source code, or maybe empirical experiments doing some math to see what kind of unit makes sense for that value.
  • Also try reading from capacity_* in case neither charge_* nor energy_* is available.
  • FYI, int_or_zero() function already handles the case of file not being found (by returning zero), so I guess we don't need that if os.path.exists.
  • Maybe we should read each one of {energy,charge,capacity}_{full,now} to a different variable and then handle if any one of them is zero. Or, if we keep reading to the same var, we should rename it to reflect it is not related to the sysfs var.
  • Add comments at the top of that function explaining the charge_*, energy_* and capacity_*.
  • Fix the remaining_seconds calculation. If no reasonable calculation is possible (for instance, when reading percentags from capacity_*), we can add a log message for that.

Thank you for your patch!
Also, just curious, what distribution do you use?

@h3xx
Copy link
Copy Markdown
Author

h3xx commented Feb 22, 2012

Using int_or_zero() on a file that doesn't exist was causing it to return 0, which is how I first noticed the bug, so it must test before it pulls the info from it otherwise it may not return the right value.

Here's a dump of all the contents of my "files" in the sysfs tree:

/sys/class/power_supply/BAT1/alarm:300000
/sys/class/power_supply/BAT1/charge_full:2247000
/sys/class/power_supply/BAT1/charge_full_design:2200000
/sys/class/power_supply/BAT1/charge_now:2243000
/sys/class/power_supply/BAT1/current_now:0
/sys/class/power_supply/BAT1/cycle_count:0
/sys/class/power_supply/BAT1/manufacturer:SANYO
/sys/class/power_supply/BAT1/model_name:AL10A31
/sys/class/power_supply/BAT1/present:1
/sys/class/power_supply/BAT1/serial_number:3789
/sys/class/power_supply/BAT1/status:Unknown
/sys/class/power_supply/BAT1/technology:Li-ion
/sys/class/power_supply/BAT1/type:Battery
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_NAME=BAT1
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_STATUS=Unknown
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_PRESENT=1
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_TECHNOLOGY=Li-ion
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_CYCLE_COUNT=0
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_VOLTAGE_MIN_DESIGN=11100000
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_VOLTAGE_NOW=12763000
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_CURRENT_NOW=0
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_CHARGE_FULL_DESIGN=2200000
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_CHARGE_FULL=2247000
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_CHARGE_NOW=2243000
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_MODEL_NAME=AL10A31
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_MANUFACTURER=SANYO
/sys/class/power_supply/BAT1/uevent:POWER_SUPPLY_SERIAL_NUMBER=3789
/sys/class/power_supply/BAT1/voltage_min_design:11100000
/sys/class/power_supply/BAT1/voltage_now:12763000

I use Slackware 13.37 with a vanilla 3.2.2 kernel.

More answers and hacking later, after sleep.

h3xx added 2 commits February 22, 2012 11:36
Caveats: `'0' or 2` will return `'0'`

{power,current}_now can contain a 0 when charging. This is normal.
@trustin
Copy link
Copy Markdown

trustin commented Jan 27, 2016

Observed the same problem with:

$ uname -a
Linux infinity 4.3.0-0.bpo.1-amd64 #1 SMP Debian 4.3.3-7~bpo8+1 (2016-01-19) x86_64 GNU/Linux
$ dpkg -l | grep linux-image-4.3
ii  linux-image-4.3.0-0.bpo.1-amd64       4.3.3-7~bpo8+1                       amd64        Linux 4.3 for 64-bit PCs
$ cat /etc/os-release 
PRETTY_NAME="Debian GNU/Linux 8 (jessie)"
NAME="Debian GNU/Linux"
VERSION_ID="8"
VERSION="8 (jessie)"
ID=debian
HOME_URL="http://www.debian.org/"
SUPPORT_URL="http://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants