From 46b7567195cef9d2266d3bc053d74efb2464810a Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 20 Jul 2020 00:04:05 -0700 Subject: [PATCH 1/9] put wall wetting inside --- firmware/controllers/algo/engine.h | 1 - firmware/controllers/algo/event_registry.h | 3 +++ .../engine_cycle/main_trigger_callback.cpp | 3 +-- unit_tests/tests/test_fuel_wall_wetting.cpp | 14 ++++++++------ 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index 8614e2580b..6ec879af4e 100644 --- a/firmware/controllers/algo/engine.h +++ b/firmware/controllers/algo/engine.h @@ -140,7 +140,6 @@ public: IgnitionEventList ignitionEvents; #endif /* EFI_ENGINE_CONTROL */ - WallFuel wallFuel[INJECTION_PIN_COUNT]; bool needToStopEngine(efitick_t nowNt) const; bool etbAutoTune = false; /** diff --git a/firmware/controllers/algo/event_registry.h b/firmware/controllers/algo/event_registry.h index 0284b7d9c6..6c1ec660b5 100644 --- a/firmware/controllers/algo/event_registry.h +++ b/firmware/controllers/algo/event_registry.h @@ -12,6 +12,7 @@ #include "scheduler.h" #include "fl_stack.h" #include "trigger_structure.h" +#include "accel_enrichment.h" #define MAX_INJECTION_OUTPUT_COUNT INJECTION_PIN_COUNT #define MAX_WIRES_COUNT 2 @@ -42,6 +43,8 @@ public: * TODO: make watchdog decrement relevant counter */ bool isScheduled = false; + + WallFuel wallFuel; }; diff --git a/firmware/controllers/engine_cycle/main_trigger_callback.cpp b/firmware/controllers/engine_cycle/main_trigger_callback.cpp index ea43db5c0d..420852ca6f 100644 --- a/firmware/controllers/engine_cycle/main_trigger_callback.cpp +++ b/firmware/controllers/engine_cycle/main_trigger_callback.cpp @@ -180,8 +180,7 @@ void handleFuelInjectionEvent(int injEventIndex, InjectionEvent *event, * x2 or /2? */ - size_t injectorIndex = event->outputs[0]->injectorIndex; - const floatms_t injectionDuration = ENGINE(wallFuel[injectorIndex]).adjust(ENGINE(injectionDuration) PASS_ENGINE_PARAMETER_SUFFIX); + const floatms_t injectionDuration = event->wallFuel.adjust(ENGINE(injectionDuration) PASS_ENGINE_PARAMETER_SUFFIX); #if EFI_PRINTF_FUEL_DETAILS if (printFuelDebug) { printf("fuel index=%d injectionDuration=%.2fms adjusted=%.2fms\n", diff --git a/unit_tests/tests/test_fuel_wall_wetting.cpp b/unit_tests/tests/test_fuel_wall_wetting.cpp index aa7e1bf548..54acc45c17 100644 --- a/unit_tests/tests/test_fuel_wall_wetting.cpp +++ b/unit_tests/tests/test_fuel_wall_wetting.cpp @@ -17,9 +17,11 @@ TEST(fuel, testWallWettingEnrichmentMath) { engine->rpmCalculator.setRpmValue(3000 PASS_ENGINE_PARAMETER_SUFFIX); + WallFuel wallFuel; + // each invocation of 'adjust' changes WallWetting internal state - ASSERT_NEAR(16.6666, ENGINE(wallFuel[0]).adjust(10.0 PASS_ENGINE_PARAMETER_SUFFIX), EPS4D); - ASSERT_NEAR(16.198, ENGINE(wallFuel[0]).adjust(10.0 PASS_ENGINE_PARAMETER_SUFFIX), EPS4D); + ASSERT_NEAR(16.6666, wallFuel.adjust(10.0 PASS_ENGINE_PARAMETER_SUFFIX), EPS4D); + ASSERT_NEAR(16.198, wallFuel.adjust(10.0 PASS_ENGINE_PARAMETER_SUFFIX), EPS4D); } TEST(fuel, testWallWettingEnrichmentScheduling) { @@ -38,11 +40,11 @@ TEST(fuel, testWallWettingEnrichmentScheduling) { int expectedInvocationCounter = 1; for (int i = 0; i < 4; i++) { - ASSERT_EQ(expectedInvocationCounter, ENGINE(wallFuel[i]).invocationCounter); + ASSERT_EQ(expectedInvocationCounter, ENGINE(injectionEvents.elements[i]).wallFuel.invocationCounter); } // Cylinder 5 doesn't exist - shouldn't have been called! - ASSERT_EQ(0, ENGINE(wallFuel[5]).invocationCounter); + ASSERT_EQ(0, ENGINE(injectionEvents.elements[5]).wallFuel.invocationCounter); eth.engine.periodicFastCallback(PASS_ENGINE_PARAMETER_SIGNATURE); eth.engine.periodicFastCallback(PASS_ENGINE_PARAMETER_SIGNATURE); @@ -50,9 +52,9 @@ TEST(fuel, testWallWettingEnrichmentScheduling) { // still same 1 per cylinder - wall wetting is NOT invoked from 'periodicFastCallback' for (int i = 0; i < 4; i++) { - ASSERT_EQ(expectedInvocationCounter, ENGINE(wallFuel[i]).invocationCounter); + ASSERT_EQ(expectedInvocationCounter, ENGINE(injectionEvents.elements[i]).wallFuel.invocationCounter); } // Cylinder 5 doesn't exist - shouldn't have been called! - ASSERT_EQ(0, ENGINE(wallFuel[5]).invocationCounter); + ASSERT_EQ(0, ENGINE(injectionEvents.elements[5]).wallFuel.invocationCounter); } From a1d39b8de6b9249d69a3603d41d725dfd4869576 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 20 Jul 2020 00:55:45 -0700 Subject: [PATCH 2/9] more spots --- firmware/console/binary/tunerstudio.cpp | 2 +- firmware/console/status_loop.cpp | 5 +++-- firmware/controllers/engine_controller.cpp | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/firmware/console/binary/tunerstudio.cpp b/firmware/console/binary/tunerstudio.cpp index ebe0970982..b05a22c134 100644 --- a/firmware/console/binary/tunerstudio.cpp +++ b/firmware/console/binary/tunerstudio.cpp @@ -276,7 +276,7 @@ static const void * getStructAddr(int structId) { case LDS_ENGINE_STATE_INDEX: return static_cast(&engine->engineState); case LDS_FUEL_TRIM_STATE_INDEX: - return static_cast(&engine->wallFuel[0]); + return static_cast(&engine->injectionEvents.elements[0].wallFuel); case LDS_TRIGGER_CENTRAL_STATE_INDEX: return static_cast(&engine->triggerCentral); case LDS_TRIGGER_STATE_STATE_INDEX: diff --git a/firmware/console/status_loop.cpp b/firmware/console/status_loop.cpp index 0b678d1985..6dc6995335 100644 --- a/firmware/console/status_loop.cpp +++ b/firmware/console/status_loop.cpp @@ -562,11 +562,12 @@ void updateTunerStudioState(TunerStudioOutputChannels *tsOutputChannels DECLARE_ // 148 tsOutputChannels->fuelTankLevel = engine->sensors.fuelTankLevel; // 160 - tsOutputChannels->wallFuelAmount = ENGINE(wallFuel[0]).getWallFuel(); + const auto& wallFuel = ENGINE(injectionEvents.elements[0].wallFuel); + tsOutputChannels->wallFuelAmount = wallFuel.getWallFuel(); // 164 tsOutputChannels->iatCorrection = ENGINE(engineState.running.intakeTemperatureCoefficient); // 168 - tsOutputChannels->wallFuelCorrection = ENGINE(wallFuel[0]).wallFuelCorrection; + tsOutputChannels->wallFuelCorrection = wallFuel.wallFuelCorrection; // 184 tsOutputChannels->cltCorrection = ENGINE(engineState.running.coolantTemperatureCoefficient); // 188 diff --git a/firmware/controllers/engine_controller.cpp b/firmware/controllers/engine_controller.cpp index 6246a560b1..3d60580522 100644 --- a/firmware/controllers/engine_controller.cpp +++ b/firmware/controllers/engine_controller.cpp @@ -208,9 +208,9 @@ static void resetAccel(void) { engine->engineLoadAccelEnrichment.resetAE(); engine->tpsAccelEnrichment.resetAE(); - for (unsigned int i = 0; i < sizeof(engine->wallFuel) / sizeof(engine->wallFuel[0]); i++) + for (unsigned int i = 0; i < efi::size(engine->injectionEvents.elements); i++) { - engine->wallFuel[i].resetWF(); + engine->injectionEvents.elements[i].wallFuel.resetWF(); } } From 0f0d18afc81384f87ce9c5ad4585ea55ef897cf8 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 20 Jul 2020 01:03:13 -0700 Subject: [PATCH 3/9] s --- firmware/console/status_loop.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/firmware/console/status_loop.cpp b/firmware/console/status_loop.cpp index 6dc6995335..c6539cd435 100644 --- a/firmware/console/status_loop.cpp +++ b/firmware/console/status_loop.cpp @@ -564,10 +564,11 @@ void updateTunerStudioState(TunerStudioOutputChannels *tsOutputChannels DECLARE_ // 160 const auto& wallFuel = ENGINE(injectionEvents.elements[0].wallFuel); tsOutputChannels->wallFuelAmount = wallFuel.getWallFuel(); - // 164 - tsOutputChannels->iatCorrection = ENGINE(engineState.running.intakeTemperatureCoefficient); // 168 tsOutputChannels->wallFuelCorrection = wallFuel.wallFuelCorrection; + + // 164 + tsOutputChannels->iatCorrection = ENGINE(engineState.running.intakeTemperatureCoefficient); // 184 tsOutputChannels->cltCorrection = ENGINE(engineState.running.coolantTemperatureCoefficient); // 188 From 9c736a2c3d5f3ff9d97d9a386cf7a4252f74ed57 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 20 Jul 2020 03:29:43 -0700 Subject: [PATCH 4/9] test --- firmware/controllers/algo/fuel_math.cpp | 2 +- firmware/controllers/system/efi_gpio.h | 2 + .../injection_mode_transition.cpp | 96 ++++++++++++++++--- 3 files changed, 84 insertions(+), 16 deletions(-) diff --git a/firmware/controllers/algo/fuel_math.cpp b/firmware/controllers/algo/fuel_math.cpp index ac57b4e043..c744702a62 100644 --- a/firmware/controllers/algo/fuel_math.cpp +++ b/firmware/controllers/algo/fuel_math.cpp @@ -36,7 +36,7 @@ EXTERN_ENGINE; fuel_Map3D_t fuelMap("fuel"); -static fuel_Map3D_t fuelPhaseMap("fl ph"); +fuel_Map3D_t fuelPhaseMap("fl ph"); extern fuel_Map3D_t veMap; extern afr_Map3D_t afrMap; extern baroCorr_Map3D_t baroCorrMap; diff --git a/firmware/controllers/system/efi_gpio.h b/firmware/controllers/system/efi_gpio.h index 8971e0cfa9..6bbe0d1034 100644 --- a/firmware/controllers/system/efi_gpio.h +++ b/firmware/controllers/system/efi_gpio.h @@ -111,6 +111,8 @@ public: void open(); void close(); + int8_t getOverlappingCounter() const { return overlappingCounter; } + // todo: re-implement this injectorIndex via address manipulation to reduce memory usage? int8_t injectorIndex; diff --git a/unit_tests/tests/ignition_injection/injection_mode_transition.cpp b/unit_tests/tests/ignition_injection/injection_mode_transition.cpp index 31894e7583..62e2acbe14 100644 --- a/unit_tests/tests/ignition_injection/injection_mode_transition.cpp +++ b/unit_tests/tests/ignition_injection/injection_mode_transition.cpp @@ -7,37 +7,103 @@ #include "engine_test_helper.h" +static void doRevolution(EngineTestHelper& eth, int periodMs) { + float halfToothTime = (periodMs / 6.0f) / 2; + + eth.fireRise(halfToothTime); + eth.fireFall(halfToothTime); + eth.fireRise(halfToothTime); + eth.fireFall(halfToothTime); + eth.fireRise(halfToothTime); + eth.fireFall(halfToothTime); + + // now missing tooth + eth.fireRise(halfToothTime); + eth.fireFall(3 * halfToothTime); + + // This tooth is the sync point! + eth.fireRise(halfToothTime); + eth.fireFall(halfToothTime); +} // https://github.com/rusefi/rusefi/issues/1592 TEST(fuelControl, transitionIssue1592) { - WITH_ENGINE_TEST_HELPER(TEST_ENGINE); setupSimpleTestEngineWithMafAndTT_ONE_trigger(ð, IM_SEQUENTIAL); - eth.fireTriggerEvents2(4 /* count */ , 600 /* ms */); + // This is easiest to trip on a wheel that requires sync + engineConfiguration->trigger.customTotalToothCount = 6; + engineConfiguration->trigger.customSkippedToothCount = 1; + eth.setTriggerType(TT_TOOTHED_WHEEL PASS_ENGINE_PARAMETER_SUFFIX); + engineConfiguration->ambiguousOperationMode = FOUR_STROKE_CAM_SENSOR; + engineConfiguration->isFasterEngineSpinUpEnabled = true; - ASSERT_EQ(CRANKING, engine->rpmCalculator.getState()); - ASSERT_EQ( 100, GET_RPM()) << "spinning-RPM#1"; + engineConfiguration->fuelAlgorithm = LM_ALPHA_N; - ASSERT_EQ(IM_SIMULTANEOUS, ENGINE(getCurrentInjectionMode(PASS_ENGINE_PARAMETER_SIGNATURE))); + extern fuel_Map3D_t fuelMap; + fuelMap.setAll(13); + extern fuel_Map3D_t fuelPhaseMap; + fuelPhaseMap.setAll(0); + + engineConfiguration->globalTriggerAngleOffset = 20; + + // Yes, this is a ton of fuel but it makes the repro easier + engineConfiguration->cranking.baseFuel = 89; + engineConfiguration->cranking.rpm = 500; + + // Test the transition from batch cranking to sequential running + engineConfiguration->crankingInjectionMode = IM_BATCH; + engineConfiguration->twoWireBatchInjection = true; - eth.fireRise(150); - eth.fireFall(150); + // First sync point will schedule cranking pulse since we're in "faster spin up" mode + doRevolution(eth, 150); - eth.fireRise(150); - eth.fireFall(150); + { + // Injector 2 should be scheduled to open then close + void* inj2 = reinterpret_cast(&engine->injectionEvents.elements[1]); - eth.fireRise(150); - eth.fireFall(150); + ASSERT_EQ(engine->executor.size(), 2); - ASSERT_EQ( 400, GET_RPM()) << "running-RPM#1"; + // Check that the action is correct - we don't care about the timing necessarily + auto sched_open = engine->executor.getForUnitTest(0); + ASSERT_EQ(sched_open->action.getArgument(), inj2); + ASSERT_EQ(sched_open->action.getCallback(), &turnInjectionPinHigh); - ASSERT_EQ(IM_SIMULTANEOUS, ENGINE(getCurrentInjectionMode(PASS_ENGINE_PARAMETER_SIGNATURE))); + auto sched_close = engine->executor.getForUnitTest(1); + // Next action should be closing the same injector + ASSERT_EQ(sched_close->action.getArgument(), inj2); + ASSERT_EQ(sched_close->action.getCallback(), &turnInjectionPinLow); + } + // Execute the first of those two events - the injector opens, but doesn't yet close. + engine->executor.executeAll(getTimeNowUs() + MS2US(35)); - eth.writeEvents("transitionIssue1592.logicdata"); + // Check that queue got shorter, and overlap counters were incremented on injectors 2/3 (batch mode, remember?) + { + // Check that it was exec'd + ASSERT_EQ(engine->executor.size(), 1); + // Injectors 2/3 should currently be open + EXPECT_EQ(enginePins.injectors[0].getOverlappingCounter(), 0); + EXPECT_EQ(enginePins.injectors[1].getOverlappingCounter(), 1); + EXPECT_EQ(enginePins.injectors[2].getOverlappingCounter(), 1); + EXPECT_EQ(enginePins.injectors[3].getOverlappingCounter(), 0); + } + + // Second sync point will transition to running + // This needs to reset overlapping state as it may reschedule injector openings + doRevolution(eth, 150); + + // Injectors should all be closed immediately after mode change + EXPECT_EQ(enginePins.injectors[0].getOverlappingCounter(), 0); + + // !!!!!!!!! BUG !!!!!!!!!!!!!!! + // These next two should be equal to 0, not 1 + EXPECT_EQ(enginePins.injectors[1].getOverlappingCounter(), 1); + EXPECT_EQ(enginePins.injectors[2].getOverlappingCounter(), 1); + // !!!!!!!!! BUG !!!!!!!!!!!!!!! + + EXPECT_EQ(enginePins.injectors[3].getOverlappingCounter(), 0); } - From 52d69abf4ab1722a8241cd657fce0cf5ee48c698 Mon Sep 17 00:00:00 2001 From: rusefi Date: Mon, 20 Jul 2020 09:04:29 -0400 Subject: [PATCH 5/9] logicdata into unit tests --- firmware/console/binary/tooth_logger.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firmware/console/binary/tooth_logger.cpp b/firmware/console/binary/tooth_logger.cpp index c13942d80f..86313f8739 100644 --- a/firmware/console/binary/tooth_logger.cpp +++ b/firmware/console/binary/tooth_logger.cpp @@ -61,7 +61,7 @@ int getCompositeRecordCount() { int copyCompositeEvents(CompositeEvent *events) { for (int i = 0;i < NextIdx;i++) { CompositeEvent *event = &events[i]; - event->timestamp = buffer[i].timestamp; + event->timestamp = SWAP_UINT32(buffer[i].timestamp); event->primaryTrigger = buffer[i].priLevel; event->secondaryTrigger = buffer[i].secLevel; event->trg = buffer[i].trigger; From de66f4dc6ef7d9ecc63bcca2f833bd55c9b10b67 Mon Sep 17 00:00:00 2001 From: rusefi Date: Mon, 20 Jul 2020 11:16:51 -0400 Subject: [PATCH 6/9] unit test framework improvements for smoother event execution --- unit_tests/.gitignore | 3 +- unit_tests/engine_test_helper.cpp | 35 ++++++++++++++++--- unit_tests/engine_test_helper.h | 16 +++++++-- .../injection_mode_transition.cpp | 8 ++--- 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/unit_tests/.gitignore b/unit_tests/.gitignore index 30469214fb..f6319ed794 100644 --- a/unit_tests/.gitignore +++ b/unit_tests/.gitignore @@ -2,4 +2,5 @@ build/ gcov_working_area triggers -triggers.txt \ No newline at end of file +triggers.txt +*.logicdata diff --git a/unit_tests/engine_test_helper.cpp b/unit_tests/engine_test_helper.cpp index 9d03274804..437e5104df 100644 --- a/unit_tests/engine_test_helper.cpp +++ b/unit_tests/engine_test_helper.cpp @@ -103,12 +103,28 @@ void EngineTestHelper::writeEvents(const char *fileName) { /** * mock a change of time and fire single RISE front event + * DEPRECATED many usages should be migrated to */ void EngineTestHelper::fireRise(float delayMs) { moveTimeForwardUs(MS2US(delayMs)); firePrimaryTriggerRise(); } +void EngineTestHelper::smartFireRise(float delayMs) { + moveTimeForwardUs(MS2US(delayMs)); + firePrimaryTriggerRise(); +} + +void EngineTestHelper::fireFall(float delayMs) { + moveTimeForwardUs(MS2US(delayMs)); + firePrimaryTriggerFall(); +} + +void EngineTestHelper::smartFireFall(float delayMs) { + moveTimeForwardUs(MS2US(delayMs)); + firePrimaryTriggerFall(); +} + /** * fire single RISE front event */ @@ -120,11 +136,6 @@ void EngineTestHelper::firePrimaryTriggerRise() { engine->triggerCentral.handleShaftSignal(SHAFT_PRIMARY_RISING, nowNt, engine, engine->engineConfigurationPtr, &persistentConfig); } -void EngineTestHelper::fireFall(float delayMs) { - moveTimeForwardUs(MS2US(delayMs)); - firePrimaryTriggerFall(); -} - void EngineTestHelper::firePrimaryTriggerFall() { efitick_t nowNt = getTimeNowNt(); Engine *engine = &this->engine; @@ -149,6 +160,13 @@ void EngineTestHelper::fireTriggerEvents2(int count, float durationMs) { } } +void EngineTestHelper::smartFireTriggerEvents2(int count, float durationMs) { + for (int i = 0; i < count; i++) { + smartFireRise(durationMs); + smartFireFall(durationMs); + } +} + void EngineTestHelper::clearQueue() { engine.executor.executeAll(99999999); // this is needed to clear 'isScheduled' flag ASSERT_EQ( 0, engine.executor.size()) << "Failed to clearQueue"; @@ -169,6 +187,13 @@ void EngineTestHelper::moveTimeForwardUs(int deltaTimeUs) { timeNowUs += deltaTimeUs; } +void EngineTestHelper::smartMoveTimeForwardUs(int deltaTimeUs) { + if (printTriggerDebug || printFuelDebug) { + printf("moveTimeForwardUs %.1fms\r\n", deltaTimeUs / 1000.0); + } + timeNowUs += deltaTimeUs; +} + efitimeus_t EngineTestHelper::getTimeNowUs(void) { return timeNowUs; } diff --git a/unit_tests/engine_test_helper.h b/unit_tests/engine_test_helper.h index 494c533907..0e2e1b0c78 100644 --- a/unit_tests/engine_test_helper.h +++ b/unit_tests/engine_test_helper.h @@ -36,8 +36,22 @@ public: void applyTriggerWaveform(); void setTriggerType(trigger_type_e trigger DECLARE_ENGINE_PARAMETER_SUFFIX); + /** + * DEPRECATED these methods do not execute events on the queue + */ void fireRise(float delayMs); void fireFall(float delayMs); + void moveTimeForwardUs(int deltaTimeUs); + void fireTriggerEvents2(int count, float delayMs); + + /** + * these methods execute events while moving time forward + * todo: better naming convention? + */ + void smartFireRise(float delayMs); + void smartFireFall(float delayMs); + void smartMoveTimeForwardUs(int deltaTimeUs); + void smartFireTriggerEvents2(int count, float delayMs); /** * See also #fireRise() which would also move time forward @@ -49,7 +63,6 @@ public: void firePrimaryTriggerFall(); void fireTriggerEvents(int count); void fireTriggerEventsWithDuration(float delayMs); - void fireTriggerEvents2(int count, float delayMs); void clearQueue(); scheduling_s * assertEvent5(const char *msg, int index, void *callback, efitime_t expectedTimestamp); @@ -65,7 +78,6 @@ public: int executeActions(); void moveTimeForwardMs(float deltaTimeMs); - void moveTimeForwardUs(int deltaTimeUs); efitimeus_t getTimeNowUs(void); void writeEvents(const char *fileName); diff --git a/unit_tests/tests/ignition_injection/injection_mode_transition.cpp b/unit_tests/tests/ignition_injection/injection_mode_transition.cpp index 31894e7583..71d6799169 100644 --- a/unit_tests/tests/ignition_injection/injection_mode_transition.cpp +++ b/unit_tests/tests/ignition_injection/injection_mode_transition.cpp @@ -14,7 +14,7 @@ TEST(fuelControl, transitionIssue1592) { WITH_ENGINE_TEST_HELPER(TEST_ENGINE); setupSimpleTestEngineWithMafAndTT_ONE_trigger(ð, IM_SEQUENTIAL); - eth.fireTriggerEvents2(4 /* count */ , 600 /* ms */); + eth.smartFireTriggerEvents2(4 /* count */ , 600 /* ms */); ASSERT_EQ(CRANKING, engine->rpmCalculator.getState()); ASSERT_EQ( 100, GET_RPM()) << "spinning-RPM#1"; @@ -22,13 +22,13 @@ TEST(fuelControl, transitionIssue1592) { ASSERT_EQ(IM_SIMULTANEOUS, ENGINE(getCurrentInjectionMode(PASS_ENGINE_PARAMETER_SIGNATURE))); - eth.fireRise(150); + eth.smartFireRise(150); eth.fireFall(150); - eth.fireRise(150); + eth.smartFireRise(150); eth.fireFall(150); - eth.fireRise(150); + eth.smartFireRise(150); eth.fireFall(150); ASSERT_EQ( 400, GET_RPM()) << "running-RPM#1"; From 2a45c9cbe85d989d8fe1ae560ad29f9f5bcb6bc7 Mon Sep 17 00:00:00 2001 From: rusefi Date: Mon, 20 Jul 2020 12:45:26 -0400 Subject: [PATCH 7/9] unit test framework improvements for smoother event execution --- firmware/console/binary/tooth_logger.cpp | 7 ++-- unit_tests/engine_test_helper.cpp | 32 +++++++++++++++---- unit_tests/global_execution_queue.cpp | 4 +++ unit_tests/global_execution_queue.h | 3 +- unit_tests/logicdata.cpp | 4 +-- unit_tests/logicdata.h | 2 +- unit_tests/logicdata_sandbox.cpp | 8 ++--- .../injection_mode_transition.cpp | 5 +-- 8 files changed, 43 insertions(+), 22 deletions(-) diff --git a/firmware/console/binary/tooth_logger.cpp b/firmware/console/binary/tooth_logger.cpp index 86313f8739..e1acef8a3e 100644 --- a/firmware/console/binary/tooth_logger.cpp +++ b/firmware/console/binary/tooth_logger.cpp @@ -28,7 +28,7 @@ typedef struct __attribute__ ((packed)) { // unfortunately all these fields are required by TS... bool priLevel : 1; bool secLevel : 1; - bool trigger : 1; + bool isTDC : 1; bool sync : 1; bool coil : 1; bool injector : 1; @@ -64,7 +64,7 @@ int copyCompositeEvents(CompositeEvent *events) { event->timestamp = SWAP_UINT32(buffer[i].timestamp); event->primaryTrigger = buffer[i].priLevel; event->secondaryTrigger = buffer[i].secLevel; - event->trg = buffer[i].trigger; + event->isTDC = buffer[i].isTDC; event->sync = buffer[i].sync; event->coil = buffer[i].coil; event->injector = buffer[i].injector; @@ -81,7 +81,7 @@ static void SetNextCompositeEntry(efitick_t timestamp, bool trigger1, bool trigg buffer[NextIdx].timestamp = SWAP_UINT32(nowUs); buffer[NextIdx].priLevel = trigger1; buffer[NextIdx].secLevel = trigger2; - buffer[NextIdx].trigger = isTDC; + buffer[NextIdx].isTDC = isTDC; buffer[NextIdx].sync = engine->triggerCentral.triggerState.shaft_is_synchronized; buffer[NextIdx].coil = coil; buffer[NextIdx].injector = injector; @@ -144,6 +144,7 @@ void LogTriggerTopDeadCenter(efitick_t timestamp DECLARE_ENGINE_PARAMETER_SUFFIX return; } SetNextCompositeEntry(timestamp, trigger1, trigger2, true PASS_ENGINE_PARAMETER_SUFFIX); + SetNextCompositeEntry(timestamp + 10, trigger1, trigger2, false PASS_ENGINE_PARAMETER_SUFFIX); } void LogTriggerCoilState(efitick_t timestamp, bool state DECLARE_ENGINE_PARAMETER_SUFFIX) { diff --git a/unit_tests/engine_test_helper.cpp b/unit_tests/engine_test_helper.cpp index 437e5104df..0ac210e7e8 100644 --- a/unit_tests/engine_test_helper.cpp +++ b/unit_tests/engine_test_helper.cpp @@ -111,7 +111,7 @@ void EngineTestHelper::fireRise(float delayMs) { } void EngineTestHelper::smartFireRise(float delayMs) { - moveTimeForwardUs(MS2US(delayMs)); + smartMoveTimeForwardUs(MS2US(delayMs)); firePrimaryTriggerRise(); } @@ -121,7 +121,7 @@ void EngineTestHelper::fireFall(float delayMs) { } void EngineTestHelper::smartFireFall(float delayMs) { - moveTimeForwardUs(MS2US(delayMs)); + smartMoveTimeForwardUs(MS2US(delayMs)); firePrimaryTriggerFall(); } @@ -187,11 +187,31 @@ void EngineTestHelper::moveTimeForwardUs(int deltaTimeUs) { timeNowUs += deltaTimeUs; } +/** + * this method executed all pending events wile + */ void EngineTestHelper::smartMoveTimeForwardUs(int deltaTimeUs) { if (printTriggerDebug || printFuelDebug) { - printf("moveTimeForwardUs %.1fms\r\n", deltaTimeUs / 1000.0); + printf("smartMoveTimeForwardUs %.1fms\r\n", deltaTimeUs / 1000.0); } - timeNowUs += deltaTimeUs; + int targetTime = timeNowUs + deltaTimeUs; + + while (true) { + scheduling_s* nextScheduledEvent = engine.executor.getHead(); + if (nextScheduledEvent == nullptr) { + // nothing pending - we are done here + break; + } + int nextEventTime = nextScheduledEvent->momentX; + if (nextEventTime > targetTime) { + // next event is too far in the future + break; + } + timeNowUs = nextEventTime; + engine.executor.executeAll(timeNowUs); + } + + timeNowUs = targetTime; } efitimeus_t EngineTestHelper::getTimeNowUs(void) { @@ -233,9 +253,9 @@ static AngleBasedEvent * getElementAtIndexForUnitText(int index, Engine *engine) index--; } #if EFI_UNIT_TEST - firmwareError(OBD_PCM_Processor_Fault, "getForUnitText: null"); + firmwareError(OBD_PCM_Processor_Fault, "getElementAtIndexForUnitText: null"); #endif /* EFI_UNIT_TEST */ - return NULL; + return nullptr; } AngleBasedEvent * EngineTestHelper::assertTriggerEvent(const char *msg, diff --git a/unit_tests/global_execution_queue.cpp b/unit_tests/global_execution_queue.cpp index 4b20117b3d..3b8ca5bd4e 100644 --- a/unit_tests/global_execution_queue.cpp +++ b/unit_tests/global_execution_queue.cpp @@ -35,6 +35,10 @@ int TestExecutor::size() { return schedulingQueue.size(); } +scheduling_s* TestExecutor::getHead() { + return schedulingQueue.getHead(); +} + scheduling_s* TestExecutor::getForUnitTest(int index) { return schedulingQueue.getElementAtIndexForUnitText(index); } diff --git a/unit_tests/global_execution_queue.h b/unit_tests/global_execution_queue.h index c5ecec4435..f52cf0875a 100644 --- a/unit_tests/global_execution_queue.h +++ b/unit_tests/global_execution_queue.h @@ -18,7 +18,8 @@ public: void clear(); int executeAll(efitime_t now); int size(); - scheduling_s* getForUnitTest(int index); + scheduling_s * getHead(); + scheduling_s * getForUnitTest(int index); void setMockExecutor(ExecutorInterface* exec); private: diff --git a/unit_tests/logicdata.cpp b/unit_tests/logicdata.cpp index cd7b5f629e..b2da4a2d85 100644 --- a/unit_tests/logicdata.cpp +++ b/unit_tests/logicdata.cpp @@ -37,7 +37,7 @@ #define MAX_STRING_SIZE 40 -static char channelNames[][MAX_STRING_SIZE] = { "Primary", "Secondary", "Trg", +static char channelNames[][MAX_STRING_SIZE] = { "Primary", "Secondary", "TDC", "Sync", "Coil", "Injector", "Channel 6", "Channel 7" }; static int CHANNEL_FLAGS[] = { 0x13458b, 0x0000ff, 0x00a0f9, 0x00ffff, 0x00ff00, @@ -378,7 +378,7 @@ static int getChannelState(int ch, CompositeEvent *event) { case 1: return event->secondaryTrigger; case 2: - return event->trg; + return event->isTDC; case 3: return event->sync; case 4: diff --git a/unit_tests/logicdata.h b/unit_tests/logicdata.h index 249877487d..b29be49a9b 100644 --- a/unit_tests/logicdata.h +++ b/unit_tests/logicdata.h @@ -11,7 +11,7 @@ struct CompositeEvent { int timestamp; bool primaryTrigger; bool secondaryTrigger; - bool trg; + bool isTDC; bool sync; bool coil; bool injector; diff --git a/unit_tests/logicdata_sandbox.cpp b/unit_tests/logicdata_sandbox.cpp index 57a871d577..2f5056296d 100644 --- a/unit_tests/logicdata_sandbox.cpp +++ b/unit_tests/logicdata_sandbox.cpp @@ -8,13 +8,12 @@ static CompositeEvent events[100]; - -void setEvent(CompositeEvent *events, int index, - int timestamp, bool primaryTrigger, bool secondaryTrigger, bool trg, bool sync, bool coil, bool injector) { +static void setEvent(CompositeEvent *events, int index, + int timestamp, bool primaryTrigger, bool secondaryTrigger, bool isTDC, bool sync, bool coil, bool injector) { events[index].timestamp = timestamp; events[index].primaryTrigger = primaryTrigger; events[index].secondaryTrigger = secondaryTrigger; - events[index].trg = trg; + events[index].isTDC = isTDC; events[index].sync = sync; events[index].coil = coil; events[index].injector = injector; @@ -23,7 +22,6 @@ void setEvent(CompositeEvent *events, int index, void runLogicdataSandbox() { printf(".logicdata Sandbox 20200719\n"); - int index = 0; setEvent(events, index++, 10, false, false, false, false, false, false); setEvent(events, index++, 20, true, false, true, false, false, false); diff --git a/unit_tests/tests/ignition_injection/injection_mode_transition.cpp b/unit_tests/tests/ignition_injection/injection_mode_transition.cpp index 71d6799169..1a39069b4a 100644 --- a/unit_tests/tests/ignition_injection/injection_mode_transition.cpp +++ b/unit_tests/tests/ignition_injection/injection_mode_transition.cpp @@ -36,8 +36,5 @@ TEST(fuelControl, transitionIssue1592) { ASSERT_EQ(IM_SIMULTANEOUS, ENGINE(getCurrentInjectionMode(PASS_ENGINE_PARAMETER_SIGNATURE))); - eth.writeEvents("transitionIssue1592.logicdata"); - + eth.writeEvents("fuel_schedule_transition_issue_1592.logicdata"); } - - From d52fb0b16831aca40be76a8acd868c82482eac39 Mon Sep 17 00:00:00 2001 From: rusefi Date: Mon, 20 Jul 2020 13:38:33 -0400 Subject: [PATCH 8/9] better unit test logging --- .../engine_cycle/main_trigger_callback.cpp | 18 +++++++++++++---- .../controllers/engine_cycle/spark_logic.cpp | 20 ++++++++++++------- .../controllers/trigger/trigger_central.cpp | 2 +- unit_tests/efifeatures.h | 2 +- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/firmware/controllers/engine_cycle/main_trigger_callback.cpp b/firmware/controllers/engine_cycle/main_trigger_callback.cpp index ea43db5c0d..1877f312f0 100644 --- a/firmware/controllers/engine_cycle/main_trigger_callback.cpp +++ b/firmware/controllers/engine_cycle/main_trigger_callback.cpp @@ -97,7 +97,9 @@ void InjectorOutputPin::open() { overlappingCounter++; #if FUEL_MATH_EXTREME_LOGGING - printf("turnInjectionPinHigh %s %d %d\r\n", name, overlappingCounter, (int)getTimeNowUs()); + if (printFuelDebug) { + printf("turnInjectionPinHigh %s %d %d\r\n", name, overlappingCounter, (int)getTimeNowUs()); + } #endif /* FUEL_MATH_EXTREME_LOGGING */ if (overlappingCounter > 1) { @@ -106,7 +108,9 @@ void InjectorOutputPin::open() { // * this is another kind of overlap which happens in case of a small duty cycle after a large duty cycle // */ #if FUEL_MATH_EXTREME_LOGGING - printf("overlapping, no need to touch pin %s %d\r\n", output->name, (int)getTimeNowUs()); + if (printFuelDebug) { + printf("overlapping, no need to touch pin %s %d\r\n", name, (int)getTimeNowUs()); + } #endif /* FUEL_MATH_EXTREME_LOGGING */ } else { setHigh(); @@ -135,13 +139,17 @@ void turnInjectionPinHigh(InjectionEvent *event) { void InjectorOutputPin::close() { #if FUEL_MATH_EXTREME_LOGGING - printf("InjectorOutputPin::close %s %d %d\r\n", name, overlappingCounter, (int)getTimeNowUs()); + if (printFuelDebug) { + printf("InjectorOutputPin::close %s %d %d\r\n", name, overlappingCounter, (int)getTimeNowUs()); + } #endif /* FUEL_MATH_EXTREME_LOGGING */ overlappingCounter--; if (overlappingCounter > 0) { #if FUEL_MATH_EXTREME_LOGGING + if (printFuelDebug) { printf("was overlapping, no need to touch pin %s %d\r\n", name, (int)getTimeNowUs()); + } #endif /* FUEL_MATH_EXTREME_LOGGING */ } else { setLow(); @@ -315,7 +323,9 @@ static ALWAYS_INLINE void handleFuel(const bool limitedFuel, uint32_t trgEventIn } #if FUEL_MATH_EXTREME_LOGGING - scheduleMsg(logger, "handleFuel ind=%d %d", trgEventIndex, getRevolutionCounter()); + if (printFuelDebug) { + scheduleMsg(logger, "handleFuel ind=%d %d", trgEventIndex, getRevolutionCounter()); + } #endif /* FUEL_MATH_EXTREME_LOGGING */ ENGINE(tpsAccelEnrichment.onNewValue(Sensor::get(SensorType::Tps1).value_or(0) PASS_ENGINE_PARAMETER_SUFFIX)); diff --git a/firmware/controllers/engine_cycle/spark_logic.cpp b/firmware/controllers/engine_cycle/spark_logic.cpp index 0f852e4b99..5827e45ee8 100644 --- a/firmware/controllers/engine_cycle/spark_logic.cpp +++ b/firmware/controllers/engine_cycle/spark_logic.cpp @@ -25,6 +25,10 @@ EXTERN_ENGINE; extern bool verboseMode; #endif /* EFI_UNIT_TEST */ +#if EFI_PRINTF_FUEL_DETAILS || FUEL_MATH_EXTREME_LOGGING + extern bool printFuelDebug; +#endif // EFI_PRINTF_FUEL_DETAILS + static cyclic_buffer ignitionErrorDetection; static Logging *logger; @@ -47,7 +51,7 @@ static void fireSparkBySettingPinLow(IgnitionEvent *event, IgnitionOutputPin *ou #if SPARK_EXTREME_LOGGING scheduleMsg(logger, "spark goes low %d %s %d current=%d cnt=%d id=%d", getRevolutionCounter(), output->name, (int)getTimeNowUs(), output->currentLogicValue, output->outOfOrder, event->sparkId); -#endif /* FUEL_MATH_EXTREME_LOGGING */ +#endif /* SPARK_EXTREME_LOGGING */ /** * there are two kinds of 'out-of-order' @@ -119,7 +123,9 @@ static void prepareCylinderIgnitionSchedule(angle_t dwellAngleDuration, floatms_ event->dwellPosition.setAngle(dwellStartAngle PASS_ENGINE_PARAMETER_SUFFIX); #if FUEL_MATH_EXTREME_LOGGING - printf("addIgnitionEvent %s ind=%d\n", output->name, event->dwellPosition.triggerEventIndex); + if (printFuelDebug) { + printf("addIgnitionEvent %s ind=%d\n", output->name, event->dwellPosition.triggerEventIndex); + } // scheduleMsg(logger, "addIgnitionEvent %s ind=%d", output->name, event->dwellPosition->eventIndex); #endif /* FUEL_MATH_EXTREME_LOGGING */ } @@ -223,7 +229,7 @@ static void startDwellByTurningSparkPinHigh(IgnitionEvent *event, IgnitionOutput #if SPARK_EXTREME_LOGGING scheduleMsg(logger, "spark goes high %d %s %d current=%d cnt=%d id=%d", getRevolutionCounter(), output->name, (int)getTimeNowUs(), output->currentLogicValue, output->outOfOrder, event->sparkId); -#endif /* FUEL_MATH_EXTREME_LOGGING */ +#endif /* SPARK_EXTREME_LOGGING */ if (output->outOfOrder) { output->outOfOrder = false; @@ -308,7 +314,7 @@ bool scheduleOrQueue(AngleBasedEvent *event, if (isPending) { #if SPARK_EXTREME_LOGGING scheduleMsg(logger, "isPending thus not adding to queue index=%d rev=%d now=%d", trgEventIndex, getRevolutionCounter(), (int)getTimeNowUs()); -#endif /* FUEL_MATH_EXTREME_LOGGING */ +#endif /* SPARK_EXTREME_LOGGING */ } else { LL_APPEND2(ENGINE(angleBasedEventsHead), event, nextToothEvent); } @@ -350,7 +356,7 @@ static ALWAYS_INLINE void handleSparkEvent(bool limitedSpark, uint32_t trgEventI #if SPARK_EXTREME_LOGGING scheduleMsg(logger, "scheduling sparkUp ind=%d %d %s now=%d %d later id=%d", trgEventIndex, getRevolutionCounter(), event->getOutputForLoggins()->name, (int)getTimeNowUs(), (int)chargeDelayUs, event->sparkId); -#endif /* FUEL_MATH_EXTREME_LOGGING */ +#endif /* SPARK_EXTREME_LOGGING */ /** @@ -382,7 +388,7 @@ static ALWAYS_INLINE void handleSparkEvent(bool limitedSpark, uint32_t trgEventI } else { #if SPARK_EXTREME_LOGGING scheduleMsg(logger, "to queue sparkDown ind=%d %d %s now=%d for id=%d", trgEventIndex, getRevolutionCounter(), event->getOutputForLoggins()->name, (int)getTimeNowUs(), event->sparkEvent.position.triggerEventIndex); -#endif /* FUEL_MATH_EXTREME_LOGGING */ +#endif /* SPARK_EXTREME_LOGGING */ } @@ -461,7 +467,7 @@ static void scheduleAllSparkEventsUntilNextTriggerTooth(uint32_t trgEventIndex, #if SPARK_EXTREME_LOGGING scheduleMsg(logger, "time to invoke ind=%d %d %d", trgEventIndex, getRevolutionCounter(), (int)getTimeNowUs()); -#endif /* FUEL_MATH_EXTREME_LOGGING */ +#endif /* SPARK_EXTREME_LOGGING */ scheduleByAngle( sDown, diff --git a/firmware/controllers/trigger/trigger_central.cpp b/firmware/controllers/trigger/trigger_central.cpp index 149a052436..aa6d9d1404 100644 --- a/firmware/controllers/trigger/trigger_central.cpp +++ b/firmware/controllers/trigger/trigger_central.cpp @@ -480,7 +480,7 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta #if TRIGGER_EXTREME_LOGGING scheduleMsg(logger, "trigger %d %d %d", triggerIndexForListeners, getRevolutionCounter(), (int)getTimeNowUs()); -#endif /* FUEL_MATH_EXTREME_LOGGING */ +#endif /* TRIGGER_EXTREME_LOGGING */ /** * Here we invoke all the listeners - the main engine control logic is inside these listeners diff --git a/unit_tests/efifeatures.h b/unit_tests/efifeatures.h index 29679ab606..bb96796992 100644 --- a/unit_tests/efifeatures.h +++ b/unit_tests/efifeatures.h @@ -36,7 +36,7 @@ #define EFI_GPIO_HARDWARE TRUE -#define FUEL_MATH_EXTREME_LOGGING FALSE +#define FUEL_MATH_EXTREME_LOGGING TRUE #define EFI_DEFAILED_LOGGING FALSE From 1e8a9862f14ad8a00e1e573517f4d375ad6ac07a Mon Sep 17 00:00:00 2001 From: rusefi Date: Mon, 20 Jul 2020 14:25:32 -0400 Subject: [PATCH 9/9] unit test framework improvements for smoother event execution --- .../ignition_injection/test_dwell_corner_case_issue_796.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/unit_tests/tests/ignition_injection/test_dwell_corner_case_issue_796.cpp b/unit_tests/tests/ignition_injection/test_dwell_corner_case_issue_796.cpp index 9c2cd55dd0..db280b2ca6 100644 --- a/unit_tests/tests/ignition_injection/test_dwell_corner_case_issue_796.cpp +++ b/unit_tests/tests/ignition_injection/test_dwell_corner_case_issue_796.cpp @@ -21,11 +21,12 @@ TEST(scheduler, dwellIssue796) { ASSERT_EQ(300000, ENGINE(rpmCalculator.oneDegreeUs) * 180); // with just a bit much time between events integer RPM goes down one full percent - eth.fireRise(601); - eth.fireFall(600); + eth.smartFireRise(601); + eth.smartFireFall(600); ASSERT_NEAR( 99, GET_RPM(), EPS3D) << "spinning-RPM#2"; // while integer RPM value is 1% away from rpm=100, below oneDegreeUs is much closer to RPM=100 value ASSERT_EQ(300250, (int)(ENGINE(rpmCalculator.oneDegreeUs) * 180)); + eth.writeEvents("dwell_issue_1592.logicdata"); }