[bug#34246] [PATCH 0/3] Improve the upower-service style.

[bug#34246] [PATCH 0/3] Improve the upower-service style.

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]

These patches update the style of the upower-service. Improving and
documenting the <upower-configuration> record, and deprecating the
upower-service procedure.

Directly using record types as default values for service types, along
with default values for the fields in the record type is generally more
flexible and configurable than using procedures for service
configuration. It means that the configuration for the service can be
changed programatically by generating new configuration based off of the
original configuration, rather than having to rerun the procedure that
created the configuration in the first place.


Christopher Baines (3):
  services: Improve the upower-configuration record.
  services: Improve the upower-service-type.
  services: desktop: Deprecate the upower-service procedure.

 doc/guix.texi            | 79 ++++++++++++++++++++++++++--------
 gnu/services/desktop.scm | 92 +++++++++++++++++++++++++++-------------
 2 files changed, 123 insertions(+), 48 deletions(-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

[bug#34246] [PATCH 1/3] services: Improve the upower-configuration record.

[Christopher Baines]

Copy the defaults from the upower-service procedure to the
<upower-configuration> record type. This will allow making it the default
value for the upower-service-type, and deprecating the procedure. Export the
field accessors so that the <upower-configuration> record type becomes more
usable.

* gnu/services/desktop.scm (<upower-configuration>): Export it.
(upower-configuration-upower, upower-configuration-watts-up-pro?,
upower-configuration-poll-batteries?, upower-configuration-ignore-lid?,
upower-configuration-use-percentage-for-policy?,
upower-configuration-percentage-low, upower-configuration-percentage-critical,
upower-configuration-percentage-action, upower-configuration-time-low,
upower-configuration-time-critical, upower-configuration-time-action,
upower-configuration-critical-power-action): Add default and export.
---
 gnu/services/desktop.scm | 55 +++++++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 15 deletions(-)

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index fbeabf1162..f51ac4d74c 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -7,6 +7,7 @@
 ;;; Copyright © 2017 Nils Gillmann <ng0@n0.is>
 ;;; Copyright © 2018 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2017, 2019 Christopher Baines <mail@cbaines.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -56,8 +57,22 @@
   #:use-module (guix gexp)
   #:use-module (srfi srfi-1)
   #:use-module (ice-9 match)
-  #:export (upower-configuration
+  #:export (<upower-configuration>
+            upower-configuration
             upower-configuration?
+            upower-configuration-upower
+            upower-configuration-watts-up-pro?
+            upower-configuration-poll-batteries?
+            upower-configuration-ignore-lid?
+            upower-configuration-use-percentage-for-policy?
+            upower-configuration-percentage-low
+            upower-configuration-percentage-critical
+            upower-configuration-percentage-action
+            upower-configuration-time-low
+            upower-configuration-time-critical
+            upower-configuration-time-action
+            upower-configuration-critical-power-action
+
             upower-service
             upower-service-type
 
@@ -173,23 +188,33 @@ is set to @var{value} when the bus daemon launches it."
 ;;; Upower D-Bus service.
 ;;;
 
-;; TODO: Export.
 (define-record-type* <upower-configuration>
   upower-configuration make-upower-configuration
   upower-configuration?
-  (upower        upower-configuration-upower
-                 (default upower))
-  (watts-up-pro? upower-configuration-watts-up-pro?)
-  (poll-batteries? upower-configuration-poll-batteries?)
-  (ignore-lid? upower-configuration-ignore-lid?)
-  (use-percentage-for-policy? upower-configuration-use-percentage-for-policy?)
-  (percentage-low upower-configuration-percentage-low)
-  (percentage-critical upower-configuration-percentage-critical)
-  (percentage-action upower-configuration-percentage-action)
-  (time-low upower-configuration-time-low)
-  (time-critical upower-configuration-time-critical)
-  (time-action upower-configuration-time-action)
-  (critical-power-action upower-configuration-critical-power-action))
+  (upower                     upower-configuration-upower
+                              (default upower))
+  (watts-up-pro?              upower-configuration-watts-up-pro?
+                              (default #f))
+  (poll-batteries?            upower-configuration-poll-batteries?
+                              (default #t))
+  (ignore-lid?                upower-configuration-ignore-lid?
+                              (default #f))
+  (use-percentage-for-policy? upower-configuration-use-percentage-for-policy?
+                              (default #f))
+  (percentage-low             upower-configuration-percentage-low
+                              (default 10))
+  (percentage-critical        upower-configuration-percentage-critical
+                              (default 3))
+  (percentage-action          upower-configuration-percentage-action
+                              (default 2))
+  (time-low                   upower-configuration-time-low
+                              (default 1200))
+  (time-critical              upower-configuration-time-critical
+                              (default 300))
+  (time-action                upower-configuration-time-action
+                              (default 120))
+  (critical-power-action      upower-configuration-critical-power-action
+                              (default 'hybrid-sleep)))
 
 (define* upower-configuration-file
   ;; Return an upower-daemon configuration file.
-- 
2.20.1

[bug#34246] [PATCH 2/3] services: Improve the upower-service-type.

[Christopher Baines]

Add a description and default value. Switch the documentation to mention the
service-type and the configuration record, rather than the upower-service
procedure.

* gnu/services/desktop.scm (upower-service-type)[description, default-value]:
Define these fields.
(%desktop-services): Change (upower-service) to (service upower-service-type).
* doc/guix.texi (Desktop Services): Update the upower service documentation.
---
 doc/guix.texi            | 92 ++++++++++++++++++++++++++++++++--------
 gnu/services/desktop.scm | 10 ++++-
 2 files changed, 82 insertions(+), 20 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 972a6a7762..2ff120d5a3 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34,6 +34,7 @@ Copyright @copyright{} 2016, 2017 Nils Gillmann@*
 Copyright @copyright{} 2016, 2017, 2018 Jan Nieuwenhuizen@*
 Copyright @copyright{} 2016 Julien Lepiller@*
 Copyright @copyright{} 2016 Alex ter Weele@*
+Copyright @copyright{} 2016, 2017, 2018, 2019 Christopher Baines@*
 Copyright @copyright{} 2017, 2018 Clément Lassieur@*
 Copyright @copyright{} 2017, 2018 Mathieu Othacehe@*
 Copyright @copyright{} 2017 Federico Beffa@*
@@ -14230,24 +14231,79 @@ capabilities to ordinary users.  For example, an ordinary user can be granted
 the capability to suspend the system if the user is logged in locally.
 @end deffn
 
-@deffn {Scheme Procedure} upower-service [#:upower @var{upower}] @
-                         [#:watts-up-pro? #f] @
-                         [#:poll-batteries? #t] @
-                         [#:ignore-lid? #f] @
-                         [#:use-percentage-for-policy? #f] @
-                         [#:percentage-low 10] @
-                         [#:percentage-critical 3] @
-                         [#:percentage-action 2] @
-                         [#:time-low 1200] @
-                         [#:time-critical 300] @
-                         [#:time-action 120] @
-                         [#:critical-power-action 'hybrid-sleep]
-Return a service that runs @uref{http://upower.freedesktop.org/,
-@command{upowerd}}, a system-wide monitor for power consumption and battery
-levels, with the given configuration settings.  It implements the
-@code{org.freedesktop.UPower} D-Bus interface, and is notably used by
-GNOME.
-@end deffn
+@defvr {Scheme Variable} upower-service-type
+Service that runs @uref{http://upower.freedesktop.org/, @command{upowerd}}, a
+system-wide monitor for power consumption and battery levels, with the given
+configuration settings.
+
+It implements the @code{org.freedesktop.UPower} D-Bus interface, and is
+notably used by GNOME.
+@end defvr
+
+@deftp {Data Type} upower-configuration
+Data type representation the configuration for UPower.
+
+@table @asis
+
+@item @code{upower} (default: @var{upower})
+Package to use for @code{upower}.
+
+@item @code{watts-up-pro?} (default: @code{#f})
+Enable the Watts Up Pro device.
+
+@item @code{poll-batteries?} (default: @code{#t})
+Enable polling the kernel for battery level changes.
+
+@item @code{ignore-lid?} (default: @code{#f})
+Ignore the lid state, this can be useful if it's incorrect on a device.
+
+@item @code{use-percentage-for-policy?} (default: @code{#f})
+Whether battery percentage based policy should be used.  The default is to use
+the time left, change to @code{#t} to use the percentage.
+
+@item @code{percentage-low} (default: @code{10})
+When @code{use-percentage-for-policy?} is @code{#t}, this sets the percentage
+at which the battery is considered low.
+
+@item @code{percentage-critical} (default: @code{3})
+When @code{use-percentage-for-policy?} is @code{#t}, this sets the percentage
+at which the battery is considered critical.
+
+@item @code{percentage-action} (default: @code{2})
+When @code{use-percentage-for-policy?} is @code{#t}, this sets the percentage
+at which action will be taken.
+
+@item @code{time-low} (default: @code{1200})
+When @code{use-time-for-policy?} is @code{#f}, this sets the time remaining in
+seconds at which the battery is considered low.
+
+@item @code{time-critical} (default: @code{300})
+When @code{use-time-for-policy?} is @code{#f}, this sets the time remaining in
+seconds at which the battery is considered critical.
+
+@item @code{time-action} (default: @code{120})
+When @code{use-time-for-policy?} is @code{#f}, this sets the time remaining in
+seconds at which action will be taken.
+
+@item @code{critical-power-action} (default: @code{'hybrid-sleep})
+The action taken when @code{percentage-action} or @code{time-action} is
+reached (depending on the configuration of @code{use-percentage-for-policy?}).
+
+Possible values are:
+
+@itemize @bullet
+@item
+@code{'power-off}
+
+@item
+@code{'hibernate}
+
+@item
+@code{'hybrid-sleep}.
+@end itemize
+
+@end table
+@end deftp
 
 @deffn {Scheme Procedure} udisks-service [#:udisks @var{udisks}]
 Return a service for @uref{http://udisks.freedesktop.org/docs/latest/,
diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index f51ac4d74c..896084d2d5 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -272,6 +272,11 @@ is set to @var{value} when the bus daemon launches it."
 (define upower-service-type
   (let ((upower-package (compose list upower-configuration-upower)))
     (service-type (name 'upower)
+                  (description
+                   "Run @command{upowerd}}, a system-wide monitor for power
+consumption and battery levels, with the given configuration settings.  It
+implements the @code{org.freedesktop.UPower} D-Bus interface, and is notably
+used by GNOME.")
                   (extensions
                    (list (service-extension dbus-root-service-type
                                             upower-dbus-service)
@@ -284,7 +289,8 @@ is set to @var{value} when the bus daemon launches it."
 
                          ;; Make the 'upower' command visible.
                          (service-extension profile-service-type
-                                            upower-package))))))
+                                            upower-package)))
+                  (default-value (upower-configuration)))))
 
 (define* (upower-service #:key (upower upower)
                          (watts-up-pro? #f)
@@ -1013,7 +1019,7 @@ as expected.")))
          (service wpa-supplicant-service-type)    ;needed by NetworkManager
          (service avahi-service-type)
          (udisks-service)
-         (upower-service)
+         (service upower-service-type)
          (accountsservice-service)
          (colord-service)
          (geoclue-service)
-- 
2.20.1

[bug#34246] [PATCH 3/3] services: desktop: Deprecate the upower-service procedure.

[Christopher Baines]

This has now been replaced by the upower-service-type and
<upower-configuration> record.

* gnu/services/desktop.scm (upower-service): Deprecate this procedure.
---
 gnu/services/desktop.scm | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index 896084d2d5..2264a3e8aa 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -50,6 +50,7 @@
   #:use-module (gnu packages libusb)
   #:use-module (gnu packages mate)
   #:use-module (gnu packages enlightenment)
+  #:use-module (guix deprecation)
   #:use-module (guix records)
   #:use-module (guix packages)
   #:use-module (guix store)
@@ -292,18 +293,18 @@ used by GNOME.")
                                             upower-package)))
                   (default-value (upower-configuration)))))
 
-(define* (upower-service #:key (upower upower)
-                         (watts-up-pro? #f)
-                         (poll-batteries? #t)
-                         (ignore-lid? #f)
-                         (use-percentage-for-policy? #f)
-                         (percentage-low 10)
-                         (percentage-critical 3)
-                         (percentage-action 2)
-                         (time-low 1200)
-                         (time-critical 300)
-                         (time-action 120)
-                         (critical-power-action 'hybrid-sleep))
+(define-deprecated (upower-service #:key (upower upower)
+                                   (watts-up-pro? #f)
+                                   (poll-batteries? #t)
+                                   (ignore-lid? #f)
+                                   (use-percentage-for-policy? #f)
+                                   (percentage-low 10)
+                                   (percentage-critical 3)
+                                   (percentage-action 2)
+                                   (time-low 1200)
+                                   (time-critical 300)
+                                   (time-action 120)
+                                   (critical-power-action 'hybrid-sleep))
   "Return a service that runs @uref{http://upower.freedesktop.org/,
 @command{upowerd}}, a system-wide monitor for power consumption and battery
 levels, with the given configuration settings.  It implements the
-- 
2.20.1

[bug#34246] [PATCH 0/3] Improve the upower-service style.

[Ludovic Courtès]

Hello!

Christopher Baines <mail@cbaines.net> skribis:

> These patches update the style of the upower-service. Improving and
> documenting the <upower-configuration> record, and deprecating the
> upower-service procedure.
>
> Directly using record types as default values for service types, along
> with default values for the fields in the record type is generally more
> flexible and configurable than using procedures for service
> configuration. It means that the configuration for the service can be
> changed programatically by generating new configuration based off of the
> original configuration, rather than having to rerun the procedure that
> created the configuration in the first place.
>
>
> Christopher Baines (3):
>   services: Improve the upower-configuration record.
>   services: Improve the upower-service-type.
>   services: desktop: Deprecate the upower-service procedure.

All three patches LGTM.  Thanks for taking the time to do this!  :-)

Ludo’.

bug#34246: [PATCH 0/3] Improve the upower-service style.

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1114 bytes --]


Ludovic Courtès <ludo@gnu.org> writes:

> Hello!
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> These patches update the style of the upower-service. Improving and
>> documenting the <upower-configuration> record, and deprecating the
>> upower-service procedure.
>>
>> Directly using record types as default values for service types, along
>> with default values for the fields in the record type is generally more
>> flexible and configurable than using procedures for service
>> configuration. It means that the configuration for the service can be
>> changed programatically by generating new configuration based off of the
>> original configuration, rather than having to rerun the procedure that
>> created the configuration in the first place.
>>
>>
>> Christopher Baines (3):
>>   services: Improve the upower-configuration record.
>>   services: Improve the upower-service-type.
>>   services: desktop: Deprecate the upower-service procedure.
>
> All three patches LGTM.  Thanks for taking the time to do this!  :-)

Great, I've pushed these patches to master now :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]