GNU bug report logs - #76289
[PATCH 0/2] Add speakersafetyd system service.

Previous Next

Package: guix-patches;

Reported by: Roman Scherer <roman <at> burningswell.com>

Date: Fri, 14 Feb 2025 13:56:01 UTC

Severity: normal

Tags: patch

Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>

To reply to this bug, email your comments to 76289 AT debbugs.gnu.org.
There is no need to reopen the bug first.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org:
bug#76289; Package guix-patches. (Fri, 14 Feb 2025 13:56:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Roman Scherer <roman <at> burningswell.com>:
New bug report received and forwarded. Copy sent to ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org. (Fri, 14 Feb 2025 13:56:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Roman Scherer <roman <at> burningswell.com>
To: guix-patches <at> gnu.org
Cc: Roman Scherer <roman <at> burningswell.com>, efraim <at> flashner.co.il
Subject: [PATCH 0/2] Add speakersafetyd system service.
Date: Fri, 14 Feb 2025 14:55:26 +0100
Hello Guix,

this patch series adds a system service for speakersafetyd [1], a userspace
daemon written in Rust that implements an analogue of the Texas Instruments
Smart Amp speaker protection model. It is developed and used by the Asahi
Linux project to protect the speakers on Apple Silicon devices.

The patch series contains 2 patches:

- The first one adjusts the SHAREDIR in the speakersafetyd package I submitted
  earlier this year. The shared dir was wrong forcing users to specify it with
  the --config-path option. Now just running speakersafetyd without any
  options does the right thing, instead of complaining it could not find the
  config.

- The second patch is the system service and its documentation.

Could you please review the patch series?

Thanks, Roman.

[1] https://github.com/AsahiLinux/speakersafetyd/

Roman Scherer (2):
  gnu: speakersafetyd: Use correct shared directory.
  services: Add speakersafetyd service.

 doc/guix.texi              | 41 +++++++++++++++++++++++++
 gnu/packages/rust-apps.scm |  2 +-
 gnu/services/sound.scm     | 61 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 102 insertions(+), 2 deletions(-)


base-commit: 4b5f0408e66392ab745dc0f7830732217d88f17d
--
2.48.1




Information forwarded to divya <at> subvertising.org, efraim <at> flashner.co.il, guix-patches <at> gnu.org:
bug#76289; Package guix-patches. (Fri, 14 Feb 2025 13:59:02 GMT) Full text and rfc822 format available.

Message #8 received at 76289 <at> debbugs.gnu.org (full text, mbox):

From: Roman Scherer <roman <at> burningswell.com>
To: 76289 <at> debbugs.gnu.org
Cc: Roman Scherer <roman <at> burningswell.com>, efraim <at> flashner.co.il
Subject: [PATCH 1/2] gnu: speakersafetyd: Use correct shared directory.
Date: Fri, 14 Feb 2025 14:58:09 +0100
* gnu/packages/rust-apps.scm (speakersafetyd): Adjust shared directory.

Change-Id: If52576364f54394a2930d2d8750446acba389f6c
---
 gnu/packages/rust-apps.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/packages/rust-apps.scm b/gnu/packages/rust-apps.scm
index f8da401bf7..90ec1ece2e 100644
--- a/gnu/packages/rust-apps.scm
+++ b/gnu/packages/rust-apps.scm
@@ -3203,7 +3203,7 @@ (define-public speakersafetyd
               (setenv "UNITDIR" (string-append #$output "/lib/systemd/system"))
               (setenv "UDEVDIR" (string-append #$output "/lib/udev/rules.d"))
               (setenv "TMPFILESDIR" (string-append #$output "/usr/lib/tmpfiles.d"))
-              (setenv "SHAREDIR" (string-append #$output "/usr/share"))
+              (setenv "SHAREDIR" (string-append #$output "/share"))
               (setenv "VARDIR" (string-append #$output "/var"))
               (invoke "make" "install-data"))))))
     (inputs (list alsa-lib))
-- 
2.48.1





Information forwarded to ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org:
bug#76289; Package guix-patches. (Fri, 14 Feb 2025 13:59:02 GMT) Full text and rfc822 format available.

Message #11 received at 76289 <at> debbugs.gnu.org (full text, mbox):

From: Roman Scherer <roman <at> burningswell.com>
To: 76289 <at> debbugs.gnu.org
Cc: Roman Scherer <roman <at> burningswell.com>, efraim <at> flashner.co.il
Subject: [PATCH 2/2] services: Add speakersafetyd service.
Date: Fri, 14 Feb 2025 14:58:10 +0100
* gnu/services/sound.scm (speakersafetyd-service-type) New variable.
* doc/guix.texi: Document the speakersafetyd service.

Change-Id: Ib8fa19b056a2036019ae7c199d81e1139664e951
---
 doc/guix.texi          | 41 ++++++++++++++++++++++++++++
 gnu/services/sound.scm | 61 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index bd66adf326..3b82df5196 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -26575,6 +26575,47 @@ Sound Services
 
 @end defvar
 
+@subsubheading Speaker Safety Daemon System Service
+
+@uref{https://github.com/AsahiLinux/speakersafetyd, Speaker Safety
+Daemon} is a userspace daemon that implements an analogue of the Texas
+Instruments Smart Amp speaker protection model.  It can be used to
+protect the speakers on Apple Silicon devices.
+
+@defvar speakersafetyd-service-type
+This is the type for the @code{speakersafetyd} system service, whose
+value is a @command{speakersafetyd-configuration} record.
+
+@lisp
+(service speakersafetyd-service-type)
+@end lisp
+
+See below for details about @code{speakersafetyd-configuration}.
+@end defvar
+
+@deftp {Data Type} speakersafetyd-configuration
+Data type representing the configuration for @code{speakersafetyd-service}.
+
+@table @asis
+@item @code{blackbox-path} (default: @code{"/var/lib/speakersafetyd/blackbox"})
+The path to a directory to which "blackbox" files are written when the
+speakers are getting too hot.  The blackbox files contain audio and
+debug information which the developers of @code{speakersafetyd} might
+ask for when reporting bugs.
+
+@item @code{config-path} (default: @code{(file-append speakersafetyd "/share/speakersafetyd")})
+Path to the base directory as a G-expression (@pxref{G-Expressions})
+that contains the configuration files of the speaker models.
+
+@item @code{max-reduction} (default: @code{7})
+Maximum gain reduction before panicing, useful for debugging.
+
+@item @code{package} (default: @var{speakersafetyd})
+The Speaker Safety Daemon package to use.
+
+@end table
+@end deftp
+
 @node File Search Services
 @subsection File Search Services
 
diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
index 8ca7acd737..fb8a8d3d17 100644
--- a/gnu/services/sound.scm
+++ b/gnu/services/sound.scm
@@ -35,6 +35,7 @@ (define-module (gnu services sound)
   #:use-module (gnu packages audio)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages pulseaudio)
+  #:use-module (gnu packages rust-apps)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
   #:export (alsa-configuration
@@ -56,7 +57,15 @@ (define-module (gnu services sound)
             ladspa-configuration
             ladspa-configuration?
             ladspa-configuration-plugins
-            ladspa-service-type))
+            ladspa-service-type
+
+            speakersafetyd-configuration
+            speakersafetyd-configuration-blackbox-path
+            speakersafetyd-configuration-config-path
+            speakersafetyd-configuration-max-reduction
+            speakersafetyd-configuration-package
+            speakersafetyd-configuration?
+            speakersafetyd-service-type))
 
 ;;; Commentary:
 ;;;
@@ -263,4 +272,54 @@ (define ladspa-service-type
    (default-value (ladspa-configuration))
    (description "Configure LADSPA plugins.")))
 
+
+;;;
+;;; Speaker Safety Daemon
+;;;
+
+(define-record-type* <speakersafetyd-configuration>
+  speakersafetyd-configuration
+  make-speakersafetyd-configuration
+  speakersafetyd-configuration?
+  (blackbox-path speakersafetyd-configuration-blackbox-path
+                 (default "/var/lib/speakersafetyd/blackbox"))
+  (config-path speakersafetyd-configuration-config-path
+               (default (file-append speakersafetyd "/share/speakersafetyd")))
+  (max-reduction speakersafetyd-configuration-max-reduction
+                 (default 7))
+  (package speakersafetyd-configuration-package
+           (default speakersafetyd)))
+
+(define (speakersafetyd-shepherd-service config)
+  (let ((blackbox-path (speakersafetyd-configuration-blackbox-path config))
+        (config-path (speakersafetyd-configuration-config-path config))
+        (max-reduction (speakersafetyd-configuration-max-reduction config))
+        (package (speakersafetyd-configuration-package config)))
+    (shepherd-service
+     (documentation "Speaker saftey daemon")
+     (provision '(speakersafetyd))
+     (requirement '(udev))
+     (start #~(make-forkexec-constructor
+               (list #$(file-append package "/bin/speakersafetyd")
+                     "--config-path" #$config-path
+                     "--blackbox-path" #$blackbox-path
+                     "--max-reduction" (number->string #$max-reduction))))
+     (stop #~(make-kill-destructor)))))
+
+(define speakersafetyd-service-type
+  (service-type
+   (name 'speakersafetyd)
+   (description "Speaker Saftey Daemon")
+   (extensions
+    (list (service-extension
+           profile-service-type
+           (compose list speakersafetyd-configuration-package))
+          (service-extension
+           shepherd-root-service-type
+           (compose list speakersafetyd-shepherd-service))
+          (service-extension
+           udev-service-type
+           (compose list speakersafetyd-configuration-package))))
+   (default-value (speakersafetyd-configuration))))
+
 ;;; sound.scm ends here
-- 
2.48.1





Information forwarded to guix-patches <at> gnu.org:
bug#76289; Package guix-patches. (Sat, 15 Feb 2025 01:19:01 GMT) Full text and rfc822 format available.

Message #14 received at 76289 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Roman Scherer <roman <at> burningswell.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 76289 <at> debbugs.gnu.org, efraim <at> flashner.co.il
Subject: Re: [bug#76289] [PATCH 2/2] services: Add speakersafetyd service.
Date: Sat, 15 Feb 2025 10:18:10 +0900
Hi Roman,

Roman Scherer <roman <at> burningswell.com> writes:

> * gnu/services/sound.scm (speakersafetyd-service-type) New variable.
> * doc/guix.texi: Document the speakersafetyd service.

Interesting!

> Change-Id: Ib8fa19b056a2036019ae7c199d81e1139664e951
> ---
>  doc/guix.texi          | 41 ++++++++++++++++++++++++++++
>  gnu/services/sound.scm | 61 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index bd66adf326..3b82df5196 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -26575,6 +26575,47 @@ Sound Services
>
>  @end defvar
>
> +@subsubheading Speaker Safety Daemon System Service
> +
> +@uref{https://github.com/AsahiLinux/speakersafetyd, Speaker Safety
> +Daemon} is a userspace daemon that implements an analogue of the Texas
> +Instruments Smart Amp speaker protection model.  It can be used to
> +protect the speakers on Apple Silicon devices.
> +
> +@defvar speakersafetyd-service-type
> +This is the type for the @code{speakersafetyd} system service, whose
> +value is a @command{speakersafetyd-configuration} record.
> +
> +@lisp
> +(service speakersafetyd-service-type)
> +@end lisp
> +
> +See below for details about @code{speakersafetyd-configuration}.
> +@end defvar
> +
> +@deftp {Data Type} speakersafetyd-configuration
> +Data type representing the configuration for @code{speakersafetyd-service}.
> +
> +@table @asis
> +@item @code{blackbox-path} (default: @code{"/var/lib/speakersafetyd/blackbox"})
> +The path to a directory to which "blackbox" files are written when the
> +speakers are getting too hot.  The blackbox files contain audio and
> +debug information which the developers of @code{speakersafetyd} might
> +ask for when reporting bugs.
> +
> +@item @code{config-path} (default: @code{(file-append speakersafetyd "/share/speakersafetyd")})
> +Path to the base directory as a G-expression (@pxref{G-Expressions})
> +that contains the configuration files of the speaker models.
> +
> +@item @code{max-reduction} (default: @code{7})
> +Maximum gain reduction before panicing, useful for debugging.
> +
> +@item @code{package} (default: @var{speakersafetyd})
> +The Speaker Safety Daemon package to use.
> +
> +@end table
> +@end deftp
> +
>  @node File Search Services
>  @subsection File Search Services
>
> diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
> index 8ca7acd737..fb8a8d3d17 100644
> --- a/gnu/services/sound.scm
> +++ b/gnu/services/sound.scm
> @@ -35,6 +35,7 @@ (define-module (gnu services sound)
>    #:use-module (gnu packages audio)
>    #:use-module (gnu packages linux)
>    #:use-module (gnu packages pulseaudio)
> +  #:use-module (gnu packages rust-apps)
>    #:use-module (ice-9 match)
>    #:use-module (srfi srfi-1)
>    #:export (alsa-configuration
> @@ -56,7 +57,15 @@ (define-module (gnu services sound)
>              ladspa-configuration
>              ladspa-configuration?
>              ladspa-configuration-plugins
> -            ladspa-service-type))
> +            ladspa-service-type
> +
> +            speakersafetyd-configuration
> +            speakersafetyd-configuration-blackbox-path
> +            speakersafetyd-configuration-config-path
> +            speakersafetyd-configuration-max-reduction
> +            speakersafetyd-configuration-package
> +            speakersafetyd-configuration?
> +            speakersafetyd-service-type))
>
>  ;;; Commentary:
>  ;;;
> @@ -263,4 +272,54 @@ (define ladspa-service-type
>     (default-value (ladspa-configuration))
>     (description "Configure LADSPA plugins.")))
>
> +
> +;;;
> +;;; Speaker Safety Daemon
> +;;;
> +
> +(define-record-type* <speakersafetyd-configuration>
> +  speakersafetyd-configuration
> +  make-speakersafetyd-configuration
> +  speakersafetyd-configuration?
> +  (blackbox-path speakersafetyd-configuration-blackbox-path
> +                 (default "/var/lib/speakersafetyd/blackbox"))

Since these values are not serialized, we are free to name them the way
we like; so they should follow the GNU and Guix conventions, namely:

I'd use blackbox-directory.

> +  (config-path speakersafetyd-configuration-config-path
> +               (default (file-append speakersafetyd
> "/share/speakersafetyd")))

I'd use configuration-directory.

> +  (max-reduction speakersafetyd-configuration-max-reduction
> +                 (default 7))

I'd use maximum-gain-reduction

> +  (package speakersafetyd-configuration-package
> +           (default speakersafetyd)))

I'd use the more conventional
speakersafetyd-configuration-speakersafetyd (using the name of the
package as the field name).

Our related conventions here are roughly:

1. Prefer full name instead of abbreviation, especially in public APIs
2. Path always used to denote a multi-entries search path like 'PATH'
and friends.  Use 'file name', 'file' or 'directory' instead.

It's also more conventional to use the package name for the package
object field, so here I'd use
'speakersafetyd-configuration-speakersafetyd'.

Could you please also use 'define-configuration/no-serialization' from
(gnu services configuration) ?  It validates the type of each field and
produces useful user error messages in case these are incorrect.

> +(define (speakersafetyd-shepherd-service config)
> +  (let ((blackbox-path (speakersafetyd-configuration-blackbox-path config))
> +        (config-path (speakersafetyd-configuration-config-path config))
> +        (max-reduction (speakersafetyd-configuration-max-reduction config))
> +        (package (speakersafetyd-configuration-package config)))

nitpick: I'd unbox each value using match-record; which will make things a bit
more concise.

> +    (shepherd-service
> +     (documentation "Speaker saftey daemon")

s/saftey/safety/

> +     (provision '(speakersafetyd))
> +     (requirement '(udev))
> +     (start #~(make-forkexec-constructor
> +               (list #$(file-append package "/bin/speakersafetyd")
> +                     "--config-path" #$config-path
> +                     "--blackbox-path" #$blackbox-path
> +                     "--max-reduction" (number->string #$max-reduction))))
> +     (stop #~(make-kill-destructor)))))
> +
> +(define speakersafetyd-service-type
> +  (service-type
> +   (name 'speakersafetyd)
> +   (description "Speaker Saftey Daemon")
>
s/Saftey/Safety/ The project has a better description, which can be
adapted to something like "@command{speakersafetyd} is a user space
daemon that implements an analogue of the Texas Instruments Smart Amp
speaker protection model."

> +   (extensions
> +    (list (service-extension
> +           profile-service-type
> +           (compose list speakersafetyd-configuration-package))
> +          (service-extension
> +           shepherd-root-service-type
> +           (compose list speakersafetyd-shepherd-service))
> +          (service-extension
> +           udev-service-type
> +           (compose list speakersafetyd-configuration-package))))
> +   (default-value (speakersafetyd-configuration))))

Nitpick, but I'd order these from most critical to less critical
ordering, such that te root service type is extended first, then the
udev-service-type, then the profile.

The rest LGTM.  Could you please send a v2?

-- 
Thanks,
Maxim




Information forwarded to divya <at> subvertising.org, efraim <at> flashner.co.il, guix-patches <at> gnu.org:
bug#76289; Package guix-patches. (Sat, 15 Feb 2025 09:03:02 GMT) Full text and rfc822 format available.

Message #17 received at 76289 <at> debbugs.gnu.org (full text, mbox):

From: Roman Scherer <roman <at> burningswell.com>
To: 76289 <at> debbugs.gnu.org
Cc: Roman Scherer <roman <at> burningswell.com>
Subject: [PATCH v2 1/2] gnu: speakersafetyd: Use correct shared directory.
Date: Sat, 15 Feb 2025 10:01:53 +0100
* gnu/packages/rust-apps.scm (speakersafetyd): Adjust shared directory.

Change-Id: If52576364f54394a2930d2d8750446acba389f6c
---
 gnu/packages/rust-apps.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/packages/rust-apps.scm b/gnu/packages/rust-apps.scm
index f8da401bf7..90ec1ece2e 100644
--- a/gnu/packages/rust-apps.scm
+++ b/gnu/packages/rust-apps.scm
@@ -3203,7 +3203,7 @@ (define-public speakersafetyd
               (setenv "UNITDIR" (string-append #$output "/lib/systemd/system"))
               (setenv "UDEVDIR" (string-append #$output "/lib/udev/rules.d"))
               (setenv "TMPFILESDIR" (string-append #$output "/usr/lib/tmpfiles.d"))
-              (setenv "SHAREDIR" (string-append #$output "/usr/share"))
+              (setenv "SHAREDIR" (string-append #$output "/share"))
               (setenv "VARDIR" (string-append #$output "/var"))
               (invoke "make" "install-data"))))))
     (inputs (list alsa-lib))

base-commit: 4b5f0408e66392ab745dc0f7830732217d88f17d
-- 
2.48.1





Information forwarded to ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org:
bug#76289; Package guix-patches. (Sat, 15 Feb 2025 09:03:03 GMT) Full text and rfc822 format available.

Message #20 received at 76289 <at> debbugs.gnu.org (full text, mbox):

From: Roman Scherer <roman <at> burningswell.com>
To: 76289 <at> debbugs.gnu.org
Cc: Roman Scherer <roman <at> burningswell.com>
Subject: [PATCH v2 2/2] services: Add speakersafetyd service.
Date: Sat, 15 Feb 2025 10:01:54 +0100
* gnu/services/sound.scm (speakersafetyd-service-type) New variable.
* doc/guix.texi: Document the speakersafetyd service.

Change-Id: Ib8fa19b056a2036019ae7c199d81e1139664e951
---
 doc/guix.texi          | 42 +++++++++++++++++++++++++++
 gnu/services/sound.scm | 65 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index bd66adf326..ec5c7ab5cf 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -26575,6 +26575,48 @@ Sound Services
 
 @end defvar
 
+@subsubheading Speaker Safety Daemon System Service
+
+@uref{https://github.com/AsahiLinux/speakersafetyd, Speaker Safety
+Daemon} is a user-space daemon that implements an analogue of the Texas
+Instruments Smart Amp speaker protection model.  It can be used to
+protect the speakers on Apple Silicon devices.
+
+@defvar speakersafetyd-service-type
+This is the type for the @code{speakersafetyd} system service, whose
+value is a @command{speakersafetyd-configuration} record.
+
+@lisp
+(service speakersafetyd-service-type)
+@end lisp
+
+See below for details about @code{speakersafetyd-configuration}.
+@end defvar
+
+@deftp {Data Type} speakersafetyd-configuration
+Available @code{speakersafetyd-configuration} fields are:
+
+@table @asis
+@item @code{blackbox-directory} (default: @code{"/var/lib/speakersafetyd/blackbox"}) (type: string)
+The directory to which blackbox files are written when the speakers are
+getting too hot.  The blackbox files contain audio and debug information
+which the developers of @code{speakersafetyd} might ask for when
+reporting bugs.
+
+@item @code{configuration-directory} (type: file-like)
+The base directory as a G-expression (@pxref{G-Expressions}) that
+contains the configuration files of the speaker models.
+
+@item @code{maximum-gain-reduction} (default: @code{7}) (type: integer)
+Maximum gain reduction before panicing, useful for debugging.
+
+@item @code{speakersafetyd} (default: @code{speakersafetyd}) (type: file-like)
+The Speaker Safety Daemon package to use.
+
+@end table
+
+@end deftp
+
 @node File Search Services
 @subsection File Search Services
 
diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
index 8ca7acd737..9418969e9e 100644
--- a/gnu/services/sound.scm
+++ b/gnu/services/sound.scm
@@ -35,6 +35,7 @@ (define-module (gnu services sound)
   #:use-module (gnu packages audio)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages pulseaudio)
+  #:use-module (gnu packages rust-apps)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
   #:export (alsa-configuration
@@ -56,7 +57,15 @@ (define-module (gnu services sound)
             ladspa-configuration
             ladspa-configuration?
             ladspa-configuration-plugins
-            ladspa-service-type))
+            ladspa-service-type
+
+            speakersafetyd-configuration
+            speakersafetyd-configuration-blackbox-directory
+            speakersafetyd-configuration-directory
+            speakersafetyd-configuration-maximum-gain-reduction
+            speakersafetyd-configuration-speakersafetyd
+            speakersafetyd-configuration?
+            speakersafetyd-service-type))
 
 ;;; Commentary:
 ;;;
@@ -263,4 +272,58 @@ (define ladspa-service-type
    (default-value (ladspa-configuration))
    (description "Configure LADSPA plugins.")))
 
+
+;;;
+;;; Speaker Safety Daemon
+;;;
+
+(define-configuration/no-serialization speakersafetyd-configuration
+  (blackbox-directory
+   (string "/var/lib/speakersafetyd/blackbox")
+   "The directory to which blackbox files are written when the speakers are
+getting too hot.  The blackbox files contain audio and debug information which
+the developers of @code{speakersafetyd} might ask for when reporting bugs.")
+  (configuration-directory
+   (file-like (file-append speakersafetyd "/share/speakersafetyd"))
+   "The base directory as a G-expression (@pxref{G-Expressions}) that contains
+the configuration files of the speaker models.")
+  (maximum-gain-reduction
+   (integer 7)
+   "Maximum gain reduction before panicing, useful for debugging.")
+  (speakersafetyd
+   (file-like speakersafetyd)
+   "The Speaker Safety Daemon package to use."))
+
+(define speakersafetyd-shepherd-service
+  (match-record-lambda <speakersafetyd-configuration>
+      (blackbox-directory configuration-directory maximum-gain-reduction speakersafetyd)
+    (shepherd-service
+     (documentation "Run the speaker safety daemon")
+     (provision '(speakersafetyd))
+     (requirement '(udev))
+     (start #~(make-forkexec-constructor
+               (list #$(file-append speakersafetyd "/bin/speakersafetyd")
+                     "--config-path" #$configuration-directory
+                     "--blackbox-path" #$blackbox-directory
+                     "--max-reduction" (number->string #$maximum-gain-reduction))))
+     (stop #~(make-kill-destructor)))))
+
+(define speakersafetyd-service-type
+  (service-type
+   (name 'speakersafetyd)
+   (description "Run @command{speakersafetyd}, a user space daemon that
+implements an analogue of the Texas Instruments Smart Amp speaker protection
+model.  It can be used to protect the speakers on Apple Silicon devices.")
+   (extensions
+    (list (service-extension
+           shepherd-root-service-type
+           (compose list speakersafetyd-shepherd-service))
+          (service-extension
+           udev-service-type
+           (compose list speakersafetyd-configuration-speakersafetyd))
+          (service-extension
+           profile-service-type
+           (compose list speakersafetyd-configuration-speakersafetyd))))
+   (default-value (speakersafetyd-configuration))))
+
 ;;; sound.scm ends here
-- 
2.48.1





Information forwarded to guix-patches <at> gnu.org:
bug#76289; Package guix-patches. (Sat, 15 Feb 2025 09:16:02 GMT) Full text and rfc822 format available.

Message #23 received at 76289 <at> debbugs.gnu.org (full text, mbox):

From: Roman Scherer <roman <at> burningswell.com>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Roman Scherer <roman <at> burningswell.com>,
 Ludovic Courtès <ludo <at> gnu.org>, 76289 <at> debbugs.gnu.org,
 efraim <at> flashner.co.il
Subject: Re: [bug#76289] [PATCH 2/2] services: Add speakersafetyd service.
Date: Sat, 15 Feb 2025 10:15:21 +0100
[Message part 1 (text/plain, inline)]
Hi Maxim,

thanks for your review. I hope I addressed your comments. Additionally:

- I used the configuration->documentation command to generate
  documentation from the configuration record

- I discovered match-record-lambda and use this instead of match-record

Could you have another look please?

Thank you, Roman.

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

> Hi Roman,
>
> Roman Scherer <roman <at> burningswell.com> writes:
>
>> * gnu/services/sound.scm (speakersafetyd-service-type) New variable.
>> * doc/guix.texi: Document the speakersafetyd service.
>
> Interesting!
>
>> Change-Id: Ib8fa19b056a2036019ae7c199d81e1139664e951
>> ---
>>  doc/guix.texi          | 41 ++++++++++++++++++++++++++++
>>  gnu/services/sound.scm | 61 +++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 101 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/guix.texi b/doc/guix.texi
>> index bd66adf326..3b82df5196 100644
>> --- a/doc/guix.texi
>> +++ b/doc/guix.texi
>> @@ -26575,6 +26575,47 @@ Sound Services
>>
>>  @end defvar
>>
>> +@subsubheading Speaker Safety Daemon System Service
>> +
>> +@uref{https://github.com/AsahiLinux/speakersafetyd, Speaker Safety
>> +Daemon} is a userspace daemon that implements an analogue of the Texas
>> +Instruments Smart Amp speaker protection model.  It can be used to
>> +protect the speakers on Apple Silicon devices.
>> +
>> +@defvar speakersafetyd-service-type
>> +This is the type for the @code{speakersafetyd} system service, whose
>> +value is a @command{speakersafetyd-configuration} record.
>> +
>> +@lisp
>> +(service speakersafetyd-service-type)
>> +@end lisp
>> +
>> +See below for details about @code{speakersafetyd-configuration}.
>> +@end defvar
>> +
>> +@deftp {Data Type} speakersafetyd-configuration
>> +Data type representing the configuration for @code{speakersafetyd-service}.
>> +
>> +@table @asis
>> +@item @code{blackbox-path} (default: @code{"/var/lib/speakersafetyd/blackbox"})
>> +The path to a directory to which "blackbox" files are written when the
>> +speakers are getting too hot.  The blackbox files contain audio and
>> +debug information which the developers of @code{speakersafetyd} might
>> +ask for when reporting bugs.
>> +
>> +@item @code{config-path} (default: @code{(file-append speakersafetyd "/share/speakersafetyd")})
>> +Path to the base directory as a G-expression (@pxref{G-Expressions})
>> +that contains the configuration files of the speaker models.
>> +
>> +@item @code{max-reduction} (default: @code{7})
>> +Maximum gain reduction before panicing, useful for debugging.
>> +
>> +@item @code{package} (default: @var{speakersafetyd})
>> +The Speaker Safety Daemon package to use.
>> +
>> +@end table
>> +@end deftp
>> +
>>  @node File Search Services
>>  @subsection File Search Services
>>
>> diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
>> index 8ca7acd737..fb8a8d3d17 100644
>> --- a/gnu/services/sound.scm
>> +++ b/gnu/services/sound.scm
>> @@ -35,6 +35,7 @@ (define-module (gnu services sound)
>>    #:use-module (gnu packages audio)
>>    #:use-module (gnu packages linux)
>>    #:use-module (gnu packages pulseaudio)
>> +  #:use-module (gnu packages rust-apps)
>>    #:use-module (ice-9 match)
>>    #:use-module (srfi srfi-1)
>>    #:export (alsa-configuration
>> @@ -56,7 +57,15 @@ (define-module (gnu services sound)
>>              ladspa-configuration
>>              ladspa-configuration?
>>              ladspa-configuration-plugins
>> -            ladspa-service-type))
>> +            ladspa-service-type
>> +
>> +            speakersafetyd-configuration
>> +            speakersafetyd-configuration-blackbox-path
>> +            speakersafetyd-configuration-config-path
>> +            speakersafetyd-configuration-max-reduction
>> +            speakersafetyd-configuration-package
>> +            speakersafetyd-configuration?
>> +            speakersafetyd-service-type))
>>
>>  ;;; Commentary:
>>  ;;;
>> @@ -263,4 +272,54 @@ (define ladspa-service-type
>>     (default-value (ladspa-configuration))
>>     (description "Configure LADSPA plugins.")))
>>
>> +
>> +;;;
>> +;;; Speaker Safety Daemon
>> +;;;
>> +
>> +(define-record-type* <speakersafetyd-configuration>
>> +  speakersafetyd-configuration
>> +  make-speakersafetyd-configuration
>> +  speakersafetyd-configuration?
>> +  (blackbox-path speakersafetyd-configuration-blackbox-path
>> +                 (default "/var/lib/speakersafetyd/blackbox"))
>
> Since these values are not serialized, we are free to name them the way
> we like; so they should follow the GNU and Guix conventions, namely:
>
> I'd use blackbox-directory.
>
>> +  (config-path speakersafetyd-configuration-config-path
>> +               (default (file-append speakersafetyd
>> "/share/speakersafetyd")))
>
> I'd use configuration-directory.
>
>> +  (max-reduction speakersafetyd-configuration-max-reduction
>> +                 (default 7))
>
> I'd use maximum-gain-reduction
>
>> +  (package speakersafetyd-configuration-package
>> +           (default speakersafetyd)))
>
> I'd use the more conventional
> speakersafetyd-configuration-speakersafetyd (using the name of the
> package as the field name).
>
> Our related conventions here are roughly:
>
> 1. Prefer full name instead of abbreviation, especially in public APIs
> 2. Path always used to denote a multi-entries search path like 'PATH'
> and friends.  Use 'file name', 'file' or 'directory' instead.
>
> It's also more conventional to use the package name for the package
> object field, so here I'd use
> 'speakersafetyd-configuration-speakersafetyd'.
>
> Could you please also use 'define-configuration/no-serialization' from
> (gnu services configuration) ?  It validates the type of each field and
> produces useful user error messages in case these are incorrect.
>
>> +(define (speakersafetyd-shepherd-service config)
>> +  (let ((blackbox-path (speakersafetyd-configuration-blackbox-path config))
>> +        (config-path (speakersafetyd-configuration-config-path config))
>> +        (max-reduction (speakersafetyd-configuration-max-reduction config))
>> +        (package (speakersafetyd-configuration-package config)))
>
> nitpick: I'd unbox each value using match-record; which will make things a bit
> more concise.
>
>> +    (shepherd-service
>> +     (documentation "Speaker saftey daemon")
>
> s/saftey/safety/
>
>> +     (provision '(speakersafetyd))
>> +     (requirement '(udev))
>> +     (start #~(make-forkexec-constructor
>> +               (list #$(file-append package "/bin/speakersafetyd")
>> +                     "--config-path" #$config-path
>> +                     "--blackbox-path" #$blackbox-path
>> +                     "--max-reduction" (number->string #$max-reduction))))
>> +     (stop #~(make-kill-destructor)))))
>> +
>> +(define speakersafetyd-service-type
>> +  (service-type
>> +   (name 'speakersafetyd)
>> +   (description "Speaker Saftey Daemon")
>>
> s/Saftey/Safety/ The project has a better description, which can be
> adapted to something like "@command{speakersafetyd} is a user space
> daemon that implements an analogue of the Texas Instruments Smart Amp
> speaker protection model."
>
>> +   (extensions
>> +    (list (service-extension
>> +           profile-service-type
>> +           (compose list speakersafetyd-configuration-package))
>> +          (service-extension
>> +           shepherd-root-service-type
>> +           (compose list speakersafetyd-shepherd-service))
>> +          (service-extension
>> +           udev-service-type
>> +           (compose list speakersafetyd-configuration-package))))
>> +   (default-value (speakersafetyd-configuration))))
>
> Nitpick, but I'd order these from most critical to less critical
> ordering, such that te root service type is extended first, then the
> udev-service-type, then the profile.
>
> The rest LGTM.  Could you please send a v2?
[signature.asc (application/pgp-signature, inline)]

Information forwarded to divya <at> subvertising.org, efraim <at> flashner.co.il, guix-patches <at> gnu.org:
bug#76289; Package guix-patches. (Sat, 15 Feb 2025 09:25:02 GMT) Full text and rfc822 format available.

Message #26 received at 76289 <at> debbugs.gnu.org (full text, mbox):

From: Roman Scherer <roman <at> burningswell.com>
To: 76289 <at> debbugs.gnu.org
Cc: Roman Scherer <roman <at> burningswell.com>
Subject: [PATCH v3 1/2] gnu: speakersafetyd: Use correct shared directory.
Date: Sat, 15 Feb 2025 10:24:34 +0100
* gnu/packages/rust-apps.scm (speakersafetyd): Adjust shared directory.

Change-Id: If52576364f54394a2930d2d8750446acba389f6c
---
 gnu/packages/rust-apps.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/packages/rust-apps.scm b/gnu/packages/rust-apps.scm
index f8da401bf7..90ec1ece2e 100644
--- a/gnu/packages/rust-apps.scm
+++ b/gnu/packages/rust-apps.scm
@@ -3203,7 +3203,7 @@ (define-public speakersafetyd
               (setenv "UNITDIR" (string-append #$output "/lib/systemd/system"))
               (setenv "UDEVDIR" (string-append #$output "/lib/udev/rules.d"))
               (setenv "TMPFILESDIR" (string-append #$output "/usr/lib/tmpfiles.d"))
-              (setenv "SHAREDIR" (string-append #$output "/usr/share"))
+              (setenv "SHAREDIR" (string-append #$output "/share"))
               (setenv "VARDIR" (string-append #$output "/var"))
               (invoke "make" "install-data"))))))
     (inputs (list alsa-lib))

base-commit: 4b5f0408e66392ab745dc0f7830732217d88f17d
-- 
2.48.1





Information forwarded to ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org:
bug#76289; Package guix-patches. (Sat, 15 Feb 2025 09:25:02 GMT) Full text and rfc822 format available.

Message #29 received at 76289 <at> debbugs.gnu.org (full text, mbox):

From: Roman Scherer <roman <at> burningswell.com>
To: 76289 <at> debbugs.gnu.org
Cc: Roman Scherer <roman <at> burningswell.com>
Subject: [PATCH v3 2/2] services: Add speakersafetyd service.
Date: Sat, 15 Feb 2025 10:24:35 +0100
* gnu/services/sound.scm (speakersafetyd-service-type) New variable.
* doc/guix.texi: Document the speakersafetyd service.

Change-Id: Ib8fa19b056a2036019ae7c199d81e1139664e951
---
 doc/guix.texi          | 42 +++++++++++++++++++++++++++
 gnu/services/sound.scm | 65 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index bd66adf326..ec5c7ab5cf 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -26575,6 +26575,48 @@ Sound Services
 
 @end defvar
 
+@subsubheading Speaker Safety Daemon System Service
+
+@uref{https://github.com/AsahiLinux/speakersafetyd, Speaker Safety
+Daemon} is a user-space daemon that implements an analogue of the Texas
+Instruments Smart Amp speaker protection model.  It can be used to
+protect the speakers on Apple Silicon devices.
+
+@defvar speakersafetyd-service-type
+This is the type for the @code{speakersafetyd} system service, whose
+value is a @command{speakersafetyd-configuration} record.
+
+@lisp
+(service speakersafetyd-service-type)
+@end lisp
+
+See below for details about @code{speakersafetyd-configuration}.
+@end defvar
+
+@deftp {Data Type} speakersafetyd-configuration
+Available @code{speakersafetyd-configuration} fields are:
+
+@table @asis
+@item @code{blackbox-directory} (default: @code{"/var/lib/speakersafetyd/blackbox"}) (type: string)
+The directory to which blackbox files are written when the speakers are
+getting too hot.  The blackbox files contain audio and debug information
+which the developers of @code{speakersafetyd} might ask for when
+reporting bugs.
+
+@item @code{configuration-directory} (type: file-like)
+The base directory as a G-expression (@pxref{G-Expressions}) that
+contains the configuration files of the speaker models.
+
+@item @code{maximum-gain-reduction} (default: @code{7}) (type: integer)
+Maximum gain reduction before panicing, useful for debugging.
+
+@item @code{speakersafetyd} (default: @code{speakersafetyd}) (type: file-like)
+The Speaker Safety Daemon package to use.
+
+@end table
+
+@end deftp
+
 @node File Search Services
 @subsection File Search Services
 
diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
index 8ca7acd737..bf7d3cc6b0 100644
--- a/gnu/services/sound.scm
+++ b/gnu/services/sound.scm
@@ -35,6 +35,7 @@ (define-module (gnu services sound)
   #:use-module (gnu packages audio)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages pulseaudio)
+  #:use-module (gnu packages rust-apps)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
   #:export (alsa-configuration
@@ -56,7 +57,15 @@ (define-module (gnu services sound)
             ladspa-configuration
             ladspa-configuration?
             ladspa-configuration-plugins
-            ladspa-service-type))
+            ladspa-service-type
+
+            speakersafetyd-configuration
+            speakersafetyd-configuration-blackbox-directory
+            speakersafetyd-configuration-configuration-directory
+            speakersafetyd-configuration-maximum-gain-reduction
+            speakersafetyd-configuration-speakersafetyd
+            speakersafetyd-configuration?
+            speakersafetyd-service-type))
 
 ;;; Commentary:
 ;;;
@@ -263,4 +272,58 @@ (define ladspa-service-type
    (default-value (ladspa-configuration))
    (description "Configure LADSPA plugins.")))
 
+
+;;;
+;;; Speaker Safety Daemon
+;;;
+
+(define-configuration/no-serialization speakersafetyd-configuration
+  (blackbox-directory
+   (string "/var/lib/speakersafetyd/blackbox")
+   "The directory to which blackbox files are written when the speakers are
+getting too hot.  The blackbox files contain audio and debug information which
+the developers of @code{speakersafetyd} might ask for when reporting bugs.")
+  (configuration-directory
+   (file-like (file-append speakersafetyd "/share/speakersafetyd"))
+   "The base directory as a G-expression (@pxref{G-Expressions}) that contains
+the configuration files of the speaker models.")
+  (maximum-gain-reduction
+   (integer 7)
+   "Maximum gain reduction before panicing, useful for debugging.")
+  (speakersafetyd
+   (file-like speakersafetyd)
+   "The Speaker Safety Daemon package to use."))
+
+(define speakersafetyd-shepherd-service
+  (match-record-lambda <speakersafetyd-configuration>
+      (blackbox-directory configuration-directory maximum-gain-reduction speakersafetyd)
+    (shepherd-service
+     (documentation "Run the speaker safety daemon")
+     (provision '(speakersafetyd))
+     (requirement '(udev))
+     (start #~(make-forkexec-constructor
+               (list #$(file-append speakersafetyd "/bin/speakersafetyd")
+                     "--config-path" #$configuration-directory
+                     "--blackbox-path" #$blackbox-directory
+                     "--max-reduction" (number->string #$maximum-gain-reduction))))
+     (stop #~(make-kill-destructor)))))
+
+(define speakersafetyd-service-type
+  (service-type
+   (name 'speakersafetyd)
+   (description "Run @command{speakersafetyd}, a user space daemon that
+implements an analogue of the Texas Instruments Smart Amp speaker protection
+model.  It can be used to protect the speakers on Apple Silicon devices.")
+   (extensions
+    (list (service-extension
+           shepherd-root-service-type
+           (compose list speakersafetyd-shepherd-service))
+          (service-extension
+           udev-service-type
+           (compose list speakersafetyd-configuration-speakersafetyd))
+          (service-extension
+           profile-service-type
+           (compose list speakersafetyd-configuration-speakersafetyd))))
+   (default-value (speakersafetyd-configuration))))
+
 ;;; sound.scm ends here
-- 
2.48.1





Information forwarded to guix-patches <at> gnu.org:
bug#76289; Package guix-patches. (Sat, 15 Feb 2025 09:27:02 GMT) Full text and rfc822 format available.

Message #32 received at 76289 <at> debbugs.gnu.org (full text, mbox):

From: Roman Scherer <roman <at> burningswell.com>
To: Roman Scherer <roman <at> burningswell.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 76289 <at> debbugs.gnu.org,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, efraim <at> flashner.co.il
Subject: Re: [bug#76289] [PATCH 2/2] services: Add speakersafetyd service.
Date: Sat, 15 Feb 2025 10:25:55 +0100
[Message part 1 (text/plain, inline)]
And I just send a v3, because I exported the wrong symbol.

Roman Scherer <roman <at> burningswell.com> writes:

> Hi Maxim,
>
> thanks for your review. I hope I addressed your comments. Additionally:
>
> - I used the configuration->documentation command to generate
>   documentation from the configuration record
>
> - I discovered match-record-lambda and use this instead of match-record
>
> Could you have another look please?
>
> Thank you, Roman.
>
> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:
>
>> Hi Roman,
>>
>> Roman Scherer <roman <at> burningswell.com> writes:
>>
>>> * gnu/services/sound.scm (speakersafetyd-service-type) New variable.
>>> * doc/guix.texi: Document the speakersafetyd service.
>>
>> Interesting!
>>
>>> Change-Id: Ib8fa19b056a2036019ae7c199d81e1139664e951
>>> ---
>>>  doc/guix.texi          | 41 ++++++++++++++++++++++++++++
>>>  gnu/services/sound.scm | 61 +++++++++++++++++++++++++++++++++++++++++-
>>>  2 files changed, 101 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/doc/guix.texi b/doc/guix.texi
>>> index bd66adf326..3b82df5196 100644
>>> --- a/doc/guix.texi
>>> +++ b/doc/guix.texi
>>> @@ -26575,6 +26575,47 @@ Sound Services
>>>
>>>  @end defvar
>>>
>>> +@subsubheading Speaker Safety Daemon System Service
>>> +
>>> +@uref{https://github.com/AsahiLinux/speakersafetyd, Speaker Safety
>>> +Daemon} is a userspace daemon that implements an analogue of the Texas
>>> +Instruments Smart Amp speaker protection model.  It can be used to
>>> +protect the speakers on Apple Silicon devices.
>>> +
>>> +@defvar speakersafetyd-service-type
>>> +This is the type for the @code{speakersafetyd} system service, whose
>>> +value is a @command{speakersafetyd-configuration} record.
>>> +
>>> +@lisp
>>> +(service speakersafetyd-service-type)
>>> +@end lisp
>>> +
>>> +See below for details about @code{speakersafetyd-configuration}.
>>> +@end defvar
>>> +
>>> +@deftp {Data Type} speakersafetyd-configuration
>>> +Data type representing the configuration for @code{speakersafetyd-service}.
>>> +
>>> +@table @asis
>>> +@item @code{blackbox-path} (default: @code{"/var/lib/speakersafetyd/blackbox"})
>>> +The path to a directory to which "blackbox" files are written when the
>>> +speakers are getting too hot.  The blackbox files contain audio and
>>> +debug information which the developers of @code{speakersafetyd} might
>>> +ask for when reporting bugs.
>>> +
>>> +@item @code{config-path} (default: @code{(file-append speakersafetyd "/share/speakersafetyd")})
>>> +Path to the base directory as a G-expression (@pxref{G-Expressions})
>>> +that contains the configuration files of the speaker models.
>>> +
>>> +@item @code{max-reduction} (default: @code{7})
>>> +Maximum gain reduction before panicing, useful for debugging.
>>> +
>>> +@item @code{package} (default: @var{speakersafetyd})
>>> +The Speaker Safety Daemon package to use.
>>> +
>>> +@end table
>>> +@end deftp
>>> +
>>>  @node File Search Services
>>>  @subsection File Search Services
>>>
>>> diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
>>> index 8ca7acd737..fb8a8d3d17 100644
>>> --- a/gnu/services/sound.scm
>>> +++ b/gnu/services/sound.scm
>>> @@ -35,6 +35,7 @@ (define-module (gnu services sound)
>>>    #:use-module (gnu packages audio)
>>>    #:use-module (gnu packages linux)
>>>    #:use-module (gnu packages pulseaudio)
>>> +  #:use-module (gnu packages rust-apps)
>>>    #:use-module (ice-9 match)
>>>    #:use-module (srfi srfi-1)
>>>    #:export (alsa-configuration
>>> @@ -56,7 +57,15 @@ (define-module (gnu services sound)
>>>              ladspa-configuration
>>>              ladspa-configuration?
>>>              ladspa-configuration-plugins
>>> -            ladspa-service-type))
>>> +            ladspa-service-type
>>> +
>>> +            speakersafetyd-configuration
>>> +            speakersafetyd-configuration-blackbox-path
>>> +            speakersafetyd-configuration-config-path
>>> +            speakersafetyd-configuration-max-reduction
>>> +            speakersafetyd-configuration-package
>>> +            speakersafetyd-configuration?
>>> +            speakersafetyd-service-type))
>>>
>>>  ;;; Commentary:
>>>  ;;;
>>> @@ -263,4 +272,54 @@ (define ladspa-service-type
>>>     (default-value (ladspa-configuration))
>>>     (description "Configure LADSPA plugins.")))
>>>
>>> +
>>> +;;;
>>> +;;; Speaker Safety Daemon
>>> +;;;
>>> +
>>> +(define-record-type* <speakersafetyd-configuration>
>>> +  speakersafetyd-configuration
>>> +  make-speakersafetyd-configuration
>>> +  speakersafetyd-configuration?
>>> +  (blackbox-path speakersafetyd-configuration-blackbox-path
>>> +                 (default "/var/lib/speakersafetyd/blackbox"))
>>
>> Since these values are not serialized, we are free to name them the way
>> we like; so they should follow the GNU and Guix conventions, namely:
>>
>> I'd use blackbox-directory.
>>
>>> +  (config-path speakersafetyd-configuration-config-path
>>> +               (default (file-append speakersafetyd
>>> "/share/speakersafetyd")))
>>
>> I'd use configuration-directory.
>>
>>> +  (max-reduction speakersafetyd-configuration-max-reduction
>>> +                 (default 7))
>>
>> I'd use maximum-gain-reduction
>>
>>> +  (package speakersafetyd-configuration-package
>>> +           (default speakersafetyd)))
>>
>> I'd use the more conventional
>> speakersafetyd-configuration-speakersafetyd (using the name of the
>> package as the field name).
>>
>> Our related conventions here are roughly:
>>
>> 1. Prefer full name instead of abbreviation, especially in public APIs
>> 2. Path always used to denote a multi-entries search path like 'PATH'
>> and friends.  Use 'file name', 'file' or 'directory' instead.
>>
>> It's also more conventional to use the package name for the package
>> object field, so here I'd use
>> 'speakersafetyd-configuration-speakersafetyd'.
>>
>> Could you please also use 'define-configuration/no-serialization' from
>> (gnu services configuration) ?  It validates the type of each field and
>> produces useful user error messages in case these are incorrect.
>>
>>> +(define (speakersafetyd-shepherd-service config)
>>> +  (let ((blackbox-path (speakersafetyd-configuration-blackbox-path config))
>>> +        (config-path (speakersafetyd-configuration-config-path config))
>>> +        (max-reduction (speakersafetyd-configuration-max-reduction config))
>>> +        (package (speakersafetyd-configuration-package config)))
>>
>> nitpick: I'd unbox each value using match-record; which will make things a bit
>> more concise.
>>
>>> +    (shepherd-service
>>> +     (documentation "Speaker saftey daemon")
>>
>> s/saftey/safety/
>>
>>> +     (provision '(speakersafetyd))
>>> +     (requirement '(udev))
>>> +     (start #~(make-forkexec-constructor
>>> +               (list #$(file-append package "/bin/speakersafetyd")
>>> +                     "--config-path" #$config-path
>>> +                     "--blackbox-path" #$blackbox-path
>>> +                     "--max-reduction" (number->string #$max-reduction))))
>>> +     (stop #~(make-kill-destructor)))))
>>> +
>>> +(define speakersafetyd-service-type
>>> +  (service-type
>>> +   (name 'speakersafetyd)
>>> +   (description "Speaker Saftey Daemon")
>>>
>> s/Saftey/Safety/ The project has a better description, which can be
>> adapted to something like "@command{speakersafetyd} is a user space
>> daemon that implements an analogue of the Texas Instruments Smart Amp
>> speaker protection model."
>>
>>> +   (extensions
>>> +    (list (service-extension
>>> +           profile-service-type
>>> +           (compose list speakersafetyd-configuration-package))
>>> +          (service-extension
>>> +           shepherd-root-service-type
>>> +           (compose list speakersafetyd-shepherd-service))
>>> +          (service-extension
>>> +           udev-service-type
>>> +           (compose list speakersafetyd-configuration-package))))
>>> +   (default-value (speakersafetyd-configuration))))
>>
>> Nitpick, but I'd order these from most critical to less critical
>> ordering, such that te root service type is extended first, then the
>> udev-service-type, then the profile.
>>
>> The rest LGTM.  Could you please send a v2?
[signature.asc (application/pgp-signature, inline)]

Reply sent to Maxim Cournoyer <maxim.cournoyer <at> gmail.com>:
You have taken responsibility. (Sat, 15 Feb 2025 14:36:01 GMT) Full text and rfc822 format available.

Notification sent to Roman Scherer <roman <at> burningswell.com>:
bug acknowledged by developer. (Sat, 15 Feb 2025 14:36:02 GMT) Full text and rfc822 format available.

Message #37 received at 76289-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Roman Scherer <roman <at> burningswell.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 76289-done <at> debbugs.gnu.org
Subject: Re: [bug#76289] [PATCH v3 2/2] services: Add speakersafetyd service.
Date: Sat, 15 Feb 2025 23:35:24 +0900
Hi,

Roman Scherer <roman <at> burningswell.com> writes:

> * gnu/services/sound.scm (speakersafetyd-service-type) New variable.
> * doc/guix.texi: Document the speakersafetyd service.
>
> Change-Id: Ib8fa19b056a2036019ae7c199d81e1139664e951

I've made the following small adjustments:

--8<---------------cut here---------------start------------->8---
modified   doc/guix.texi
@@ -26593,6 +26593,7 @@ Sound Services
 See below for details about @code{speakersafetyd-configuration}.
 @end defvar
 
+@c %start of fragment
 @deftp {Data Type} speakersafetyd-configuration
 Available @code{speakersafetyd-configuration} fields are:
 
@@ -26608,14 +26609,14 @@ Sound Services
 contains the configuration files of the speaker models.
 
 @item @code{maximum-gain-reduction} (default: @code{7}) (type: integer)
-Maximum gain reduction before panicing, useful for debugging.
+Maximum gain reduction before panicking, useful for debugging.
 
 @item @code{speakersafetyd} (default: @code{speakersafetyd}) (type: file-like)
 The Speaker Safety Daemon package to use.
 
 @end table
-
 @end deftp
+@c %end of fragment
 
 @node File Search Services
 @subsection File Search Services
modified   gnu/services/sound.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2020 Liliana Marie Prikler <liliana.prikler <at> gmail.com>
 ;;; Copyright © 2020 Marius Bakke <mbakke <at> fastmail.com>
 ;;; Copyright © 2022 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2025 Roman Scherer <roman <at> burningswell.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -274,7 +275,7 @@ (define ladspa-service-type
 
 
 ;;;
-;;; Speaker Safety Daemon
+;;; Speaker Safety Daemon.
 ;;;
 
 (define-configuration/no-serialization speakersafetyd-configuration
@@ -289,7 +290,7 @@ (define-configuration/no-serialization speakersafetyd-configuration
 the configuration files of the speaker models.")
   (maximum-gain-reduction
    (integer 7)
-   "Maximum gain reduction before panicing, useful for debugging.")
+   "Maximum gain reduction before panicking, useful for debugging.")
   (speakersafetyd
    (file-like speakersafetyd)
    "The Speaker Safety Daemon package to use."))

--8<---------------cut here---------------end--------------->8---

And expound the changelog a bit to:

--8<---------------cut here---------------start------------->8---
services: Add speakersafetyd service.

* gnu/services/sound.scm (speakersafetyd-shepherd-service)
(speakersafetyd-configuration)
(speakersafetyd-service-type): New variables.
* doc/guix.texi (Sound Services): Document it.
--8<---------------cut here---------------end--------------->8---

And pushed as commit af643735a5.

Thank you for your contribution!  Closing by replying to
76289-done <at> debbugs.gnu.org in CC.

-- 
Thanks,
Maxim




Information forwarded to guix-patches <at> gnu.org:
bug#76289; Package guix-patches. (Sat, 15 Feb 2025 14:38:02 GMT) Full text and rfc822 format available.

Message #40 received at 76289-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Roman Scherer <roman <at> burningswell.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>, efraim <at> flashner.co.il,
 76289-done <at> debbugs.gnu.org
Subject: Re: [bug#76289] [PATCH 2/2] services: Add speakersafetyd service.
Date: Sat, 15 Feb 2025 23:37:09 +0900
Hi Roman,

Roman Scherer <roman <at> burningswell.com> writes:

[...]

>> thanks for your review. I hope I addressed your comments. Additionally:
>>
>> - I used the configuration->documentation command to generate
>>   documentation from the configuration record

That's how it's currently intended to be used (more automation would be
welcome :-))

>> - I discovered match-record-lambda and use this instead of match-record

Neat, I ddn't know about it.

>> Could you have another look please?

It's now merged.

-- 
Thanks,
Maxim




Information forwarded to guix-patches <at> gnu.org:
bug#76289; Package guix-patches. (Sat, 15 Feb 2025 14:48:02 GMT) Full text and rfc822 format available.

Message #43 received at 76289-done <at> debbugs.gnu.org (full text, mbox):

From: Roman Scherer <roman <at> burningswell.com>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Roman Scherer <roman <at> burningswell.com>,
 Ludovic Courtès <ludo <at> gnu.org>, efraim <at> flashner.co.il,
 76289-done <at> debbugs.gnu.org
Subject: Re: [bug#76289] [PATCH 2/2] services: Add speakersafetyd service.
Date: Sat, 15 Feb 2025 15:46:55 +0100
[Message part 1 (text/plain, inline)]
Hi Maxim,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

> Hi Roman,
>
> Roman Scherer <roman <at> burningswell.com> writes:
>
> [...]
>
>>> thanks for your review. I hope I addressed your comments. Additionally:
>>>
>>> - I used the configuration->documentation command to generate
>>>   documentation from the configuration record
>
> That's how it's currently intended to be used (more automation would be
> welcome :-))

+1

>>> - I discovered match-record-lambda and use this instead of match-record
>
> Neat, I ddn't know about it.
>
>>> Could you have another look please?
>
> It's now merged.

Nice, thank you!
[signature.asc (application/pgp-signature, inline)]

This bug report was last modified 16 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.