From c59c80e99d6a07dffd383f867c3b49e48f6aac02 Mon Sep 17 00:00:00 2001
From: Markus Ressel <mail@markusressel.de>
Date: Mon, 23 Sep 2024 06:46:41 +0200
Subject: [PATCH] refactoring and cleanup

---
 internal/control_loop/common.go   |  7 +++++--
 internal/control_loop/direct.go   | 16 ++++++++++++----
 internal/control_loop/pid.go      |  9 ++++-----
 internal/controller/controller.go |  2 +-
 4 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/internal/control_loop/common.go b/internal/control_loop/common.go
index 1322383..ce05b13 100644
--- a/internal/control_loop/common.go
+++ b/internal/control_loop/common.go
@@ -1,6 +1,9 @@
 package control_loop
 
+// ControlLoop defines how a FanController approaches the target value of its curve.
 type ControlLoop interface {
-	// Loop advances the control loop
-	Loop(target float64, measured float64) float64
+	// Cycle advances the control loop to the next step and returns the new pwm value.
+	// Note: multiple calls to Loop may not result in the same output, as
+	// the control loop may take time into account or have other stateful properties.
+	Cycle(target int, lastSetPwm int) int
 }
diff --git a/internal/control_loop/direct.go b/internal/control_loop/direct.go
index 78bfa76..fffc1eb 100644
--- a/internal/control_loop/direct.go
+++ b/internal/control_loop/direct.go
@@ -2,6 +2,7 @@ package control_loop
 
 import (
 	"github.com/markusressel/fan2go/internal/util"
+	"math"
 	"time"
 )
 
@@ -27,7 +28,7 @@ func NewDirectControlLoop(
 	}
 }
 
-func (l *DirectControlLoop) Loop(target float64, measured float64) float64 {
+func (l *DirectControlLoop) Cycle(target int, measured int) int {
 	loopTime := time.Now()
 
 	dt := loopTime.Sub(l.lastTime).Seconds()
@@ -37,15 +38,22 @@ func (l *DirectControlLoop) Loop(target float64, measured float64) float64 {
 	// the pwm adjustment depends on the direction and
 	// the time-based change speed limit.
 	maxPwmChangeThiStep := float64(l.maxPwmChangePerCycle) * dt
-	err := target - measured
+	err := float64(target - measured)
 	// we can be above or below the target pwm value,
 	// so we substract or add at most the max pwm change,
 	// capped to having reached the target
+	var stepTarget float64
 	if err > 0 {
 		// below desired speed, add pwms
-		return util.Coerce(maxPwmChangeThiStep, 0, err)
+		stepTarget = util.Coerce(maxPwmChangeThiStep, 0, err)
 	} else {
 		// above or at desired speed, subtract pwms
-		return util.Coerce(-maxPwmChangeThiStep, err, 0)
+		stepTarget = util.Coerce(-maxPwmChangeThiStep, err, 0)
 	}
+
+	// ensure we are within sane bounds
+	coerced := util.Coerce(stepTarget, 0, 255)
+	result := int(math.Round(coerced))
+
+	return result
 }
diff --git a/internal/control_loop/pid.go b/internal/control_loop/pid.go
index 35daded..b15195d 100644
--- a/internal/control_loop/pid.go
+++ b/internal/control_loop/pid.go
@@ -30,13 +30,12 @@ func NewPidControlLoop(
 	}
 }
 
-func (l *PidControlLoop) Loop(target float64, lastSetPwm float64) float64 {
-	result := l.pidLoop.Loop(target, lastSetPwm)
+func (l *PidControlLoop) Cycle(target int, lastSetPwm int) int {
+	result := l.pidLoop.Loop(float64(target), float64(lastSetPwm))
 
-	// TODO: verify this
 	// ensure we are within sane bounds
-	coerced := util.Coerce(lastSetPwm+result, 0, 255)
+	coerced := util.Coerce(float64(lastSetPwm)+result, 0, 255)
 	stepTarget := int(math.Round(coerced))
 
-	return float64(stepTarget)
+	return stepTarget
 }
diff --git a/internal/controller/controller.go b/internal/controller/controller.go
index 66eac6f..270d02f 100644
--- a/internal/controller/controller.go
+++ b/internal/controller/controller.go
@@ -248,7 +248,7 @@ func (f *PidFanController) UpdateFanSpeed() error {
 	target := f.calculateTargetPwm()
 
 	// the target pwm, approaching the actual target smoothly
-	stepTarget := int(f.controlLoop.Loop(float64(target), float64(lastSetPwm)))
+	stepTarget := f.controlLoop.Cycle(target, lastSetPwm)
 
 	if target >= 0 {
 		_ = trySetManualPwm(f.fan)