Move ETB control to a thread (#1374)

* switch to thread

* actually use default frequency define

* crank ADC too

* make space in RAM

* remove TS field

* this should work for test

* fix dt

* re-resize ram

firmware/config/boards/microrusefi/board_configuration.cpp

@@ -130,8 +130,6 @@ static void setupEtb() {
 
 	// we only have pwm/dir, no dira/dirb
 	engineConfiguration->etb_use_two_wires = false;
-
-	engineConfiguration->etbFreq = 800;
 }
 
 static void setupDefaultSensorInputs() {

firmware/config/boards/proteus/board_configuration.cpp

@@ -112,7 +112,6 @@ static void setupEtb() {
 
 	// we only have pwm/dir, no dira/dirb
 	engineConfiguration->etb_use_two_wires = false;
-	engineConfiguration->etbFreq = 800;
 }
 
 static void setupDefaultSensorInputs() {

firmware/config/boards/skeleton/board_configuration.cpp

@@ -69,8 +69,6 @@ static void setupTle9201Etb() {
 	engineConfiguration->etbIo[0].controlPin1 = GPIOC_7;
 	engineConfiguration->etbIo[0].directionPin1 = GPIOA_8;
 	engineConfiguration->etbIo[0].directionPin2 = GPIO_UNASSIGNED;
-	// PWM frequency needs to be configured to match the physical part
-	engineConfiguration->etbFreq = 800;
 }
 
 // Configure key sensors inputs.

firmware/controllers/actuators/electronic_throttle.cpp

@@ -133,10 +133,6 @@ void EtbController::showStatus(Logging* logger) {
 	m_pid.showPidStatus(logger, "ETB");
 }
 
-int EtbController::getPeriodMs() {
-	return GET_PERIOD_LIMITED(&engineConfiguration->etb);
-}
-
 expected<percent_t> EtbController::observePlant() const {
 	return Sensor::get(indexToTpsSensor(m_myIndex));
 }
@@ -291,7 +287,7 @@ expected<percent_t> EtbController::getClosedLoop(percent_t target, percent_t act
 		return getClosedLoopAutotune(actualThrottlePosition);
 	} else {
 		// Normal case - use PID to compute closed loop part
-		return m_pid.getOutput(target, actualThrottlePosition);
+		return m_pid.getOutput(target, actualThrottlePosition, 1.0f / ETB_LOOP_FREQUENCY);
 	}
 }
 
@@ -313,7 +309,7 @@ void EtbController::setOutput(expected<percent_t> outputValue) {
 	}
 }
 
-void EtbController::PeriodicTask() {
+void EtbController::update(efitick_t nowNt) {
 #if EFI_TUNER_STUDIO
 	// Only debug throttle #0
 	if (m_myIndex == 0) {
@@ -347,7 +343,7 @@ void EtbController::PeriodicTask() {
 		m_pid.showPidStatus(&logger, "ETB");
 	}
 
-	update();
+	ClosedLoopController::update();
 
 	DISPLAY_STATE(Engine)
 	DISPLAY_TEXT(Electronic_Throttle);
@@ -390,8 +386,23 @@ void EtbController::PeriodicTask() {
 /* DISPLAY_ENDIF */
 }
 
+#if !EFI_UNIT_TEST
+#include "periodic_thread_controller.h"
+struct EtbImpl final : public EtbController, public PeriodicController<512> {
+	EtbImpl() : PeriodicController("ETB", NORMALPRIO + 3, ETB_LOOP_FREQUENCY) {}
+
+	void PeriodicTask(efitick_t nowNt) override {
+		EtbController::update(nowNt);
+	}
+
+	void start() override {
+		Start();
+	}
+};
+
 // real implementation (we mock for some unit tests)
-EtbController etbControllers[ETB_COUNT];
+EtbImpl etbControllers[ETB_COUNT];
+#endif
 
 static void showEthInfo(void) {
 #if EFI_PROD_CODE
@@ -531,27 +542,31 @@ void setDefaultEtbParameters(DECLARE_CONFIG_PARAMETER_SIGNATURE) {
 		}
 	}
 
+	engineConfiguration->etbFreq = DEFAULT_ETB_PWM_FREQUENCY;
 
-	engineConfiguration->throttlePedalUpVoltage = 0; // that's voltage, not ADC like with TPS
-	engineConfiguration->throttlePedalWOTVoltage = 6; // that's voltage, not ADC like with TPS
+	// voltage, not ADC like with TPS
+	engineConfiguration->throttlePedalUpVoltage = 0;
+	engineConfiguration->throttlePedalWOTVoltage = 5;
 
 	engineConfiguration->etb = {
 		1,		// Kp
 		10,		// Ki
 		0.05,	// Kd
 		0,		// offset
-		(1000 / DEFAULT_ETB_LOOP_FREQUENCY),
+		0,		// Update rate, unused
 		-100, 100 // min/max
 	};
 
-	engineConfiguration->etb_iTermMin = -300;
-	engineConfiguration->etb_iTermMax = 300;
+	engineConfiguration->etb_iTermMin = -30;
+	engineConfiguration->etb_iTermMax = 30;
 }
 
 void onConfigurationChangeElectronicThrottleCallback(engine_configuration_s *previousConfiguration) {
+#if !EFI_UNIT_TEST
 	for (int i = 0; i < ETB_COUNT; i++) {
 		etbControllers[i].onConfigurationChange(&previousConfiguration->etb);
 	}
+#endif
 }
 
 #if EFI_PROD_CODE && 0
@@ -648,7 +663,7 @@ void doInitElectronicThrottle(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
 	etbPidReset(PASS_ENGINE_PARAMETER_SIGNATURE);
 
 	for (int i = 0 ; i < engine->etbActualCount; i++) {
-		engine->etbControllers[i]->Start();
+		engine->etbControllers[i]->start();
 	}
 }
 
@@ -657,9 +672,11 @@ void initElectronicThrottle(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
 		return;
 	}
 
+#if !EFI_UNIT_TEST
 	for (int i = 0; i < ETB_COUNT; i++) {
 		engine->etbControllers[i] = &etbControllers[i];
 	}
+#endif
 
 	doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE);
 }

firmware/controllers/actuators/electronic_throttle.h

@@ -8,34 +8,33 @@
 #pragma once
 
 // https://en.wikipedia.org/wiki/Nyquist%E2%80%93Shannon_sampling_theorem
-#define DEFAULT_ETB_LOOP_FREQUENCY 200
-#define DEFAULT_ETB_PWM_FREQUENCY 300
+#define ETB_LOOP_FREQUENCY 500
+#define DEFAULT_ETB_PWM_FREQUENCY 800
 
 #include "engine.h"
 #include "closed_loop_controller.h"
 #include "expected.h"
-#include "periodic_task.h"
 
 class DcMotor;
 class Logging;
 
-class IEtbController : public PeriodicTimerController, public ClosedLoopController<percent_t, percent_t> {
+class IEtbController : public ClosedLoopController<percent_t, percent_t> {
 public:
 	DECLARE_ENGINE_PTR;
 	virtual void init(DcMotor *motor, int ownIndex, pid_s *pidParameters, const ValueProvider3D* pedalMap) = 0;
 	virtual void reset() = 0;
 	virtual void setIdlePosition(percent_t pos) = 0;
+	virtual void start() = 0;
 };
 
-class EtbController final : public IEtbController {
+class EtbController : public IEtbController {
 public:
 	void init(DcMotor *motor, int ownIndex, pid_s *pidParameters, const ValueProvider3D* pedalMap) override;
 	void setIdlePosition(percent_t pos) override;
-
-	// PeriodicTimerController implementation
-	int getPeriodMs() override;
-	void PeriodicTask() override;
 	void reset() override;
+	void start() override {}
+
+	void update(efitick_t nowNt);
 
 	// Called when the configuration may have changed.  Controller will
 	// reset if necessary.

firmware/controllers/engine_controller.cpp

@@ -732,7 +732,7 @@ void initEngineContoller(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX)
 // help to notice when RAM usage goes up - if a code change adds to RAM usage these variables would fail
 // linking process which is the way to raise the alarm
 #ifndef RAM_UNUSED_SIZE
-#define RAM_UNUSED_SIZE 16000
+#define RAM_UNUSED_SIZE 14500
 #endif
 #ifndef CCM_UNUSED_SIZE
 #define CCM_UNUSED_SIZE 2900

firmware/hw_layer/adc_inputs.cpp

@@ -398,7 +398,7 @@ int getSlowAdcCounter() {
 class SlowAdcController : public PeriodicController<256> {
 public:
 	SlowAdcController() 
-		: PeriodicController("ADC", NORMALPRIO + 5, 200)
+		: PeriodicController("ADC", NORMALPRIO + 5, 500)
 	{
 	}
 

firmware/tunerstudio/rusefi.input

@@ -2781,7 +2781,6 @@ cmd_set_engine_type_default					= "w\x00\x31\x00\x00"
 		field = "pFactor",								etb_pFactor, {throttlePedalPositionAdcChannel != 16}
 		field = "iFactor",								etb_iFactor, {throttlePedalPositionAdcChannel != 16}
 		field = "dFactor",								etb_dFactor, {throttlePedalPositionAdcChannel != 16}
-		field = "control period",						etb_periodMs, {throttlePedalPositionAdcChannel != 16}
 		field = "pid min",								etb_minValue, {throttlePedalPositionAdcChannel != 16}
 		field = "pid max",								etb_maxValue, {throttlePedalPositionAdcChannel != 16}
 		field = "iTermMin",								etb_iTermMin, {throttlePedalPositionAdcChannel != 16}

unit_tests/mocks.h

@@ -7,13 +7,9 @@
 
 class MockEtb : public IEtbController {
 public:
-	// PeriodicTimerController mocks
-	MOCK_METHOD(void, PeriodicTask, (), (override));
-	MOCK_METHOD(int, getPeriodMs, (), (override));
-
 	// IEtbController mocks
 	MOCK_METHOD(void, reset, (), ());
-	MOCK_METHOD(void, Start, (), (override));
+	MOCK_METHOD(void, start, (), (override));
 	MOCK_METHOD(void, init, (DcMotor* motor, int ownIndex, pid_s* pidParameters, const ValueProvider3D* pedalMap), (override));
 	MOCK_METHOD(void, setIdlePosition, (percent_t pos), (override));
 

unit_tests/tests/test_etb.cpp

@@ -47,7 +47,7 @@ TEST(etb, initializationSingleThrottle) {
 	// Expect mock0 to be init with index 0, and PID params
 	EXPECT_CALL(mocks[0], init(_, 0, &engineConfiguration->etb, Ne(nullptr)));
 	EXPECT_CALL(mocks[0], reset);
-	EXPECT_CALL(mocks[0], Start);
+	EXPECT_CALL(mocks[0], start);
 
 	// We do not expect throttle #2 to be initialized
 
@@ -72,12 +72,12 @@ TEST(etb, initializationDualThrottle) {
 	// Expect mock0 to be init with index 0, and PID params
 	EXPECT_CALL(mocks[0], init(_, 0, &engineConfiguration->etb, Ne(nullptr)));
 	EXPECT_CALL(mocks[0], reset);
-	EXPECT_CALL(mocks[0], Start);
+	EXPECT_CALL(mocks[0], start);
 
 	// Expect mock1 to be init with index 2, and PID params
 	EXPECT_CALL(mocks[1], init(_, 1, &engineConfiguration->etb, Ne(nullptr)));
 	EXPECT_CALL(mocks[1], reset);
-	EXPECT_CALL(mocks[1], Start);
+	EXPECT_CALL(mocks[1], start);
 
 	doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE);
 }