diff --git a/firmware/controllers/algo/event_registry.cpp b/firmware/controllers/algo/event_registry.cpp index 36d6215eda..656c0f48f3 100644 --- a/firmware/controllers/algo/event_registry.cpp +++ b/firmware/controllers/algo/event_registry.cpp @@ -42,6 +42,7 @@ IgnitionEvent::IgnitionEvent() { next = NULL; output = NULL; advance = NAN; + sparkId = 0; } //void registerActuatorEventWhat(InjectionEventList *list, int eventIndex, OutputSignal *actuator, float angleOffset) { diff --git a/firmware/controllers/algo/event_registry.h b/firmware/controllers/algo/event_registry.h index dd67fa7d1d..768b638e59 100644 --- a/firmware/controllers/algo/event_registry.h +++ b/firmware/controllers/algo/event_registry.h @@ -47,13 +47,14 @@ public: class IgnitionEvent { public: IgnitionEvent(); - NamedOutputPin *output; + IgnitionOutputPin *output; scheduling_s signalTimerUp; scheduling_s signalTimerDown; angle_t advance; event_trigger_position_s dwellPosition; event_trigger_position_s sparkPosition; IgnitionEvent *next; + int sparkId; char *name; }; diff --git a/firmware/controllers/math/engine_math.cpp b/firmware/controllers/math/engine_math.cpp index bd1af929fa..9144483877 100644 --- a/firmware/controllers/math/engine_math.cpp +++ b/firmware/controllers/math/engine_math.cpp @@ -93,8 +93,7 @@ void setSingleCoilDwell(engine_configuration_s *engineConfiguration) { #if EFI_ENGINE_CONTROL || defined(__DOXYGEN__) - -static void addIgnitionEvent(angle_t localAdvance, angle_t dwellAngle, IgnitionEventList *list, NamedOutputPin *output DECLARE_ENGINE_PARAMETER_S) { +static void addIgnitionEvent(angle_t localAdvance, angle_t dwellAngle, IgnitionEventList *list, IgnitionOutputPin *output DECLARE_ENGINE_PARAMETER_S) { IgnitionEvent *event = list->add(); if (!isPinAssigned(output)) { @@ -126,7 +125,7 @@ void initializeIgnitionActions(angle_t advance, angle_t dwellAngle, angle_t localAdvance = -advance + ENGINE(angleExtra[i]); int index = ENGINE(ignitionPin[i]); int cylinderIndex = ID2INDEX(getCylinderId(CONFIG(specs.firingOrder), index)); - NamedOutputPin *output = &enginePins.coils[cylinderIndex]; + IgnitionOutputPin *output = &enginePins.coils[cylinderIndex]; addIgnitionEvent(localAdvance, dwellAngle, list, output PASS_ENGINE_PARAMETER); diff --git a/firmware/controllers/system/efiGpio.cpp b/firmware/controllers/system/efiGpio.cpp index 8c52af0fdb..086b6d5798 100644 --- a/firmware/controllers/system/efiGpio.cpp +++ b/firmware/controllers/system/efiGpio.cpp @@ -72,6 +72,7 @@ IgnitionOutputPin::IgnitionOutputPin() { void IgnitionOutputPin::reset() { outOfOrderCounter = 0; + sparkId = 0; } OutputPin::OutputPin() { diff --git a/firmware/controllers/system/efiGpio.h b/firmware/controllers/system/efiGpio.h index 9084dc0484..941ef216f0 100644 --- a/firmware/controllers/system/efiGpio.h +++ b/firmware/controllers/system/efiGpio.h @@ -60,6 +60,7 @@ class IgnitionOutputPin : public NamedOutputPin { public: IgnitionOutputPin(); void reset(); + int sparkId; int outOfOrderCounter; // https://sourceforge.net/p/rusefi/tickets/319/ }; diff --git a/firmware/controllers/trigger/spark_logic.cpp b/firmware/controllers/trigger/spark_logic.cpp index 64047bceae..bf84ec45fd 100644 --- a/firmware/controllers/trigger/spark_logic.cpp +++ b/firmware/controllers/trigger/spark_logic.cpp @@ -24,11 +24,23 @@ int isIgnitionTimingError(void) { return ignitionErrorDetection.sum(6) > 4; } -void turnSparkPinLow(IgnitionOutputPin *output) { +void turnSparkPinLow(IgnitionEvent *event) { + IgnitionOutputPin *output = event->output; #if SPARK_EXTREME_LOGGING || defined(__DOXYGEN__) - scheduleMsg(logger, "spark goes low %d %s %d", getRevolutionCounter(), output->name, (int)getTimeNowUs()); + scheduleMsg(logger, "spark goes low %d %s %d current=%d cnt=%d id=%d", getRevolutionCounter(), output->name, (int)getTimeNowUs(), + output->currentLogicValue, output->outOfOrderCounter, event->sparkId); #endif /* FUEL_MATH_EXTREME_LOGGING */ + /** + * there are two kinds of 'out-of-order' + * 1) low goes before high, everything is fine after words + * + * 2) we have an un-matched low followed by legit pairs + * + */ + + output->sparkId = event->sparkId; + if (!output->currentLogicValue) { warning(CUSTOM_ERR_6149, "out-of-order coil off %s", output->name); output->outOfOrderCounter++; @@ -42,15 +54,19 @@ void turnSparkPinLow(IgnitionOutputPin *output) { #endif /* EFI_PROD_CODE */ } -void turnSparkPinHigh(IgnitionOutputPin *output) { +void turnSparkPinHigh(IgnitionEvent *event) { + IgnitionOutputPin *output = event->output; #if SPARK_EXTREME_LOGGING || defined(__DOXYGEN__) - scheduleMsg(logger, "spark goes high %d %s %d", getRevolutionCounter(), output->name, (int)getTimeNowUs()); + scheduleMsg(logger, "spark goes high %d %s %d current=%d cnt=%d id=%d", getRevolutionCounter(), output->name, (int)getTimeNowUs(), + output->currentLogicValue, output->outOfOrderCounter, event->sparkId); #endif /* FUEL_MATH_EXTREME_LOGGING */ if (output->outOfOrderCounter > 0) { - // let's save this coil if things do not look right output->outOfOrderCounter--; - return; + if (output->sparkId == event->sparkId) { + // let's save this coil if things do not look right + return; + } } turnPinHigh(output); @@ -61,6 +77,8 @@ void turnSparkPinHigh(IgnitionOutputPin *output) { #endif /* EFI_PROD_CODE */ } +static int globalSparkIdCoutner = 0; + static ALWAYS_INLINE void handleSparkEvent(bool limitedSpark, uint32_t trgEventIndex, IgnitionEvent *iEvent, int rpm DECLARE_ENGINE_PARAMETER_S) { @@ -81,6 +99,8 @@ static ALWAYS_INLINE void handleSparkEvent(bool limitedSpark, uint32_t trgEventI return; } + iEvent->sparkId = globalSparkIdCoutner++; + /** * We are alternating two event lists in order to avoid a potential issue around revolution boundary * when an event is scheduled within the next revolution. @@ -88,12 +108,13 @@ static ALWAYS_INLINE void handleSparkEvent(bool limitedSpark, uint32_t trgEventI scheduling_s * sUp = &iEvent->signalTimerUp; scheduling_s * sDown = &iEvent->signalTimerDown; + /** * The start of charge is always within the current trigger event range, so just plain time-based scheduling */ if (!limitedSpark) { #if SPARK_EXTREME_LOGGING || defined(__DOXYGEN__) - scheduleMsg(logger, "scheduling sparkUp ind=%d %d %s now=%d %d later", trgEventIndex, getRevolutionCounter(), iEvent->output->name, (int)getTimeNowUs(), (int)chargeDelayUs); + scheduleMsg(logger, "scheduling sparkUp ind=%d %d %s now=%d %d later", trgEventIndex, getRevolutionCounter(), iEvent->output->name, (int)getTimeNowUs(), (int)chargeDelayUs); #endif /* FUEL_MATH_EXTREME_LOGGING */ /** @@ -101,7 +122,7 @@ static ALWAYS_INLINE void handleSparkEvent(bool limitedSpark, uint32_t trgEventI * This way we make sure that coil dwell started while spark was enabled would fire and not burn * the coil. */ - scheduleTask(true, "spark up", sUp, chargeDelayUs, (schfunc_t) &turnSparkPinHigh, iEvent->output); + scheduleTask(true, "spark up", sUp, chargeDelayUs, (schfunc_t) &turnSparkPinHigh, iEvent); } /** * Spark event is often happening during a later trigger event timeframe @@ -123,7 +144,7 @@ static ALWAYS_INLINE void handleSparkEvent(bool limitedSpark, uint32_t trgEventI scheduleMsg(logger, "scheduling sparkDown ind=%d %d %s now=%d %d later", trgEventIndex, getRevolutionCounter(), iEvent->output->name, (int)getTimeNowUs(), (int)timeTillIgnitionUs); #endif /* FUEL_MATH_EXTREME_LOGGING */ - scheduleTask(true, "spark1 down", sDown, (int) timeTillIgnitionUs, (schfunc_t) &turnSparkPinLow, iEvent->output); + scheduleTask(true, "spark1 down", sDown, (int) timeTillIgnitionUs, (schfunc_t) &turnSparkPinLow, iEvent); } else { #if SPARK_EXTREME_LOGGING || defined(__DOXYGEN__) scheduleMsg(logger, "to queue sparkDown ind=%d %d %s %d for %d", trgEventIndex, getRevolutionCounter(), iEvent->output->name, (int)getTimeNowUs(), iEvent->sparkPosition.eventIndex); @@ -210,7 +231,7 @@ void handleSpark(int revolutionIndex, bool limitedSpark, uint32_t trgEventIndex, float timeTillIgnitionUs = ENGINE(rpmCalculator.oneDegreeUs) * current->sparkPosition.angleOffset; - scheduleTask(true, "spark 2down", sDown, (int) timeTillIgnitionUs, (schfunc_t) &turnSparkPinLow, current->output); + scheduleTask(true, "spark 2down", sDown, (int) timeTillIgnitionUs, (schfunc_t) &turnSparkPinLow, current); } } diff --git a/firmware/controllers/trigger/spark_logic.h b/firmware/controllers/trigger/spark_logic.h index 419867eeec..9c2bd88c60 100644 --- a/firmware/controllers/trigger/spark_logic.h +++ b/firmware/controllers/trigger/spark_logic.h @@ -14,7 +14,7 @@ int isInjectionEnabled(engine_configuration_s *engineConfiguration); void handleSpark(int revolutionIndex, bool limitedSpark, uint32_t trgEventIndex, int rpm, IgnitionEventList *list DECLARE_ENGINE_PARAMETER_S); void initSparkLogic(Logging *sharedLogger); -void turnSparkPinHigh(IgnitionOutputPin *output); -void turnSparkPinLow(IgnitionOutputPin *output); +void turnSparkPinHigh(IgnitionEvent *event); +void turnSparkPinLow(IgnitionEvent *event); #endif /* CONTROLLERS_TRIGGER_SPARK_LOGIC_H_ */ diff --git a/unit_tests/test_trigger_decoder.cpp b/unit_tests/test_trigger_decoder.cpp index 8745e67f78..4d502dc8ad 100644 --- a/unit_tests/test_trigger_decoder.cpp +++ b/unit_tests/test_trigger_decoder.cpp @@ -365,12 +365,12 @@ void testRpmCalculator(void) { assertREqualsM("Call@0", (void*)ev0->callback, (void*)turnSparkPinHigh); assertEqualsM("ev 0", st + 944, ev0->momentX); - assertEqualsLM("o 0", (long)&enginePins.coils[0], (long)ev0->param); + assertEqualsLM("coil 0", (long)&enginePins.coils[0], (long)((IgnitionEvent*)ev0->param)->output); scheduling_s *ev1 = schedulingQueue.getForUnitText(1); assertREqualsM("Call@1", (void*)ev1->callback, (void*)turnSparkPinLow); assertEqualsM("ev 1", st + 1444, ev1->momentX); - assertEqualsLM("o 1", (long)&enginePins.coils[0], (long)ev1->param); + assertEqualsLM("coil 1", (long)&enginePins.coils[0], (long)((IgnitionEvent*)ev1->param)->output); } @@ -1178,4 +1178,49 @@ void testSparkReverseOrderBug319(void) { schedulingQueue.executeAll(timeNow); assertEqualsM("out-of-order #2", 0, enginePins.coils[3].outOfOrderCounter); + + printf("*************************************************** now let's have a good engine cycle and confirm things work\r\n"); + + timeNow += MS2US(20); + eth.firePrimaryTriggerRise(); + schedulingQueue.executeAll(timeNow); + + assertEqualsM("RPM#2", 545, eth.engine.rpmCalculator.getRpm(PASS_ENGINE_PARAMETER_F)); + + assertEqualsM("out-of-order #3", 0, enginePins.coils[3].outOfOrderCounter); + + timeNow += MS2US(20); + eth.firePrimaryTriggerFall(); + schedulingQueue.executeAll(timeNow); + assertEqualsM("out-of-order #4", 1, enginePins.coils[3].outOfOrderCounter); + + printf("*************************************************** (rpm is back) now let's have a good engine cycle and confirm things work\r\n"); + + timeNow += MS2US(20); + eth.firePrimaryTriggerRise(); + schedulingQueue.executeAll(timeNow); + + assertEqualsM("RPM#3", 3000, eth.engine.rpmCalculator.getRpm(PASS_ENGINE_PARAMETER_F)); + + assertEqualsM("out-of-order #5", 1, enginePins.coils[3].outOfOrderCounter); + + timeNow += MS2US(20); + eth.firePrimaryTriggerFall(); + schedulingQueue.executeAll(timeNow); + assertEqualsM("out-of-order #6", 0, enginePins.coils[3].outOfOrderCounter); + + printf("*************************************************** (rpm is back 2) now let's have a good engine cycle and confirm things work\r\n"); + + timeNow += MS2US(20); + eth.firePrimaryTriggerRise(); + schedulingQueue.executeAll(timeNow); + + assertEqualsM("RPM#4", 3000, eth.engine.rpmCalculator.getRpm(PASS_ENGINE_PARAMETER_F)); + + assertEqualsM("out-of-order #7", 0, enginePins.coils[3].outOfOrderCounter); + + timeNow += MS2US(20); + eth.firePrimaryTriggerFall(); + schedulingQueue.executeAll(timeNow); + assertEqualsM("out-of-order #8", 0, enginePins.coils[3].outOfOrderCounter); }