Various battery.el improvements (bug#41808)

* lisp/battery.el: Mention BSD support in Commentary.  Don't load
preloaded lisp/emacs-lisp/timer.el.
(battery--files): New function.
(battery--find-linux-sysfs-batteries): Use it and make fewer
syscalls.
(battery-status-function): Perform GNU/Linux checks in increasing
order of obsolescence: sysfs, ACPI, and then APM.  Simplify Darwin
check.  Add :version tag now that battery-upower is the default.
(battery-echo-area-format, battery-mode-line-format): Mention %s.
(battery-load-low, battery-load-critical): New faces.
(battery-update): Display battery-mode-line-format even if
percentage is N/A.  Apply faces battery-load-low or
battery-load-critical according to the percentage, but append them
so they don't override user customizations.  Update all mode lines
since we are in global-mode-string.
(battery-linux-proc-apm-regexp): Mark as obsolete, replacing with...
(battery--linux-proc-apm): ...this new rx definition.
(battery-linux-proc-apm): Use it.  Fix indentation.  Simplify.
(battery--acpi-rate, battery--acpi-capacity): New rx definitions.
(battery-linux-proc-acpi): Use them.  Fix pathological whitespace
regexps.  Simplify.
(battery-linux-sysfs): Fix docstring and indentation.  Reduce number
of file searches.  Simplify.
(battery-bsd-apm): Fix docstring.  Simplify.
(battery-pmset): Fix docstring.  Simplify ID regexp.

* lisp/emacs-lisp/rx.el (rx-define): Indent as a defun.

* test/lisp/battery-tests.el (battery-linux-proc-apm-regexp): Test
new battery--linux-proc-apm rx definition.
(battery-acpi-rate-regexp, battery-acpi-capacity-regexp): New tests.

3 files changed, 229 insertions, 175 deletions

diff --git a/lisp/battery.el b/lisp/battery.el
index 38728196507..f6f70b2f16d 100644
--- a/lisp/battery.el
+++ b/lisp/battery.el
@@ -23,18 +23,18 @@
 
 ;;; Commentary:
 
-;; There is at present support for GNU/Linux, macOS, and Windows.
+;; There is at present support for GNU/Linux, BSD, macOS, and Windows.
 ;; This library supports:
 ;; - UPower (https://upower.freedesktop.org) via D-Bus API.
-;; - the `/sys/class/power_supply/' files of Linux >= 2.6.39.
-;; - the `/proc/acpi/' directory structure of Linux 2.4.20 and 2.6.
-;; - the `/proc/apm' file format of Linux version 1.3.58 or newer.
+;; - The `/sys/class/power_supply/' files of Linux >= 2.6.39.
+;; - The `/proc/acpi/' directory structure of Linux 2.4.20 and 2.6.
+;; - The `/proc/apm' file format of Linux version 1.3.58 or newer.
+;; - BSD by using the `apm' program.
 ;; - Darwin (macOS) by using the `pmset' program.
 ;; - Windows via the GetSystemPowerStatus API call.
 
 ;;; Code:
 
-(require 'timer)
 (require 'dbus)
 (eval-when-compile (require 'cl-lib))
 
@@ -60,37 +60,39 @@ If set to nil, then autodetect `:battery' device."
 (defconst battery-upower-dbus-service "org.freedesktop.UPower"
   "Well-known UPower service name for the D-Bus system.")
 
+(defun battery--files (dir)
+  "Return a list of absolute file names in DIR or nil on error.
+Value does not include \".\" or \"..\"."
+  (ignore-errors (directory-files dir t directory-files-no-dot-files-regexp)))
+
 (defun battery--find-linux-sysfs-batteries ()
-  (let ((dirs nil))
-    (dolist (file (directory-files "/sys/class/power_supply/" t))
-      (when (and (or (file-directory-p file)
-                     (file-symlink-p file))
-                 (file-exists-p (expand-file-name "capacity" file)))
-        (push file dirs)))
+  "Return a list of all sysfs battery directories."
+  (let (dirs)
+    (dolist (dir (battery--files "/sys/class/power_supply/"))
+      (when (file-exists-p (expand-file-name "capacity" dir))
+        (push dir dirs)))
     (nreverse dirs)))
 
 (defcustom battery-status-function
   (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)
+              (battery--find-linux-sysfs-batteries))
+         #'battery-linux-sysfs)
         ((and (eq system-type 'gnu/linux)
               (file-directory-p "/proc/acpi/battery"))
          #'battery-linux-proc-acpi)
         ((and (eq system-type 'gnu/linux)
-              (file-directory-p "/sys/class/power_supply/")
-              (battery--find-linux-sysfs-batteries))
-         #'battery-linux-sysfs)
+              (file-readable-p "/proc/apm"))
+         #'battery-linux-proc-apm)
         ((and (eq system-type 'berkeley-unix)
               (file-executable-p "/usr/sbin/apm"))
          #'battery-bsd-apm)
         ((and (eq system-type 'darwin)
-              (condition-case nil
-                  (with-temp-buffer
-                    (and (eq (call-process "pmset" nil t nil "-g" "ps") 0)
-                         (> (buffer-size) 0)))
-                (error nil)))
+              (ignore-errors
+                (with-temp-buffer
+                  (and (eq (call-process "pmset" nil t nil "-g" "ps") 0)
+                       (not (bobp))))))
          #'battery-pmset)
         ((fboundp 'w32-battery-status)
          #'w32-battery-status))
@@ -102,6 +104,7 @@ Its cons cells are of the form
 
 CONVERSION is the character code of a \"conversion specification\"
 introduced by a `%' character in a control string."
+  :version "28.1"
   :type '(choice (const nil) function))
 
 (defcustom battery-echo-area-format
@@ -113,12 +116,13 @@ string are substituted as defined by the current value of the variable
 `battery-status-function'.  Here are the ones generally available:
 %c Current capacity (mAh or mWh)
 %r Current rate of charge or discharge
+%L AC line status (verbose)
 %B Battery status (verbose)
 %b Battery status: empty means high, `-' means low,
    `!' means critical, and `+' means charging
 %d Temperature (in degrees Celsius)
-%L AC line status (verbose)
 %p Battery load percentage
+%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'
@@ -137,7 +141,7 @@ The full `format-spec' formatting syntax is supported."
   :type 'integer)
 
 (defcustom battery-mode-line-format
-  (cond ((eq battery-status-function 'battery-linux-proc-acpi)
+  (cond ((eq battery-status-function #'battery-linux-proc-acpi)
          "[%b%p%%,%d°C]")
         (battery-status-function
          "[%b%p%%]"))
@@ -148,12 +152,13 @@ string are substituted as defined by the current value of the variable
 `battery-status-function'.  Here are the ones generally available:
 %c Current capacity (mAh or mWh)
 %r Current rate of charge or discharge
+%L AC line status (verbose)
 %B Battery status (verbose)
 %b Battery status: empty means high, `-' means low,
    `!' means critical, and `+' means charging
 %d Temperature (in degrees Celsius)
-%L AC line status (verbose)
 %p Battery load percentage
+%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'
@@ -176,6 +181,18 @@ A battery load percentage below this number is considered low."
 A battery load percentage below this number is considered critical."
   :type 'integer)
 
+(defface battery-load-low
+  '((t :inherit warning))
+  "Face used in mode line string when battery load is low.
+See the option `battery-load-low'."
+  :version "28.1")
+
+(defface battery-load-critical
+  '((t :inherit error))
+  "Face used in mode line string when battery load is critical.
+See the option `battery-load-critical'."
+  :version "28.1")
+
 (defvar battery-update-timer nil
   "Interval timer object.")
 
@@ -208,7 +225,7 @@ seconds."
                 (delq 'battery-mode-line-string global-mode-string))
         (add-to-list 'global-mode-string 'battery-mode-line-string t)
         (setq battery-update-timer (run-at-time nil battery-update-interval
-                                                'battery-update-handler))
+                                                #'battery-update-handler))
         (battery-update))
     (message "Battery status not available")
     (setq display-battery-mode nil)))
@@ -220,34 +237,42 @@ seconds."
 (defun battery-update ()
   "Update battery status information in the mode line."
   (let* ((data (and battery-status-function (funcall battery-status-function)))
-         (percentage (car (read-from-string (cdr (assq ?p data))))))
-    (setq battery-mode-line-string
-          (propertize (if (and battery-mode-line-format
-                               (numberp percentage)
-                               (<= percentage battery-mode-line-limit))
-                          (battery-format battery-mode-line-format data)
-                        "")
-                      'face
-                      (and (numberp percentage)
-                           (<= percentage battery-load-critical)
-                           'error)
-                      'help-echo "Battery status information")))
-  (force-mode-line-update))
+         (percentage (car (read-from-string (cdr (assq ?p data)))))
+         (res (and battery-mode-line-format
+                   (or (not (numberp percentage))
+                       (<= percentage battery-mode-line-limit))
+                   (battery-format battery-mode-line-format data)))
+         (len (length res)))
+    (unless (zerop len)
+      (cond ((not (numberp percentage)))
+            ((< percentage battery-load-critical)
+             (add-face-text-property 0 len 'battery-load-critical t res))
+            ((< percentage battery-load-low)
+             (add-face-text-property 0 len 'battery-load-low t res)))
+      (put-text-property 0 len 'help-echo "Battery status information" res))
+    (setq battery-mode-line-string (or res "")))
+  (force-mode-line-update t))
+
 
 ;;; `/proc/apm' interface for Linux.
 
-(defconst battery-linux-proc-apm-regexp
-  (concat "^\\([^ ]+\\)"                ; Driver version.
-          " \\([^ ]+\\)"                ; APM BIOS version.
-          " 0x\\([0-9a-f]+\\)"          ; APM BIOS flags.
-          " 0x\\([0-9a-f]+\\)"          ; AC line status.
-          " 0x\\([0-9a-f]+\\)"          ; Battery status.
-          " 0x\\([0-9a-f]+\\)"          ; Battery flags.
-          " \\(-?[0-9]+\\)%"            ; Load percentage.
-          " \\(-?[0-9]+\\)"             ; Remaining time.
-          " \\(.*\\)"                   ; Time unit.
-          "$")
+;; Regular expression matching contents of `/proc/apm'.
+(rx-define battery--linux-proc-apm
+  (: bol   (group (+ (not ?\s)))        ; Driver version.
+     " "   (group (+ (not ?\s)))        ; APM BIOS version.
+     " 0x" (group (+ xdigit))           ; APM BIOS flags.
+     " 0x" (group (+ xdigit))           ; AC line status.
+     " 0x" (group (+ xdigit))           ; Battery status.
+     " 0x" (group (+ xdigit))           ; Battery flags.
+     " "   (group (? ?-) (+ digit)) ?%  ; Load percentage.
+     " "   (group (? ?-) (+ digit))     ; Remaining time.
+     " "   (group (* nonl))             ; Time unit
+     eol))
+
+(defconst battery-linux-proc-apm-regexp (rx battery--linux-proc-apm)
   "Regular expression matching contents of `/proc/apm'.")
+(make-obsolete-variable 'battery-linux-proc-apm-regexp
+                        "it is no longer used." "28.1")
 
 (defun battery-linux-proc-apm ()
   "Get APM status information from Linux (the kernel).
@@ -267,12 +292,12 @@ The following %-sequences are provided:
 %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 (driver-version bios-version bios-interface line-status
-        battery-status battery-status-symbol load-percentage
-        seconds minutes hours remaining-time tem)
+  (let ( driver-version bios-version bios-interface line-status
+         battery-status battery-status-symbol load-percentage
+         seconds minutes hours remaining-time tem )
     (with-temp-buffer
       (ignore-errors (insert-file-contents "/proc/apm"))
-      (when (re-search-forward battery-linux-proc-apm-regexp)
+      (when (re-search-forward (rx battery--linux-proc-apm) nil t)
         (setq driver-version (match-string 1))
         (setq bios-version (match-string 2))
         (setq tem (string-to-number (match-string 3) 16))
@@ -285,9 +310,7 @@ The following %-sequences are provided:
           (cond ((= tem 0) (setq line-status "off-line"))
                 ((= tem 1) (setq line-status "on-line"))
                 ((= tem 2) (setq line-status "on backup")))
-          (setq tem (string-to-number (match-string 6) 16))
-          (if (= tem 255)
-              (setq battery-status "N/A")
+          (unless (= (string-to-number (match-string 6) 16) 255)
             (setq tem (string-to-number (match-string 5) 16))
             (cond ((= tem 0) (setq battery-status "high"
                                    battery-status-symbol ""))
@@ -304,7 +327,7 @@ The following %-sequences are provided:
             (setq minutes (/ seconds 60)
                   hours (/ seconds 3600))
             (setq remaining-time
-                  (format "%d:%02d" hours (- minutes (* 60 hours))))))))
+                  (format "%d:%02d" hours (% minutes 60)))))))
     (list (cons ?v (or driver-version "N/A"))
           (cons ?V (or bios-version "N/A"))
           (cons ?I (or bios-interface "N/A"))
@@ -312,27 +335,31 @@ The following %-sequences are provided:
           (cons ?B (or battery-status "N/A"))
           (cons ?b (or battery-status-symbol ""))
           (cons ?p (or load-percentage "N/A"))
-          (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"))
+          (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")))))
 
 
 ;;; `/proc/acpi/' interface for Linux.
 
+(rx-define battery--acpi-rate (&rest hour)
+  (: (group (+ digit)) " " (group ?m (in "AW") hour)))
+(rx-define battery--acpi-capacity (battery--acpi-rate ?h))
+
 (defun battery-linux-proc-acpi ()
   "Get ACPI status information from Linux (the kernel).
-This function works only with the `/proc/acpi/' format introduced
-in Linux version 2.4.20 and 2.6.0.
+This function works only with the `/proc/acpi/' interface
+introduced in Linux version 2.4.20 and 2.6.0.
 
 The following %-sequences are provided:
 %c Current capacity (mAh)
-%r Current rate
+%r Current rate of charge or discharge
+%L AC line status (verbose)
 %B Battery status (verbose)
 %b Battery status, empty means high, `-' means low,
    `!' means critical, and `+' means charging
 %d Temperature (in degrees Celsius)
-%L AC line status (verbose)
 %p Battery load percentage
 %m Remaining time (to charge or discharge) in minutes
 %h Remaining time (to charge or discharge) in hours
@@ -348,45 +375,51 @@ The following %-sequences are provided:
     ;; information together since displaying for a variable amount of
     ;; batteries seems overkill for format-strings.
     (with-temp-buffer
-      (dolist (dir (ignore-errors (directory-files "/proc/acpi/battery/"
-                                                   t "\\`[^.]")))
-        (erase-buffer)
-        (ignore-errors (insert-file-contents (expand-file-name "state" dir)))
-        (when (re-search-forward "present: +yes$" nil t)
-          (and (re-search-forward "charging state: +\\(.*\\)$" nil t)
+      (dolist (dir (battery--files "/proc/acpi/battery/"))
+        (ignore-errors
+          (insert-file-contents (expand-file-name "state" dir) nil nil nil t))
+        (goto-char (point-min))
+        (when (re-search-forward (rx "present:" (+ space) "yes" eol) nil t)
+          (and (re-search-forward (rx "charging state:" (+ space)
+                                      (group (not space) (* nonl)) eol)
+                                  nil t)
                (member charging-state '("unknown" "charged" nil))
                ;; On most multi-battery systems, most of the time only one
                ;; battery is "charging"/"discharging", the others are
                ;; "unknown".
                (setq charging-state (match-string 1)))
-          (when (re-search-forward "present rate: +\\([0-9]+\\) \\(m[AW]\\)$"
+          (when (re-search-forward (rx "present rate:" (+ space)
+                                       (battery--acpi-rate) eol)
                                    nil t)
             (setq rate (+ (or rate 0) (string-to-number (match-string 1))))
             (when (> rate 0)
-              (setq rate-type (or (and rate-type
-                                     (if (string= rate-type (match-string 2))
-                                         rate-type
-                                       (error
-                                        "Inconsistent rate types (%s vs. %s)"
-                                        rate-type (match-string 2))))
-                                  (match-string 2)))))
-          (when (re-search-forward "remaining capacity: +\\([0-9]+\\) m[AW]h$"
+              (cond ((not rate-type)
+                     (setq rate-type (match-string 2)))
+                    ((not (string= rate-type (match-string 2)))
+                     (error "Inconsistent rate types (%s vs. %s)"
+                            rate-type (match-string 2))))))
+          (when (re-search-forward (rx "remaining capacity:" (+ space)
+                                       battery--acpi-capacity eol)
                                    nil t)
             (setq capacity
                   (+ (or capacity 0) (string-to-number (match-string 1))))))
         (goto-char (point-max))
         (ignore-errors (insert-file-contents (expand-file-name "info" dir)))
-        (when (re-search-forward "present: +yes$" nil t)
-          (when (re-search-forward "design capacity: +\\([0-9]+\\) m[AW]h$"
+        (when (re-search-forward (rx "present:" (+ space) "yes" eol) nil t)
+          (when (re-search-forward (rx "design capacity:" (+ space)
+                                       battery--acpi-capacity eol)
                                    nil t)
             (cl-incf design-capacity (string-to-number (match-string 1))))
-          (when (re-search-forward "last full capacity: +\\([0-9]+\\) m[AW]h$"
+          (when (re-search-forward (rx "last full capacity:" (+ space)
+                                       battery--acpi-capacity eol)
                                    nil t)
             (cl-incf last-full-capacity (string-to-number (match-string 1))))
-          (when (re-search-forward
-                 "design capacity warning: +\\([0-9]+\\) m[AW]h$" nil t)
+          (when (re-search-forward (rx "design capacity warning:" (+ space)
+                                       battery--acpi-capacity eol)
+                                   nil t)
             (cl-incf warn (string-to-number (match-string 1))))
-          (when (re-search-forward "design capacity low: +\\([0-9]+\\) m[AW]h$"
+          (when (re-search-forward (rx "design capacity low:" (+ space)
+                                       battery--acpi-capacity eol)
                                    nil t)
             (cl-incf low (string-to-number (match-string 1)))))))
     (setq full-capacity (if (> last-full-capacity 0)
@@ -400,79 +433,70 @@ The following %-sequences are provided:
                                    60)
                                 rate))
                hours (/ minutes 60)))
-    (list (cons ?c (or (and capacity (number-to-string capacity)) "N/A"))
+    (list (cons ?c (if capacity (number-to-string capacity) "N/A"))
           (cons ?L (or (battery-search-for-one-match-in-files
-                        (mapcar (lambda (e) (concat e "/state"))
-                                (ignore-errors
-                                  (directory-files "/proc/acpi/ac_adapter/"
-                                                   t "\\`[^.]")))
-                        "state: +\\(.*\\)$" 1)
-
+                        (mapcar (lambda (d) (expand-file-name "state" d))
+                                (battery--files "/proc/acpi/ac_adapter/"))
+                        (rx "state:" (+ space) (group (not space) (* nonl)) eol)
+                        1)
                        "N/A"))
           (cons ?d (or (battery-search-for-one-match-in-files
-                        (mapcar (lambda (e) (concat e "/temperature"))
-                                (ignore-errors
-                                  (directory-files "/proc/acpi/thermal_zone/"
-                                                   t "\\`[^.]")))
-                        "temperature: +\\([0-9]+\\) C$" 1)
-
+                        (mapcar (lambda (d) (expand-file-name "temperature" d))
+                                (battery--files "/proc/acpi/thermal_zone/"))
+                        (rx "temperature:" (+ space) (group (+ digit)) " C" eol)
+                        1)
                        "N/A"))
-          (cons ?r (or (and rate (concat (number-to-string rate) " "
-                                         rate-type)) "N/A"))
+          (cons ?r (if rate
+                       (concat (number-to-string rate) " " rate-type)
+                     "N/A"))
           (cons ?B (or charging-state "N/A"))
-          (cons ?b (or (and (string= charging-state "charging") "+")
-                       (and capacity (< capacity low) "!")
-                       (and capacity (< capacity warn) "-")
-                       ""))
-          (cons ?h (or (and hours (number-to-string hours)) "N/A"))
-          (cons ?m (or (and minutes (number-to-string minutes)) "N/A"))
-          (cons ?t (or (and minutes
-                            (format "%d:%02d" hours (- minutes (* 60 hours))))
-                       "N/A"))
-          (cons ?p (or (and full-capacity capacity
-                            (> full-capacity 0)
-                            (number-to-string
-                             (floor (* 100 capacity) full-capacity)))
-                       "N/A")))))
+          (cons ?b (cond ((string= charging-state "charging") "+")
+                         ((and capacity (< capacity low)) "!")
+                         ((and capacity (< capacity warn)) "-")
+                         ("")))
+          (cons ?h (if hours (number-to-string hours) "N/A"))
+          (cons ?m (if minutes (number-to-string minutes) "N/A"))
+          (cons ?t (if minutes (format "%d:%02d" hours (% minutes 60)) "N/A"))
+          (cons ?p (if (and full-capacity capacity (> full-capacity 0))
+                       (number-to-string (floor (* 100 capacity) full-capacity))
+                     "N/A")))))
 
 
 ;;; `/sys/class/power_supply/BATN' interface for Linux.
 
 (defun battery-linux-sysfs ()
-  "Get ACPI status information from Linux kernel.
+  "Get sysfs status information from Linux kernel.
 This function works only with the new `/sys/class/power_supply/'
-format introduced in Linux version 2.4.25.
+interface introduced in Linux version 2.4.25.
 
 The following %-sequences are provided:
 %c Current capacity (mAh or mWh)
-%r Current rate
+%r Current rate of charge or discharge
+%L Power source (verbose)
 %B Battery status (verbose)
 %b Battery status, empty means high, `-' means low,
    `!' means critical, and `+' means charging
 %d Temperature (in degrees Celsius)
 %p Battery load percentage
-%L AC line status (verbose)
 %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 (charging-state temperature hours percentage-now
-        ;; Some batteries report charges and current, other energy and power.
+  (let (;; Some batteries report charges and current, others energy and power.
         ;; In order to reliably be able to combine those data, we convert them
         ;; all to energy/power (since we can't combine different charges if
         ;; they're not at the same voltage).
         (energy-full 0.0)
         (energy-now 0.0)
         (power-now 0.0)
-        (voltage-now 10.8))    ;Arbitrary default, in case the info is missing.
+        (voltage-now 10.8) ; Arbitrary default, in case the info is missing.
+        charging-state temperature hours percentage-now)
     ;; SysFS provides information about each battery present in the
     ;; system in a separate subdirectory.  We are going to merge the
     ;; available information together.
     (with-temp-buffer
-      (dolist (dir (ignore-errors
-                     (battery--find-linux-sysfs-batteries)))
-        (erase-buffer)
-        (ignore-errors (insert-file-contents
-                        (expand-file-name "uevent" dir)))
+      (dolist (dir (battery--find-linux-sysfs-batteries))
+        (ignore-errors
+          (insert-file-contents (expand-file-name "uevent" dir) nil nil nil t))
         (goto-char (point-min))
         (when (re-search-forward
                "POWER_SUPPLY_VOLTAGE_NOW=\\([0-9]*\\)$" nil t)
@@ -508,7 +532,7 @@ The following %-sequences are provided:
                                            voltage-now))
                    (cl-incf energy-now  (* (string-to-number now-string)
                                            voltage-now)))
-                  ((and (progn (goto-char (point-min)) t)
+                  ((and (goto-char (point-min))
                         (re-search-forward
                          "POWER_SUPPLY_ENERGY_FULL=\\([0-9]*\\)$" nil t)
                         (setq full-string (match-string 1))
@@ -517,7 +541,6 @@ The following %-sequences are provided:
                         (setq now-string (match-string 1)))
                    (cl-incf energy-full (string-to-number full-string))
                    (cl-incf energy-now  (string-to-number now-string)))))
-          (goto-char (point-min))
           (unless (zerop power-now)
             (let ((remaining (if (string= charging-state "Discharging")
                                  energy-now
@@ -525,9 +548,9 @@ The following %-sequences are provided:
               (setq hours (/ remaining power-now)))))))
     (when (and (> energy-full 0) (> energy-now 0))
       (setq percentage-now (/ (* 100 energy-now) energy-full)))
-    (list (cons ?c (cond ((or (> energy-full 0) (> energy-now 0))
-                          (number-to-string (/ energy-now voltage-now)))
-                         (t "N/A")))
+    (list (cons ?c (if (or (> energy-full 0) (> energy-now 0))
+                       (number-to-string (/ energy-now voltage-now))
+                     "N/A"))
           (cons ?r (if (> power-now 0.0)
                        (format "%.1f" (/ power-now 1000000.0))
                      "N/A"))
@@ -538,27 +561,20 @@ The following %-sequences are provided:
                      "N/A"))
           (cons ?d (or temperature "N/A"))
           (cons ?B (or charging-state "N/A"))
-          (cons ?b (or (and (string= charging-state "Charging") "+")
-                       (and percentage-now (< percentage-now battery-load-critical) "!")
-                       (and percentage-now (< percentage-now battery-load-low) "-")
-                       ""))
-          (cons ?p (cond
-                    ((and percentage-now (format "%.1f" percentage-now)))
-                    (t "N/A")))
-          (cons ?L (cond
-                    ((battery-search-for-one-match-in-files
-                      (list "/sys/class/power_supply/AC/online"
-                            "/sys/class/power_supply/ACAD/online"
-                            "/sys/class/power_supply/ADP1/online")
-                      "1" 0)
-                     "AC")
-                    ((battery-search-for-one-match-in-files
-                      (list "/sys/class/power_supply/AC/online"
-                            "/sys/class/power_supply/ACAD/online"
-                            "/sys/class/power_supply/ADP1/online")
-                      "0" 0)
-                     "BAT")
-                    (t "N/A"))))))
+          (cons ?b (cond ((string= charging-state "Charging") "+")
+                         ((not percentage-now) "")
+                         ((< percentage-now battery-load-critical) "!")
+                         ((< percentage-now battery-load-low) "-")
+                         ("")))
+          (cons ?p (if percentage-now (format "%.1f" percentage-now) "N/A"))
+          (cons ?L (pcase (battery-search-for-one-match-in-files
+                           '("/sys/class/power_supply/AC/online"
+                             "/sys/class/power_supply/ACAD/online"
+                             "/sys/class/power_supply/ADP1/online")
+                           (rx (in "01")) 0)
+                     ("0" "BAT")
+                     ("1" "AC")
+                     (_ "N/A"))))))
 
 
 ;;; `upowerd' interface.
@@ -681,19 +697,20 @@ The following %-sequences are provided:
 
 
 ;;; `apm' interface for BSD.
+
 (defun battery-bsd-apm ()
   "Get APM status information from BSD apm binary.
 The following %-sequences are provided:
+%P Advanced power saving mode state (verbose)
 %L AC line status (verbose)
 %B Battery status (verbose)
 %b Battery status, empty means high, `-' means low,
- `!' means critical, and `+' means charging
-%P Advanced power saving mode state (verbose)
-%p Battery charge percentage
-%s Remaining battery charge time in seconds
-%m Remaining battery charge time in minutes
-%h Remaining battery charge time in hours
-%t Remaining battery charge time in the form `h:min'"
+   `!' means critical, and `+' means charging
+%p Battery load percentage
+%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* ((os-name (car (split-string
                         ;; FIXME: Can't we use something like `system-type'?
                         (shell-command-to-string "/usr/bin/uname"))))
@@ -759,7 +776,7 @@ The following %-sequences are provided:
         (setq seconds (string-to-number battery-life)
               minutes (truncate seconds 60)))
       (setq hours (truncate minutes 60)
-            remaining-time (format "%d:%02d" hours (mod minutes 60))))
+            remaining-time (format "%d:%02d" hours (% minutes 60))))
     (list (cons ?L (or line-status "N/A"))
           (cons ?B (or (car battery-status) "N/A"))
           (cons ?b (or (cdr battery-status) "N/A"))
@@ -767,9 +784,9 @@ The following %-sequences are provided:
                        "N/A"
                      battery-percentage))
           (cons ?P (or apm-mode "N/A"))
-          (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"))
+          (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")))))
 
 
@@ -784,21 +801,25 @@ The following %-sequences are provided:
 %b Battery status, empty means high, `-' means low,
    `!' means critical, and `+' means charging
 %p Battery load percentage
-%h Remaining time in hours
-%m Remaining time in minutes
-%t Remaining time in the form `h:min'"
-  (let (power-source load-percentage battery-status battery-status-symbol
-        remaining-time hours minutes)
+%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 ( power-source load-percentage battery-status battery-status-symbol
+         remaining-time hours minutes )
     (with-temp-buffer
       (ignore-errors (call-process "pmset" nil t nil "-g" "ps"))
       (goto-char (point-min))
-      (when (re-search-forward "\\(?:Currentl?y\\|Now\\) drawing from '\\(AC\\|Battery\\) Power'" nil t)
+      (when (re-search-forward ;; Handle old typo in output.
+             "\\(?:Currentl?y\\|Now\\) drawing from '\\(AC\\|Battery\\) Power'"
+             nil t)
         (setq power-source (match-string 1))
-        (when (re-search-forward "^ -InternalBattery-0\\([ \t]+(id=[0-9]+)\\)*[ \t]+" nil t)
+        (when (re-search-forward (rx bol " -InternalBattery-0" (+ space)
+                                     (* "(id=" (+ digit) ")" (+ space)))
+                                 nil t)
           (when (looking-at "\\([0-9]\\{1,3\\}\\)%")
             (setq load-percentage (match-string 1))
             (goto-char (match-end 0))
-            (cond ((looking-at "; charging")
+            (cond ((looking-at-p "; charging")
                    (setq battery-status "charging"
                          battery-status-symbol "+"))
                   ((< (string-to-number load-percentage) battery-load-critical)
diff --git a/lisp/emacs-lisp/rx.el b/lisp/emacs-lisp/rx.el
index aa4b2addd47..88bb0a8bd6c 100644
--- a/lisp/emacs-lisp/rx.el
+++ b/lisp/emacs-lisp/rx.el
@@ -1381,7 +1381,7 @@ To make local rx extensions, use `rx-let' for `rx',
 For more details, see Info node `(elisp) Extending Rx'.
 
 \(fn NAME [(ARGS...)] RX)"
-  (declare (indent 1))
+  (declare (indent defun))
   `(eval-and-compile
      (put ',name 'rx-definition ',(rx--make-binding name definition))
      ',name))
diff --git a/test/lisp/battery-tests.el b/test/lisp/battery-tests.el
index 4cb7470d884..92ab013f040 100644
--- a/test/lisp/battery-tests.el
+++ b/test/lisp/battery-tests.el
@@ -22,9 +22,9 @@
 (require 'battery)
 
 (ert-deftest battery-linux-proc-apm-regexp ()
-  "Test `battery-linux-proc-apm-regexp'."
+  "Test `rx' definition `battery--linux-proc-apm'."
   (let ((str "1.16 1.2 0x07 0x01 0xff 0x80 -1% -1 ?"))
-    (should (string-match battery-linux-proc-apm-regexp str))
+    (should (string-match (rx battery--linux-proc-apm) str))
     (should (equal (match-string 0 str) str))
     (should (equal (match-string 1 str) "1.16"))
     (should (equal (match-string 2 str) "1.2"))
@@ -36,7 +36,7 @@
     (should (equal (match-string 8 str) "-1"))
     (should (equal (match-string 9 str) "?")))
   (let ((str "1.16 1.2 0x03 0x00 0x00 0x01 99% 1792 min"))
-    (should (string-match battery-linux-proc-apm-regexp str))
+    (should (string-match (rx battery--linux-proc-apm) str))
     (should (equal (match-string 0 str) str))
     (should (equal (match-string 1 str) "1.16"))
     (should (equal (match-string 2 str) "1.2"))
@@ -48,6 +48,39 @@
     (should (equal (match-string 8 str) "1792"))
     (should (equal (match-string 9 str) "min"))))
 
+(ert-deftest battery-acpi-rate-regexp ()
+  "Test `rx' definition `battery--acpi-rate'."
+  (let ((str "01 mA"))
+    (should (string-match (rx (battery--acpi-rate)) str))
+    (should (equal (match-string 0 str) str))
+    (should (equal (match-string 1 str) "01"))
+    (should (equal (match-string 2 str) "mA")))
+  (let ((str "23 mW"))
+    (should (string-match (rx (battery--acpi-rate)) str))
+    (should (equal (match-string 0 str) str))
+    (should (equal (match-string 1 str) "23"))
+    (should (equal (match-string 2 str) "mW")))
+  (let ((str "23 mWh"))
+    (should (string-match (rx (battery--acpi-rate)) str))
+    (should (equal (match-string 0 str) "23 mW"))
+    (should (equal (match-string 1 str) "23"))
+    (should (equal (match-string 2 str) "mW")))
+  (should-not (string-match (rx (battery--acpi-rate) eos) "45 mWh")))
+
+(ert-deftest battery-acpi-capacity-regexp ()
+  "Test `rx' definition `battery--acpi-capacity'."
+  (let ((str "01 mAh"))
+    (should (string-match (rx battery--acpi-capacity) str))
+    (should (equal (match-string 0 str) str))
+    (should (equal (match-string 1 str) "01"))
+    (should (equal (match-string 2 str) "mAh")))
+  (let ((str "23 mWh"))
+    (should (string-match (rx battery--acpi-capacity) str))
+    (should (equal (match-string 0 str) str))
+    (should (equal (match-string 1 str) "23"))
+    (should (equal (match-string 2 str) "mWh")))
+  (should-not (string-match (rx battery--acpi-capacity eos) "45 mW")))
+
 (ert-deftest battery-format ()
   "Test `battery-format'."
   (should (equal (battery-format "" ()) ""))