From 0dfba2a655de31d7e546f50f925bac585e095c21 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Fri, 23 Sep 2022 14:30:10 -0700 Subject: [PATCH] remove ignitionPin (#4608) * remove ignitionPin * now that's gone * BMW test --- firmware/controllers/algo/engine.cpp | 1 - firmware/controllers/algo/engine.h | 7 ---- .../engine_cycle/rpm_calculator.cpp | 4 -- .../controllers/engine_cycle/spark_logic.cpp | 32 +++++++++++++++- firmware/controllers/math/engine_math.cpp | 38 +------------------ firmware/controllers/math/engine_math.h | 6 --- .../test_ignition_scheduling.cpp | 23 ++++++----- 7 files changed, 46 insertions(+), 65 deletions(-) diff --git a/firmware/controllers/algo/engine.cpp b/firmware/controllers/algo/engine.cpp index 96b4c1e578..6ec853454a 100644 --- a/firmware/controllers/algo/engine.cpp +++ b/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(); } diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index f5d55f7bcf..ac014604db 100644 --- a/firmware/controllers/algo/engine.h +++ b/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 diff --git a/firmware/controllers/engine_cycle/rpm_calculator.cpp b/firmware/controllers/engine_cycle/rpm_calculator.cpp index 8c803bc2e4..d03a41be09 100644 --- a/firmware/controllers/engine_cycle/rpm_calculator.cpp +++ b/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(); } /** diff --git a/firmware/controllers/engine_cycle/spark_logic.cpp b/firmware/controllers/engine_cycle/spark_logic.cpp index eb1f81f8b1..cd032e8162 100644 --- a/firmware/controllers/engine_cycle/spark_logic.cpp +++ b/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()->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]; diff --git a/firmware/controllers/math/engine_math.cpp b/firmware/controllers/math/engine_math.cpp index d2b22a456d..a7ccf0d05a 100644 --- a/firmware/controllers/math/engine_math.cpp +++ b/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 diff --git a/firmware/controllers/math/engine_math.h b/firmware/controllers/math/engine_math.h index 5b2baf0fff..70e1e3d7f7 100644 --- a/firmware/controllers/math/engine_math.h +++ b/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); diff --git a/unit_tests/tests/ignition_injection/test_ignition_scheduling.cpp b/unit_tests/tests/ignition_injection/test_ignition_scheduling.cpp index 7899b694a0..304f6bf91b 100644 --- a/unit_tests/tests/ignition_injection/test_ignition_scheduling.cpp +++ b/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]);