On Sat, Jan 25 2020, Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote:
> The diff below improves the way apmd -z/-Z may trigger.
>
> I think the current behavior is bogus, incrementing and checking
> apmtimeout like this doesn't make much sense.
>
> Here's a proposal:
> - on APM_POWER_CHANGE events, check the battery level and trigger
>   autoaction if needed.  This should be enough to make autoaction just
>   work with drivers like acpibat(4).
> - on kevent timeout (10mn by default now, maybe too long), also check
>   the battery level and suspend if needed.  This should be useful only
>   if your battery driver doesn't send any APM_POWER_CHANGE event.
>
> While here I also tweaked the warning.

This has been committed, thanks Ted.

> Some more context:
> - a subsequent diff would reorder the code to handle similarly the "!rv"
>   and "ev->ident == ctl_fd" paths

Diff below.

> - I think we want some throttling mechanism, like wait for 1mn after we
>   resume before autosuspending again.  But I want to fix obvious
>   problems first.
[...]

Further unify the handling of kevent(2) timeouts and apm events:
fake an APM_POWER_CHANGE event on timeouts.

I think assert(3) is appropriate here but could be convinced otherwise.

ok?


Index: apmd.c
===================================================================
--- apmd.c.orig
+++ apmd.c
@@ -37,6 +37,7 @@
 #include <sys/event.h>
 #include <sys/time.h>
 #include <sys/sysctl.h>
+#include <assert.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <syslog.h>
@@ -497,7 +498,7 @@ main(int argc, char *argv[])
                error("kevent", NULL);
 
        for (;;) {
-               int rv;
+               int rv, event, index;
 
                sts = ts;
 
@@ -528,6 +529,46 @@ main(int argc, char *argv[])
                        continue;
                } else if (rv == 0) {
                        /* wakeup for timeout: take status */
+                       event = APM_POWER_CHANGE;
+                       index = -1;
+               } else {
+                       assert(rv == 1 && ev->ident == ctl_fd);
+                       event = APM_EVENT_TYPE(ev->data);
+                       index = APM_EVENT_INDEX(ev->data);
+               }
+
+               logmsg(LOG_DEBUG, "apmevent %04x index %d", event, index);
+
+               switch (event) {
+               case APM_SUSPEND_REQ:
+               case APM_USER_SUSPEND_REQ:
+               case APM_CRIT_SUSPEND_REQ:
+               case APM_BATTERY_LOW:
+                       suspends++;
+                       break;
+               case APM_USER_STANDBY_REQ:
+               case APM_STANDBY_REQ:
+                       standbys++;
+                       break;
+               case APM_USER_HIBERNATE_REQ:
+                       hibernates++;
+                       break;
+#if 0
+               case APM_CANCEL:
+                       suspends = standbys = 0;
+                       break;
+#endif
+               case APM_NORMAL_RESUME:
+               case APM_CRIT_RESUME:
+               case APM_SYS_STANDBY_RESUME:
+                       powerbak = power_status(ctl_fd, 0, &pinfo);
+                       if (powerstatus != powerbak) {
+                               powerstatus = powerbak;
+                               powerchange = 1;
+                       }
+                       resumes++;
+                       break;
+               case APM_POWER_CHANGE:
                        powerbak = power_status(ctl_fd, 0, &pinfo);
                        if (powerstatus != powerbak) {
                                powerstatus = powerbak;
@@ -548,63 +589,9 @@ main(int argc, char *argv[])
                                else
                                        hibernates++;
                        }
-               } else if (ev->ident == ctl_fd) {
-                       logmsg(LOG_DEBUG, "apmevent %04x index %d",
-                           (int)APM_EVENT_TYPE(ev->data),
-                           (int)APM_EVENT_INDEX(ev->data));
-
-                       switch (APM_EVENT_TYPE(ev->data)) {
-                       case APM_SUSPEND_REQ:
-                       case APM_USER_SUSPEND_REQ:
-                       case APM_CRIT_SUSPEND_REQ:
-                       case APM_BATTERY_LOW:
-                               suspends++;
-                               break;
-                       case APM_USER_STANDBY_REQ:
-                       case APM_STANDBY_REQ:
-                               standbys++;
-                               break;
-                       case APM_USER_HIBERNATE_REQ:
-                               hibernates++;
-                               break;
-#if 0
-                       case APM_CANCEL:
-                               suspends = standbys = 0;
-                               break;
-#endif
-                       case APM_NORMAL_RESUME:
-                       case APM_CRIT_RESUME:
-                       case APM_SYS_STANDBY_RESUME:
-                               powerbak = power_status(ctl_fd, 0, &pinfo);
-                               if (powerstatus != powerbak) {
-                                       powerstatus = powerbak;
-                                       powerchange = 1;
-                               }
-                               resumes++;
-                               break;
-                       case APM_POWER_CHANGE:
-                               powerbak = power_status(ctl_fd, 0, &pinfo);
-                               if (powerstatus != powerbak) {
-                                       powerstatus = powerbak;
-                                       powerchange = 1;
-                               }
-
-                               if (!powerstatus && autoaction &&
-                                   autolimit > (int)pinfo.battery_life) {
-                                       logmsg(LOG_NOTICE,
-                                           "estimated battery life %d%%"
-                                           " below configured limit %d%%",
-                                           pinfo.battery_life, autolimit);
-
-                                       if (autoaction == AUTO_SUSPEND)
-                                               suspends++;
-                                       else
-                                               hibernates++;
-                               }
-                               break;
-                       default:
-                               ;
-                       }
+                       break;
+               default:
+                       ;
                }
 
                if ((standbys || suspends) && noacsleep &&

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to