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.

Some more context:
- a subsequent diff would reorder the code to handle similarly the "!rv"
  and "ev->ident == ctl_fd" paths
- 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.
- if battery is still lower than your autoaction limit, your system
  might go back to sleep once if you attempt a resume after plugging
  your AC cable.  Another glitch, another mail to tech@.

Thoughts?  oks?


Index: apmd.c
===================================================================
RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v
retrieving revision 1.91
diff -u -p -r1.91 apmd.c
--- apmd.c      2 Nov 2019 00:41:36 -0000       1.91
+++ apmd.c      25 Jan 2020 18:05:08 -0000
@@ -380,7 +380,6 @@ main(int argc, char *argv[])
        int noacsleep = 0;
        struct timespec ts = {TIMO, 0}, sts = {0, 0};
        struct apm_power_info pinfo;
-       time_t apmtimeout = 0;
        const char *sockname = _PATH_APM_SOCKET;
        const char *errstr;
        int kq, nchanges;
@@ -502,13 +501,10 @@ main(int argc, char *argv[])
 
                sts = ts;
 
-               apmtimeout += 1;
                if ((rv = kevent(kq, NULL, 0, ev, 1, &sts)) == -1)
                        break;
 
-               if (apmtimeout >= ts.tv_sec) {
-                       apmtimeout = 0;
-
+               if (!rv) {
                        /* wakeup for timeout: take status */
                        powerbak = power_status(ctl_fd, 0, &pinfo);
                        if (powerstatus != powerbak) {
@@ -519,8 +515,8 @@ main(int argc, char *argv[])
                        if (!powerstatus && autoaction &&
                            autolimit > (int)pinfo.battery_life) {
                                logmsg(LOG_NOTICE,
-                                   "estimated battery life %d%%, "
-                                   "autoaction limit set to %d%% .",
+                                   "estimated battery life %d%%"
+                                   " below configured limit %d%%",
                                    pinfo.battery_life,
                                    autolimit
                                );
@@ -530,10 +526,8 @@ main(int argc, char *argv[])
                                else
                                        hibernate(ctl_fd);
                        }
-               }
-
-               if (!rv)
                        continue;
+               }
 
                if (ev->ident == ctl_fd) {
                        suspends = standbys = hibernates = resumes = 0;
@@ -575,6 +569,19 @@ main(int argc, char *argv[])
                                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:


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

Reply via email to