bug#63620: 30.0.50; [Feature Request] run hooks on sleep/wake

bug#63620: 30.0.50; [Feature Request] run hooks on sleep/wake

[Andrew Cohen]

I am playing around with various network things (mostly vpn related) and
need Emacs to take certain actions when my laptop sleeps and wakes. This
is easy to do using the existing dbus support (the relevant code is
below, which runs hooks on sleeping and waking) The question is whether
this is useful enough to add to Emacs, and if so the best place to put
it.

Not too long ago, Eric Abrahamsen added gnus-dbus.el which is similar,
although for a single purpose: to close gnus server network connections
on sleep. This has a single entry point for the user: set
'gnus-dbus-close-on-sleep to t to enable the feature. 


Options:
1. Do nothing (this isn't useful enough to change anything, and I can just
keep using my own code).

2. Add a small package dbus-sleep.el. I would also remove gnus-dbus.el
but leave the variable 'gnus-dbus-close-on-sleep and use it to control
the installation of appropriate gnus functions to the new hooks.

3. 2. Add the new code to the existing dbus.el. I would also remove
gnus-dbus.el but leave the variable 'gnus-dbus-close-on-sleep and use it
to control the installation of appropriate functions to the new hooks.

I mostly favor adding it to dbus.el. The argument against: dbus.el is
focused on providing language bindings for the D-Bus API rather than a
user-feature. The removal of gnus-dbus.el shouldn't cause any problem
since I would maintain the same functionality enabled by the same
variable, so no user-visible change. 

Advice?


;;; Code:

