Skip to content

Commit 00e9a4a

Browse files
committed
fix(cli): restore fan settings when fan init is interrupted
The measurement leaves the fan in manual mode at whatever PWM value was probed last. Interrupting the CLI initialization with SIGINT/SIGTERM killed the process without any cleanup, leaving the fan stuck there. Split RunInitialization() into a public wrapper that captures the fan state and owns the restore on any failure, and a private worker holding the sequence itself. The initialization sequence is context-aware, so the CLI cancels the context via signal.NotifyContext on SIGINT/SIGTERM and the controller restores the fan state on its own, without the CLI knowing anything about fan state internals.
1 parent f5055f9 commit 00e9a4a

5 files changed

Lines changed: 119 additions & 32 deletions

File tree

cmd/fan/init.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
package fan
22

33
import (
4+
"context"
5+
"os"
6+
"os/signal"
7+
"syscall"
8+
49
"github.com/markusressel/fan2go/internal"
510
"github.com/markusressel/fan2go/internal/configuration"
611
"github.com/markusressel/fan2go/internal/control_loop"
@@ -56,7 +61,10 @@ var initCmd = &cobra.Command{
5661
return err
5762
}
5863

59-
_, err = fanController.RunInitialization()
64+
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
65+
defer stop()
66+
67+
_, err = fanController.RunInitialization(ctx)
6068
if err == nil {
6169
ui.Success("Done!")
6270
// print measured fan curve

internal/controller/controller.go

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ type FanController interface {
4646

4747
// UpdateCurve dynamically updates the curve reference
4848
UpdateCurve(curve curves.SpeedCurve)
49+
50+
// RunInitialization runs the fan initialization sequence.
51+
RunInitialization(ctx context.Context) (map[int]float64, error)
4952
}
5053

5154
type FanStateSnapshot struct {
@@ -231,7 +234,7 @@ func (f *DefaultFanController) Run(ctx context.Context) error {
231234
// wait a bit to gather monitoring data
232235
time.Sleep(2*time.Second + configuration.CurrentConfig.TempSensorPollingRate)
233236

234-
fanPwmData, err := f.runInitializationIfNeeded()
237+
fanPwmData, err := f.runInitializationIfNeeded(ctx)
235238
if err != nil {
236239
return err
237240
}
@@ -344,7 +347,7 @@ func (f *DefaultFanController) Run(ctx context.Context) error {
344347
return err
345348
}
346349

347-
func (f *DefaultFanController) runInitializationIfNeeded() (map[int]float64, error) {
350+
func (f *DefaultFanController) runInitializationIfNeeded(ctx context.Context) (map[int]float64, error) {
348351
fan := f.fan
349352
// check if we have data for this fan in persistence,
350353
// if not we need to run the initialization sequence
@@ -354,7 +357,7 @@ func (f *DefaultFanController) runInitializationIfNeeded() (map[int]float64, err
354357
config := fan.GetConfig()
355358
if config.HwMon != nil || config.Nvidia != nil {
356359
ui.Warning("Fan '%s' has not yet been analyzed, starting initialization sequence...", fan.GetId())
357-
fanCurveData, err := f.RunInitialization()
360+
fanCurveData, err := f.RunInitialization(ctx)
358361
if err != nil {
359362
return nil, err
360363
}
@@ -368,25 +371,32 @@ func (f *DefaultFanController) runInitializationIfNeeded() (map[int]float64, err
368371
return fanRpmData, err
369372
}
370373

371-
func (f *DefaultFanController) RunInitialization() (map[int]float64, error) {
372-
fan := f.fan
373-
374-
// the `fan init` CLI command calls this directly (without Run()), so the original fan state may not have been captured yet
374+
func (f *DefaultFanController) RunInitialization(ctx context.Context) (map[int]float64, error) {
375375
err := f.storeCurrentFanState()
376376
if err != nil {
377377
return nil, err
378378
}
379379

380-
err = f.computeFanSpecificMappings()
380+
curveData, err := f.runInitialization(ctx)
381+
if err != nil {
382+
f.restoreControlMode()
383+
return nil, err
384+
}
385+
return curveData, nil
386+
}
387+
388+
func (f *DefaultFanController) runInitialization(ctx context.Context) (map[int]float64, error) {
389+
fan := f.fan
390+
391+
err := f.computeFanSpecificMappings()
381392
if err != nil {
382393
ui.Error("Fan %s: Error computing fan specific mappings: %v", fan.GetId(), err)
383394
return nil, err
384395
}
385396

386397
fanAnalyzer := NewFanCurveAnalyzer(f)
387-
curveData, err := fanAnalyzer.RunInitializationSequence()
398+
curveData, err := fanAnalyzer.RunInitializationSequence(ctx)
388399
if err != nil {
389-
f.restoreControlMode()
390400
return nil, err
391401
}
392402

@@ -407,7 +417,6 @@ func (f *DefaultFanController) RunInitialization() (map[int]float64, error) {
407417

408418
fanRpmData, err := f.persistence.LoadFanRpmData(fan)
409419
if err != nil {
410-
f.restoreControlMode()
411420
return nil, err
412421
}
413422

internal/controller/controller_test.go

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2121,7 +2121,7 @@ func TestRunInitialization_FailedInit_RestoresOriginalControlMode(t *testing.T)
21212121
updateRate: time.Duration(100),
21222122
}
21232123

2124-
_, err := controller.RunInitialization()
2124+
_, err := controller.RunInitialization(context.Background())
21252125

21262126
assert.Error(t, err)
21272127
assert.NotContains(t, fan.controlModeHistory, fans.ControlModeDisabled,
@@ -2130,6 +2130,50 @@ func TestRunInitialization_FailedInit_RestoresOriginalControlMode(t *testing.T)
21302130
"failed init must restore the original (automatic) control mode")
21312131
}
21322132

2133+
func TestRunInitialization_Cancelled_RestoresOriginalControlMode(t *testing.T) {
2134+
originalConfig := configuration.CurrentConfig
2135+
defer func() {
2136+
configuration.CurrentConfig = originalConfig
2137+
}()
2138+
configuration.CurrentConfig.FanController.PwmSetDelay = 1 * time.Millisecond
2139+
// non-zero so the cancelled context is the only ready channel in sleepWithContext
2140+
configuration.CurrentConfig.FanResponseDelay = 1
2141+
configuration.CurrentConfig.Analysis.SampleCount = 1
2142+
configuration.CurrentConfig.Analysis.SampleDelay = 0
2143+
configuration.CurrentConfig.Analysis.SettleTimeout = 0
2144+
2145+
fan := &mockFanForFailedInit{
2146+
MockFan: MockFan{
2147+
ID: "fan",
2148+
RPM: 0,
2149+
ControlMode: fans.ControlModeAutomatic,
2150+
PwmMap: &configuration.PwmMapConfig{Identity: &configuration.PwmMapIdentityConfig{}},
2151+
SetPwmToGetPwmMap: &configuration.SetPwmToGetPwmMapConfig{
2152+
Identity: &configuration.SetPwmToGetPwmMapIdentityConfig{},
2153+
},
2154+
ControlModeConfig: &configuration.ControlModeConfig{
2155+
OnExit: &configuration.OnExitConfig{Restore: &configuration.OnExitRestoreConfig{}},
2156+
},
2157+
},
2158+
}
2159+
controller := DefaultFanController{
2160+
persistence: mockPersistence{hasPwmMap: false},
2161+
fan: fan,
2162+
updateRate: time.Duration(100),
2163+
}
2164+
2165+
ctx, cancel := context.WithCancel(context.Background())
2166+
cancel()
2167+
2168+
_, err := controller.RunInitialization(ctx)
2169+
2170+
assert.True(t, errors.Is(err, context.Canceled))
2171+
assert.NotContains(t, fan.controlModeHistory, fans.ControlModeDisabled,
2172+
"the zero value of ControlMode must never be written to the fan")
2173+
assert.Equal(t, fans.ControlModeAutomatic, fan.ControlMode,
2174+
"cancelled init must restore the original (automatic) control mode")
2175+
}
2176+
21332177
// --- setPwmToGetPwmMap / pwmMap edge-case tests ---
21342178

21352179
func TestFanController_ComputeSetPwmToGetPwmMap_AllWritesFail(t *testing.T) {

internal/controller/fan_analysis.go

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package controller
22

33
import (
4+
"context"
45
"fmt"
56
"math"
67
"sort"
@@ -14,6 +15,15 @@ import (
1415

1516
const pwmMismatchRetries = 2
1617

18+
func sleepWithContext(ctx context.Context, d time.Duration) error {
19+
select {
20+
case <-ctx.Done():
21+
return ctx.Err()
22+
case <-time.After(d):
23+
return nil
24+
}
25+
}
26+
1727
type FanCurveAnalyzer struct {
1828
fanController *DefaultFanController
1929
}
@@ -26,7 +36,7 @@ func NewFanCurveAnalyzer(
2636
}
2737
}
2838

29-
func (f *FanCurveAnalyzer) RunInitializationSequence() (rpmCurve map[int]float64, err error) {
39+
func (f *FanCurveAnalyzer) RunInitializationSequence(ctx context.Context) (rpmCurve map[int]float64, err error) {
3040
fan := f.fanController.fan
3141

3242
if !fan.Supports(fans.FeatureRpmSensor) {
@@ -51,21 +61,21 @@ func (f *FanCurveAnalyzer) RunInitializationSequence() (rpmCurve map[int]float64
5161
initStarted := time.Now()
5262

5363
startBoundaryStarted := time.Now()
54-
startIdx, _, err := f.detectStartBoundary(fan, distinct, curveData)
64+
startIdx, _, err := f.detectStartBoundary(ctx, fan, distinct, curveData)
5565
if err != nil {
5666
return nil, err
5767
}
5868
ui.Info("Fan %s: Boundary discovery (start) finished in %s", fan.GetId(), time.Since(startBoundaryStarted).Round(time.Millisecond))
5969

6070
maxBoundaryStarted := time.Now()
61-
maxIdx, _, err := f.detectMaxBoundary(fan, distinct, startIdx, curveData)
71+
maxIdx, _, err := f.detectMaxBoundary(ctx, fan, distinct, startIdx, curveData)
6272
if err != nil {
6373
return nil, err
6474
}
6575
ui.Info("Fan %s: Boundary discovery (max) finished in %s", fan.GetId(), time.Since(maxBoundaryStarted).Round(time.Millisecond))
6676

6777
interiorStarted := time.Now()
68-
curveData, err = f.sampleInteriorCoarse(fan, distinct, startIdx, maxIdx, curveData)
78+
curveData, err = f.sampleInteriorCoarse(ctx, fan, distinct, startIdx, maxIdx, curveData)
6979
if err != nil {
7080
return nil, err
7181
}
@@ -82,6 +92,7 @@ func (f *FanCurveAnalyzer) RunInitializationSequence() (rpmCurve map[int]float64
8292
}
8393

8494
func (f *FanCurveAnalyzer) detectStartBoundary(
95+
ctx context.Context,
8596
fan fans.Fan,
8697
distinct []int,
8798
curveData map[int]float64,
@@ -95,7 +106,7 @@ func (f *FanCurveAnalyzer) detectStartBoundary(
95106
for low <= high {
96107
mid := (low + high) / 2
97108
pwm := distinct[mid]
98-
rpm, measureErr := f.measureAtPwm(fan, pwm, configuration.CurrentConfig.Analysis.SettleTimeout)
109+
rpm, measureErr := f.measureAtPwm(ctx, fan, pwm, configuration.CurrentConfig.Analysis.SettleTimeout)
99110
if measureErr != nil {
100111
return 0, 0, measureErr
101112
}
@@ -134,6 +145,7 @@ func (f *FanCurveAnalyzer) detectStartBoundary(
134145
}
135146

136147
func (f *FanCurveAnalyzer) detectMaxBoundary(
148+
ctx context.Context,
137149
fan fans.Fan,
138150
distinct []int,
139151
startIdx int,
@@ -162,7 +174,7 @@ func (f *FanCurveAnalyzer) detectMaxBoundary(
162174
ui.Info("Fan %s: Discovering max boundary...", fan.GetId())
163175
for idx := lastIdx; idx >= topStartIdx; idx -= step {
164176
pwm := distinct[idx]
165-
rpm, measureErr := f.measureAtPwm(fan, pwm, 0)
177+
rpm, measureErr := f.measureAtPwm(ctx, fan, pwm, 0)
166178
if measureErr != nil {
167179
return 0, 0, measureErr
168180
}
@@ -186,7 +198,7 @@ func (f *FanCurveAnalyzer) detectMaxBoundary(
186198
pwm := distinct[idx]
187199
rpm, exists := fastScan[pwm]
188200
if !exists {
189-
measureRpm, measureErr := f.measureAtPwm(fan, pwm, 0)
201+
measureRpm, measureErr := f.measureAtPwm(ctx, fan, pwm, 0)
190202
if measureErr != nil {
191203
return 0, 0, measureErr
192204
}
@@ -204,7 +216,7 @@ func (f *FanCurveAnalyzer) detectMaxBoundary(
204216
}
205217

206218
if roughIdx >= 0 {
207-
confirmedIdx, confirmedPwm, confirmErr := f.confirmMaxBoundary(fan, distinct, startIdx, roughIdx, curveData)
219+
confirmedIdx, confirmedPwm, confirmErr := f.confirmMaxBoundary(ctx, fan, distinct, startIdx, roughIdx, curveData)
208220
if confirmErr != nil {
209221
return 0, 0, confirmErr
210222
}
@@ -219,6 +231,7 @@ func (f *FanCurveAnalyzer) detectMaxBoundary(
219231
}
220232

221233
func (f *FanCurveAnalyzer) confirmMaxBoundary(
234+
ctx context.Context,
222235
fan fans.Fan,
223236
distinct []int,
224237
startIdx int,
@@ -246,7 +259,7 @@ func (f *FanCurveAnalyzer) confirmMaxBoundary(
246259
peakConfirmed := 0.0
247260
for _, idx := range confirmIndices {
248261
pwm := distinct[idx]
249-
rpm, measureErr := f.measureAtPwm(fan, pwm, configuration.CurrentConfig.Analysis.SettleTimeout)
262+
rpm, measureErr := f.measureAtPwm(ctx, fan, pwm, configuration.CurrentConfig.Analysis.SettleTimeout)
250263
if measureErr != nil {
251264
return 0, 0, measureErr
252265
}
@@ -284,6 +297,7 @@ func (f *FanCurveAnalyzer) confirmMaxBoundary(
284297
}
285298

286299
func (f *FanCurveAnalyzer) sampleInteriorCoarse(
300+
ctx context.Context,
287301
fan fans.Fan,
288302
distinct []int,
289303
startIdx int,
@@ -308,7 +322,7 @@ func (f *FanCurveAnalyzer) sampleInteriorCoarse(
308322
continue
309323
}
310324
settleTimeout := settleTimeoutForPwmJump(lastMeasuredPwm, pwm, configuration.CurrentConfig.Analysis.SettleTimeout)
311-
rpm, err := f.measureAtPwm(fan, pwm, settleTimeout)
325+
rpm, err := f.measureAtPwm(ctx, fan, pwm, settleTimeout)
312326
if err != nil {
313327
return curveData, err
314328
}
@@ -394,13 +408,15 @@ func (f *FanCurveAnalyzer) rpmCurveMeasurementCleanup(curveData map[int]float64)
394408
// expected value after setting it even after waiting FanResponseDelay (indicates the hardware ignored the request).
395409
// If settleTimeout > 0, waitForFanToSettle is called with that timeout (used for large PWM steps).
396410
// If settleTimeout == 0, a plain FanResponseDelay sleep is used instead (sufficient for small steps).
397-
func (f *FanCurveAnalyzer) measureAtPwm(fan fans.Fan, pwm int, settleTimeout time.Duration) (float64, error) {
411+
func (f *FanCurveAnalyzer) measureAtPwm(ctx context.Context, fan fans.Fan, pwm int, settleTimeout time.Duration) (float64, error) {
398412
actualPwm := f.fanController.applyPwmMapToTarget(pwm)
399413
matchedPwm := false
400414
for attempt := 0; attempt <= pwmMismatchRetries; attempt++ {
401415
if attempt > 0 {
402416
// The reported PWM may lag behind the requested value on some hardware, so wait FanResponseDelay before retrying.
403-
time.Sleep(time.Duration(configuration.CurrentConfig.FanResponseDelay) * time.Second)
417+
if err := sleepWithContext(ctx, time.Duration(configuration.CurrentConfig.FanResponseDelay)*time.Second); err != nil {
418+
return 0, err
419+
}
404420
}
405421
err := f.fanController.setPwm(actualPwm)
406422
if err != nil {
@@ -427,10 +443,14 @@ func (f *FanCurveAnalyzer) measureAtPwm(fan fans.Fan, pwm int, settleTimeout tim
427443
}
428444

429445
if settleTimeout > 0 {
430-
f.waitForFanToSettle(fan, settleTimeout)
446+
if err := f.waitForFanToSettle(ctx, fan, settleTimeout); err != nil {
447+
return 0, err
448+
}
431449
} else {
432450
// Small PWM step — a response-delay sleep is sufficient.
433-
time.Sleep(time.Duration(configuration.CurrentConfig.FanResponseDelay) * time.Second)
451+
if err := sleepWithContext(ctx, time.Duration(configuration.CurrentConfig.FanResponseDelay)*time.Second); err != nil {
452+
return 0, err
453+
}
434454
}
435455

436456
sampleCount := configuration.CurrentConfig.Analysis.SampleCount
@@ -441,7 +461,9 @@ func (f *FanCurveAnalyzer) measureAtPwm(fan fans.Fan, pwm int, settleTimeout tim
441461
samples := make([]float64, 0, sampleCount)
442462
for i := 0; i < sampleCount; i++ {
443463
if i > 0 {
444-
time.Sleep(sampleDelay)
464+
if err := sleepWithContext(ctx, sampleDelay); err != nil {
465+
return 0, err
466+
}
445467
}
446468
r, err := fan.GetRpm()
447469
if err != nil {
@@ -459,7 +481,7 @@ func (f *FanCurveAnalyzer) measureAtPwm(fan fans.Fan, pwm int, settleTimeout tim
459481
// waitForFanToSettle waits until the fan's RPM readings are stable (requiredConsecutive consecutive
460482
// readings with diff <= MaxRpmDiffForSettledFan). If timeout > 0 and the deadline is exceeded, a
461483
// warning is logged and the function returns early rather than blocking forever.
462-
func (f *FanCurveAnalyzer) waitForFanToSettle(fan fans.Fan, timeout time.Duration) {
484+
func (f *FanCurveAnalyzer) waitForFanToSettle(ctx context.Context, fan fans.Fan, timeout time.Duration) error {
463485
const requiredConsecutive = 3
464486
const settleSampleInterval = 500 * time.Millisecond
465487
const settleWindowSize = 8
@@ -484,9 +506,11 @@ func (f *FanCurveAnalyzer) waitForFanToSettle(fan fans.Fan, timeout time.Duratio
484506
for consecutiveStable < requiredConsecutive {
485507
if timeout > 0 && time.Now().After(deadline) {
486508
ui.Warning("Fan %s did not settle within %v, continuing anyway (%d/%d stable readings)", fan.GetId(), timeout, consecutiveStable, requiredConsecutive)
487-
return
509+
return nil
510+
}
511+
if err := sleepWithContext(ctx, settleSampleInterval); err != nil {
512+
return err
488513
}
489-
time.Sleep(settleSampleInterval)
490514

491515
currentRpm, err := fan.GetRpm()
492516
if err != nil {
@@ -519,6 +543,7 @@ func (f *FanCurveAnalyzer) waitForFanToSettle(fan fans.Fan, timeout time.Duratio
519543
fan.GetId(), consecutiveStable, requiredConsecutive, meanNow, rangeNow)
520544
}
521545
ui.Debug("Fan %s has settled (%d consecutive stable readings)", fan.GetId(), requiredConsecutive)
546+
return nil
522547
}
523548

524549
func evaluateAdaptiveSettling(window []float64, prevMean *float64, prevRange *float64, baseThreshold float64) (bool, float64, float64) {

0 commit comments

Comments
 (0)