From cdfd2909c02242b91ddd4dda00defe9a1f19f8d9 Mon Sep 17 00:00:00 2001
From: Markus Ressel <mail@markusressel.de>
Date: Mon, 23 Sep 2024 07:35:05 +0200
Subject: [PATCH] added validation, deprecate ControlLoop

---
 cmd/fan/init.go                      |  6 +-----
 internal/backend.go                  |  5 ++++-
 internal/configuration/fans.go       |  8 +++++++-
 internal/configuration/validation.go | 15 +++++++++++++++
 internal/control_loop/pid.go         | 11 +----------
 5 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/cmd/fan/init.go b/cmd/fan/init.go
index 2b13f2d..4647b6e 100644
--- a/cmd/fan/init.go
+++ b/cmd/fan/init.go
@@ -31,11 +31,7 @@ var initCmd = &cobra.Command{
 		fanController := controller.NewFanController(
 			p,
 			fan,
-			control_loop.NewPidControlLoop(
-				0.03,
-				0.002,
-				0.0005,
-			),
+			control_loop.NewDirectControlLoop(nil),
 			configuration.CurrentConfig.ControllerAdjustmentTickRate)
 
 		ui.Info("Deleting existing data for fan '%s'...", fan.GetId())
diff --git a/internal/backend.go b/internal/backend.go
index 837a832..ba813b3 100644
--- a/internal/backend.go
+++ b/internal/backend.go
@@ -230,8 +230,9 @@ func initializeObjects(pers persistence.Persistence) map[fans.Fan]controller.Fan
 
 		var controlLoop control_loop.ControlLoop
 
-		// TODO: check compatibility fallback
+		// compatibility fallback
 		if config.ControlLoop != nil {
+			ui.Warning("Using deprecated control loop configuration for fan %s. Please update your configuration to use the new control algorithm configuration.", config.ID)
 			controlLoop = control_loop.NewPidControlLoop(
 				config.ControlLoop.P,
 				config.ControlLoop.I,
@@ -247,6 +248,8 @@ func initializeObjects(pers persistence.Persistence) map[fans.Fan]controller.Fan
 			controlLoop = control_loop.NewDirectControlLoop(
 				&config.ControlAlgorithm.Direct.MaxPwmChangePerCycle,
 			)
+		} else {
+			controlLoop = control_loop.NewDirectControlLoop(nil)
 		}
 
 		fanController := controller.NewFanController(pers, fan, controlLoop, updateRate)
diff --git a/internal/configuration/fans.go b/internal/configuration/fans.go
index 2582cc2..54cd709 100644
--- a/internal/configuration/fans.go
+++ b/internal/configuration/fans.go
@@ -15,7 +15,13 @@ type FanConfig struct {
 	HwMon            *HwMonFanConfig         `json:"hwMon,omitempty"`
 	File             *FileFanConfig          `json:"file,omitempty"`
 	Cmd              *CmdFanConfig           `json:"cmd,omitempty"`
-	ControlLoop      *ControlLoopConfig      `json:"controlLoop,omitempty"`
+
+	// ControlLoop is a configuration for a PID control loop.
+	//
+	// Deprecated: HeaderMap exists for historical compatibility
+	// and should not be used. To access the headers returned by a handler,
+	// use the Response.Header map as returned by the Result method.
+	ControlLoop *ControlLoopConfig `json:"controlLoop,omitempty"`
 }
 
 type ControlAlgorithm string
diff --git a/internal/configuration/validation.go b/internal/configuration/validation.go
index cec0f6d..d329e43 100644
--- a/internal/configuration/validation.go
+++ b/internal/configuration/validation.go
@@ -265,6 +265,21 @@ func validateFans(config *Configuration) error {
 			return fmt.Errorf("fan %s: no curve definition with id '%s' found", fanConfig.ID, fanConfig.Curve)
 		}
 
+		if fanConfig.ControlAlgorithm != nil {
+			if fanConfig.ControlAlgorithm.Direct != nil {
+				if fanConfig.ControlAlgorithm.Direct.MaxPwmChangePerCycle <= 0 {
+					return fmt.Errorf("fan %s: invalid maxPwmChangePerCycle, must be > 0", fanConfig.ID)
+				}
+			}
+
+			if fanConfig.ControlAlgorithm.Pid != nil {
+				pidConfig := fanConfig.ControlAlgorithm.Pid
+				if pidConfig.P == 0 && pidConfig.I == 0 && pidConfig.D == 0 {
+					return fmt.Errorf("fan %s: all PID constants are zero", fanConfig.ID)
+				}
+			}
+		}
+
 		if fanConfig.HwMon != nil {
 			if (fanConfig.HwMon.Index != 0 && fanConfig.HwMon.RpmChannel != 0) || (fanConfig.HwMon.Index == 0 && fanConfig.HwMon.RpmChannel == 0) {
 				return fmt.Errorf("fan %s: must have one of index or rpmChannel, must be >= 1", fanConfig.ID)
diff --git a/internal/control_loop/pid.go b/internal/control_loop/pid.go
index b15195d..a30bdb7 100644
--- a/internal/control_loop/pid.go
+++ b/internal/control_loop/pid.go
@@ -10,21 +10,12 @@ type PidControlLoop struct {
 	pidLoop *util.PidLoop
 }
 
-// NewPidControlLoop creates a PidControlLoop, which is a very simple control that Pidly applies the given
-// target pwm. It can also be used to gracefully approach the target by
-// utilizing the "maxPwmChangePerCycle" property.
+// NewPidControlLoop creates a PidControlLoop, which uses a PID loop to approach the target.
 func NewPidControlLoop(
 	p float64,
 	i float64,
 	d float64,
 ) *PidControlLoop {
-	// TODO: somehow incorporate default values
-	//pidLoop = util.NewPidLoop(
-	//	0.03,
-	//	0.002,
-	//	0.0005,
-	//)
-
 	return &PidControlLoop{
 		pidLoop: util.NewPidLoop(p, i, d),
 	}