From 94d37857fa927168327a33acb385aff603bf01c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=85smund=20V=C3=A5ge=20Fannemel?=
 <34712686+asmfstatoil@users.noreply.github.com>
Date: Mon, 4 Dec 2023 19:35:42 +0100
Subject: [PATCH] feat: assure no negative Ti and Td (#872)

* feat: assure no negative Ti and Td

* feat: also check for Kp
test: added tests
---
 .../ControllerDeviceBaseClass.java            | 37 ++++++-
 .../ControllerDeviceBaseClassTest.java        | 98 +++++++++++++++++++
 2 files changed, 130 insertions(+), 5 deletions(-)
 create mode 100644 src/test/java/neqsim/processSimulation/controllerDevice/ControllerDeviceBaseClassTest.java

diff --git a/src/main/java/neqsim/processSimulation/controllerDevice/ControllerDeviceBaseClass.java b/src/main/java/neqsim/processSimulation/controllerDevice/ControllerDeviceBaseClass.java
index 1486f94d3c..65c6cfe725 100644
--- a/src/main/java/neqsim/processSimulation/controllerDevice/ControllerDeviceBaseClass.java
+++ b/src/main/java/neqsim/processSimulation/controllerDevice/ControllerDeviceBaseClass.java
@@ -7,6 +7,8 @@
 package neqsim.processSimulation.controllerDevice;
 
 import java.util.UUID;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import neqsim.processSimulation.measurementDevice.MeasurementDeviceInterface;
 import neqsim.util.NamedBaseClass;
 
@@ -20,6 +22,7 @@
  */
 public class ControllerDeviceBaseClass extends NamedBaseClass implements ControllerDeviceInterface {
   private static final long serialVersionUID = 1000;
+  static Logger logger = LogManager.getLogger(ControllerDeviceBaseClass.class);
 
   /**
    * Unique identifier of which solve/run call was last called successfully.
@@ -48,7 +51,18 @@ public class ControllerDeviceBaseClass extends NamedBaseClass implements Control
    * </p>
    */
   public ControllerDeviceBaseClass() {
-    super("controller");
+    this("controller");
+  }
+
+  /**
+   * <p>
+   * Constructor for ControllerDeviceBaseClass.
+   * </p>
+   *
+   * @param name Name of PID controller object
+   */
+  public ControllerDeviceBaseClass(String name) {
+    super(name);
   }
 
   /** {@inheritDoc} */
@@ -77,8 +91,9 @@ public void runTransient(double initResponse, double dt, UUID id) {
         transmitter.getMeasuredPercentValue() - (controllerSetPoint - transmitter.getMinimumValue())
             / (transmitter.getMaximumValue() - transmitter.getMinimumValue()) * 100;
 
-    if (Ti > 0)
+    if (Ti != 0) {
       TintValue += Kp / Ti * error * dt;
+    }
     double TderivValue = Kp * Td * (error - oldError) / dt;
     response = initResponse + propConstant * (Kp * error + TintValue + TderivValue);
     // System.out.println("error " + error + " %");
@@ -142,7 +157,11 @@ public double getKp() {
    * @param Kp Proportional gain of PID controller
    */
   public void setKp(double Kp) {
-    this.Kp = Kp;
+    if (Kp >= 0) {
+      this.Kp = Kp;
+    } else {
+      logger.warn("Negative Kp is not allowed. Use setReverseActing.");
+    }
   }
 
   /** {@inheritDoc} */
@@ -172,7 +191,11 @@ public double getTi() {
    * @param Ti Integral time in seconds
    */
   public void setTi(double Ti) {
-    this.Ti = Ti;
+    if (Ti >= 0) {
+      this.Ti = Ti;
+    } else {
+      logger.warn("Negative Ti is not allowed.");
+    }
   }
 
   /**
@@ -194,6 +217,10 @@ public double getTd() {
    * @param Td Derivative time in seconds
    */
   public void setTd(double Td) {
-    this.Td = Td;
+    if (Td >= 0) {
+      this.Td = Td;
+    } else {
+      logger.warn("Negative Td is not allowed.");
+    }
   }
 }
diff --git a/src/test/java/neqsim/processSimulation/controllerDevice/ControllerDeviceBaseClassTest.java b/src/test/java/neqsim/processSimulation/controllerDevice/ControllerDeviceBaseClassTest.java
new file mode 100644
index 0000000000..010ede2656
--- /dev/null
+++ b/src/test/java/neqsim/processSimulation/controllerDevice/ControllerDeviceBaseClassTest.java
@@ -0,0 +1,98 @@
+package neqsim.processSimulation.controllerDevice;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+public class ControllerDeviceBaseClassTest {
+
+  static ControllerDeviceBaseClass c;
+
+  @BeforeAll
+  static void setUp() {
+    c = new ControllerDeviceBaseClass("testPID");
+  }
+
+  @Test
+  void testSetControllerParameters() {
+    double zero = 0;
+    double positive = 9.9;
+    double negative = -0.1;
+
+    c.setControllerParameters(zero, zero, zero);
+
+    Assertions.assertEquals(zero, c.getKp());
+    Assertions.assertEquals(zero, c.getTd());
+    Assertions.assertEquals(zero, c.getTi());
+
+    c.setControllerParameters(positive, positive, positive);
+    Assertions.assertEquals(positive, c.getKp());
+    Assertions.assertEquals(positive, c.getTd());
+    Assertions.assertEquals(positive, c.getTi());
+
+    c.setControllerParameters(negative, positive, positive);
+    Assertions.assertEquals(positive, c.getKp());
+    Assertions.assertEquals(positive, c.getTd());
+    Assertions.assertEquals(positive, c.getTi());
+
+    c.setControllerParameters(positive, negative, positive);
+    Assertions.assertEquals(positive, c.getKp());
+    Assertions.assertEquals(positive, c.getTd());
+    Assertions.assertEquals(positive, c.getTi());
+
+    c.setControllerParameters(positive, positive, negative);
+    Assertions.assertEquals(positive, c.getKp());
+    Assertions.assertEquals(positive, c.getTd());
+    Assertions.assertEquals(positive, c.getTi());
+  }
+
+  @Test
+  void testGetKp() {
+    double kp = c.getKp();
+    Assertions.assertEquals(c.getKp(), kp);
+  }
+
+  @Test
+  void testGetTd() {
+    double td = c.getTd();
+    Assertions.assertEquals(c.getTd(), td);
+  }
+
+  @Test
+  void testGetTi() {
+    double ti = c.getTi();
+    Assertions.assertEquals(c.getTi(), ti);
+  }
+
+  @Test
+  void testGetUnit() {
+    String unit = c.getUnit();
+    Assertions.assertEquals(c.getUnit(), unit);
+  }
+
+  @Test
+  void testIsReverseActing() {
+    boolean isReverse = c.isReverseActing();
+    Assertions.assertEquals(isReverse, c.isReverseActing());
+  }
+
+  @Test
+  void testSetReverseActing() {
+    boolean testValue = true;
+    boolean oldValue = c.isReverseActing();
+    c.setReverseActing(testValue);
+    Assertions.assertEquals(testValue, c.isReverseActing());
+    c.setReverseActing(oldValue);
+    Assertions.assertEquals(oldValue, c.isReverseActing());
+  }
+
+  @Test
+  void testSetUnit() {
+    String testUnit = "test";
+    String oldUnit = c.getUnit();
+    c.setUnit(testUnit);
+    Assertions.assertEquals(testUnit, c.getUnit());
+    c.setUnit(oldUnit);
+    Assertions.assertEquals(oldUnit, c.getUnit());
+  }
+}