bug#25559: Trouble getting battery status

bug#25559: Trouble getting battery status

[Rahul Martim Juliato]

Hello!

battery.el gives me an error, where is not possible to find my battery
info directory. 

I'm on a 32 bit ARM, Arch Linux with 3.8.11 kernel, Samsung Chromebook,
and on the proc directoy this is what I have:

~ $ ls -l /sys/class/power_supply/
total 0
lrwxrwxrwx   1 root           root     0 2017-01-27 21:48 sbs-104-000b -> ../../devices/12ca0000.i2c/i2c-4/i2c-104/104-000b/power_supply/sbs-104-000b
lrwxrwxrwx   1 root           root     0 2017-01-27 21:47 tps65090-ac -> ../../devices/12ca0000.i2c/i2c-4/i2c-104/104-0048/tps65090-charger/power_supply/tps65090-ac
~ $ 

That means, I have 2 sub-directories, and battery info is inside
sbs-104-000b.

Taking a look into battery.el, it seems that the program does not tests
for inner directories possibility.

It would be nice for this to look for it, or at least give me possibility to
point the exactly location in a variable.

Thanks,

Rahul

bug#25559: Trouble getting battery status

[Lars Ingebrigtsen]

Rahul Martim Juliato <rahuljuliato@gmail.com> writes:

> battery.el gives me an error, where is not possible to find my battery
> info directory. 
>
> I'm on a 32 bit ARM, Arch Linux with 3.8.11 kernel, Samsung Chromebook,
> and on the proc directoy this is what I have:
>
> ~ $ ls -l /sys/class/power_supply/
> total 0
> lrwxrwxrwx 1 root root 0 2017-01-27 21:48 sbs-104-000b ->
> ../../devices/12ca0000.i2c/i2c-4/i2c-104/104-000b/power_supply/sbs-104-000b
> lrwxrwxrwx 1 root root 0 2017-01-27 21:47 tps65090-ac ->
> ../../devices/12ca0000.i2c/i2c-4/i2c-104/104-0048/tps65090-charger/power_supply/tps65090-ac
> ~ $ 
>
> That means, I have 2 sub-directories, and battery info is inside
> sbs-104-000b.
>
> Taking a look into battery.el, it seems that the program does not tests
> for inner directories possibility.

It looks like it just looks for batteries named BATx:

(defcustom battery-linux-sysfs-regexp "[bB][aA][tT][0-9]?$"

That seems unnecessary -- surely we can just look into all the
directories there and see whether it looks like we have a battery
directory?  The patch below does this, but perhaps it's problematic.  Do
all power_supply/<battery> directories have a "capacity" file, or does
that vary by Linux kernel version?

diff --git a/lisp/battery.el b/lisp/battery.el
index 7037d07dcf..9a09093d48 100644
--- a/lisp/battery.el
+++ b/lisp/battery.el
@@ -38,13 +38,6 @@ battery
   :prefix "battery-"
   :group 'hardware)
 
-(defcustom battery-linux-sysfs-regexp "[bB][aA][tT][0-9]?$"
-  "Regexp for folder names to be searched under
-  /sys/class/power_supply/ that contain battery information."
-  :version "26.1"
-  :type 'regexp
-  :group 'battery)
-
 (defcustom battery-upower-device "battery_BAT1"
   "Upower battery device name."
   :version "26.1"
@@ -60,8 +53,8 @@ battery-status-function
 	 #'battery-linux-proc-acpi)
 	((and (eq system-type 'gnu/linux)
 	      (file-directory-p "/sys/class/power_supply/")
-	      (directory-files "/sys/class/power_supply/" nil
-                               battery-linux-sysfs-regexp))
+	      (directory-files-recursively "/sys/class/power_supply/"
+                                           "\\`capacity\\'" nil nil t))
 	 #'battery-linux-sysfs)
 	((and (eq system-type 'berkeley-unix)
 	      (file-executable-p "/usr/sbin/apm"))
@@ -449,9 +442,10 @@ battery-linux-sysfs
     ;; available information together.
     (with-temp-buffer
       (dolist (dir (ignore-errors
-		    (directory-files
-		     "/sys/class/power_supply/" t
-                     battery-linux-sysfs-regexp)))
+                     (mapcar
+                      #'file-name-directory
+	              (directory-files-recursively
+                       "/sys/class/power_supply/" "\\`capacity\\'" nil nil t))))
 	(erase-buffer)
 	(ignore-errors (insert-file-contents
 			(expand-file-name "uevent" dir)))


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#25559: Trouble getting battery status

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> It looks like it just looks for batteries named BATx:
>
> (defcustom battery-linux-sysfs-regexp "[bB][aA][tT][0-9]?$"
>
> That seems unnecessary -- surely we can just look into all the
> directories there and see whether it looks like we have a battery
> directory?  The patch below does this, but perhaps it's problematic.  Do
> all power_supply/<battery> directories have a "capacity" file, or does
> that vary by Linux kernel version?

There were no comments, and I've tested the patch on three different
GNU/Linux systems, and it seems to work OK.  But like I said, I'm not
extremely confident about this change -- if this leads to problems,
feel free to revert.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no