remove ignitionPin (#4608)

* remove ignitionPin

* now that's gone

* BMW test

firmware/controllers/algo/engine.cpp

@@ -270,7 +270,6 @@ void Engine::reset() {
 	 * it's important for fixAngle() that engineCycle field never has zero
 	 */
 	engineState.engineCycle = getEngineCycle(FOUR_STROKE_CRANK_SENSOR);
-	memset(&ignitionPin, 0, sizeof(ignitionPin));
 	resetLua();
 }
 

firmware/controllers/algo/engine.h

@@ -271,13 +271,6 @@ public:
 
 	void resetEngineSnifferIfInTestMode();
 
-	/**
-	 * pre-calculated reference to which output pin should be used for
-	 * given sequence index within engine cycle
-	 * todo: update documentation
-	 */
-	int ignitionPin[MAX_CYLINDER_COUNT];
-
 	EngineState engineState;
 	/**
 	 * idle blip is a development tool: alternator PID research for instance have benefited from a repetitive change of RPM

firmware/controllers/engine_cycle/rpm_calculator.cpp

@@ -249,10 +249,6 @@ void RpmCalculator::setSpinningUp(efitick_t nowNt) {
 	if (isSpinningUp()) {
 		engine->triggerCentral.triggerState.setLastEventTimeForInstantRpm(nowNt);
 	}
-	/**
-	 * Update ignition pin indices if needed. Here we potentially switch to wasted spark temporarily.
-	 */
-	prepareIgnitionPinIndices();
 }
 
 /**

firmware/controllers/engine_cycle/spark_logic.cpp

@@ -69,6 +69,31 @@ static void fireSparkBySettingPinLow(IgnitionEvent *event, IgnitionOutputPin *ou
 		} \
 }
 
+/**
+ * @param cylinderIndex from 0 to cylinderCount, not cylinder number
+ */
+static int getIgnitionPinForIndex(int cylinderIndex, ignition_mode_e ignitionMode) {
+	switch (ignitionMode) {
+	case IM_ONE_COIL:
+		return 0;
+	case IM_WASTED_SPARK: {
+		if (engineConfiguration->specs.cylindersCount == 1) {
+			// we do not want to divide by zero
+			return 0;
+		}
+		return cylinderIndex % (engineConfiguration->specs.cylindersCount / 2);
+	}
+	case IM_INDIVIDUAL_COILS:
+		return cylinderIndex;
+	case IM_TWO_COILS:
+		return cylinderIndex % 2;
+
+	default:
+		firmwareError(CUSTOM_OBD_IGNITION_MODE, "Invalid ignition mode getIgnitionPinForIndex(): %d", engineConfiguration->ignitionMode);
+		return 0;
+	}
+}
+
 static void prepareCylinderIgnitionSchedule(angle_t dwellAngleDuration, floatms_t sparkDwell, IgnitionEvent *event) {
 	// todo: clean up this implementation? does not look too nice as is.
 
@@ -84,7 +109,10 @@ static void prepareCylinderIgnitionSchedule(angle_t dwellAngleDuration, floatms_
 		+ engine->module<KnockController>()->getKnockRetard();
 
 	efiAssertVoid(CUSTOM_SPARK_ANGLE_1, !cisnan(sparkAngle), "sparkAngle#1");
-	const int index = engine->ignitionPin[event->cylinderIndex];
+
+	auto ignitionMode = getCurrentIgnitionMode();
+
+	const int index = getIgnitionPinForIndex(event->cylinderIndex, ignitionMode);
 	const int coilIndex = ID2INDEX(getCylinderId(index));
 	IgnitionOutputPin *output = &enginePins.coils[coilIndex];
 
@@ -94,7 +122,7 @@ static void prepareCylinderIgnitionSchedule(angle_t dwellAngleDuration, floatms_
 	//  - we are running wasted spark, and have "two wire" mode enabled
 	//  - We are running sequential mode, but we're cranking, so we should run in two wire wasted mode (not one wire wasted)
 	bool isTwoWireWasted = engineConfiguration->twoWireBatchIgnition || (engineConfiguration->ignitionMode == IM_INDIVIDUAL_COILS);
-	if (getCurrentIgnitionMode() == IM_WASTED_SPARK && isTwoWireWasted) {
+	if (ignitionMode == IM_WASTED_SPARK && isTwoWireWasted) {
 		int secondIndex = index + engineConfiguration->specs.cylindersCount / 2;
 		int secondCoilIndex = ID2INDEX(getCylinderId(secondIndex));
 		secondOutput = &enginePins.coils[secondCoilIndex];

firmware/controllers/math/engine_math.cpp

@@ -374,39 +374,6 @@ size_t getNextFiringCylinderId(size_t prevCylinderId) {
 	return 1;
 }
 
-/**
- * @param cylinderIndex from 0 to cylinderCount, not cylinder number
- */
-static int getIgnitionPinForIndex(int cylinderIndex) {
-	switch (getCurrentIgnitionMode()) {
-	case IM_ONE_COIL:
-		return 0;
-	case IM_WASTED_SPARK: {
-		if (engineConfiguration->specs.cylindersCount == 1) {
-			// we do not want to divide by zero
-			return 0;
-		}
-		return cylinderIndex % (engineConfiguration->specs.cylindersCount / 2);
-	}
-	case IM_INDIVIDUAL_COILS:
-		return cylinderIndex;
-	case IM_TWO_COILS:
-		return cylinderIndex % 2;
-
-	default:
-		firmwareError(CUSTOM_OBD_IGNITION_MODE, "Invalid ignition mode getIgnitionPinForIndex(): %d", engineConfiguration->ignitionMode);
-		return 0;
-	}
-}
-
-void prepareIgnitionPinIndices() {
-#if EFI_ENGINE_CONTROL
-	for (size_t cylinderIndex = 0; cylinderIndex < engineConfiguration->specs.cylindersCount; cylinderIndex++) {
-		engine->ignitionPin[cylinderIndex] = getIgnitionPinForIndex(cylinderIndex);
-	}
-#endif /* EFI_ENGINE_CONTROL */
-}
-
 /**
  * @return IM_WASTED_SPARK if in SPINNING mode and IM_INDIVIDUAL_COILS setting
  * @return engineConfiguration->ignitionMode otherwise
@@ -415,7 +382,8 @@ ignition_mode_e getCurrentIgnitionMode() {
 	ignition_mode_e ignitionMode = engineConfiguration->ignitionMode;
 #if EFI_SHAFT_POSITION_INPUT
 	// In spin-up cranking mode we don't have full phase sync info yet, so wasted spark mode is better
-	if (ignitionMode == IM_INDIVIDUAL_COILS) {
+	// However, only do this on even cylinder count engines: odd cyl count doesn't fire at all
+	if (ignitionMode == IM_INDIVIDUAL_COILS && (engineConfiguration->specs.cylindersCount % 2 == 0)) {
 		bool missingPhaseInfoForSequential = 
 			!engine->triggerCentral.triggerState.hasSynchronizedPhase();
 
@@ -442,8 +410,6 @@ void prepareOutputSignals() {
 	}
 #endif /* EFI_UNIT_TEST */
 
-	prepareIgnitionPinIndices();
-
 	engine->triggerCentral.triggerShape.prepareShape(engine->triggerCentral.triggerFormDetails);
 
 	// Fuel schedule may now be completely wrong, force a reset

firmware/controllers/math/engine_math.h

@@ -46,12 +46,6 @@ floatms_t getSparkDwell(int rpm);
 
 ignition_mode_e getCurrentIgnitionMode();
 
-/**
- * This lightweight method is invoked in case of a configuration change or initialization.
- * But also it's used for "Spinning-up to Cranking" transition.
- */
-void prepareIgnitionPinIndices();
-
 size_t getCylinderId(size_t index);
 size_t getNextFiringCylinderId(size_t prevCylinderId);
 

unit_tests/tests/ignition_injection/test_ignition_scheduling.cpp

@@ -13,19 +13,24 @@ using ::testing::_;
 TEST(ignition, twoCoils) {
 	EngineTestHelper eth(FRANKENSO_BMW_M73_F);
 
-	// first one to fire uses first coil
-	ASSERT_EQ(engine->ignitionPin[ID2INDEX(1)], 0);
-	ASSERT_EQ(engine->ignitionPin[ID2INDEX(2)], 1);
-	ASSERT_EQ(engine->ignitionPin[ID2INDEX(3)], 0);
-	ASSERT_EQ(engine->ignitionPin[ID2INDEX(4)], 1);
-
-	ASSERT_EQ(engine->ignitionPin[ID2INDEX(11)], 0);
-	ASSERT_EQ(engine->ignitionPin[ID2INDEX(12)], 1);
-
 	// let's recalculate with zero timing so that we can focus on relation advance between cylinders
 	setArrayValues(engine->engineState.timingAdvance, 0.0f);
 	initializeIgnitionActions();
 
+	// first one to fire uses first coil
+	EXPECT_EQ(engine->ignitionEvents.elements[0].cylinderNumber, 0);
+	EXPECT_EQ(engine->ignitionEvents.elements[1].cylinderNumber, 6);
+	EXPECT_EQ(engine->ignitionEvents.elements[2].cylinderNumber, 0);
+	EXPECT_EQ(engine->ignitionEvents.elements[3].cylinderNumber, 6);
+	EXPECT_EQ(engine->ignitionEvents.elements[4].cylinderNumber, 0);
+	EXPECT_EQ(engine->ignitionEvents.elements[5].cylinderNumber, 6);
+	EXPECT_EQ(engine->ignitionEvents.elements[6].cylinderNumber, 0);
+	EXPECT_EQ(engine->ignitionEvents.elements[7].cylinderNumber, 6);
+	EXPECT_EQ(engine->ignitionEvents.elements[8].cylinderNumber, 0);
+	EXPECT_EQ(engine->ignitionEvents.elements[9].cylinderNumber, 6);
+	EXPECT_EQ(engine->ignitionEvents.elements[10].cylinderNumber, 0);
+	EXPECT_EQ(engine->ignitionEvents.elements[11].cylinderNumber, 6);
+
 	ASSERT_EQ(engine->ignitionEvents.elements[0].sparkAngle, 0);
 	ASSERT_EQ((void*)engine->ignitionEvents.elements[0].outputs[0], (void*)&enginePins.coils[0]);