Trigger cleanup init (#4222)

* extra parameter

* name decoders in constructor

* s

firmware/controllers/algo/engine.cpp

@@ -107,15 +107,13 @@ static operation_mode_e lookupOperationMode() {
 }
 
 static void initVvtShape(TriggerWaveform& shape, const TriggerConfiguration& config, TriggerDecoderBase &initState) {
-	shape.initializeTriggerWaveform(
+	shape.initializeTriggerWaveform(FOUR_STROKE_CAM_SENSOR, config);
-			lookupOperationMode(),
-			engineConfiguration->vvtCamSensorUseRise, config);
 
 	shape.initializeSyncPoint(initState, config);
 }
 
 void Engine::updateTriggerWaveform() {
-	static TriggerDecoderBase initState;
+	static TriggerDecoderBase initState("init");
 
 	// Re-read config in case it's changed
 	primaryTriggerConfiguration.update();
@@ -127,9 +125,7 @@ void Engine::updateTriggerWaveform() {
 	// we have a confusing threading model so some synchronization would not hurt
 	chibios_rt::CriticalSectionLocker csl;
 
-	TRIGGER_WAVEFORM(initializeTriggerWaveform(
+	TRIGGER_WAVEFORM(initializeTriggerWaveform(lookupOperationMode(), primaryTriggerConfiguration));
-			lookupOperationMode(),
-			engineConfiguration->useOnlyRisingEdgeForTrigger, primaryTriggerConfiguration));
 
 	/**
 	 * this is only useful while troubleshooting a new trigger shape in the field
@@ -160,15 +156,9 @@ void Engine::updateTriggerWaveform() {
 		calculateTriggerSynchPoint(engine->triggerCentral.triggerShape,
 				initState);
 
-		engine->triggerCentral.triggerState.name = "TRG";
 		engine->engineCycleEventCount = TRIGGER_WAVEFORM(getLength());
 	}
 
-	engine->triggerCentral.vvtState[0][0].name = "VVT B1 Int";
-	engine->triggerCentral.vvtState[0][1].name = "VVT B1 Exh";
-	engine->triggerCentral.vvtState[1][0].name = "VVT B2 Int";
-	engine->triggerCentral.vvtState[1][1].name = "VVT B2 Exh";
-
 	for (int camIndex = 0; camIndex < CAMS_PER_BANK; camIndex++) {
 		// todo: should 'vvtWithRealDecoder' be used here?
 		if (engineConfiguration->vvtMode[camIndex] != VVT_INACTIVE) {

firmware/controllers/trigger/decoders/trigger_structure.cpp

@@ -446,7 +446,7 @@ void TriggerWaveform::setThirdTriggerSynchronizationGap(float syncRatio) {
 /**
  * External logger is needed because at this point our logger is not yet initialized
  */
-void TriggerWaveform::initializeTriggerWaveform(operation_mode_e triggerOperationMode, bool useOnlyRisingEdgeForTrigger, const TriggerConfiguration& triggerConfig) {
+void TriggerWaveform::initializeTriggerWaveform(operation_mode_e triggerOperationMode, const TriggerConfiguration& triggerConfig) {
 
 #if EFI_PROD_CODE
 	efiAssertVoid(CUSTOM_ERR_6641, getCurrentRemainingStack() > EXPECTED_REMAINING_STACK, "init t");
@@ -455,6 +455,7 @@ void TriggerWaveform::initializeTriggerWaveform(operation_mode_e triggerOperatio
 
 	shapeDefinitionError = false;
 
+	bool useOnlyRisingEdgeForTrigger = triggerConfig.UseOnlyRisingEdgeForTrigger;
 	this->useOnlyRisingEdgeForTriggerTemp = useOnlyRisingEdgeForTrigger;
 
 	switch (triggerConfig.TriggerType.type) {

firmware/controllers/trigger/decoders/trigger_structure.h

@@ -73,8 +73,7 @@ class TriggerConfiguration;
 class TriggerWaveform {
 public:
 	TriggerWaveform();
-	void initializeTriggerWaveform(operation_mode_e triggerOperationMode,
+	void initializeTriggerWaveform(operation_mode_e triggerOperationMode, const TriggerConfiguration& triggerConfig);
-			bool useOnlyRisingEdgeForTrigger, const TriggerConfiguration& triggerConfig);
 	void setShapeDefinitionError(bool value);
 
 	/**

firmware/controllers/trigger/trigger_central.cpp

@@ -43,7 +43,8 @@ static scheduling_s debugToggleScheduling;
 TriggerCentral::TriggerCentral() :
 		vvtEventRiseCounter(),
 		vvtEventFallCounter(),
-		vvtPosition()
+		vvtPosition(),
+		triggerState("TRG")
 {
 	memset(&hwEventCounters, 0, sizeof(hwEventCounters));
 	triggerState.resetTriggerState();

firmware/controllers/trigger/trigger_central.h

@@ -113,7 +113,23 @@ public:
 
 	TriggerWaveform triggerShape;
 
-	VvtTriggerDecoder vvtState[BANKS_COUNT][CAMS_PER_BANK];
+	VvtTriggerDecoder vvtState[BANKS_COUNT][CAMS_PER_BANK] = {
+		{
+			"VVT B1 Int",
+#if CAMS_PER_BANK >= 2
+			"VVT B1 Exh"
+#endif
+		},
+#if BANKS_COUNT >= 2
+		{
+			"VVT B2 Int",
+#if CAMS_PER_BANK >= 2
+			"VVT B1 Exh"
+#endif
+		}
+#endif
+	};
+
 	TriggerWaveform vvtShape[CAMS_PER_BANK];
 
 	TriggerFormDetails triggerFormDetails;

firmware/controllers/trigger/trigger_decoder.cpp

@@ -38,7 +38,9 @@
 #include "sensor_chart.h"
 #endif
 
-TriggerDecoderBase::TriggerDecoderBase() {
+TriggerDecoderBase::TriggerDecoderBase(const char* name)
+	: name(name)
+{
 	resetTriggerState();
 }
 
@@ -90,10 +92,12 @@ void TriggerDecoderBase::resetCurrentCycleState() {
 
 #if EFI_SHAFT_POSITION_INPUT
 
-PrimaryTriggerDecoder::PrimaryTriggerDecoder() :
+PrimaryTriggerDecoder::PrimaryTriggerDecoder(const char* name)
-		//https://en.cppreference.com/w/cpp/language/zero_initialization
+	: TriggerDecoderBase(name)
-		timeOfLastEvent(), instantRpmValue()
+	//https://en.cppreference.com/w/cpp/language/zero_initialization
-		{
+	, timeOfLastEvent()
+	, instantRpmValue()
+{
 }
 
 #if ! EFI_PROD_CODE
@@ -343,7 +347,7 @@ static trigger_value_e eventType[6] = { TV_FALL, TV_RISE, TV_FALL, TV_RISE, TV_F
 
 #define nextTriggerEvent() \
  { \
-	if (triggerConfiguration.UseOnlyRisingEdgeForTrigger) {currentCycle.current_index++;} \
+	if (useOnlyRisingEdgeForTrigger) {currentCycle.current_index++;} \
 	currentCycle.current_index++; \
 	PRINT_INC_INDEX; \
 }

firmware/controllers/trigger/trigger_decoder.h

@@ -62,7 +62,7 @@ struct TriggerDecodeResult {
  */
 class TriggerDecoderBase : public trigger_state_s {
 public:
-	TriggerDecoderBase();
+	TriggerDecoderBase(const char* name);
 	/**
 	 * current trigger processing index, between zero and #size
 	 */
@@ -108,7 +108,7 @@ public:
 	efitick_t toothed_previous_time;
 
 	current_cycle_state_s currentCycle;
-	const char *name = nullptr;
+	const char* const name;
 
 	/**
 	 * how many times since ECU reboot we had unexpected number of teeth in trigger cycle
@@ -165,7 +165,7 @@ private:
  */
 class PrimaryTriggerDecoder : public TriggerDecoderBase {
 public:
-	PrimaryTriggerDecoder();
+	PrimaryTriggerDecoder(const char* name);
 	void resetTriggerState() override;
 
 	angle_t syncEnginePhase(int divider, int remainder, angle_t engineCycle);
@@ -223,7 +223,10 @@ private:
 	bool m_hasSynchronizedPhase = false;
 };
 
-class VvtTriggerDecoder : public TriggerDecoderBase { };
+class VvtTriggerDecoder : public TriggerDecoderBase {
+public:
+	VvtTriggerDecoder(const char* name) : TriggerDecoderBase(name) { }
+};
 
 angle_t getEngineCycle(operation_mode_e operationMode);
 

unit_tests/tests/trigger/test_trigger_decoder.cpp

@@ -64,7 +64,7 @@ static void testDodgeNeonDecoder() {
 	TriggerWaveform * shape = &eth.engine.triggerCentral.triggerShape;
 	ASSERT_EQ(8, shape->getTriggerWaveformSynchPointIndex());
 
-	TriggerDecoderBase state;
+	TriggerDecoderBase state("test");
 
 	ASSERT_FALSE(state.getShaftSynchronized()) << "1 shaft_is_synchronized";
 
@@ -111,7 +111,7 @@ static void assertTriggerPosition(event_trigger_position_s *position, int eventI
 TEST(trigger, testSomethingWeird) {
 	EngineTestHelper eth(FORD_INLINE_6_1995);
 
-	TriggerDecoderBase state_;
+	TriggerDecoderBase state_("test");
 	TriggerDecoderBase *sta = &state_;
 
 	const auto& triggerConfiguration = engine->primaryTriggerConfiguration;

unit_tests/tests/trigger/test_trigger_decoder_2.cpp

@@ -29,12 +29,14 @@ private:
 };
 
 struct MockTriggerDecoder : public TriggerDecoderBase {
+	MockTriggerDecoder() : TriggerDecoderBase("mock") { }
+
 	MOCK_METHOD(void, onTriggerError, (), (override));
 };
 
 static auto makeTriggerShape(operation_mode_e mode, const TriggerConfiguration& config) {
 	TriggerWaveform shape;
-	shape.initializeTriggerWaveform(mode, true, config);
+	shape.initializeTriggerWaveform(mode, config);
 
 	return shape;
 }