diff --git a/internal/monitor/monitor.go b/internal/monitor/monitor.go index 7e9f560..f046314 100644 --- a/internal/monitor/monitor.go +++ b/internal/monitor/monitor.go @@ -105,6 +105,7 @@ type Monitor struct { armVolts float64 inhibited bool firedShut bool + retries int } // New returns a Monitor with the given policy. @@ -245,6 +246,26 @@ func (m *Monitor) evaluateShutdown(r sensor.Reading) ([]Log, bool) { r.Volts, r.SOC, m.armCount)}}, true } +// ActuationFailed reports that the poweroff command did not succeed. +// +// It clears the fired latch so a later qualifying tick tries again. Without +// this, one failed `systemctl poweroff` — a transient polkit denial, a busy +// D-Bus, logind restarting — would silently disarm the feature for the rest of +// the discharge, and the battery would reach cell cutoff and hard-cut the +// machine. That is precisely the outcome shutdown exists to prevent, so a +// failed attempt must not be treated as a completed one. +// +// The arming state is left intact: the battery is still draining, so the next +// qualifying tick retries one interval later rather than restarting the whole +// sample window. +func (m *Monitor) ActuationFailed() { + m.firedShut = false + m.retries++ +} + +// Retries reports how many times actuation has failed and been re-armed. +func (m *Monitor) Retries() int { return m.retries } + func (m *Monitor) disarm(reason string) []Log { m.inhibited = false if !m.armed { diff --git a/internal/monitor/monitor_test.go b/internal/monitor/monitor_test.go index 2d6cb05..24e8f43 100644 --- a/internal/monitor/monitor_test.go +++ b/internal/monitor/monitor_test.go @@ -270,6 +270,61 @@ func TestWithoutDischargeGuardAFalseACReadingIsEnough(t *testing.T) { } } +// A failed poweroff must not count as a completed one. If it latched, the +// battery would reach cell cutoff and the HAT would hard-cut the machine — +// exactly what the feature prevents. +func TestFailedActuationRetriesOnTheNextTick(t *testing.T) { + m := New(shutdownConfig()) + settle(t, m) + + for i := 0; i < 3; i++ { + m.Tick(onBattery(i), nil) + } + if !m.Tick(onBattery(3), nil).Shutdown { + t.Fatal("expected the first shutdown") + } + + // systemctl poweroff failed: polkit denied it, or D-Bus was busy. + m.ActuationFailed() + + res := m.Tick(onBattery(4), nil) + if !res.Shutdown { + t.Fatal("a failed poweroff must be retried on the next qualifying tick") + } + if m.Retries() != 1 { + t.Errorf("Retries() = %d, want 1", m.Retries()) + } + + // Still latched while the retry is outstanding. + if m.Tick(onBattery(5), nil).Shutdown { + t.Error("must not fire again before the second attempt is reported failed") + } +} + +// Retrying must not defeat the safety guards: if AC returns between attempts, +// the machine must stay up. +func TestRetryStillRespectsACReturn(t *testing.T) { + m := New(shutdownConfig()) + settle(t, m) + + for i := 0; i < 3; i++ { + m.Tick(onBattery(i), nil) + } + if !m.Tick(onBattery(3), nil).Shutdown { + t.Fatal("expected the first shutdown") + } + m.ActuationFailed() + + if m.Tick(onMains(), nil).Shutdown { + t.Fatal("AC returned; a pending retry must not power the machine off") + } + for i := 4; i < 7; i++ { + if m.Tick(onBattery(i), nil).Shutdown { + t.Fatalf("fired at tick %d; the window must restart after AC returned", i) + } + } +} + func TestLowSOCAloneCanArm(t *testing.T) { m := New(shutdownConfig()) settle(t, m) diff --git a/main.go b/main.go index b9e23d8..2fefec4 100644 --- a/main.go +++ b/main.go @@ -168,7 +168,14 @@ func tick(ctx context.Context, src sensor.Sensor, mon *monitor.Monitor, opts *op } if res.Shutdown { - powerOff(opts.shutdownCmd, log) + if err := powerOff(opts.shutdownCmd, log); err != nil { + // Re-arm. A failed poweroff must not count as a completed one, or + // the battery reaches cell cutoff and the HAT hard-cuts the + // machine — the exact outcome this feature prevents. + mon.ActuationFailed() + log.Error("poweroff failed; will retry on the next qualifying tick", + "attempt", mon.Retries(), "err", err) + } } } @@ -247,24 +254,25 @@ func startWatchdog(ctx context.Context, lastTick *atomic.Int64, interval time.Du return func() { close(done) } } -func powerOff(command string, log *slog.Logger) { +// powerOff runs the configured poweroff command and reports whether it +// succeeded. The caller re-arms on failure. +func powerOff(command string, log *slog.Logger) error { fields := strings.Fields(command) if len(fields) == 0 { - log.Error("shutdown requested but -shutdown-cmd is empty") - return - } - - if _, err := daemon.SdNotify(false, daemon.SdNotifyStopping); err != nil { - log.Warn("could not notify systemd that we are stopping", "err", err) + return fmt.Errorf("-shutdown-cmd is empty") } + // Deliberately NOT sending SdNotifyStopping before the attempt. Announcing + // a stop that then fails leaves systemd believing the unit is going away + // while it keeps running. logind ends the session on a successful poweroff + // regardless. out, err := exec.Command(fields[0], fields[1:]...).CombinedOutput() if err != nil { - // Do not retry. A poweroff that fails needs a human, and a retry loop - // would fork repeatedly while the battery drains. - log.Error("poweroff command failed", - "cmd", command, "err", err, "output", strings.TrimSpace(string(out))) - return + if trimmed := strings.TrimSpace(string(out)); trimmed != "" { + return fmt.Errorf("%s: %w: %s", command, err, trimmed) + } + return fmt.Errorf("%s: %w", command, err) } log.Warn("poweroff requested", "cmd", command) + return nil }