Re: [PATCH] battery.el, upower fixes

[Evgeny Zajcev <lg.zevlg@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1433 bytes --]

ср, 5 февр. 2020 г. в 06:03, Stefan Monnier <monnier@iro.umontreal.ca>:

> >> No: `M-x battery` will emit the warning.
> > So `battery-upower` should check the correctness of the
> > `battery-upower-device` on every call?  It would make it heavier, but of
> > course more reliable, because user might change the value of the
> > `batter-upower-device` in runtime
> >
> > We might have `nil` values for the `battery-upower-device` and
> > `battery-upower-line-power-device` meaning "autodetect".  This will
> require
> > additional call to D-Bus (battery-upower-device-list) on every call to
> > `battery-upower`, probably this is OK.  If user want extra speed he just
> > set right values for that vars.  Sounds good?
>
> Right, we have to decouple the custom vars `battery-upower-device` and
> `battery-upower-line-power-device` from the actual list of devices.
> We can do this by recomputing the actual list (and checking its
> validity) every time time.
>
> Or if the performance impact matters, we can do this computation on the
> first call and then check every time that the custom vars haven't
> changed (and ask for D-Bus to calls us back when the list needs to be
> changed).
>

Not much impact actually.  Here is the updated battery.el against the
master this time.
Also added "%b" format-spec, so 'M-x display-battery-mode RET' shows "+" in
case AC is on-line

Thanks


-- 
lg

