Fix autoconfigure for weird PWMs

README.md

@@ -448,7 +448,6 @@ Flags:
   -v, --verbose         More verbose output
 
 Use "fan2go [command] --help" for more information about a command.
-
 > sudo fan2go
 ```
 
@@ -615,6 +614,10 @@ configuration.
 To reduce the risk of runnin the whole system on low fan speeds for such a long period of time, you can force fan2go to
 initialize only one fan at a time, using the `runFanInitializationInParallel: false` config option.
 
+Some PWM controllers or fans may require more time to react to PWM changes. If fan2go is failing to characterize a fan,
+you can try increasing the fan response delay by passing `--fan-response-delay <seconds>` to the `fan init` command or
+by setting `fanResponseDelay` in the config. The default value is 2 seconds.
+
 ## Monitoring
 
 Temperature and RPM sensors are polled continuously at the rate specified by the `tempSensorPollingRate` config option.

cmd/fan/init.go

@@ -7,6 +7,7 @@ import (
 	"github.com/markusressel/fan2go/internal/ui"
 	"github.com/markusressel/fan2go/internal/util"
 	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
 )
 
 var initCmd = &cobra.Command{
@@ -59,5 +60,7 @@ var initCmd = &cobra.Command{
 }
 
 func init() {
+	initCmd.Flags().IntP("fan-response-delay", "e", 2, "Delay in seconds to wait before checking that a fan has responded to a control change")
+	_ = viper.BindPFlag("FanResponseDelay", initCmd.Flags().Lookup("fan-response-delay"))
 	Command.AddCommand(initCmd)
 }

cmd/root.go

@@ -2,9 +2,10 @@ package cmd
 
 import (
 	"fmt"
-	"github.com/pterm/pterm/putils"
 	"os"
 
+	"github.com/pterm/pterm/putils"
+
 	"github.com/markusressel/fan2go/cmd/config"
 	"github.com/markusressel/fan2go/cmd/curve"
 	"github.com/markusressel/fan2go/cmd/fan"

fan2go.yaml

@@ -8,6 +8,10 @@ runFanInitializationInParallel: false
 # Note: This parameter is only used for initial analysis of fan curve
 #       and has no effect during normal operation
 maxRpmDiffForSettledFan: 10
+# The time in seconds to wait before checking that a fan has responded to a control change
+# Note: This parameter is only used for initial analysis of fan curve
+#       and has no effect during normal operation
+fanResponseDelay: 2
 
 # The rate to poll temperature sensors at
 tempSensorPollingRate: 200ms

internal/configuration/config.go

@@ -1,18 +1,20 @@
 package configuration
 
 import (
+	"os"
+	"time"
+
 	"github.com/markusressel/fan2go/internal/ui"
 	"github.com/mitchellh/go-homedir"
 	"github.com/spf13/viper"
-	"os"
-	"time"
 )
 
 type Configuration struct {
 	DbPath string `json:"dbPath"`
 
 	RunFanInitializationInParallel bool    `json:"runFanInitializationInParallel"`
 	MaxRpmDiffForSettledFan        float64 `json:"maxRpmDiffForSettledFan"`
+	FanResponseDelay               int     `json:"fanResponseDelay"`
 
 	TempSensorPollingRate time.Duration `json:"tempSensorPollingRate"`
 	TempRollingWindowSize int           `json:"tempRollingWindowSize"`
@@ -62,6 +64,7 @@ func setDefaultValues() {
 	viper.SetDefault("dbpath", "/etc/fan2go/fan2go.db")
 	viper.SetDefault("RunFanInitializationInParallel", true)
 	viper.SetDefault("MaxRpmDiffForSettledFan", 10.0)
+	viper.SetDefault("FanResponseDelay", 2)
 	viper.SetDefault("TempSensorPollingRate", 200*time.Millisecond)
 	viper.SetDefault("TempRollingWindowSize", 10)
 	viper.SetDefault("RpmPollingRate", 1*time.Second)

internal/controller/controller.go

@@ -3,19 +3,23 @@ package controller
 import (
 	"context"
 	"fmt"
+	"math"
+	"sort"
+	"sync"
+	"time"
+
 	"github.com/markusressel/fan2go/internal/configuration"
 	"github.com/markusressel/fan2go/internal/curves"
 	"github.com/markusressel/fan2go/internal/fans"
 	"github.com/markusressel/fan2go/internal/persistence"
 	"github.com/markusressel/fan2go/internal/ui"
 	"github.com/markusressel/fan2go/internal/util"
 	"github.com/oklog/run"
-	"math"
-	"sort"
-	"sync"
-	"time"
 )
 
+// Amount of time to wait between a set-pwm and get-pwm. Used during fan initial calibration.
+const pwmSetGetDelay time.Duration = 5 * time.Millisecond
+
 var InitializationSequenceMutex sync.Mutex
 
 type FanControllerStatistics struct {
@@ -55,7 +59,7 @@ type PidFanController struct {
 	originalPwmValue int
 	// the last pwm value that was set to the fan, **before** applying the pwmMap to it
 	lastSetPwm *int
-	// a list of all pwm values where setPwm(x) != setPwm(y) for the controlled fan
+	// a list of all pre-pwmMap pwm values where setPwm(x) != setPwm(y) for the controlled fan
 	pwmValuesWithDistinctTarget []int
 	// a map of x -> getPwm() where x is setPwm(x) for the controlled fan
 	pwmMap map[int]int
@@ -281,33 +285,31 @@ func (f *PidFanController) RunInitializationSequence() (err error) {
 	curveData := map[int]float64{}
 
 	initialMeasurement := true
-	for pwm := range f.pwmValuesWithDistinctTarget {
+	for _, pwm := range f.pwmValuesWithDistinctTarget {
 		// set a pwm
 		err = f.setPwm(pwm)
 		if err != nil {
 			ui.Error("Unable to run initialization sequence on %s: %v", fan.GetId(), err)
 			return err
 		}
-
+		expectedPwm := f.pwmMap[pwm]
+		time.Sleep(pwmSetGetDelay)
 		actualPwm, err := fan.GetPwm()
 		if err != nil {
 			ui.Error("Fan %s: Unable to measure current PWM", fan.GetId())
 			return err
 		}
-		if actualPwm != pwm {
-			ui.Debug("Fan %s: Actual PWM value differs from requested one, skipping. Requested: %d Actual: %d", fan.GetId(), pwm, actualPwm)
+		if actualPwm != expectedPwm {
+			ui.Debug("Fan %s: Actual PWM value differs from requested one, skipping: requested: %d, expected: %d, actual: %d", fan.GetId(), pwm, expectedPwm, actualPwm)
 			continue
 		}
 
 		if initialMeasurement {
 			initialMeasurement = false
 			f.waitForFanToSettle(fan)
 		} else {
-			// wait a bit to allow the fan speed to settle.
-			// since most sensors are update only each second,
-			// we wait double that to make sure we get
-			// the most recent measurement
-			time.Sleep(2 * time.Second)
+			// wait a bit to allow the fan speed to settle
+			time.Sleep(time.Duration(configuration.CurrentConfig.FanResponseDelay) * time.Second)
 		}
 
 		rpm, err := fan.GetRpm()
@@ -421,7 +423,7 @@ func (f *PidFanController) calculateTargetPwm() int {
 
 	if f.lastSetPwm != nil && f.pwmMap != nil {
 		lastSetPwm := *(f.lastSetPwm)
-		expected := f.mapToClosestDistinct(lastSetPwm)
+		expected := f.pwmMap[f.findClosestDistinctTarget(lastSetPwm)]
 		if currentPwm, err := fan.GetPwm(); err == nil {
 			if currentPwm != expected {
 				f.stats.UnexpectedPwmValueCount += 1
@@ -463,16 +465,17 @@ func (f *PidFanController) calculateTargetPwm() int {
 func (f *PidFanController) setPwm(target int) (err error) {
 	current, err := f.fan.GetPwm()
 
-	closestAvailable := f.mapToClosestDistinct(target)
+	closestTarget := f.findClosestDistinctTarget(target)
+	closestExpected := f.pwmMap[closestTarget]
 
 	f.lastSetPwm = &target
 	if err == nil {
-		if closestAvailable == current {
+		if closestExpected == current {
 			// nothing to do
 			return nil
 		}
 	}
-	return f.fan.SetPwm(closestAvailable)
+	return f.fan.SetPwm(closestTarget)
 }
 
 func (f *PidFanController) waitForFanToSettle(fan fans.Fan) {
@@ -499,9 +502,8 @@ func (f *PidFanController) waitForFanToSettle(fan fans.Fan) {
 	ui.Debug("Fan %s has settled (current RPM max diff: %f)", fan.GetId(), measuredRpmDiffMax)
 }
 
-func (f *PidFanController) mapToClosestDistinct(target int) int {
-	closest := util.FindClosest(target, f.pwmValuesWithDistinctTarget)
-	return f.pwmMap[closest]
+func (f *PidFanController) findClosestDistinctTarget(target int) int {
+	return util.FindClosest(target, f.pwmValuesWithDistinctTarget)
 }
 
 // computePwmMap computes a mapping between "requested pwm value" -> "actual set pwm value"
@@ -561,7 +563,7 @@ func (f *PidFanController) computePwmMapAutomatically() {
 	pwmMap := map[int]int{}
 	for i := fans.MaxPwmValue; i >= fans.MinPwmValue; i-- {
 		_ = fan.SetPwm(i)
-		time.Sleep(10 * time.Millisecond)
+		time.Sleep(pwmSetGetDelay)
 		pwm, err := fan.GetPwm()
 		if err != nil {
 			ui.Warning("Error reading PWM value of fan %s: %v", fan.GetId(), err)

internal/controller/controller_test.go

@@ -467,6 +467,6 @@ func TestFanController_UpdateFanSpeed_FanCurveGaps(t *testing.T) {
 	// THEN
 	assert.Equal(t, 54, targetPwm)
 
-	closestTarget := controller.mapToClosestDistinct(targetPwm)
-	assert.Equal(t, 50, closestTarget)
+	closestTarget := controller.findClosestDistinctTarget(targetPwm)
+	assert.Equal(t, 58, closestTarget)
 }