Skip to content

Commit

Permalink
Fix autoconfigure for weird PWMs
Browse files Browse the repository at this point in the history
This patch fixes fan autoconfiguration for weird PWMs that have non-1:1 value maps.
In particular, it fixes the construction of the RPM curve data to properly use the
PWM value map computed just before.

This patch also adds a config option and flag to set the delay between setting
a PWM value and observing the fan RPM sensor to support a wide range of
controllers and fans. A long delay can lead to system overheating. The default
is 2 seconds.

This patch also changes a few other delays in the initialization code, namely
the delay between setting a PWM value and observing the PWM value (to detect
controllers that alter/quantize/adjust PWM values). It is hardcoded to 5
milliseconds.
  • Loading branch information
jfroy committed Mar 23, 2023
1 parent f5f0d87 commit c09860d
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 28 deletions.
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,6 @@ Flags:
-v, --verbose More verbose output
Use "fan2go [command] --help" for more information about a command.
> sudo fan2go
```

Expand Down Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions cmd/fan/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)
}
3 changes: 2 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 5 additions & 2 deletions internal/configuration/config.go
Original file line number Diff line number Diff line change
@@ -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"`
Expand Down Expand Up @@ -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)
Expand Down
46 changes: 24 additions & 22 deletions internal/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit c09860d

Please sign in to comment.