From 5cb90d0e1b57ca6b9edee20ea32f5d9f899ed9ce Mon Sep 17 00:00:00 2001 From: rusefi Date: Sun, 13 Oct 2019 09:59:06 -0400 Subject: [PATCH 01/10] preparation for #961 more unified access to pre-calculated value --- firmware/controllers/algo/engine2.cpp | 4 +--- firmware/controllers/obd2.cpp | 2 +- firmware/controllers/trigger/main_trigger_callback.cpp | 4 ++-- unit_tests/tests/test_fuel_map.cpp | 5 ++--- unit_tests/tests/test_logic_expression.cpp | 3 ++- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/firmware/controllers/algo/engine2.cpp b/firmware/controllers/algo/engine2.cpp index 28c71b01a1..adeb59c4b3 100644 --- a/firmware/controllers/algo/engine2.cpp +++ b/firmware/controllers/algo/engine2.cpp @@ -223,9 +223,7 @@ void EngineState::periodicFastCallback(DECLARE_ENGINE_PARAMETER_SIGNATURE) { void EngineState::updateTChargeK(int rpm, float tps DECLARE_ENGINE_PARAMETER_SUFFIX) { #if EFI_ENGINE_CONTROL - float coolantC = ENGINE(sensors.clt); - float intakeC = ENGINE(sensors.iat); - float newTCharge = getTCharge(rpm, tps, coolantC, intakeC PASS_ENGINE_PARAMETER_SUFFIX); + float newTCharge = getTCharge(rpm, tps, getCoolantTemperature(), getIntakeAirTemperature() PASS_ENGINE_PARAMETER_SUFFIX); // convert to microsecs and then to seconds efitick_t curTime = getTimeNowNt(); float secsPassed = (float)NT2US(curTime - timeSinceLastTChargeK) / 1000000.0f; diff --git a/firmware/controllers/obd2.cpp b/firmware/controllers/obd2.cpp index 65b4dfbf4b..86707408f8 100644 --- a/firmware/controllers/obd2.cpp +++ b/firmware/controllers/obd2.cpp @@ -37,7 +37,7 @@ #include "tps.h" #include "engine_math.h" #include "fuel_math.h" -#include "allsensors.h" +#include "thermistors.h" extern CANTxFrame txmsg; diff --git a/firmware/controllers/trigger/main_trigger_callback.cpp b/firmware/controllers/trigger/main_trigger_callback.cpp index 61727a5878..29b8f63883 100644 --- a/firmware/controllers/trigger/main_trigger_callback.cpp +++ b/firmware/controllers/trigger/main_trigger_callback.cpp @@ -337,7 +337,7 @@ static ALWAYS_INLINE void handleFuelInjectionEvent(int injEventIndex, InjectionE static void fuelClosedLoopCorrection(DECLARE_ENGINE_PARAMETER_SIGNATURE) { #if ! EFI_UNIT_TEST if (GET_RPM_VALUE < CONFIG(fuelClosedLoopRpmThreshold) || - ENGINE(sensors.clt) < CONFIG(fuelClosedLoopCltThreshold) || + getCoolantTemperature() < CONFIG(fuelClosedLoopCltThreshold) || getTPS(PASS_ENGINE_PARAMETER_SIGNATURE) > CONFIG(fuelClosedLoopTpsThreshold) || ENGINE(sensors.currentAfr) < CONFIGB(fuelClosedLoopAfrLowThreshold) || ENGINE(sensors.currentAfr) > engineConfiguration->fuelClosedLoopAfrHighThreshold) { @@ -578,7 +578,7 @@ void startPrimeInjectionPulse(DECLARE_ENGINE_PARAMETER_SIGNATURE) { // If 'primeInjFalloffTemperature' is not specified (by default), we have a prime pulse deactivation at zero celsius degrees, which is okay. const float maxPrimeInjAtTemperature = -40.0f; // at this temperature the pulse is maximal. floatms_t pulseLength = interpolateClamped(maxPrimeInjAtTemperature, CONFIG(startOfCrankingPrimingPulse), - CONFIG(primeInjFalloffTemperature), 0.0f, ENGINE(sensors.clt)); + CONFIG(primeInjFalloffTemperature), 0.0f, getCoolantTemperature()); if (pulseLength > 0) { startSimultaniousInjection(engine); efitimeus_t turnOffDelayUs = (efitimeus_t)efiRound(MS2US(pulseLength), 1.0f); diff --git a/unit_tests/tests/test_fuel_map.cpp b/unit_tests/tests/test_fuel_map.cpp index 0b2b7e021c..43b7c72c53 100644 --- a/unit_tests/tests/test_fuel_map.cpp +++ b/unit_tests/tests/test_fuel_map.cpp @@ -85,11 +85,10 @@ TEST(misc, testFuelMap) { setFlatInjectorLag(0 PASS_CONFIG_PARAMETER_SUFFIX); - float iat = getIntakeAirTemperature(); - ASSERT_FALSE(cisnan(iat)); + ASSERT_FALSE(cisnan(getIntakeAirTemperature())); float iatCorrection = getIatFuelCorrection(-KELV PASS_ENGINE_PARAMETER_SUFFIX); ASSERT_EQ( 2, iatCorrection) << "IAT"; - engine->sensors.clt = getCoolantTemperature(); + ASSERT_FALSE(cisnan(getCoolantTemperature())); float cltCorrection = getCltFuelCorrection(PASS_ENGINE_PARAMETER_SIGNATURE); ASSERT_EQ( 1, cltCorrection) << "CLT"; float injectorLag = getInjectorLag(getVBatt(PASS_ENGINE_PARAMETER_SIGNATURE) PASS_ENGINE_PARAMETER_SUFFIX); diff --git a/unit_tests/tests/test_logic_expression.cpp b/unit_tests/tests/test_logic_expression.cpp index fe6c21b18e..59003b9fac 100644 --- a/unit_tests/tests/test_logic_expression.cpp +++ b/unit_tests/tests/test_logic_expression.cpp @@ -11,6 +11,7 @@ #include "fsio_impl.h" #include "cli_registry.h" #include "engine_test_helper.h" +#include "thermistors.h" #define TEST_POOL_SIZE 256 @@ -19,7 +20,7 @@ float getEngineValue(le_action_e action DECLARE_ENGINE_PARAMETER_SUFFIX) { case LE_METHOD_FAN: return engine->fsioState.mockFan; case LE_METHOD_COOLANT: - return engine->sensors.clt; + return getCoolantTemperature(); case LE_METHOD_RPM: return engine->fsioState.mockRpm; case LE_METHOD_CRANKING_RPM: From 38db34fe58592b78f1ce3b4488b872d026722188 Mon Sep 17 00:00:00 2001 From: rusefi Date: Sun, 13 Oct 2019 09:59:43 -0400 Subject: [PATCH 02/10] sweet unit test hack --- unit_tests/unit_test_framework.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/unit_tests/unit_test_framework.h b/unit_tests/unit_test_framework.h index 808b6c6044..66e16cbe15 100644 --- a/unit_tests/unit_test_framework.h +++ b/unit_tests/unit_test_framework.h @@ -12,6 +12,9 @@ #include "gtest/gtest.h" #include "gmock/gmock.h" +// This lets us inspect private state from unit tests +#define private public + #define EPS1D 0.1 #define EPS2D 0.01 #define EPS3D 0.001 From 12137fc7cd0f7ec808be2e73c20e39d9a7d80c6f Mon Sep 17 00:00:00 2001 From: rusefi Date: Mon, 14 Oct 2019 06:18:08 -0400 Subject: [PATCH 03/10] preparing for #974 code comments & refactoring --- firmware/controllers/map_averaging.cpp | 7 +++- firmware/controllers/map_averaging.h | 3 ++ .../controllers/trigger/rpm_calculator.cpp | 33 ++++++------------- firmware/controllers/trigger/rpm_calculator.h | 4 +-- firmware/controllers/trigger/spark_logic.cpp | 9 +++++ firmware/development/engine_sniffer.cpp | 4 +-- firmware/development/sensor_chart.cpp | 2 +- firmware/development/wave_analyzer.cpp | 2 +- unit_tests/tests/test_trigger_decoder.cpp | 18 +++++----- 9 files changed, 43 insertions(+), 39 deletions(-) diff --git a/firmware/controllers/map_averaging.cpp b/firmware/controllers/map_averaging.cpp index cc781f6e30..ef97130c20 100644 --- a/firmware/controllers/map_averaging.cpp +++ b/firmware/controllers/map_averaging.cpp @@ -236,6 +236,9 @@ void refreshMapAveragingPreCalc(DECLARE_ENGINE_PARAMETER_SIGNATURE) { for (int i = 0; i < engineConfiguration->specs.cylindersCount; i++) { angle_t cylinderOffset = getEngineCycle(engine->getOperationMode(PASS_ENGINE_PARAMETER_SIGNATURE)) * i / engineConfiguration->specs.cylindersCount; efiAssertVoid(CUSTOM_ERR_6692, !cisnan(cylinderOffset), "cylinderOffset"); + // part of this formula related to specific cylinder offset is never changing - we can + // move the loop into start-up calculation and not have this loop as part of periodic calculation + // todo: change the logic as described above in order to reduce periodic CPU usage? float cylinderStart = start + cylinderOffset - offsetAngle + tdcPosition(); fixAngle(cylinderStart, "cylinderStart", CUSTOM_ERR_6562); engine->engineState.mapAveragingStart[i] = cylinderStart; @@ -297,7 +300,9 @@ static void mapAveragingTriggerCallback(trigger_event_e ckpEventType, fixAngle(samplingEnd, "samplingEnd", CUSTOM_ERR_6563); // only if value is already prepared - int structIndex = engine->rpmCalculator.getRevolutionCounter() % 2; + int structIndex = getRevolutionCounter() % 2; + // at the moment we schedule based on time prediction based on current RPM and angle + // we are loosing precision in case of changing RPM - the further away is the event the worse is precision // todo: schedule this based on closest trigger event, same as ignition works scheduleByAngle(rpm, &startTimer[i][structIndex], samplingStart, startAveraging, NULL, &engine->rpmCalculator); diff --git a/firmware/controllers/map_averaging.h b/firmware/controllers/map_averaging.h index 08274f6296..7da86b64ac 100644 --- a/firmware/controllers/map_averaging.h +++ b/firmware/controllers/map_averaging.h @@ -18,7 +18,10 @@ void mapAveragingAdcCallback(adcsample_t newValue); void initMapAveraging(Logging *sharedLogger, Engine *engine); void refreshMapAveragingPreCalc(DECLARE_ENGINE_PARAMETER_SIGNATURE); + +#if EFI_TUNER_STUDIO void postMapState(TunerStudioOutputChannels *tsOutputChannels); +#endif #endif /* EFI_MAP_AVERAGING */ diff --git a/firmware/controllers/trigger/rpm_calculator.cpp b/firmware/controllers/trigger/rpm_calculator.cpp index 2927627e39..76b7d47cad 100644 --- a/firmware/controllers/trigger/rpm_calculator.cpp +++ b/firmware/controllers/trigger/rpm_calculator.cpp @@ -6,8 +6,11 @@ * Actual getRpm() is calculated once per crankshaft revolution, based on the amount of time passed * since the start of previous shaft revolution. * + * We also have 'instant RPM' logic separate from this 'cycle RPM' logic. Open question is why do we not use + * instant RPM instead of cycle RPM more often. + * * @date Jan 1, 2013 - * @author Andrey Belomutskiy, (c) 2012-2018 + * @author Andrey Belomutskiy, (c) 2012-2019 */ #include "global.h" @@ -21,8 +24,7 @@ #if EFI_PROD_CODE #include "os_util.h" -#include "engine.h" -#endif +#endif /* EFI_PROD_CODE */ #if EFI_SENSOR_CHART #include "sensor_chart.h" @@ -85,8 +87,6 @@ extern bool hasFirmwareErrorFlag; static Logging * logger; -int revolutionCounterSinceBootForUnitTest = 0; - RpmCalculator::RpmCalculator() { #if !EFI_PROD_CODE mockRpm = MOCK_UNDEFINED; @@ -96,7 +96,6 @@ RpmCalculator::RpmCalculator() { // we need this initial to have not_running at first invocation lastRpmEventTimeNt = (efitime_t) -10 * US2NT(US_PER_SECOND_LL); - revolutionCounterSinceBootForUnitTest = 0; } /** @@ -178,12 +177,9 @@ spinning_state_e RpmCalculator::getState() const { void RpmCalculator::onNewEngineCycle() { revolutionCounterSinceBoot++; revolutionCounterSinceStart++; -#if EFI_UNIT_TEST - revolutionCounterSinceBootForUnitTest = revolutionCounterSinceBoot; -#endif /* EFI_UNIT_TEST */ } -uint32_t RpmCalculator::getRevolutionCounter(void) const { +uint32_t RpmCalculator::getRevolutionCounterM(void) const { return revolutionCounterSinceBoot; } @@ -230,9 +226,7 @@ void RpmCalculator::setSpinningUp(efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFI void rpmShaftPositionCallback(trigger_event_e ckpSignalType, uint32_t index DECLARE_ENGINE_PARAMETER_SUFFIX) { efitick_t nowNt = getTimeNowNt(); -#if EFI_PROD_CODE efiAssertVoid(CUSTOM_ERR_6632, getCurrentRemainingStack() > 256, "lowstckRCL"); -#endif RpmCalculator *rpmState = &engine->rpmCalculator; @@ -315,7 +309,7 @@ static void tdcMarkCallback(trigger_event_e ckpSignalType, (void) ckpSignalType; bool isTriggerSynchronizationPoint = index0 == 0; if (isTriggerSynchronizationPoint && ENGINE(isEngineChartEnabled)) { - int revIndex2 = engine->rpmCalculator.getRevolutionCounter() % 2; + int revIndex2 = getRevolutionCounter() % 2; int rpm = GET_RPM(); // todo: use tooth event-based scheduling, not just time-based scheduling if (isValidRpm(rpm)) { @@ -326,12 +320,6 @@ static void tdcMarkCallback(trigger_event_e ckpSignalType, } #endif -#if EFI_PROD_CODE || EFI_SIMULATOR -int getRevolutionCounter() { - return engine->rpmCalculator.getRevolutionCounter(); -} -#endif - /** * @return Current crankshaft angle, 0 to 720 for four-stroke */ @@ -361,7 +349,6 @@ void initRpmCalculator(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX) { addTriggerEventListener(rpmShaftPositionCallback, "rpm reporter", engine); } -#if EFI_PROD_CODE || EFI_SIMULATOR /** * Schedules a callback 'angle' degree of crankshaft from now. * The callback would be executed once after the duration of time which @@ -369,14 +356,14 @@ void initRpmCalculator(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX) { */ void scheduleByAngle(int rpm, scheduling_s *timer, angle_t angle, schfunc_t callback, void *param, RpmCalculator *calc DECLARE_ENGINE_PARAMETER_SUFFIX) { + // todo: remove 'calc' parameter UNUSED(rpm); efiAssertVoid(CUSTOM_ANGLE_NAN, !cisnan(angle), "NaN angle?"); efiAssertVoid(CUSTOM_ERR_6634, isValidRpm(rpm), "RPM check expected"); - float delayUs = calc->oneDegreeUs * angle; + float delayUs = ENGINE(rpmCalculator.oneDegreeUs) * angle; efiAssertVoid(CUSTOM_ERR_6635, !cisnan(delayUs), "NaN delay?"); - engine->executor.scheduleForLater(timer, (int) delayUs, callback, param); + ENGINE(executor.scheduleForLater(timer, (int) delayUs, callback, param)); } -#endif #else RpmCalculator::RpmCalculator() { diff --git a/firmware/controllers/trigger/rpm_calculator.h b/firmware/controllers/trigger/rpm_calculator.h index c07853d013..eec0a6ff31 100644 --- a/firmware/controllers/trigger/rpm_calculator.h +++ b/firmware/controllers/trigger/rpm_calculator.h @@ -89,7 +89,7 @@ public: * This method is invoked once per engine cycle right after we calculate new RPM value */ void onNewEngineCycle(); - uint32_t getRevolutionCounter(void) const; + uint32_t getRevolutionCounterM(void) const; void setRpmValue(int value DECLARE_ENGINE_PARAMETER_SUFFIX); /** * The same as setRpmValue() but without state change. @@ -158,7 +158,7 @@ void initRpmCalculator(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX); float getCrankshaftAngleNt(efitime_t timeNt DECLARE_ENGINE_PARAMETER_SUFFIX); -int getRevolutionCounter(void); +#define getRevolutionCounter() ENGINE(rpmCalculator.getRevolutionCounterM()) #if EFI_ENGINE_SNIFFER #define addEngineSnifferEvent(name, msg) if (ENGINE(isEngineChartEnabled)) { waveChart.addEvent3((name), (msg)); } diff --git a/firmware/controllers/trigger/spark_logic.cpp b/firmware/controllers/trigger/spark_logic.cpp index 314973f559..3d6f0dd220 100644 --- a/firmware/controllers/trigger/spark_logic.cpp +++ b/firmware/controllers/trigger/spark_logic.cpp @@ -36,6 +36,10 @@ int isIgnitionTimingError(void) { } static void fireSparkBySettingPinLow(IgnitionEvent *event, IgnitionOutputPin *output) { +#if EFI_UNIT_TEST + Engine *engine = event->engine; +#endif /* EFI_UNIT_TEST */ + #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); @@ -161,7 +165,12 @@ if (engineConfiguration->debugMode == DBG_DWELL_METRIC) { } static void startDwellByTurningSparkPinHigh(IgnitionEvent *event, IgnitionOutputPin *output) { +#if EFI_UNIT_TEST + Engine *engine = event->engine; + EXPAND_Engine; +#endif /* EFI_UNIT_TEST */ + // todo: no reason for this to be disabled in unit_test mode?! #if ! EFI_UNIT_TEST if (GET_RPM_VALUE > 2 * engineConfiguration->cranking.rpm) { diff --git a/firmware/development/engine_sniffer.cpp b/firmware/development/engine_sniffer.cpp index dca046ed26..66fa82edef 100644 --- a/firmware/development/engine_sniffer.cpp +++ b/firmware/development/engine_sniffer.cpp @@ -77,7 +77,7 @@ static uint32_t skipUntilEngineCycle = 0; #if ! EFI_UNIT_TEST extern WaveChart waveChart; static void resetNow(void) { - skipUntilEngineCycle = engine->rpmCalculator.getRevolutionCounter() + 3; + skipUntilEngineCycle = getRevolutionCounter() + 3; waveChart.reset(); } #endif @@ -169,7 +169,7 @@ void WaveChart::addEvent3(const char *name, const char * msg) { if (!ENGINE(isEngineChartEnabled)) { return; } - if (skipUntilEngineCycle != 0 && ENGINE(rpmCalculator.getRevolutionCounter()) < skipUntilEngineCycle) + if (skipUntilEngineCycle != 0 && getRevolutionCounter() < skipUntilEngineCycle) return; #if EFI_SIMULATOR // todo: add UI control to enable this for firmware if desired diff --git a/firmware/development/sensor_chart.cpp b/firmware/development/sensor_chart.cpp index b728af810a..83e26fb806 100644 --- a/firmware/development/sensor_chart.cpp +++ b/firmware/development/sensor_chart.cpp @@ -39,7 +39,7 @@ void scAddData(float angle, float value) { return; } - if (engine->rpmCalculator.getRevolutionCounter() % engineConfiguration->sensorChartFrequency != 0) { + if (getRevolutionCounter() % engineConfiguration->sensorChartFrequency != 0) { /** * We are here if we do NOT need to add an event to the analog chart */ diff --git a/firmware/development/wave_analyzer.cpp b/firmware/development/wave_analyzer.cpp index 9227fe2b64..0f5a1ee8ac 100644 --- a/firmware/development/wave_analyzer.cpp +++ b/firmware/development/wave_analyzer.cpp @@ -80,7 +80,7 @@ void WaveReader::onFallEvent() { efitick_t width = nowUs - widthEventTimeUs; last_wave_high_widthUs = width; - int revolutionCounter = engine->rpmCalculator.getRevolutionCounter(); + int revolutionCounter = getRevolutionCounter(); totalOnTimeAccumulatorUs += width; if (currentRevolutionCounter != revolutionCounter) { diff --git a/unit_tests/tests/test_trigger_decoder.cpp b/unit_tests/tests/test_trigger_decoder.cpp index 46547edc26..44119a1618 100644 --- a/unit_tests/tests/test_trigger_decoder.cpp +++ b/unit_tests/tests/test_trigger_decoder.cpp @@ -584,7 +584,7 @@ static void setTestBug299(EngineTestHelper *eth) { // inj #0 |.......#|........|.......#|........| // inj #1 |........|.......#|........|.......#| ASSERT_EQ( 4, engine->executor.size()) << "qs#00"; - ASSERT_EQ( 3, engine->rpmCalculator.getRevolutionCounter()) << "rev cnt#3"; + ASSERT_EQ( 3, getRevolutionCounter()) << "rev cnt#3"; eth->assertInjectorUpEvent("setTestBug299: 1@0", 0, MS2US(8.5), 0); eth->assertInjectorDownEvent("@1", 1, MS2US(10), 0); eth->assertInjectorUpEvent("1@2", 2, MS2US(18.5), 1); @@ -607,7 +607,7 @@ static void setTestBug299(EngineTestHelper *eth) { // inj #0 |.......#|........|.......#|........| // inj #1 |........|.......#|........|.......#| ASSERT_EQ( 8, engine->executor.size()) << "qs#0"; - ASSERT_EQ( 3, engine->rpmCalculator.getRevolutionCounter()) << "rev cnt#3"; + ASSERT_EQ( 3, getRevolutionCounter()) << "rev cnt#3"; eth->assertInjectorUpEvent("02@0", 0, MS2US(-11.5), 0); eth->assertInjectorDownEvent("@1", 1, MS2US(-10), 0); eth->assertInjectorUpEvent("@2", 2, MS2US(-1.5), 1); @@ -650,7 +650,7 @@ static void setTestBug299(EngineTestHelper *eth) { // inj #0 |.......#|........|........|........| // inj #1 |........|.......#|........|........| ASSERT_EQ( 4, engine->executor.size()) << "qs#0-2"; - ASSERT_EQ( 4, engine->rpmCalculator.getRevolutionCounter()) << "rev cnt#4"; + ASSERT_EQ( 4, getRevolutionCounter()) << "rev cnt#4"; eth->assertInjectorUpEvent("0@0", 0, MS2US(8.5), 0); eth->assertInjectorDownEvent("0@1", 1, MS2US(10), 0); eth->assertInjectorUpEvent("0@2", 2, MS2US(18.5), 1); @@ -706,9 +706,9 @@ void doTestFuelSchedulerBug299smallAndMedium(int startUpDelayMs) { ASSERT_EQ( 0, engine->executor.size()) << "qs#1#2"; - ASSERT_EQ( 4, engine->rpmCalculator.getRevolutionCounter()) << "rev cnt#4#0"; + ASSERT_EQ( 4, getRevolutionCounter()) << "rev cnt#4#0"; eth.firePrimaryTriggerRise(); - ASSERT_EQ( 5, engine->rpmCalculator.getRevolutionCounter()) << "rev cnt#4#1"; + ASSERT_EQ( 5, getRevolutionCounter()) << "rev cnt#4#1"; // time...|0.......|10......|20......|30......|40......|50......|60......| // inj #0 |########|##...###|########|.....###|########|........|........| // inj #1 |.....###|########|....####|########|........|........|........| @@ -726,7 +726,7 @@ void doTestFuelSchedulerBug299smallAndMedium(int startUpDelayMs) { // { // scheduling_s *ev = engine->executor.getForUnitTest(9); -// ASSERT_EQ( 5, engine->rpmCalculator.getRevolutionCounter()) << "rev cnt#4#2"; +// ASSERT_EQ( 5, getRevolutionCounter()) << "rev cnt#4#2"; // ASSERT_TRUE(ev == &engineConfiguration->fuelActuators[2].signalPair[1].signalTimerDown) << "down 50"; // } @@ -736,7 +736,7 @@ void doTestFuelSchedulerBug299smallAndMedium(int startUpDelayMs) { eth.fireFall(20); ASSERT_EQ( 8, engine->executor.size()) << "qs#2#1"; - ASSERT_EQ( 5, engine->rpmCalculator.getRevolutionCounter()) << "rev cnt#5"; + ASSERT_EQ( 5, getRevolutionCounter()) << "rev cnt#5"; // using old fuel schedule - but already wider pulses // time...|-20.....|-10.....|0.......|10......|20......|30......|40......| // inj #0 |........|.....###|########|.....###|########|........|........| @@ -781,7 +781,7 @@ void doTestFuelSchedulerBug299smallAndMedium(int startUpDelayMs) { eth.firePrimaryTriggerRise(); ASSERT_EQ( 4, engine->executor.size()) << "qs#2#2"; - ASSERT_EQ( 6, engine->rpmCalculator.getRevolutionCounter()) << "rev cnt6"; + ASSERT_EQ( 6, getRevolutionCounter()) << "rev cnt6"; // time...|-20.....|-10.....|0.......|10......|20......|30......|40......| // inj #0 |########|.....###|########|....####|........|........|........| // inj #1 |.....###|########|.....###|########|........|........|........| @@ -821,7 +821,7 @@ void doTestFuelSchedulerBug299smallAndMedium(int startUpDelayMs) { eth.firePrimaryTriggerFall(); ASSERT_EQ( 5, engine->executor.size()) << "qs#3"; - ASSERT_EQ( 6, engine->rpmCalculator.getRevolutionCounter()) << "rev cnt6"; + ASSERT_EQ( 6, getRevolutionCounter()) << "rev cnt6"; ASSERT_EQ( 0, eth.executeActions()) << "executed #6"; From 7bb8bb7f140d6d96962da164262b42e2ed3f4807 Mon Sep 17 00:00:00 2001 From: rusefi Date: Mon, 14 Oct 2019 09:09:08 -0400 Subject: [PATCH 04/10] preparing for #974 code comments & refactoring --- firmware/controllers/engine_controller.cpp | 4 +-- firmware/controllers/map_averaging.cpp | 30 ++++++++++++++-------- firmware/controllers/map_averaging.h | 2 +- unit_tests/efifeatures.h | 1 + unit_tests/main.cpp | 8 +----- 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/firmware/controllers/engine_controller.cpp b/firmware/controllers/engine_controller.cpp index dd64d509fb..c5c35ca481 100644 --- a/firmware/controllers/engine_controller.cpp +++ b/firmware/controllers/engine_controller.cpp @@ -774,7 +774,7 @@ void initEngineContoller(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX) #if EFI_MAP_AVERAGING if (engineConfiguration->isMapAveragingEnabled) { - initMapAveraging(sharedLogger, engine); + initMapAveraging(sharedLogger PASS_ENGINE_PARAMETER_SUFFIX); } #endif /* EFI_MAP_AVERAGING */ @@ -831,6 +831,6 @@ int getRusEfiVersion(void) { if (initBootloader() != 0) return 123; #endif /* EFI_BOOTLOADER_INCLUDE_CODE */ - return 20191009; + return 20191013; } #endif /* EFI_UNIT_TEST */ diff --git a/firmware/controllers/map_averaging.cpp b/firmware/controllers/map_averaging.cpp index ef97130c20..3a6cda0e5a 100644 --- a/firmware/controllers/map_averaging.cpp +++ b/firmware/controllers/map_averaging.cpp @@ -110,15 +110,19 @@ static bool isAveraging = false; static void startAveraging(void *arg) { (void) arg; efiAssertVoid(CUSTOM_ERR_6649, getCurrentRemainingStack() > 128, "lowstck#9"); +#if !EFI_UNIT_TEST bool wasLocked = lockAnyContext(); - ; +#endif // with locking we would have a consistent state mapAdcAccumulator = 0; mapMeasurementsCounter = 0; isAveraging = true; - if (!wasLocked) +#if !EFI_UNIT_TEST + if (!wasLocked) { unlockAnyContext(); - ; + } +#endif /* !EFI_UNIT_TEST */ + mapAveragingPin.setHigh(); } @@ -176,7 +180,9 @@ void mapAveragingAdcCallback(adcsample_t adcValue) { static void endAveraging(void *arg) { (void) arg; +#if ! EFI_UNIT_TEST bool wasLocked = lockAnyContext(); +#endif isAveraging = false; // with locking we would have a consistent state #if HAL_USE_ADC @@ -197,13 +203,15 @@ static void endAveraging(void *arg) { warning(CUSTOM_UNEXPECTED_MAP_VALUE, "No MAP values"); } #endif +#if ! EFI_UNIT_TEST if (!wasLocked) unlockAnyContext(); ; +#endif mapAveragingPin.setLow(); } -static void applyMapMinBufferLength() { +static void applyMapMinBufferLength(DECLARE_ENGINE_PARAMETER_SIGNATURE) { // check range mapMinBufferLength = maxI(minI(CONFIGB(mapMinBufferLength), MAX_MAP_BUFFER_LENGTH), 1); // reset index @@ -214,14 +222,14 @@ static void applyMapMinBufferLength() { } } -void postMapState(TunerStudioOutputChannels *tsOutputChannels) { #if EFI_TUNER_STUDIO +void postMapState(TunerStudioOutputChannels *tsOutputChannels) { tsOutputChannels->debugFloatField1 = v_averagedMapValue; tsOutputChannels->debugFloatField2 = engine->engineState.mapAveragingDuration; tsOutputChannels->debugFloatField3 = currentPressure; tsOutputChannels->debugIntField1 = mapMeasurementsCounter; -#endif /* EFI_TUNER_STUDIO */ } +#endif /* EFI_TUNER_STUDIO */ void refreshMapAveragingPreCalc(DECLARE_ENGINE_PARAMETER_SIGNATURE) { int rpm = GET_RPM_VALUE; @@ -271,7 +279,7 @@ static void mapAveragingTriggerCallback(trigger_event_e ckpEventType, } if (CONFIGB(mapMinBufferLength) != mapMinBufferLength) { - applyMapMinBufferLength(); + applyMapMinBufferLength(PASS_ENGINE_PARAMETER_SIGNATURE); } measurementsPerRevolution = measurementsPerRevolutionCounter; @@ -305,9 +313,9 @@ static void mapAveragingTriggerCallback(trigger_event_e ckpEventType, // we are loosing precision in case of changing RPM - the further away is the event the worse is precision // todo: schedule this based on closest trigger event, same as ignition works scheduleByAngle(rpm, &startTimer[i][structIndex], samplingStart, - startAveraging, NULL, &engine->rpmCalculator); + startAveraging, NULL, &engine->rpmCalculator PASS_ENGINE_PARAMETER_SUFFIX); scheduleByAngle(rpm, &endTimer[i][structIndex], samplingEnd, - endAveraging, NULL, &engine->rpmCalculator); + endAveraging, NULL, &engine->rpmCalculator PASS_ENGINE_PARAMETER_SUFFIX); engine->m.mapAveragingCbTime = getTimeNowLowerNt() - engine->m.beforeMapAveragingCb; } @@ -339,7 +347,7 @@ float getMap(void) { } #endif /* EFI_PROD_CODE */ -void initMapAveraging(Logging *sharedLogger, Engine *engine) { +void initMapAveraging(Logging *sharedLogger DECLARE_CONFIG_PARAMETER_SUFFIX) { logger = sharedLogger; // startTimer[0].name = "map start0"; @@ -351,7 +359,7 @@ void initMapAveraging(Logging *sharedLogger, Engine *engine) { addTriggerEventListener(&mapAveragingTriggerCallback, "MAP averaging", engine); #endif /* EFI_SHAFT_POSITION_INPUT */ addConsoleAction("faststat", showMapStats); - applyMapMinBufferLength(); + applyMapMinBufferLength(PASS_ENGINE_PARAMETER_SIGNATURE); } #else diff --git a/firmware/controllers/map_averaging.h b/firmware/controllers/map_averaging.h index 7da86b64ac..6df7c1125b 100644 --- a/firmware/controllers/map_averaging.h +++ b/firmware/controllers/map_averaging.h @@ -16,7 +16,7 @@ void mapAveragingAdcCallback(adcsample_t newValue); #endif -void initMapAveraging(Logging *sharedLogger, Engine *engine); +void initMapAveraging(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX); void refreshMapAveragingPreCalc(DECLARE_ENGINE_PARAMETER_SIGNATURE); #if EFI_TUNER_STUDIO diff --git a/unit_tests/efifeatures.h b/unit_tests/efifeatures.h index 9e00d3bc6e..64a8f46500 100644 --- a/unit_tests/efifeatures.h +++ b/unit_tests/efifeatures.h @@ -64,5 +64,6 @@ #define EFI_BOARD_TEST FALSE #define EFI_JOYSTICK FALSE +#define EFI_MAP_AVERAGING FALSE #endif /* EFIFEATURES_H_ */ diff --git a/unit_tests/main.cpp b/unit_tests/main.cpp index 62b6707033..aa1b950b9a 100644 --- a/unit_tests/main.cpp +++ b/unit_tests/main.cpp @@ -34,12 +34,6 @@ efitick_t getTimeNowNt(void) { LoggingWithStorage sharedLogger("main"); -extern int revolutionCounterSinceBootForUnitTest; - -int getRevolutionCounter(void) { - return revolutionCounterSinceBootForUnitTest; -} - extern bool printTriggerDebug; bool verboseMode = false; @@ -47,7 +41,7 @@ GTEST_API_ int main(int argc, char **argv) { // printTriggerDebug = true; // resizeMap(); - printf("Success 20190825\r\n"); + printf("Success 20191013\r\n"); printAllTriggers(); // printConvertedTable(); testing::InitGoogleTest(&argc, argv); From fc5232da697258e7e5a3ed56d97cd9b38e4d0618 Mon Sep 17 00:00:00 2001 From: rusefi Date: Mon, 14 Oct 2019 12:58:27 -0400 Subject: [PATCH 05/10] poke --- firmware/svnversion.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/firmware/svnversion.h b/firmware/svnversion.h index 6a7347951a..5963e218da 100644 --- a/firmware/svnversion.h +++ b/firmware/svnversion.h @@ -1,12 +1,12 @@ // This file was generated by Version2Header -// Wed Sep 11 18:20:00 EDT 2019 +// Mon Oct 14 12:58:12 EDT 2019 #ifndef GIT_HASH -#define GIT_HASH "33d13a78fc6eb5ba5c56835ab17b7ec1de34ed69" +#define GIT_HASH "7bb8bb7f140d6d96962da164262b42e2ed3f4807" #endif #ifndef VCS_VERSION -#define VCS_VERSION "19879" +#define VCS_VERSION "20053" #endif From 1f4a06e9934c0a5ff7fb14166510cc24b969f567 Mon Sep 17 00:00:00 2001 From: rusefi Date: Mon, 14 Oct 2019 13:10:19 -0400 Subject: [PATCH 06/10] preparing for #974 code comments & refactoring --- simulator/simulator/rusEfiFunctionalTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simulator/simulator/rusEfiFunctionalTest.cpp b/simulator/simulator/rusEfiFunctionalTest.cpp index 349f1e80cf..091e0b77dc 100644 --- a/simulator/simulator/rusEfiFunctionalTest.cpp +++ b/simulator/simulator/rusEfiFunctionalTest.cpp @@ -120,7 +120,7 @@ void rusEfiFunctionalTest(void) { initTriggerCentral(&sharedLogger); initTriggerEmulator(&sharedLogger PASS_ENGINE_PARAMETER_SUFFIX); #if EFI_MAP_AVERAGING - initMapAveraging(&sharedLogger, engine); + initMapAveraging(&sharedLogger PASS_ENGINE_PARAMETER_SUFFIX); #endif /* EFI_MAP_AVERAGING */ initMainEventListener(&sharedLogger PASS_ENGINE_PARAMETER_SUFFIX); From 5a4c7b38cd70557eaeac1f64bea6d436611a0092 Mon Sep 17 00:00:00 2001 From: rusefi Date: Mon, 14 Oct 2019 16:04:28 -0400 Subject: [PATCH 07/10] preparing for #974 code comments & refactoring --- firmware/controllers/algo/engine.cpp | 4 ---- firmware/controllers/map_averaging.cpp | 5 +---- unit_tests/global.h | 4 ++++ 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/firmware/controllers/algo/engine.cpp b/firmware/controllers/algo/engine.cpp index 7d14c191bc..41f0e44b02 100644 --- a/firmware/controllers/algo/engine.cpp +++ b/firmware/controllers/algo/engine.cpp @@ -53,10 +53,8 @@ FsioState::FsioState() { void Engine::eInitializeTriggerShape(Logging *logger DECLARE_ENGINE_PARAMETER_SUFFIX) { #if EFI_ENGINE_CONTROL && EFI_SHAFT_POSITION_INPUT -#if !EFI_UNIT_TEST // we have a confusing threading model so some synchronization would not hurt bool alreadyLocked = lockAnyContext(); -#endif /* EFI_UNIT_TEST */ TRIGGER_SHAPE(initializeTriggerShape(logger, engineConfiguration->ambiguousOperationMode, @@ -86,11 +84,9 @@ void Engine::eInitializeTriggerShape(Logging *logger DECLARE_ENGINE_PARAMETER_SU engine->engineCycleEventCount = TRIGGER_SHAPE(getLength()); } -#if !EFI_UNIT_TEST if (!alreadyLocked) { unlockAnyContext(); } -#endif /* EFI_UNIT_TEST */ if (!TRIGGER_SHAPE(shapeDefinitionError)) { prepareOutputSignals(PASS_ENGINE_PARAMETER_SIGNATURE); diff --git a/firmware/controllers/map_averaging.cpp b/firmware/controllers/map_averaging.cpp index 3a6cda0e5a..446373ca7a 100644 --- a/firmware/controllers/map_averaging.cpp +++ b/firmware/controllers/map_averaging.cpp @@ -110,18 +110,15 @@ static bool isAveraging = false; static void startAveraging(void *arg) { (void) arg; efiAssertVoid(CUSTOM_ERR_6649, getCurrentRemainingStack() > 128, "lowstck#9"); -#if !EFI_UNIT_TEST + bool wasLocked = lockAnyContext(); -#endif // with locking we would have a consistent state mapAdcAccumulator = 0; mapMeasurementsCounter = 0; isAveraging = true; -#if !EFI_UNIT_TEST if (!wasLocked) { unlockAnyContext(); } -#endif /* !EFI_UNIT_TEST */ mapAveragingPin.setHigh(); } diff --git a/unit_tests/global.h b/unit_tests/global.h index eaaf6a0b88..312f4eabb2 100644 --- a/unit_tests/global.h +++ b/unit_tests/global.h @@ -100,4 +100,8 @@ void print(const char *fmt, ...); #define CONFIG_PARAM(x) (x) +#define lockAnyContext() false + +#define unlockAnyContext() {} + #endif /* GLOBAL_H_ */ From 6629b90a7f578fbdce8a79d02c5dd05448b99bad Mon Sep 17 00:00:00 2001 From: rusefi Date: Mon, 14 Oct 2019 18:32:16 -0400 Subject: [PATCH 08/10] preparing for #974 code comments & refactoring --- firmware/controllers/engine_controller.cpp | 12 ++++++------ firmware/controllers/map_averaging.cpp | 2 +- unit_tests/efifeatures.h | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/firmware/controllers/engine_controller.cpp b/firmware/controllers/engine_controller.cpp index c5c35ca481..cbe23b3ee0 100644 --- a/firmware/controllers/engine_controller.cpp +++ b/firmware/controllers/engine_controller.cpp @@ -113,6 +113,12 @@ void mostCommonInitEngineController(Logging *sharedLogger DECLARE_ENGINE_PARAMET initElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE); #endif /* EFI_ELECTRONIC_THROTTLE_BODY */ +#if EFI_MAP_AVERAGING + if (engineConfiguration->isMapAveragingEnabled) { + initMapAveraging(sharedLogger PASS_ENGINE_PARAMETER_SUFFIX); + } +#endif /* EFI_MAP_AVERAGING */ + } EXTERN_ENGINE; @@ -772,12 +778,6 @@ void initEngineContoller(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX) initMalfunctionIndicator(); #endif /* EFI_MALFUNCTION_INDICATOR */ -#if EFI_MAP_AVERAGING - if (engineConfiguration->isMapAveragingEnabled) { - initMapAveraging(sharedLogger PASS_ENGINE_PARAMETER_SUFFIX); - } -#endif /* EFI_MAP_AVERAGING */ - initEgoAveraging(PASS_ENGINE_PARAMETER_SIGNATURE); #if EFI_ENGINE_CONTROL && EFI_SHAFT_POSITION_INPUT diff --git a/firmware/controllers/map_averaging.cpp b/firmware/controllers/map_averaging.cpp index 446373ca7a..3367676809 100644 --- a/firmware/controllers/map_averaging.cpp +++ b/firmware/controllers/map_averaging.cpp @@ -344,7 +344,7 @@ float getMap(void) { } #endif /* EFI_PROD_CODE */ -void initMapAveraging(Logging *sharedLogger DECLARE_CONFIG_PARAMETER_SUFFIX) { +void initMapAveraging(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX) { logger = sharedLogger; // startTimer[0].name = "map start0"; diff --git a/unit_tests/efifeatures.h b/unit_tests/efifeatures.h index 64a8f46500..ef90167e67 100644 --- a/unit_tests/efifeatures.h +++ b/unit_tests/efifeatures.h @@ -64,6 +64,6 @@ #define EFI_BOARD_TEST FALSE #define EFI_JOYSTICK FALSE -#define EFI_MAP_AVERAGING FALSE +#define EFI_MAP_AVERAGING TRUE #endif /* EFIFEATURES_H_ */ From dda7f4d343b596a5716d4877999c41793191cce9 Mon Sep 17 00:00:00 2001 From: rusefi Date: Tue, 15 Oct 2019 01:27:19 -0400 Subject: [PATCH 09/10] preparing for #974 code comments & refactoring --- firmware/controllers/engine_controller.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/firmware/controllers/engine_controller.cpp b/firmware/controllers/engine_controller.cpp index cbe23b3ee0..bbb3390eb9 100644 --- a/firmware/controllers/engine_controller.cpp +++ b/firmware/controllers/engine_controller.cpp @@ -91,6 +91,8 @@ #include "cj125.h" #endif /* EFI_CJ125 */ +EXTERN_ENGINE; + // this method is used by real firmware and simulator and unit test void mostCommonInitEngineController(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX) { #if !EFI_UNIT_TEST From d0596388e6200fd7cba5014bf739b9ca5a64524b Mon Sep 17 00:00:00 2001 From: rusefi Date: Tue, 15 Oct 2019 02:06:15 -0400 Subject: [PATCH 10/10] preparing for #961 better code style --- firmware/controllers/algo/fuel_math.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/firmware/controllers/algo/fuel_math.cpp b/firmware/controllers/algo/fuel_math.cpp index d6b2633120..61b8f1691b 100644 --- a/firmware/controllers/algo/fuel_math.cpp +++ b/firmware/controllers/algo/fuel_math.cpp @@ -72,12 +72,14 @@ DISPLAY(DISPLAY_IF(isCrankingState)) floatms_t getCrankingFuel3(float coolantTem DISPLAY_SENSOR(TPS); DISPLAY_TEXT(eol); - DISPLAY_TEXT(Cranking_fuel); - floatms_t crankingFuel = engine->engineState.DISPLAY_PREFIX(cranking).DISPLAY_FIELD(fuel) = baseCrankingFuel + floatms_t crankingFuel = baseCrankingFuel * engine->engineState.cranking.durationCoefficient * engine->engineState.cranking.coolantTemperatureCoefficient * engine->engineState.cranking.tpsCoefficient; + DISPLAY_TEXT(Cranking_fuel); + engine->engineState.DISPLAY_PREFIX(cranking).DISPLAY_FIELD(fuel) = crankingFuel; + if (crankingFuel <= 0) { warning(CUSTOM_ERR_ZERO_CRANKING_FUEL, "Cranking fuel value %f", crankingFuel); }