diff --git a/firmware/controllers/engine_controller.cpp b/firmware/controllers/engine_controller.cpp index a0e506b212..478fa9fbd7 100644 --- a/firmware/controllers/engine_controller.cpp +++ b/firmware/controllers/engine_controller.cpp @@ -297,7 +297,10 @@ static void periodicSlowCallback(Engine *engine) { /** * Update engine RPM state if needed (check timeouts). */ - engine->rpmCalculator.checkIfSpinning(PASS_ENGINE_PARAMETER_SIGNATURE); + bool isSpinning = engine->rpmCalculator.checkIfSpinning(PASS_ENGINE_PARAMETER_SIGNATURE); + if (!isSpinning) { + engine->rpmCalculator.setStopSpinning(PASS_ENGINE_PARAMETER_SIGNATURE); + } if (engine->rpmCalculator.isStopped(PASS_ENGINE_PARAMETER_SIGNATURE)) { #if EFI_INTERNAL_FLASH || defined(__DOXYGEN__) @@ -750,7 +753,7 @@ void initEngineContoller(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX) // help to notice when RAM usage goes up - if a code change adds to RAM usage these variables would fail // linking process which is the way to raise the alarm static char UNUSED_RAM_SIZE[10200]; -static char UNUSED_CCM_SIZE[7100] CCM_OPTIONAL; +static char UNUSED_CCM_SIZE[7000] CCM_OPTIONAL; /** * See also VCS_VERSION @@ -765,5 +768,5 @@ int getRusEfiVersion(void) { if (initBootloader() != 0) return 123; #endif /* EFI_BOOTLOADER_INCLUDE_CODE */ - return 20190120; + return 20190124; } diff --git a/firmware/controllers/trigger/rpm_calculator.cpp b/firmware/controllers/trigger/rpm_calculator.cpp index 20686d3d59..6d9a4cda43 100644 --- a/firmware/controllers/trigger/rpm_calculator.cpp +++ b/firmware/controllers/trigger/rpm_calculator.cpp @@ -88,7 +88,6 @@ bool RpmCalculator::isRunning(DECLARE_ENGINE_PARAMETER_SIGNATURE) { bool RpmCalculator::checkIfSpinning(DECLARE_ENGINE_PARAMETER_SIGNATURE) { efitick_t nowNt = getTimeNowNt(); if (ENGINE(needToStopEngine(nowNt))) { - setStopped(PASS_ENGINE_PARAMETER_SIGNATURE); return false; } @@ -102,7 +101,6 @@ bool RpmCalculator::checkIfSpinning(DECLARE_ENGINE_PARAMETER_SIGNATURE) { */ bool noTriggerEventsForTooLong = nowNt - engine->triggerCentral.previousShaftEventTimeNt >= US2NT(US_PER_SECOND_LL); if (noRpmEventsForTooLong || noTriggerEventsForTooLong) { - setStopSpinning(PASS_ENGINE_PARAMETER_SIGNATURE); return false; } @@ -194,6 +192,7 @@ void RpmCalculator::setSpinningUp(efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFI // Only a completely stopped and non-spinning engine can enter the spinning-up state. if (isStopped(PASS_ENGINE_PARAMETER_SIGNATURE) && !isSpinning) { state = SPINNING_UP; + engine->triggerCentral.triggerState.spinningEventIndex = 0; isSpinning = true; } // update variables needed by early instant RPM calc. @@ -238,8 +237,6 @@ void rpmShaftPositionCallback(trigger_event_e ckpSignalType, if (index == 0) { ENGINE(m.beforeRpmCb) = GET_TIMESTAMP(); - // WAT? we are here in case of trigger sync. why are we checking (incorrectly) - // if engine is spinning and STOPPING it (incorrectly since 'lastRpmEventTimeNt' was not assigned yet?) bool hadRpmRecently = rpmState->checkIfSpinning(PASS_ENGINE_PARAMETER_SIGNATURE); if (hadRpmRecently) { @@ -275,8 +272,11 @@ void rpmShaftPositionCallback(trigger_event_e ckpSignalType, } #endif - // Replace 'normal' RPM with instant RPM for the initial spin-up period if (rpmState->isSpinningUp(PASS_ENGINE_PARAMETER_SIGNATURE)) { + // we are here only once trigger is synchronized for the first time + // while transitioning from 'spinning' to 'running' + // Replace 'normal' RPM with instant RPM for the initial spin-up period + engine->triggerCentral.triggerState.movePreSynchTimestamps(PASS_ENGINE_PARAMETER_SIGNATURE); int prevIndex; int iRpm = engine->triggerCentral.triggerState.calculateInstantRpm(&prevIndex, nowNt PASS_ENGINE_PARAMETER_SUFFIX); // validate instant RPM - we shouldn't skip the cranking state diff --git a/firmware/controllers/trigger/trigger_decoder.cpp b/firmware/controllers/trigger/trigger_decoder.cpp index 7f3c296df9..f153ca3a8d 100644 --- a/firmware/controllers/trigger/trigger_decoder.cpp +++ b/firmware/controllers/trigger/trigger_decoder.cpp @@ -126,17 +126,21 @@ int TriggerState::getTotalRevolutionCounter() const { TriggerStateWithRunningStatistics::TriggerStateWithRunningStatistics() : //https://en.cppreference.com/w/cpp/language/zero_initialization - instantRpmValue() + timeOfLastEvent(), instantRpmValue() { - // avoid ill-defined instant RPM when the data is not gathered yet - efitime_t nowNt = getTimeNowNt(); - for (int i = 0; i < PWM_PHASE_MAX_COUNT; i++) { - timeOfLastEvent[i] = nowNt; +} + +void TriggerStateWithRunningStatistics::movePreSynchTimestamps(DECLARE_ENGINE_PARAMETER_SIGNATURE) { + // here we take timestamps of events which happened prior to synchronization and place them + // at appropriate locations + for (int i = 0; i < spinningEventIndex;i++) { + timeOfLastEvent[getTriggerSize() - i] = spinningEvents[i]; } } float TriggerStateWithRunningStatistics::calculateInstantRpm(int *prevIndex, efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) { int current_index = currentCycle.current_index; // local copy so that noone changes the value on us + timeOfLastEvent[current_index] = nowNt; /** * Here we calculate RPM based on last 90 degrees */ @@ -149,7 +153,13 @@ float TriggerStateWithRunningStatistics::calculateInstantRpm(int *prevIndex, efi // now let's get precise angle for that event angle_t prevIndexAngle = TRIGGER_SHAPE(eventAngles[*prevIndex]); - uint32_t time = nowNt - timeOfLastEvent[*prevIndex]; + efitick_t time90ago = timeOfLastEvent[*prevIndex]; + if (time90ago == 0) { + return prevInstantRpmValue; + } + // we are OK to subtract 32 bit value from more precise 64 bit since the result would 32 bit which is + // OK for small time differences like this one + uint32_t time = nowNt - time90ago; angle_t angleDiff = currentAngle - prevIndexAngle; // todo: angle diff should be pre-calculated fixAngle(angleDiff, "angleDiff", CUSTOM_ERR_6561); @@ -160,7 +170,6 @@ float TriggerStateWithRunningStatistics::calculateInstantRpm(int *prevIndex, efi float instantRpm = (60000000.0 / 360 * US_TO_NT_MULTIPLIER) * angleDiff / time; instantRpmValue[current_index] = instantRpm; - timeOfLastEvent[current_index] = nowNt; // This fixes early RPM instability based on incomplete data if (instantRpm < RPM_LOW_THRESHOLD) @@ -171,7 +180,17 @@ float TriggerStateWithRunningStatistics::calculateInstantRpm(int *prevIndex, efi } void TriggerStateWithRunningStatistics::setLastEventTimeForInstantRpm(efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) { - timeOfLastEvent[currentCycle.current_index] = nowNt; + if (shaft_is_synchronized) { + return; + } + // here we remember tooth timestamps which happen prior to synchronization + if (spinningEventIndex >= PRE_SYNC_EVENTS) { + // too many events while trying to find synchronization point + // todo: better implementation would be to shift here or use cyclic buffer so that we keep last + // 'PRE_SYNC_EVENTS' events + return; + } + spinningEvents[spinningEventIndex++] = nowNt; } void TriggerStateWithRunningStatistics::runtimeStatistics(efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) { diff --git a/firmware/controllers/trigger/trigger_decoder.h b/firmware/controllers/trigger/trigger_decoder.h index 6feff30462..8cba10ecca 100644 --- a/firmware/controllers/trigger/trigger_decoder.h +++ b/firmware/controllers/trigger/trigger_decoder.h @@ -118,6 +118,9 @@ private: }; +// we only need 90 degrees of events so /4 or maybe even /8 should work? +#define PRE_SYNC_EVENTS (PWM_PHASE_MAX_COUNT / 4) + /** * the reason for sub-class is simply to save RAM but not having statisics in the trigger initialization instance @@ -130,6 +133,10 @@ public: * timestamp of each trigger wheel tooth */ uint32_t timeOfLastEvent[PWM_PHASE_MAX_COUNT]; + + int spinningEventIndex = 0; + // todo: change the implementation to reuse 'timeOfLastEvent' + uint32_t spinningEvents[PRE_SYNC_EVENTS]; /** * instant RPM calculated at this trigger wheel tooth */ @@ -138,6 +145,7 @@ public: * Stores last non-zero instant RPM value to fix early instability */ float prevInstantRpmValue = 0; + void movePreSynchTimestamps(DECLARE_ENGINE_PARAMETER_SIGNATURE); float calculateInstantRpm(int *prevIndex, efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX); virtual void runtimeStatistics(efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX); /** diff --git a/unit_tests/tests/test_fasterEngineSpinningUp.cpp b/unit_tests/tests/test_fasterEngineSpinningUp.cpp index fbb360ccc0..75cd9ef111 100644 --- a/unit_tests/tests/test_fasterEngineSpinningUp.cpp +++ b/unit_tests/tests/test_fasterEngineSpinningUp.cpp @@ -14,9 +14,6 @@ TEST(cranking, testFasterEngineSpinningUp) { // turn on FasterEngineSpinUp mode engineConfiguration->bc.isFasterEngineSpinUpEnabled = true; - eth.moveTimeForwardMs(1000 /*ms*/); - eth.firePrimaryTriggerRise(); - // set ignition mode engineConfiguration->ignitionMode = IM_INDIVIDUAL_COILS; // set cranking threshold (used below) @@ -24,11 +21,16 @@ TEST(cranking, testFasterEngineSpinningUp) { // set sequential injection mode to test auto-change to simultaneous when spinning-up setupSimpleTestEngineWithMafAndTT_ONE_trigger(ð, IM_SEQUENTIAL); + ASSERT_EQ(IM_INDIVIDUAL_COILS, getCurrentIgnitionMode(PASS_ENGINE_PARAMETER_SIGNATURE)); + + eth.moveTimeForwardMs(1000 /*ms*/); + eth.firePrimaryTriggerRise(); + // check if it's true ASSERT_EQ(IM_SEQUENTIAL, engine->getCurrentInjectionMode(PASS_ENGINE_PARAMETER_SIGNATURE)); - ASSERT_EQ(IM_INDIVIDUAL_COILS, getCurrentIgnitionMode(PASS_ENGINE_PARAMETER_SIGNATURE)); + ASSERT_EQ(IM_WASTED_SPARK, getCurrentIgnitionMode(PASS_ENGINE_PARAMETER_SIGNATURE)); // check if the engine has the right state - ASSERT_EQ(STOPPED, engine->rpmCalculator.getState()); + ASSERT_EQ(SPINNING_UP, engine->rpmCalculator.getState()); // check RPM ASSERT_EQ( 0, GET_RPM()) << "RPM=0"; // the queue should be empty, no trigger events yet @@ -98,7 +100,7 @@ TEST(cranking, testFasterEngineSpinningUp) { eth.assertEvent5(&engine->executor, "inj end#3", 1, (void*)seTurnPinLow, timeStartUs, MS2US(60) + 27974 + 3000); } -static void doTestFasterEngineSpinningUp60_2(int startUpDelayMs, int expectedRpm) { +static void doTestFasterEngineSpinningUp60_2(int startUpDelayMs, int rpm1, int expectedRpm) { WITH_ENGINE_TEST_HELPER(TEST_ENGINE); // turn on FasterEngineSpinUp mode engineConfiguration->bc.isFasterEngineSpinUpEnabled = true; @@ -106,15 +108,17 @@ static void doTestFasterEngineSpinningUp60_2(int startUpDelayMs, int expectedRpm setupSimpleTestEngineWithMaf(ð, IM_SEQUENTIAL, TT_TOOTHED_WHEEL_60_2); eth.moveTimeForwardMs(startUpDelayMs); + // fire 30 tooth rise/fall signals eth.fireTriggerEvents2(30 /* count */, 1 /*ms*/); - eth.fireTriggerEvents2(1, 4); - EXPECT_EQ(expectedRpm, GET_RPM()) << "test RPM with " + std::to_string(startUpDelayMs) + " startUpDelayMs"; + // now fire missed tooth rise/fall + eth.fireRise(4 /*ms*/); + EXPECT_EQ(rpm1, GET_RPM()) << "test RPM: After rise " + std::to_string(startUpDelayMs); + eth.fireFall(4 /*ms*/); + EXPECT_EQ(expectedRpm, GET_RPM()) << "test RPM: with " + std::to_string(startUpDelayMs) + " startUpDelayMs"; } TEST(cranking, testFasterEngineSpinningUp60_2) { - // I do not get it. Startup delay is affecting instance RPM? - // todo: is this feature implementation issue or test framework issue? - doTestFasterEngineSpinningUp60_2(0, 220); - doTestFasterEngineSpinningUp60_2(100, 89); - doTestFasterEngineSpinningUp60_2(1000, 0); + doTestFasterEngineSpinningUp60_2(0, 288, 263); + doTestFasterEngineSpinningUp60_2(100, 288, 263); + doTestFasterEngineSpinningUp60_2(1000, 288, 263); }