Revert "Revert "Split ETB in to interface + implementation headers (#2040)""

This reverts commit 4f4bb957

firmware/console/binary/tunerstudio.cpp

@@ -262,7 +262,7 @@ static const void * getStructAddr(int structId) {
 		return static_cast<trigger_state_s*>(&engine->triggerCentral.triggerState);
 #if EFI_ELECTRONIC_THROTTLE_BODY
 	case LDS_ETB_PID_STATE_INDEX:
-		return static_cast<EtbController*>(engine->etbControllers[0])->getPidState();
+		return engine->etbControllers[0]->getPidState();
 #endif /* EFI_ELECTRONIC_THROTTLE_BODY */
 
 #ifndef EFI_IDLE_CONTROL

firmware/controllers/actuators/electronic_throttle.cpp

@@ -75,7 +75,8 @@
 
 #if EFI_ELECTRONIC_THROTTLE_BODY
 
-#include "electronic_throttle.h"
+#include "electronic_throttle_impl.h"
+#include "engine.h"
 #include "tps.h"
 #include "sensor.h"
 #include "dc_motor.h"

firmware/controllers/actuators/electronic_throttle.h

@@ -7,108 +7,9 @@
 
 #pragma once
 
-/**
+#include "engine_ptr.h"
- * Hard code ETB update speed.
- * Since this is a safety critical system with no real reason for a user to ever need to change the update rate,
- * it's locked to 500hz, along with the ADC.
- * https://en.wikipedia.org/wiki/Nyquist%E2%80%93Shannon_sampling_theorem
- */
-#define ETB_LOOP_FREQUENCY 500
-#define DEFAULT_ETB_PWM_FREQUENCY 800
-
-#include "engine.h"
 #include "closed_loop_controller.h"
-#include "expected.h"
+#include "rusefi_types.h"
-#include "sensor.h"
-
-class DcMotor;
-class Logging;
-
-class IEtbController : public ClosedLoopController<percent_t, percent_t> {
-public:
-	DECLARE_ENGINE_PTR;
-
-	// Initialize the throttle.
-	// returns true if the throttle was initialized, false otherwise.
-	virtual bool init(etb_function_e function, DcMotor *motor, pid_s *pidParameters, const ValueProvider3D* pedalMap) = 0;
-	virtual void reset() = 0;
-	virtual void setIdlePosition(percent_t pos) = 0;
-	virtual void setWastegatePosition(percent_t pos) = 0;
-	virtual void update() = 0;
-	virtual void autoCalibrateTps() = 0;
-};
-
-class EtbController : public IEtbController {
-public:
-	bool init(etb_function_e function, DcMotor *motor, pid_s *pidParameters, const ValueProvider3D* pedalMap) override;
-	void setIdlePosition(percent_t pos) override;
-	void setWastegatePosition(percent_t pos) override;
-	void reset() override;
-
-	// Update the controller's state: read sensors, send output, etc
-	void update();
-
-	// Called when the configuration may have changed.  Controller will
-	// reset if necessary.
-	void onConfigurationChange(pid_s* previousConfiguration);
-	
-	// Print this throttle's status.
-	void showStatus(Logging* logger);
-
-	// Helpers for individual parts of throttle control
-	expected<percent_t> observePlant() const override;
-
-	expected<percent_t> getSetpoint() const override;
-	expected<percent_t> getSetpointEtb() const;
-	expected<percent_t> getSetpointWastegate() const;
-	expected<percent_t> getSetpointIdleValve() const;
-
-	expected<percent_t> getOpenLoop(percent_t target) const override;
-	expected<percent_t> getClosedLoop(percent_t setpoint, percent_t observation) override;
-	expected<percent_t> getClosedLoopAutotune(percent_t actualThrottlePosition);
-
-	void setOutput(expected<percent_t> outputValue) override;
-
-	// Used to inspect the internal PID controller's state
-	const pid_state_s* getPidState() const { return &m_pid; };
-
-	// Use the throttle to automatically calibrate the relevant throttle position sensor(s).
-	void autoCalibrateTps() override;
-
-protected:
-	// This is set if an automatic TPS calibration should be run
-	bool m_isAutocal = false;
-
-	etb_function_e getFunction() const { return m_function; }
-	DcMotor* getMotor() { return m_motor; }
-
-private:
-	etb_function_e m_function = ETB_None;
-	SensorType m_positionSensor = SensorType::Invalid;
-	DcMotor *m_motor = nullptr;
-	Pid m_pid;
-	bool m_shouldResetPid = false;
-
-	// Pedal -> target map
-	const ValueProvider3D* m_pedalMap = nullptr;
-
-	float m_idlePosition = 0;
-	float m_wastegatePosition = 0;
-
-	// Autotune helpers
-	bool m_lastIsPositive = false;
-	efitick_t m_cycleStartTime = 0;
-	float m_minCycleTps = 0;
-	float m_maxCycleTps = 0;
-	// Autotune measured parameters: gain and ultimate period
-	// These are set to correct order of magnitude starting points
-	// so we converge more quickly on the correct values
-	float m_a = 8;
-	float m_tu = 0.1f; 
-
-	uint8_t m_autotuneCounter = 0;
-	uint8_t m_autotuneCurrentParam = 0;
-};
 
 void initElectronicThrottle(DECLARE_ENGINE_PARAMETER_SIGNATURE);
 void doInitElectronicThrottle(DECLARE_ENGINE_PARAMETER_SIGNATURE);
@@ -128,3 +29,24 @@ void onConfigurationChangeElectronicThrottleCallback(engine_configuration_s *pre
 void unregisterEtbPins();
 
 void etbAutocal(size_t throttleIndex);
+
+class DcMotor;
+struct pid_s;
+class ValueProvider3D;
+struct pid_state_s;
+
+class IEtbController : public ClosedLoopController<percent_t, percent_t> {
+public:
+	DECLARE_ENGINE_PTR;
+
+	// Initialize the throttle.
+	// returns true if the throttle was initialized, false otherwise.
+	virtual bool init(etb_function_e function, DcMotor *motor, pid_s *pidParameters, const ValueProvider3D* pedalMap) = 0;
+	virtual void reset() = 0;
+	virtual void setIdlePosition(percent_t pos) = 0;
+	virtual void setWastegatePosition(percent_t pos) = 0;
+	virtual void update() = 0;
+	virtual void autoCalibrateTps() = 0;
+
+	virtual const pid_state_s* getPidState() const = 0;
+};

firmware/controllers/actuators/electronic_throttle_impl.h

@@ -0,0 +1,97 @@
+/**
+ * @file	electronic_throttle_impl.h
+ *
+ * @date Dec 7, 2013
+ * @author Andrey Belomutskiy, (c) 2012-2020
+ */
+
+#pragma once
+
+// include the "public" ETB interface
+#include "electronic_throttle.h"
+
+#include "sensor.h"
+#include "pid.h"
+
+/**
+ * Hard code ETB update speed.
+ * Since this is a safety critical system with no real reason for a user to ever need to change the update rate,
+ * it's locked to 500hz, along with the ADC.
+ * https://en.wikipedia.org/wiki/Nyquist%E2%80%93Shannon_sampling_theorem
+ */
+#define ETB_LOOP_FREQUENCY 500
+#define DEFAULT_ETB_PWM_FREQUENCY 800
+
+class Logging;
+
+class EtbController : public IEtbController {
+public:
+	bool init(etb_function_e function, DcMotor *motor, pid_s *pidParameters, const ValueProvider3D* pedalMap) override;
+	void setIdlePosition(percent_t pos) override;
+	void setWastegatePosition(percent_t pos) override;
+	void reset() override;
+
+	// Update the controller's state: read sensors, send output, etc
+	void update();
+
+	// Called when the configuration may have changed.  Controller will
+	// reset if necessary.
+	void onConfigurationChange(pid_s* previousConfiguration);
+	
+	// Print this throttle's status.
+	void showStatus(Logging* logger);
+
+	// Helpers for individual parts of throttle control
+	expected<percent_t> observePlant() const override;
+
+	expected<percent_t> getSetpoint() const override;
+	expected<percent_t> getSetpointEtb() const;
+	expected<percent_t> getSetpointWastegate() const;
+	expected<percent_t> getSetpointIdleValve() const;
+
+	expected<percent_t> getOpenLoop(percent_t target) const override;
+	expected<percent_t> getClosedLoop(percent_t setpoint, percent_t observation) override;
+	expected<percent_t> getClosedLoopAutotune(percent_t actualThrottlePosition);
+
+	void setOutput(expected<percent_t> outputValue) override;
+
+	// Used to inspect the internal PID controller's state
+	const pid_state_s* getPidState() const override { return &m_pid; };
+
+	// Use the throttle to automatically calibrate the relevant throttle position sensor(s).
+	void autoCalibrateTps() override;
+
+protected:
+	// This is set if an automatic TPS calibration should be run
+	bool m_isAutocal = false;
+
+	etb_function_e getFunction() const { return m_function; }
+	DcMotor* getMotor() { return m_motor; }
+
+private:
+	etb_function_e m_function = ETB_None;
+	SensorType m_positionSensor = SensorType::Invalid;
+	DcMotor *m_motor = nullptr;
+	Pid m_pid;
+	bool m_shouldResetPid = false;
+
+	// Pedal -> target map
+	const ValueProvider3D* m_pedalMap = nullptr;
+
+	float m_idlePosition = 0;
+	float m_wastegatePosition = 0;
+
+	// Autotune helpers
+	bool m_lastIsPositive = false;
+	efitick_t m_cycleStartTime = 0;
+	float m_minCycleTps = 0;
+	float m_maxCycleTps = 0;
+	// Autotune measured parameters: gain and ultimate period
+	// These are set to correct order of magnitude starting points
+	// so we converge more quickly on the correct values
+	float m_a = 8;
+	float m_tu = 0.1f; 
+
+	uint8_t m_autotuneCounter = 0;
+	uint8_t m_autotuneCurrentParam = 0;
+};

unit_tests/mocks.h

@@ -17,6 +17,7 @@ public:
 	MOCK_METHOD(void, setIdlePosition, (percent_t pos), (override));
 	MOCK_METHOD(void, setWastegatePosition, (percent_t pos), (override));
 	MOCK_METHOD(void, autoCalibrateTps, (), (override));
+	MOCK_METHOD(const pid_state_s*, getPidState, (), (const, override));
 
 	// ClosedLoopController mocks
 	MOCK_METHOD(expected<percent_t>, getSetpoint, (), (const, override));

unit_tests/tests/test_etb.cpp

@@ -6,7 +6,7 @@
  */
 
 #include "engine_test_helper.h"
-#include "electronic_throttle.h"
+#include "electronic_throttle_impl.h"
 #include "dc_motor.h"
 #include "engine_controller.h"
 #include "sensor.h"
@@ -298,6 +298,9 @@ TEST(etb, setpointIdle) {
 TEST(etb, setpointNoPedalMap) {
 	EtbController etb;
 
+	// Must have TPS initialized for ETB setup
+	Sensor::setMockValue(SensorType::Tps1, 0.0f, true);
+
 	// Don't pass a pedal map
 	etb.init(ETB_Throttle1, nullptr, nullptr, nullptr);