[-- Attachment #1.2: Type: text/html, Size: 1940 bytes --]

[-- Attachment #2: 0001-Make-M-x-battery-RET-work-out-of-box-for-UPower-user.patch --]
[-- Type: text/x-patch, Size: 11125 bytes --]

From 9164b1da07b592585d84806a7f7afb8d4dba408e Mon Sep 17 00:00:00 2001
From: Zajcev Evgeny <zevlg@yandex.ru>
Date: Thu, 6 Feb 2020 10:35:12 +0300
Subject: [PATCH] Make 'M-x battery RET' work out-of-box for UPower users.

* battery.el (battery-upower-prop): Removed in favor for
  'battery-upower-device-property'.
  (battery-upower-device): Can be nil, meaning autodetect the battery
  device.
  (battery-upower-line-power-device): New.  line-power device.  Can be
  nil, meaning autodetect line-power device.
  (battery-status-function): Check UPower service is available to use
  'battery-upower' as status function.
  (battery-upower): Speedup.  Request D-Bus only once, fetching all
  the properties at once.  Provide string for "%b" format spec.
  (battery-upower-device-list, battery-upower-device-all-properties,
  battery-upower-device-property): New functions to work with UPower
  devices.
  (battery-upower-dbus-service, battery-upower-dbus-interface,
  battery-upower-dbus-path, battery-upower-dbus-device-interface,
  battery-upower-dbus-device-path): New constants describing UPower
  D-Bus service.
---
 lisp/battery.el | 176 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 124 insertions(+), 52 deletions(-)

diff --git a/lisp/battery.el b/lisp/battery.el
index 1d3390070c..5368e1b0e9 100644
--- a/lisp/battery.el
+++ b/lisp/battery.el
@@ -23,14 +23,16 @@
 ;;; Commentary:
 
 ;; There is at present support for GNU/Linux, macOS and Windows.  This
-;; library supports both the `/proc/apm' file format of Linux version
-;; 1.3.58 or newer and the `/proc/acpi/' directory structure of Linux
-;; 2.4.20 and 2.6.  Darwin (macOS) is supported by using the `pmset'
-;; program.  Windows is supported by the GetSystemPowerStatus API call.
+;; library supports UPower (https://upower.freedesktop.org) via D-Bus
+;; API or the `/proc/apm' file format of Linux version 1.3.58 or newer
+;; and the `/proc/acpi/' directory structure of Linux 2.4.20 and 2.6.
+;; Darwin (macOS) is supported by using the `pmset' program.  Windows
+;; is supported by the GetSystemPowerStatus API call.
 
 ;;; Code:
 
 (require 'timer)
+(require 'dbus)
 (eval-when-compile (require 'cl-lib))
 \f
 (defgroup battery nil
@@ -38,12 +40,25 @@ battery
   :prefix "battery-"
   :group 'hardware)
 
-(defcustom battery-upower-device "battery_BAT1"
-  "Upower battery device name."
-  :version "26.1"
-  :type 'string
+(defcustom battery-upower-device nil
+  "UPower device of the `:battery' type.
+Use `battery-upower-device-list' to list all available UPower devices.
+If set to nil, then autodetect `:battery' device."
+  :version "28.1"
+  :type '(choice string (const :tag "Autodetect" nil))
   :group 'battery)
 
+(defcustom battery-upower-line-power-device nil
+  "UPower device of the `:line-power' type.
+Use `battery-upower-device-list' to list all available UPower devices.
+If set to nil, then autodetect `:battery' device."
+  :version "28.1"
+  :type '(choice string (const :tag "Autodetect" nil))
+  :group 'battery)
+
+(defconst battery-upower-dbus-service "org.freedesktop.UPower"
+  "Well-known UPower service name for the D-Bus system.")
+
 (defun battery--find-linux-sysfs-batteries ()
   (let ((dirs nil))
     (dolist (file (directory-files "/sys/class/power_supply/" t))
@@ -54,7 +69,9 @@ battery--find-linux-sysfs-batteries
     (nreverse dirs)))
 
 (defcustom battery-status-function
-  (cond ((and (eq system-type 'gnu/linux)
+  (cond ((dbus-ping :system battery-upower-dbus-service)
+         #'battery-upower)
+        ((and (eq system-type 'gnu/linux)
 	      (file-readable-p "/proc/apm"))
 	 #'battery-linux-proc-apm)
 	((and (eq system-type 'gnu/linux)
@@ -537,17 +554,68 @@ battery-linux-sysfs
                     (t "N/A"))))))
 
 \f
-(declare-function dbus-get-property "dbus.el"
-                  (bus service path interface property))
-
 ;;; `upowerd' interface.
-(defsubst battery-upower-prop (pname &optional device)
+(defconst battery-upower-dbus-interface "org.freedesktop.UPower"
+  "The interface to UPower.
+See URL `https://upower.freedesktop.org/docs/'.")
+
+(defconst battery-upower-dbus-path "/org/freedesktop/UPower"
+  "D-Bus path to talk to UPower service.")
+
+(defconst battery-upower-dbus-device-interface
+  (concat battery-upower-dbus-interface ".Device")
+  "The Device interface of the UPower.
+See URL `https://upower.freedesktop.org/docs/Device.html'.")
+
+(defconst battery-upower-dbus-device-path
+  (concat battery-upower-dbus-path "/devices")
+  "D-Bus path to talk to devices part of the UPower service.")
+
+(defconst battery-upower-types
+  '((0 . :unknown) (1 . :line-power) (2 . :battery)
+    (3 . :ups) (4 . :monitor) (5 . :mouse)
+    (6 . :keyboard) (7 . :pda) (8 . :phone))
+  "Type of the device.")
+
+(defconst battery-upower-states
+  '((0 . "unknown") (1 . "charging") (2 . "discharging")
+    (3 . "empty") (4 . "fully-charged") (5 . "pending-charge")
+    (6 . "pending-discharge"))
+  "Alist of battery power states.
+Only valid for `:battery' devices.")
+
+(defun battery-upower-device-property (device property)
+  "Get value of the single PROPERTY for the UPower DEVICE."
   (dbus-get-property
-   :system
-   "org.freedesktop.UPower"
-   (concat "/org/freedesktop/UPower/devices/" (or device battery-upower-device))
-   "org.freedesktop.UPower"
-   pname))
+   :system battery-upower-dbus-service
+   (expand-file-name device battery-upower-dbus-device-path)
+   battery-upower-dbus-device-interface
+   property))
+
+(defun battery-upower-device-all-properties (device)
+  "Return value for all available properties for the UPower DEVICE."
+  (dbus-get-all-properties
+   :system battery-upower-dbus-service
+   (expand-file-name device battery-upower-dbus-device-path)
+   battery-upower-dbus-device-interface))
+
+(defun battery-upower-device-list ()
+  "Return list of all available UPower devices.
+Each element is the cons cell in form: (DEVICE . DEVICE-TYPE)."
+  (mapcar (lambda (device-path)
+            (let* ((device (file-relative-name
+                            device-path battery-upower-dbus-device-path))
+                   (type-num (battery-upower-device-property device "Type")))
+              (cons device (or (cdr (assq type-num battery-upower-types))
+                               :unknown))))
+          (dbus-call-method :system battery-upower-dbus-service
+                            battery-upower-dbus-path
+                            battery-upower-dbus-interface
+                            "EnumerateDevices")))
+
+(defun battery-upower-device-autodetect (device-type)
+  "Return first matching UPower device of DEVICE-TYPE."
+  (car (rassq device-type (battery-upower-device-list))))
 
 (defun battery-upower ()
   "Get battery status from dbus Upower interface.
@@ -559,45 +627,49 @@ battery-upower
 %p Battery load percentage
 %r Current rate
 %B Battery status (verbose)
+%b Battery status: empty means high, `-' means low,
+   `!' means critical, and `+' means charging
 %L AC line status (verbose)
 %s Remaining time (to charge or discharge) in seconds
 %m Remaining time (to charge or discharge) in minutes
 %h Remaining time (to charge or discharge) in hours
 %t Remaining time (to charge or discharge) in the form `h:min'"
-  (let ((percents (battery-upower-prop "Percentage"))
-        (time-to-empty (battery-upower-prop "TimeToEmpty"))
-        (time-to-full (battery-upower-prop "TimeToFull"))
-        (state (battery-upower-prop "State"))
-        (online (battery-upower-prop "Online" "line_power_ACAD"))
-        (energy (battery-upower-prop "Energy"))
-        (energy-rate (battery-upower-prop "EnergyRate"))
-        (battery-states '((0 . "unknown") (1 . "charging")
-                          (2 . "discharging") (3 . "empty")
-                          (4 . "fully-charged") (5 . "pending-charge")
-                          (6 . "pending-discharge")))
-        seconds minutes hours remaining-time)
-    (cond ((and online time-to-full)
-           (setq seconds time-to-full))
-          ((and (not online) time-to-empty)
-           (setq seconds time-to-empty)))
-    (when seconds
-      (setq minutes (/ seconds 60)
-            hours (/ minutes 60)
-	    remaining-time (format "%d:%02d" hours (mod minutes 60))))
-    (list (cons ?c (or (and energy
-                            (number-to-string (round (* 1000 energy))))
-                       "N/A"))
-          (cons ?p (or (and percents (number-to-string (round percents)))
-                       "N/A"))
-          (cons ?r (or (and energy-rate
-                            (concat (number-to-string energy-rate) " W"))
-                       "N/A"))
-          (cons ?B (or (and state (cdr (assoc state battery-states)))
-                       "unknown"))
-          (cons ?L (or (and online "on-line") "off-line"))
-          (cons ?s (or (and seconds (number-to-string seconds)) "N/A"))
-          (cons ?m (or (and minutes (number-to-string minutes)) "N/A"))
-          (cons ?h (or (and hours (number-to-string hours)) "N/A"))
+  (let* ((bat-device (or battery-upower-device
+                         (battery-upower-device-autodetect :battery)))
+         (bat-props (when bat-device
+                      (battery-upower-device-all-properties bat-device)))
+         (percents (cdr (assoc "Percentage" bat-props)))
+         (time-to-empty (cdr (assoc "TimeToEmpty" bat-props)))
+         (time-to-full (cdr (assoc "TimeToFull" bat-props)))
+         (state (cdr (assoc "State" bat-props)))
+         (level (cdr (assoc "BatteryLevel" bat-props)))
+         (energy (cdr (assoc "Energy" bat-props)))
+         (energy-rate (cdr (assoc "EnergyRate" bat-props)))
+         (lp-device (or battery-upower-line-power-device
+                        (battery-upower-device-autodetect :line-power)))
+         (online-p (when lp-device
+                     (battery-upower-device-property lp-device "Online")))
+         (seconds (if online-p time-to-full time-to-empty))
+         (minutes (when seconds (/ seconds 60)))
+         (hours (when minutes (/ minutes 60)))
+         (remaining-time (when hours
+                           (format "%d:%02d" hours (mod minutes 60)))))
+    (list (cons ?c (if energy (number-to-string (round (* 1000 energy))) "N/A"))
+          (cons ?p (if percents (number-to-string (round percents)) "N/A"))
+          (cons ?r (if energy-rate
+                       (concat (number-to-string energy-rate) " W")
+                     "N/A"))
+          (cons ?B (if state
+                       (cdr (assq state battery-upower-states))
+                     "unknown"))
+          (cons ?b (cond ((= level 3) "-")
+                         ((= level 4) "!")
+                         (online-p "+")
+                         (t "")))
+          (cons ?L (if online-p "on-line" (if lp-device "off-line" "unknown")))
+          (cons ?s (if seconds (number-to-string seconds) "N/A"))
+          (cons ?m (if minutes (number-to-string minutes) "N/A"))
+          (cons ?h (if hours (number-to-string hours) "N/A"))
           (cons ?t (or remaining-time "N/A")))))
 
 \f
-- 
2.17.1