From 7f5b7097e364af24b40b042b631a10e35310b881 Mon Sep 17 00:00:00 2001
From: Matthew Kennedy <matthewkennedy@outlook.com>
Date: Mon, 13 Nov 2023 13:56:03 -0800
Subject: [PATCH] tests for heater output

---
 firmware/heater_control.cpp | 16 ++++++-------
 firmware/heater_control.h   |  5 ++--
 test/tests/test_heater.cpp  | 47 +++++++++++++++++++++++++++++++++++--
 3 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/firmware/heater_control.cpp b/firmware/heater_control.cpp
index 585a1b5..9d12f42 100644
--- a/firmware/heater_control.cpp
+++ b/firmware/heater_control.cpp
@@ -31,7 +31,7 @@ HeaterState HeaterControllerBase::GetHeaterState() const
     return heaterState;
 }
 
-HeaterState HeaterControllerBase::GetNextState(HeaterAllow heaterAllowState, float batteryVoltage, float sensorTemp)
+HeaterState HeaterControllerBase::GetNextState(HeaterState currentState, HeaterAllow heaterAllowState, float batteryVoltage, float sensorTemp)
 {
     bool heaterAllowed = heaterAllowState == HeaterAllow::Allowed;
 
@@ -62,7 +62,7 @@ HeaterState HeaterControllerBase::GetNextState(HeaterAllow heaterAllowState, flo
     float closedLoopTemp = m_targetTempC - 50;
     float underheatTemp = m_targetTempC - 100;
 
-    switch (heaterState)
+    switch (currentState)
     {
         case HeaterState::Preheat:
             timeCounter--;
@@ -119,9 +119,9 @@ HeaterState HeaterControllerBase::GetNextState(HeaterAllow heaterAllowState, flo
     return heaterState;
 }
 
-float HeaterControllerBase::GetVoltageForState(float heaterEsr)
+float HeaterControllerBase::GetVoltageForState(HeaterState state, float sensorEsr)
 {
-    switch (heaterState)
+    switch (state)
     {
         case HeaterState::Preheat:
             // Max allowed during condensation phase (preheat) is 2v
@@ -141,7 +141,7 @@ float HeaterControllerBase::GetVoltageForState(float heaterEsr)
             // Negated because lower resistance -> hotter
 
             // TODO: heater PID should operate on temperature, not ESR
-            return 7.5f - heaterPid.GetOutput(m_targetEsr, heaterEsr);
+            return 7.5f - heaterPid.GetOutput(m_targetEsr, sensorEsr);
         case HeaterState::Stopped:
         case HeaterState::NoHeaterSupply:
             // Something has gone wrong, turn off the heater.
@@ -155,7 +155,7 @@ float HeaterControllerBase::GetVoltageForState(float heaterEsr)
 void HeaterControllerBase::Update(const ISampler& sampler, HeaterAllow heaterAllowState)
 {
     // Read sensor state
-    float heaterEsr = sampler.GetSensorInternalResistance();
+    float sensorEsr = sampler.GetSensorInternalResistance();
     float sensorTemperature = sampler.GetSensorTemperature();
 
     // If we haven't heard from the ECU, use the internally sensed
@@ -165,8 +165,8 @@ void HeaterControllerBase::Update(const ISampler& sampler, HeaterAllow heaterAll
                                 : GetRemoteBatteryVoltage();
 
     // Run the state machine
-    heaterState = GetNextState(heaterAllowState, batteryVoltage, sensorTemperature);
-    float heaterVoltage = GetVoltageForState(heaterEsr);
+    heaterState = GetNextState(heaterState, heaterAllowState, batteryVoltage, sensorTemperature);
+    float heaterVoltage = GetVoltageForState(heaterState, sensorEsr);
 
     // Limit to 11 volts
     if (heaterVoltage > 11) {
diff --git a/firmware/heater_control.h b/firmware/heater_control.h
index a07a1ed..cdbcc0e 100644
--- a/firmware/heater_control.h
+++ b/firmware/heater_control.h
@@ -39,9 +39,8 @@ public:
 
     virtual void SetDuty(float duty) const = 0;
 
-protected:
-    HeaterState GetNextState(HeaterAllow haeterAllowState, float batteryVoltage, float sensorTemp);
-    float GetVoltageForState(float heaterEsr);
+    HeaterState GetNextState(HeaterState currentState, HeaterAllow haeterAllowState, float batteryVoltage, float sensorTemp);
+    float GetVoltageForState(HeaterState state, float sensorEsr);
 
 private:
     Pid heaterPid =
diff --git a/test/tests/test_heater.cpp b/test/tests/test_heater.cpp
index 8d159e6..544f315 100644
--- a/test/tests/test_heater.cpp
+++ b/test/tests/test_heater.cpp
@@ -3,13 +3,56 @@
 
 #include "heater_control.h"
 
-struct MockHeater : public HeaterControllerBase {
+struct MockHeater : public HeaterControllerBase
+{
     MockHeater() : HeaterControllerBase(0) { }
 
     MOCK_METHOD(void, SetDuty, (float), (const, override));
 };
 
-TEST(Heater, Basic)
+TEST(HeaterStateOutput, Preheat)
+{
+    MockHeater dut;
+
+    // Shouldn't depend upon sensor ESR
+    EXPECT_EQ(1.5f, dut.GetVoltageForState(HeaterState::Preheat, 0));
+    EXPECT_EQ(1.5f, dut.GetVoltageForState(HeaterState::Preheat, 300));
+    EXPECT_EQ(1.5f, dut.GetVoltageForState(HeaterState::Preheat, 1000));
+}
+
+TEST(HeaterStateOutput, WarmupRamp)
+{
+    MockHeater dut;
+
+    EXPECT_EQ(1.5f, dut.GetVoltageForState(HeaterState::Preheat, 0));
+    EXPECT_EQ(1.5f, dut.GetVoltageForState(HeaterState::Preheat, 300));
+    EXPECT_EQ(1.5f, dut.GetVoltageForState(HeaterState::Preheat, 1000));
+}
+
+TEST(HeaterStateOutput, ClosedLoop)
+{
+    MockHeater dut;
+    dut.Configure(780, 300);
+
+    // At target -> zero output but with 7.5v offset
+    EXPECT_EQ(dut.GetVoltageForState(HeaterState::ClosedLoop, 300), 7.5f);
+
+    // Below target -> more voltage
+    EXPECT_GT(dut.GetVoltageForState(HeaterState::ClosedLoop, 400), 7.5f);
+
+    // Above target -> less voltage
+    EXPECT_LT(dut.GetVoltageForState(HeaterState::ClosedLoop, 200), 7.5f);
+}
+
+TEST(HeaterStateOutput, Cases)
+{
+    MockHeater dut;
+
+    EXPECT_EQ(0, dut.GetVoltageForState(HeaterState::Stopped, 0));
+    EXPECT_EQ(0, dut.GetVoltageForState(HeaterState::NoHeaterSupply, 0));
+}
+
+TEST(HeaterStateMachine, x)
 {
     MockHeater dut;
 }