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

  • Done
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • Christopher Baines
Owner
unassigned
Submitted by
Christopher Baines
Severity
normal

Debbugs page

Christopher Baines wrote 6 years ago
(address . guix-patches@gnu.org)
877eenqy8f.fsf@cbaines.net
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(-)
-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAlxQcTBfFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF
ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE
9XeXPA/9HHoavcLLnNw7bcKWzjoGI6AxMkNLj+284FgucROMW/V7+uMw+w5aALhi
THzSnnAXl4DWvSnWmm0EIyQ2Gw83weZqLlPu0GTsUIvyHqQb9UMqzVUy/S6YqUtA
mwVZXU3oRgcd+jhSe+EC6qVYjY9GNlFTNYQE7rxZUyUos7QZoxPabhPgc1RCcvTk
jm20Zu6SB5ERHD7mZI1rK+HrayQ3iCIKyses8bAjNekgk91q6Q4HykbGrVWJ2YkQ
3LkE0o9Zc4vaiisIWqcUhoTBL169N9QKZWstou+JovcpTdNZldYn/WTsjxUA607y
PJ+EMpEne5pFwOzxyrOKHTjL0WmQIbE0JeP8JbeICh8V5die5qqKg0sO94zeZKj5
RTVf+wVqqWys5cAzFtJO769Qq5LSWqH6G3Xqcpd3chtEIKbFTnFFZrVjYxfAXXH9
KvWq5y6obQABJNzWK7PlqctuFs1chD802vJRwG5lCgI1g39GmQqvS3FuNOF7dGhW
MVWWREiAl57E4AHKhtwueL1fvVhbspccb7uvr/nbQWP2Sr6a1+4rgYW0uyaJb/8V
KXwYe/0ig4i4eMf3l8vI8g2L8/ztWULewkhqnbwi/KWV1sIvvzED9SuBQlQDFoJZ
jKcdboBWWJ+YywjSCmnEGPAoSTDLImRBylHnewKFJq5xBE4oSH4=
=fOnl
-----END PGP SIGNATURE-----

Christopher Baines wrote 6 years ago
[PATCH 3/3] services: desktop: Deprecate the upower-service procedure.
(address . 34246@debbugs.gnu.org)
20190129153749.10830-3-mail@cbaines.net
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(-)

Toggle diff (45 lines)
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
Christopher Baines wrote 6 years ago
[PATCH 2/3] services: Improve the upower-service-type.
(address . 34246@debbugs.gnu.org)
20190129153749.10830-2-mail@cbaines.net
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(-)

Toggle diff (147 lines)
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
Christopher Baines wrote 6 years ago
[PATCH 1/3] services: Improve the upower-configuration record.
(address . 34246@debbugs.gnu.org)
20190129153749.10830-1-mail@cbaines.net
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(-)

Toggle diff (86 lines)
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
Ludovic Courtès wrote 6 years ago
Re: [bug#34246] [PATCH 0/3] Improve the upower-service style.
(name . Christopher Baines)(address . mail@cbaines.net)(address . 34246@debbugs.gnu.org)
8736onh15u.fsf@gnu.org
Hello!

Christopher Baines <mail@cbaines.net> skribis:

Toggle quote (18 lines)
> 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’.
Christopher Baines wrote 6 years ago
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 34246-done@debbugs.gnu.org)
87tvh34bxs.fsf@cbaines.net
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (24 lines)
> 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 :)
-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAlxoi49fFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF
ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE
9XeDsA//V4gPXv9qfM/mVzni3DgRdWakvJNwqixhn6wpiIVU06AorwTetACh+8Aj
CebA35QXeWrWAWSNdhGFYj3kB68ZONW5mKVlLncO/yGf2WF+6kbnHJOB9kt5pUZr
xqacL+nkjIRDnwkAhGiT1FWh+iAHjjXmQsKsPTcAe90CuqK0P4CcJ2k+/UiVizrd
VEurV/jRNZ8nMlsArdDODAmPLEldi2iLNcHauUSGWTNiMaHFy9ViUmIi8BhTApbk
c5+fZOXFX7HXwJLXjx5xTR0W5zZtE9Mv0Gp4/tn4M5S640tHBF9EarZ3d/DSzNU7
sVMIh4pO3SdlQ24IXJCodkE/Uni+vyOlGiYiZvVvKTPLy+Re0YYZg9CB5+tEoucF
ZErU95nHrAMP7SfQYrLWAwGfU1x++NMC/zsFJqbP0BxaerRYF2ybnVF+5xWSfGzI
F5QHrwSVuqbLBpZ4sqUlBgEPtzW14uI3s955iMr4eLX1lamjmZkL0VxGdhhNiyhR
6IrbaKvhHw8s7LdCLkrG7Af/07WROXz+m8n4Cnnv/m3ShJ7YcQoRNp9OAIHtx6H+
4EBhPkVHzzWqiL1jgFwkuiaAIWlmpAsFZ0ilt93LV+zyu5ep6L/t502gmzgGq5Dc
TxfLgsP/RSOPLuDkjsyFpdwGxuSPHa7a8cl44irZbgqJNwhZL10=
=1zYk
-----END PGP SIGNATURE-----

Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 34246@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 34246
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help