trigger decode cleanup and perf (#1853)

* trigger decode cleanup

* test config ptr patchup

* disable CDM by default

firmware/config/stm32f4ems/efifeatures.h

@@ -18,7 +18,7 @@
 #define EFI_FSIO TRUE
 
 #ifndef EFI_CDM_INTEGRATION
-#define EFI_CDM_INTEGRATION TRUE
+#define EFI_CDM_INTEGRATION FALSE
 #endif
 
 #ifndef EFI_TOOTH_LOGGER

firmware/controllers/algo/engine.cpp

@@ -94,6 +94,7 @@ trigger_type_e getVvtTriggerType(vvt_mode_e vvtMode) {
 
 void Engine::initializeTriggerWaveform(Logging *logger DECLARE_ENGINE_PARAMETER_SUFFIX) {
 	static TriggerState initState;
+	INJECT_ENGINE_REFERENCE(&initState);
 
 #if EFI_ENGINE_CONTROL && EFI_SHAFT_POSITION_INPUT
 	// we have a confusing threading model so some synchronization would not hurt
@@ -245,11 +246,11 @@ void Engine::onTriggerSignalEvent(efitick_t nowNt) {
 	lastTriggerToothEventTimeNt = nowNt;
 }
 
-Engine::Engine() : primaryTriggerConfiguration(this), vvtTriggerConfiguration(this) {
+Engine::Engine() {
 	reset();
 }
 
-Engine::Engine(persistent_config_s *config) : primaryTriggerConfiguration(this), vvtTriggerConfiguration(this) {
+Engine::Engine(persistent_config_s *config) {
 	setConfig(config);
 	reset();
 }
@@ -394,10 +395,21 @@ void Engine::OnTriggerSyncronization(bool wasSynchronized) {
 }
 #endif
 
+void Engine::injectEngineReferences() {
+	Engine *engine = this;
+	EXPAND_Engine;
+
+	INJECT_ENGINE_REFERENCE(&primaryTriggerConfiguration);
+	INJECT_ENGINE_REFERENCE(&vvtTriggerConfiguration);
+	triggerCentral.init(PASS_ENGINE_PARAMETER_SIGNATURE);
+}
+
 void Engine::setConfig(persistent_config_s *config) {
 	this->config = config;
 	engineConfigurationPtr = &config->engineConfiguration;
 	memset(config, 0, sizeof(persistent_config_s));
+
+	injectEngineReferences();
 }
 
 void Engine::printKnockState(void) {

firmware/controllers/algo/engine.h

@@ -54,33 +54,25 @@ class IEtbController;
 class IFuelComputer;
 class IInjectorModel;
 
-class PrimaryTriggerConfiguration : public TriggerConfiguration {
+class PrimaryTriggerConfiguration final : public TriggerConfiguration {
 public:
-	PrimaryTriggerConfiguration(Engine *engine);
 	bool isUseOnlyRisingEdgeForTrigger() const;
 	const char * getPrintPrefix() const;
-	bool isSilentTriggerError() const;
 	bool isVerboseTriggerSynchDetails() const;
 	debug_mode_e getDebugMode() const;
 	trigger_type_e getType() const;
-private:
-	Engine *engine;
 };
 
-class VvtTriggerConfiguration : public TriggerConfiguration {
+class VvtTriggerConfiguration final : public TriggerConfiguration {
 public:
-	VvtTriggerConfiguration(Engine *engine);
 	bool isUseOnlyRisingEdgeForTrigger() const;
 	const char * getPrintPrefix() const;
-	bool isSilentTriggerError() const;
 	bool isVerboseTriggerSynchDetails() const;
 	debug_mode_e getDebugMode() const;
 	trigger_type_e getType() const;
-private:
-	Engine *engine;
 };
 
-class Engine : public TriggerStateListener {
+class Engine final : public TriggerStateListener {
 public:
 	explicit Engine(persistent_config_s *config);
 	Engine();
@@ -398,6 +390,8 @@ private:
 	 */
 	bool isSpinning = false;
 	void reset();
+
+	void injectEngineReferences();
 };
 
 void prepareShapes(DECLARE_ENGINE_PARAMETER_SIGNATURE);

firmware/controllers/algo/engine2.cpp

@@ -251,24 +251,12 @@ void printCurrentState(Logging *logging, int seconds, const char *engineTypeName
 			DELIMETER);
 }
 
-PrimaryTriggerConfiguration::PrimaryTriggerConfiguration(Engine *engine) {
-	this->engine = engine;
-}
-
 bool PrimaryTriggerConfiguration::isUseOnlyRisingEdgeForTrigger() const {
-	return engine->engineConfigurationPtr->useOnlyRisingEdgeForTrigger;
+	return CONFIG(useOnlyRisingEdgeForTrigger);
-}
-
-debug_mode_e PrimaryTriggerConfiguration::getDebugMode() const {
-	return engine->engineConfigurationPtr->debugMode;
 }
 
 trigger_type_e PrimaryTriggerConfiguration::getType() const {
-	return engine->engineConfigurationPtr->trigger.type;
+	return CONFIG(trigger.type);
-}
-
-bool PrimaryTriggerConfiguration::isSilentTriggerError() const {
-	return engine->engineConfigurationPtr->silentTriggerError;
 }
 
 const char * PrimaryTriggerConfiguration::getPrintPrefix() const {
@@ -276,33 +264,21 @@ const char * PrimaryTriggerConfiguration::getPrintPrefix() const {
 }
 
 bool PrimaryTriggerConfiguration::isVerboseTriggerSynchDetails() const {
-	return engine->engineConfigurationPtr->verboseTriggerSynchDetails;
+	return CONFIG(verboseTriggerSynchDetails);
-}
-
-VvtTriggerConfiguration::VvtTriggerConfiguration(Engine *engine) {
-	this->engine = engine;
 }
 
 bool VvtTriggerConfiguration::isUseOnlyRisingEdgeForTrigger() const {
-	return engine->engineConfigurationPtr->vvtCamSensorUseRise;
+	return CONFIG(vvtCamSensorUseRise);
 }
 
 const char * VvtTriggerConfiguration::getPrintPrefix() const {
 	return "VVT ";
 }
 
-debug_mode_e VvtTriggerConfiguration::getDebugMode() const {
-	return engine->engineConfigurationPtr->debugMode;
-}
-
 trigger_type_e VvtTriggerConfiguration::getType() const {
 	return engine->triggerCentral.vvtTriggerType;
 }
 
-bool VvtTriggerConfiguration::isSilentTriggerError() const {
-	return engine->engineConfigurationPtr->silentTriggerError;
-}
-
 bool VvtTriggerConfiguration::isVerboseTriggerSynchDetails() const {
-	return engine->engineConfigurationPtr->verboseVVTDecoding;
+	return CONFIG(verboseVVTDecoding);
 }

firmware/controllers/trigger/trigger_central.cpp

@@ -51,6 +51,11 @@ TriggerCentral::TriggerCentral() : trigger_central_s() {
 	noiseFilter.resetAccumSignalData();
 }
 
+void TriggerCentral::init(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
+	INJECT_ENGINE_REFERENCE(&triggerState);
+	INJECT_ENGINE_REFERENCE(&vvtState);
+}
+
 void TriggerNoiseFilter::resetAccumSignalData() {
 	memset(lastSignalTimes, 0xff, sizeof(lastSignalTimes));	// = -1
 	memset(accumSignalPeriods, 0, sizeof(accumSignalPeriods));
@@ -429,8 +434,8 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta
 		if (!noiseFilter.noiseFilter(timestamp, &triggerState, signal PASS_ENGINE_PARAMETER_SUFFIX)) {
 			return;
 		}
-		const TriggerConfiguration * triggerConfiguration = &ENGINE(primaryTriggerConfiguration);
+		const TriggerConfiguration* triggerConfiguration = &ENGINE(primaryTriggerConfiguration);
-		// moved here from hwHandleShaftSignal()
+
 		if (!isUsefulSignal(signal, triggerConfiguration)) {
 			return;
 		}

firmware/controllers/trigger/trigger_central.h

@@ -34,9 +34,10 @@ public:
  * Probably not: we have an instance of TriggerState which is used for trigger initialization,
  * also composition probably better than inheritance here
  */
-class TriggerCentral : public trigger_central_s {
+class TriggerCentral final : public trigger_central_s {
 public:
 	TriggerCentral();
+	void init(DECLARE_ENGINE_PARAMETER_SIGNATURE);
 	void addEventListener(ShaftPositionListener handler, const char *name, Engine *engine);
 	void handleShaftSignal(trigger_event_e signal, efitick_t timestamp DECLARE_ENGINE_PARAMETER_SUFFIX);
 	int getHwEventCounter(int index) const;

firmware/controllers/trigger/trigger_decoder.cpp

@@ -400,10 +400,10 @@ void TriggerState::onShaftSynchronization(
  * @param nowNt current time
  */
 void TriggerState::decodeTriggerEvent(
-		const TriggerWaveform *triggerShape,
+		const TriggerWaveform* const triggerShape,
 		const TriggerStateCallback triggerCycleCallback,
-		TriggerStateListener * triggerStateListener,
+		TriggerStateListener* triggerStateListener,
-		const TriggerConfiguration * triggerConfiguration,
+		const TriggerConfiguration* const triggerConfiguration,
 		const trigger_event_e signal,
 		const efitick_t nowNt) {
 	ScopePerf perf(PE::DecodeTriggerEvent);
@@ -468,29 +468,18 @@ void TriggerState::decodeTriggerEvent(
 		nextTriggerEvent()
 		;
 	} else {
-
+#if !EFI_PROD_CODE
-#if EFI_UNIT_TEST
 		if (printTriggerTrace) {
 			printf("%s event %s %d\r\n",
 					getTrigger_type_e(triggerConfiguration->getType()),
 					getTrigger_event_e(signal),
 					nowNt);
-		}
-#endif /* EFI_UNIT_TEST */
-
-		isFirstEvent = false;
-// todo: skip a number of signal from the beginning
-
-#if EFI_PROD_CODE
-//	scheduleMsg(&logger, "from %.2f to %.2f %d %d", triggerConfig->syncRatioFrom, triggerConfig->syncRatioTo, toothDurations[0], shaftPositionState->toothDurations[1]);
-//	scheduleMsg(&logger, "ratio %.2f", 1.0 * toothDurations[0]/ shaftPositionState->toothDurations[1]);
-#else
-		if (printTriggerTrace) {
 			printf("decodeTriggerEvent ratio %.2f: current=%d previous=%d\r\n", 1.0 * toothDurations[0] / toothDurations[1],
 					toothDurations[0], toothDurations[1]);
 		}
 #endif
 
+		isFirstEvent = false;
 		bool isSynchronizationPoint;
 		bool wasSynchronized = shaft_is_synchronized;
 
@@ -521,10 +510,9 @@ void TriggerState::decodeTriggerEvent(
 		DISPLAY(DISPLAY_FIELD(vvtCamCounter));
 
 		if (triggerShape->isSynchronizationNeeded) {
-
 			currentGap = 1.0 * toothDurations[0] / toothDurations[1];
 
-			if (triggerConfiguration->getDebugMode() == DBG_TRIGGER_COUNTERS) {
+			if (CONFIG(debugMode) == DBG_TRIGGER_COUNTERS) {
 #if EFI_TUNER_STUDIO
 				tsOutputChannels.debugFloatField6 = currentGap;
 				tsOutputChannels.debugIntField3 = currentCycle.current_index;
@@ -532,25 +520,24 @@ void TriggerState::decodeTriggerEvent(
 			}
 
 			bool isSync = true;
-			for (int i = 0;i<triggerShape->gapTrackingLength;i++) {
+			for (int i = 0; i < triggerShape->gapTrackingLength; i++) {
 				bool isGapCondition = cisnan(triggerShape->syncronizationRatioFrom[i]) || (toothDurations[i] > toothDurations[i + 1] * triggerShape->syncronizationRatioFrom[i]
 					&& toothDurations[i] < toothDurations[i + 1] * triggerShape->syncronizationRatioTo[i]);
 
 				isSync &= isGapCondition;
-
 			}
+
 			isSynchronizationPoint = isSync;
 			if (isSynchronizationPoint) {
-				enginePins.debugTriggerSync.setValue(1);
+				enginePins.debugTriggerSync.toggle();
 			}
 
-
 			/**
 			 * todo: technically we can afford detailed logging even with 60/2 as long as low RPM
 			 * todo: figure out exact threshold as a function of RPM and tooth count?
 			 * Open question what is 'triggerShape->getSize()' for 60/2 is it 58 or 58*2 or 58*4?
 			 */
-			bool silentTriggerError = triggerShape->getSize() > 40 && triggerConfiguration->isSilentTriggerError();
+			bool silentTriggerError = triggerShape->getSize() > 40 && CONFIG(silentTriggerError);
 
 #if EFI_UNIT_TEST
 			actualSynchGap = 1.0 * toothDurations[0] / toothDurations[1];
@@ -595,11 +582,7 @@ void TriggerState::decodeTriggerEvent(
 							boolToString(someSortOfTriggerError));
 				}
 			}
-
-
 #endif /* EFI_PROD_CODE */
-			enginePins.debugTriggerSync.setValue(0);
-
 		} else {
 			/**
 			 * We are here in case of a wheel without synchronization - we just need to count events,
@@ -608,30 +591,20 @@ void TriggerState::decodeTriggerEvent(
 			 * in case of noise the counter could be above the expected number of events, that's why 'more or equals' and not just 'equals'
 			 */
 
-#if EFI_UNIT_TEST
-			if (printTriggerTrace) {
-				printf("decodeTriggerEvent sync=%d index=%d size=%d\r\n",
-					shaft_is_synchronized,
-					currentCycle.current_index,
-					triggerShape->getSize());
-			}
-#endif /* EFI_UNIT_TEST */
 			unsigned int endOfCycleIndex = triggerShape->getSize() - (triggerConfiguration->isUseOnlyRisingEdgeForTrigger() ? 2 : 1);
 
-
 			isSynchronizationPoint = !shaft_is_synchronized || (currentCycle.current_index >= endOfCycleIndex);
 
 #if EFI_UNIT_TEST
 			if (printTriggerTrace) {
-				printf("decodeTriggerEvent decodeTriggerEvent isSynchronizationPoint=%d index=%d size=%d\r\n",
+				printf("decodeTriggerEvent sync=%d isSynchronizationPoint=%d index=%d size=%d\r\n",
-						isSynchronizationPoint,
+					shaft_is_synchronized,
-						currentCycle.current_index,
+					isSynchronizationPoint,
-						triggerShape->getSize());
+					currentCycle.current_index,
+					triggerShape->getSize());
 			}
 #endif /* EFI_UNIT_TEST */
-
 		}
-
 #if EFI_UNIT_TEST
 		if (printTriggerTrace) {
 			printf("decodeTriggerEvent %s isSynchronizationPoint=%d index=%d %s\r\n",
@@ -642,7 +615,6 @@ void TriggerState::decodeTriggerEvent(
 #endif /* EFI_UNIT_TEST */
 
 		if (isSynchronizationPoint) {
-
 			if (triggerStateListener) {
 				triggerStateListener->OnTriggerSyncronization(wasSynchronized);
 			}

firmware/controllers/trigger/trigger_decoder.h

@@ -25,11 +25,11 @@ struct TriggerStateListener {
 
 class TriggerConfiguration {
 public:
+	DECLARE_ENGINE_PTR;
+
 	virtual bool isUseOnlyRisingEdgeForTrigger() const = 0;
-	virtual bool isSilentTriggerError() const = 0;
 	virtual bool isVerboseTriggerSynchDetails() const = 0;
 	virtual const char * getPrintPrefix() const = 0;
-	virtual debug_mode_e getDebugMode() const = 0;
 	virtual trigger_type_e getType() const = 0;
 };
 
@@ -65,6 +65,8 @@ typedef struct {
  */
 class TriggerState : public trigger_state_s {
 public:
+	DECLARE_ENGINE_PTR;
+
 	TriggerState();
 	/**
 	 * current trigger processing index, between zero and #size