deep rabbit holes are the best ones!

refactoring: encapsulation

firmware/controllers/algo/engine.cpp

@@ -100,14 +100,6 @@ trigger_type_e getVvtTriggerType(vvt_mode_e vvtMode) {
 	}
 }
 
-static operation_mode_e lookupOperationMode() {
-	if (engineConfiguration->twoStroke) {
-		return TWO_STROKE;
-	} else {
-		return engineConfiguration->skippedWheelOnCam ? FOUR_STROKE_CAM_SENSOR : FOUR_STROKE_CRANK_SENSOR;
-	}
-}
-
 static void initVvtShape(TriggerWaveform& shape, const TriggerConfiguration& config, TriggerDecoderBase &initState) {
 	shape.initializeTriggerWaveform(FOUR_STROKE_CAM_SENSOR, config);
 
@@ -118,16 +110,16 @@ void Engine::updateTriggerWaveform() {
 	static TriggerDecoderBase initState("init");
 
 	// Re-read config in case it's changed
-	primaryTriggerConfiguration.update();
+	triggerCentral.primaryTriggerConfiguration.update();
 	for (int camIndex = 0;camIndex < CAMS_PER_BANK;camIndex++) {
-		vvtTriggerConfiguration[camIndex].update();
+		triggerCentral.vvtTriggerConfiguration[camIndex].update();
 	}
 
 #if EFI_ENGINE_CONTROL && EFI_SHAFT_POSITION_INPUT
 	// we have a confusing threading model so some synchronization would not hurt
 	chibios_rt::CriticalSectionLocker csl;
 
-	TRIGGER_WAVEFORM(initializeTriggerWaveform(lookupOperationMode(), primaryTriggerConfiguration));
+	triggerCentral.triggerShape.initializeTriggerWaveform(lookupOperationMode(), triggerCentral.primaryTriggerConfiguration);
 
 	/**
 	 * this is only useful while troubleshooting a new trigger shape in the field
@@ -166,7 +158,7 @@ void Engine::updateTriggerWaveform() {
 		if (engineConfiguration->vvtMode[camIndex] != VVT_INACTIVE) {
 			initVvtShape(
 				triggerCentral.vvtShape[camIndex],
-				vvtTriggerConfiguration[camIndex],
+				triggerCentral.vvtTriggerConfiguration[camIndex],
 				initState
 			);
 		}
@@ -193,9 +185,9 @@ void Engine::periodicSlowCallback() {
 	ScopePerf perf(PE::EnginePeriodicSlowCallback);
 
 	// Re-read config in case it's changed
-	primaryTriggerConfiguration.update();
+	triggerCentral.primaryTriggerConfiguration.update();
 	for (int camIndex = 0;camIndex < CAMS_PER_BANK;camIndex++) {
-		vvtTriggerConfiguration[camIndex].update();
+		triggerCentral.vvtTriggerConfiguration[camIndex].update();
 	}
 
 	efiWatchdog();
@@ -419,9 +411,9 @@ void Engine::OnTriggerSyncronization(bool wasSynchronized, bool isDecodingError)
 #endif
 
 void Engine::injectEngineReferences() {
-	primaryTriggerConfiguration.update();
+	triggerCentral.primaryTriggerConfiguration.update();
 	for (int camIndex = 0;camIndex < CAMS_PER_BANK;camIndex++) {
-		vvtTriggerConfiguration[camIndex].update();
+		triggerCentral.vvtTriggerConfiguration[camIndex].update();
 	}
 }
 
@@ -568,23 +560,6 @@ injection_mode_e getCurrentInjectionMode() {
 	return getEngineRotationState()->isCranking() ? engineConfiguration->crankingInjectionMode : engineConfiguration->injectionMode;
 }
 
-// see also in TunerStudio project '[doesTriggerImplyOperationMode] tag
-// this is related to 'knownOperationMode' flag
-static bool doesTriggerImplyOperationMode(trigger_type_e type) {
-	switch (type) {
-		case TT_TOOTHED_WHEEL:
-		case TT_ONE:
-		case TT_3_1_CAM:
-		case TT_36_2_2_2:	// TODO: should this one be in this list?
-		case TT_TOOTHED_WHEEL_60_2:
-		case TT_TOOTHED_WHEEL_36_1:
-			// These modes could be either cam or crank speed
-			return false;
-		default:
-			return true;
-	}
-}
-
 /**
  * The idea of this method is to execute all heavy calculations in a lower-priority thread,
  * so that trigger event handler/IO scheduler tasks are faster.

firmware/controllers/algo/engine.h

@@ -91,29 +91,6 @@ class IEtbController;
 
 struct IIdleController;
 
-class PrimaryTriggerConfiguration final : public TriggerConfiguration {
-public:
-	PrimaryTriggerConfiguration() : TriggerConfiguration("TRG ") {}
-
-protected:
-	bool isUseOnlyRisingEdgeForTrigger() const override;
-	bool isVerboseTriggerSynchDetails() const override;
-	trigger_config_s getType() const override;
-};
-
-class VvtTriggerConfiguration final : public TriggerConfiguration {
-public:
-	const int index;
-
-	VvtTriggerConfiguration(const char * prefix, const int index) : TriggerConfiguration(prefix), index(index) {
-	}
-
-protected:
-	bool isUseOnlyRisingEdgeForTrigger() const override;
-	bool isVerboseTriggerSynchDetails() const override;
-	trigger_config_s getType() const override;
-};
-
 class Engine final : public TriggerStateListener {
 public:
 	Engine();
@@ -200,13 +177,6 @@ public:
 	FanControl1 fan1;
 	FanControl2 fan2;
 
-	PrimaryTriggerConfiguration primaryTriggerConfiguration;
-#if CAMS_PER_BANK == 1
-	VvtTriggerConfiguration vvtTriggerConfiguration[CAMS_PER_BANK] = {{"VVT1 ", 0}};
-#else
-	VvtTriggerConfiguration vvtTriggerConfiguration[CAMS_PER_BANK] = {{"VVT1 ", 0}, {"VVT2 ", 1}};
-#endif
-
 	efitick_t startStopStateLastPushTime = 0;
 
 #if EFI_SHAFT_POSITION_INPUT

firmware/controllers/engine_cycle/rpm_calculator.cpp

@@ -101,6 +101,31 @@ bool RpmCalculator::checkIfSpinning(efitick_t nowNt) const {
 	return true;
 }
 
+// see also in TunerStudio project '[doesTriggerImplyOperationMode] tag
+// this is related to 'knownOperationMode' flag
+static bool doesTriggerImplyOperationMode(trigger_type_e type) {
+	switch (type) {
+		case TT_TOOTHED_WHEEL:
+		case TT_ONE:
+		case TT_3_1_CAM:
+		case TT_36_2_2_2:	// TODO: should this one be in this list?
+		case TT_TOOTHED_WHEEL_60_2:
+		case TT_TOOTHED_WHEEL_36_1:
+			// These modes could be either cam or crank speed
+			return false;
+		default:
+			return true;
+	}
+}
+
+operation_mode_e lookupOperationMode() {
+	if (engineConfiguration->twoStroke) {
+		return TWO_STROKE;
+	} else {
+		return engineConfiguration->skippedWheelOnCam ? FOUR_STROKE_CAM_SENSOR : FOUR_STROKE_CRANK_SENSOR;
+	}
+}
+
 // todo: move to triggerCentral/triggerShape since has nothing to do with rotation state!
 operation_mode_e RpmCalculator::getOperationMode() const {
 	// Ignore user-provided setting for well known triggers.

firmware/controllers/engine_cycle/rpm_calculator.h

@@ -178,6 +178,7 @@ void tdcMarkCallback(
  * @brief   Initialize RPM calculator
  */
 void initRpmCalculator();
+operation_mode_e lookupOperationMode();
 
 #define getRevolutionCounter() (engine->rpmCalculator.getRevolutionCounterM())
 

firmware/controllers/trigger/trigger_central.cpp

@@ -209,7 +209,7 @@ static void logFront(bool isImportantFront, efitick_t nowNt, int index) {
 #if EFI_PROD_CODE
 		writePad("cam debug", engineConfiguration->camInputsDebug[index], 1);
 #endif /* EFI_PROD_CODE */
-		engine->executor.scheduleByTimestampNt("dbg_on", &debugToggleScheduling, nowNt + DEBUG_PIN_DELAY, &turnOffAllDebugFields);
+		getExecutorInterface()->scheduleByTimestampNt("dbg_on", &debugToggleScheduling, nowNt + DEBUG_PIN_DELAY, &turnOffAllDebugFields);
 	}
 
 	if (engineConfiguration->displayLogicLevelsInEngineSniffer && isImportantFront) {
@@ -301,7 +301,7 @@ void hwHandleVvtCamSignal(TriggerValue front, efitick_t nowNt, int index) {
 				"vvt",
 			tc->vvtShape[camIndex],
 			nullptr,
-			engine->vvtTriggerConfiguration[camIndex],
+			engine->triggerCentral.vvtTriggerConfiguration[camIndex],
 			front == TriggerValue::RISE ? SHAFT_PRIMARY_RISING : SHAFT_PRIMARY_FALLING, nowNt);
 		// yes we log data from all VVT channels into same fields for now
 		tc->triggerState.vvtSyncGapRatio = vvtDecoder.triggerSyncGapRatio;
@@ -471,7 +471,7 @@ void handleShaftSignal(int signalIndex, bool isRising, efitick_t timestamp) {
 	// for effective noise filtering, we need both signal edges, 
 	// so we pass them to handleShaftSignal() and defer this test
 	if (!engineConfiguration->useNoiselessTriggerDecoder) {
-		if (!isUsefulSignal(signal, engine->primaryTriggerConfiguration)) {
+		if (!isUsefulSignal(signal, engine->triggerCentral.primaryTriggerConfiguration)) {
 			/**
 			 * no need to process VR falls further
 			 */
@@ -660,7 +660,7 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta
 		if (!noiseFilter.noiseFilter(timestamp, &triggerState, signal)) {
 			return;
 		}
-		if (!isUsefulSignal(signal, engine->primaryTriggerConfiguration)) {
+		if (!isUsefulSignal(signal, primaryTriggerConfiguration)) {
 			return;
 		}
 	}
@@ -678,7 +678,7 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta
 			"trigger",
 			triggerShape,
 			engine,
-			engine->primaryTriggerConfiguration,
+			primaryTriggerConfiguration,
 			signal, timestamp);
 
 	// Don't propagate state if we don't know where we are

firmware/controllers/trigger/trigger_central.h

@@ -52,6 +52,13 @@ public:
 	void resetCounters();
 	void validateCamVvtCounters();
 
+	PrimaryTriggerConfiguration primaryTriggerConfiguration;
+#if CAMS_PER_BANK == 1
+	VvtTriggerConfiguration vvtTriggerConfiguration[CAMS_PER_BANK] = {{"VVT1 ", 0}};
+#else
+	VvtTriggerConfiguration vvtTriggerConfiguration[CAMS_PER_BANK] = {{"VVT1 ", 0}, {"VVT2 ", 1}};
+#endif
+
 	LocalVersionHolder triggerVersion;
 
 	angle_t mapCamPrevToothAngle = -1;

firmware/controllers/trigger/trigger_decoder.cpp

@@ -125,7 +125,7 @@ void calculateTriggerSynchPoint(
 	efiAssertVoid(CUSTOM_TRIGGER_STACK, getCurrentRemainingStack() > EXPECTED_REMAINING_STACK, "calc s");
 #endif
 	engine->triggerErrorDetection.clear();
-	shape.initializeSyncPoint(state, engine->primaryTriggerConfiguration);
+	shape.initializeSyncPoint(state, engine->triggerCentral.primaryTriggerConfiguration);
 
 	int length = shape.getLength();
 	engine->engineCycleEventCount = length;

firmware/controllers/trigger/trigger_decoder.h

@@ -41,6 +41,29 @@ protected:
 	virtual trigger_config_s getType() const = 0;
 };
 
+class PrimaryTriggerConfiguration final : public TriggerConfiguration {
+public:
+	PrimaryTriggerConfiguration() : TriggerConfiguration("TRG ") {}
+
+protected:
+	bool isUseOnlyRisingEdgeForTrigger() const override;
+	bool isVerboseTriggerSynchDetails() const override;
+	trigger_config_s getType() const override;
+};
+
+class VvtTriggerConfiguration final : public TriggerConfiguration {
+public:
+	const int index;
+
+	VvtTriggerConfiguration(const char * prefix, const int index) : TriggerConfiguration(prefix), index(index) {
+	}
+
+protected:
+	bool isUseOnlyRisingEdgeForTrigger() const override;
+	bool isVerboseTriggerSynchDetails() const override;
+	trigger_config_s getType() const override;
+};
+
 typedef struct {
 	/**
 	 * index within trigger revolution, from 0 to trigger event count

unit_tests/tests/trigger/test_trigger_decoder.cpp

@@ -39,7 +39,7 @@ static int getTriggerZeroEventIndex(engine_type_e engineType) {
 
 	initDataStructures();
 
-	const auto& triggerConfiguration = engine->primaryTriggerConfiguration;
+	const auto& triggerConfiguration = engine->triggerCentral.primaryTriggerConfiguration;
 
 	TriggerWaveform& shape = eth.engine.triggerCentral.triggerShape;
 	return eth.engine.triggerCentral.triggerState.findTriggerZeroEventIndex(shape, triggerConfiguration);
@@ -114,7 +114,7 @@ TEST(trigger, testSomethingWeird) {
 	TriggerDecoderBase state_("test");
 	TriggerDecoderBase *sta = &state_;
 
-	const auto& triggerConfiguration = engine->primaryTriggerConfiguration;
+	const auto& triggerConfiguration = engine->triggerCentral.primaryTriggerConfiguration;
 
 
 	ASSERT_FALSE(sta->shaft_is_synchronized) << "shaft_is_synchronized";