(require 'dbus)

(defcustom dbus-sleep-sleep-hook nil
  "Hook to run on sleep."
  :group 'dbus-sleep
  :type 'hook)

(defcustom dbus-sleep-wake-hook nil
  "Hook to run on wake."
  :group 'dbus-sleep
  :type 'hook)

(defvar dbus-sleep-registration-object nil
  "Object returned from `dbus-register-signal'.
This is used to unregister the signal.")

;;;###autoload
(defun dbus-sleep-enable ()
  "Use `dbus-register-signal' to close servers on sleep."
  (interactive)
  ;; Don't enable if it's already enabled.
  (when (and (featurep 'dbusbind) (not dbus-sleep-registration-object))
    (setq dbus-sleep-registration-object
          (dbus-register-signal :system
                                "org.freedesktop.login1"
                                "/org/freedesktop/login1"
                                "org.freedesktop.login1.Manager"
                                "PrepareForSleep"
                                #'dbus-sleep-handler))))

(defun dbus-sleep-disable ()
  "Unregister sleep signal."
  (interactive)
  (condition-case nil
      (dbus-unregister-object
       dbus-sleep-registration-object)
    (setq dbus-sleep-registration-object nil)
    (wrong-type-argument nil)))

(defun dbus-sleep-handler (sleep-wake)
  "Handler to execute sleep and wake functions.
SLEEP-WAKE is t on sleeping and nil on waking."
  (ignore-errors
    (if sleep-wake
        (run-hooks 'dbus-sleep-sleep-hook)
      (run-hooks 'dbus-sleep-wake-hook))))

-- 
Andrew Cohen

bug#63620: 30.0.50; [Feature Request] run hooks on sleep/wake

[Michael Albinus]

Andrew Cohen <acohen@ust.hk> writes:

Hi Andrew,

> 2. Add a small package dbus-sleep.el. I would also remove gnus-dbus.el
> but leave the variable 'gnus-dbus-close-on-sleep and use it to control
> the installation of appropriate gnus functions to the new hooks.
>
> 3. Add the new code to the existing dbus.el. I would also remove
> gnus-dbus.el but leave the variable 'gnus-dbus-close-on-sleep and use it
> to control the installation of appropriate functions to the new hooks.
>
> I mostly favor adding it to dbus.el. The argument against: dbus.el is
> focused on providing language bindings for the D-Bus API rather than a
> user-feature. The removal of gnus-dbus.el shouldn't cause any problem
> since I would maintain the same functionality enabled by the same
> variable, so no user-visible change.

Using D-Bus is just an implementation detail. What you want are
handlers, which are invoked when your laptop falls asleep or awakes.

A general package could implement it using D-Bus if available, or using
something else if there's no D-Bus. See battery.el, which uses the D-Bus
service "org.freedesktop.UPower" only if possible. Your package might be
called sleep.el or alike.

Your code uses the D-Bus service "org.freedesktop.login1", which isn't
part of the basic D-Bus spec. So it isn't suited for dbus.el anyway.

> ;;;###autoload
> (defun dbus-sleep-enable ()
>   "Use `dbus-register-signal' to close servers on sleep."
>   (interactive)

I would make it rather a global minor mode, that you can enable/disable
it easily.

>   ;; Don't enable if it's already enabled.
>   (when (and (featurep 'dbusbind) (not dbus-sleep-registration-object))

Perhaps, you check also for the service "org.freedesktop.login1", like

--8<---------------cut here---------------start------------->8---
(member "org.freedesktop.login1" (dbus-list-activatable-names :system))
--8<---------------cut here---------------end--------------->8---

This check might return nil for several reasons, like the system bus is
not running, or you don't have permissions to speak to the system bus,
or the service simply doesn't exist.

Best regards, Michael.

bug#63620: 30.0.50; [Feature Request] run hooks on sleep/wake

[Andrew Cohen]

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

Dear Michael

>>>>> "MA" == Michael Albinus <michael.albinus@gmx.de> writes:

[...]

    MA> Using D-Bus is just an implementation detail. What you want are
    MA> handlers, which are invoked when your laptop falls asleep or
    MA> awakes.

Yes, of course you are right!

    MA> A general package could implement it using D-Bus if available,
    MA> or using something else if there's no D-Bus. See battery.el,
    MA> which uses the D-Bus service "org.freedesktop.UPower" only if
    MA> possible. Your package might be called sleep.el or alike.

I am not familiar with other methods for detecting sleep/wake, so I just
have dbus at the moment. But if anyone knows of something else
(especially on windows) it can then be added.

[...]

MA> I would make it rather a global minor mode, that you can
MA> enable/disable it easily.

Yes, that is a better idea. 

[...]

    MA> Perhaps, you check also for the service
    MA> "org.freedesktop.login1", like

    MA> (member "org.freedesktop.login1" (dbus-list-activatable-names
    MA> :system))

OK, added.


I've now put everything together and modified the gnus code to use the
new sleep.el. But I remain unsure if I should remove gnus-dbus.el
entirely (everything remains the same for the user: setting
gnus-dbus-close-on-sleep to a non-nil value will enable the feature
using the global minor mode rather than gnus-dbus).  And is sleep.el
something worth adding?


[-- Attachment #2: sleep.el --]
[-- Type: application/emacs-lisp, Size: 3425 bytes --]

[-- Attachment #3: Type: text/plain, Size: 19 bytes --]



-- 
Andrew Cohen

bug#63620: 30.0.50; [Feature Request] run hooks on sleep/wake

[Michael Albinus]

Andrew Cohen <acohen@ust.hk> writes:

> Dear Michael

Hi Andrew,

sorry for the delayed answer, I've been busy ...

> I've now put everything together and modified the gnus code to use the
> new sleep.el. But I remain unsure if I should remove gnus-dbus.el
> entirely (everything remains the same for the user: setting
> gnus-dbus-close-on-sleep to a non-nil value will enable the feature
> using the global minor mode rather than gnus-dbus).  And is sleep.el
> something worth adding?

It depends whether sleep.el will be added to vanilla Emacs, or as GNU
ELPA package. It's not my decision, but I lobby for the former.

In this case, it could be used in gnus-dbus.el.

Just some few comments on the code

> ;;; sleep.el --- run hooks on sleep and wake  -*- lexical-binding:t -*-

I would be more precise. You don't mean sleeping Emacs or another
application, but you mean sleeping the machine on OS level. Say it so.

> ;;; This global minor mode enables evaluating code when the device
> ;;; running Emacs enters or leaves the sleep state.  Two hooks are
> ;;; used, sleep-sleep-hook and sleep-wake-hook, run when the system
> ;;; detects that it is going to sleep or waking up.  Currently only a
> ;;; dbus interface to detect sleep state change is implemented.

Please quote Lisp objects like `sleep-sleep-hook'. D-Bus is spelled out
in commentaries as D-Bus.

> (defgroup sleep nil
>   "Run hooks on entering/leaving the sleep state."
>   :group 'hardware)
>
> (defcustom sleep-sleep-hook nil
>   "Hook to run on entering sleep."
>   :group 'sleep
>   :type 'hook)
>
> (defcustom sleep-wake-hook nil
>   "Hook to run on leaving sleep."
>   :group 'sleep
>   :type 'hook)

These are the user visible objects. Please be precise what is going to
sleep. The machine or device.

> Run sleep-sleep-hook and sleep-wake-hook as appropriate."

Please quote sleep-sleep-hook.

>   (unless sleep-registration-object
>     (setq sleep-registration-object
>           (dbus-register-signal :system
>                                 "org.freedesktop.login1"
>                                 "/org/freedesktop/login1"
>                                 "org.freedesktop.login1.Manager"
>                                 "PrepareForSleep"
>                                 #'sleep-handler))))

I would also protect against D-Bus errors. Like

(ignore-error dbus-error
  (unless sleep-registration-object
    (setq sleep-registration-object
          (dbus-register-signal
	   :system "org.freedesktop.login1"
           "/org/freedesktop/login1" "org.freedesktop.login1.Manager"
           "PrepareForSleep" #'sleep-handler))))

>   (condition-case nil
>       (progn
>         (dbus-unregister-object
>          sleep-registration-object)
>         (setq sleep-registration-object nil))
>     (wrong-type-argument nil)))

Aka

(ignore-error (dbus-error wrong-type-argument)
  (dbus-unregister-object
   sleep-registration-object)
  (setq sleep-registration-object nil))

> When sleep-wake-mode is enabled, Emacs will execute the hooks

Please quote sleep-wake-mode.

> support dbus detection of sleep state change."

D-Bus is spelled out ...

Furthermore, it would be nice if you add

- documentation in lispref node "(elisp) System Interface". I'm not sure
  whether it is good for a new subnode, or whether it shall be
  documented in "(elisp) System Environment". This documentation should
  also give a practical example for functions in sleep-sleep-hook and
  sleep-wake-hook.

- adding the functionality to etc/NEWS

- if possible, adding tests in test/lisp/sleep-tests.el

Best regards, Michael.