diff --git a/firmware/controllers/trigger/trigger_decoder.cpp b/firmware/controllers/trigger/trigger_decoder.cpp index e50964b525..be614f2d6a 100644 --- a/firmware/controllers/trigger/trigger_decoder.cpp +++ b/firmware/controllers/trigger/trigger_decoder.cpp @@ -239,7 +239,7 @@ void PrimaryTriggerDecoder::resetInstantRpm() { void PrimaryTriggerDecoder::movePreSynchTimestamps() { // here we take timestamps of events which happened prior to synchronization and place them // at appropriate locations - auto triggerSize = getTriggerSize(); + auto triggerSize = getTriggerCentral()->triggerShape.getLength(); int eventsToCopy = minI(spinningEventIndex, triggerSize); @@ -283,8 +283,10 @@ float PrimaryTriggerDecoder::calculateInstantRpm( 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 + + // It's OK to truncate from 64b to 32b, ARM with single precision FPU uses an expensive + // software function to convert 64b int -> float, while 32b int -> float is very cheap hardware conversion + // The difference is guaranteed to be short (it's 90 degrees of engine rotation!), so it won't overflow. uint32_t time = nowNt - time90ago; angle_t angleDiff = currentAngle - prevIndexAngle; @@ -316,13 +318,18 @@ void PrimaryTriggerDecoder::setLastEventTimeForInstantRpm(efitick_t nowNt) { return; } // here we remember tooth timestamps which happen prior to synchronization - if (spinningEventIndex >= PRE_SYNC_EVENTS) { + if (spinningEventIndex >= efi::size(spinningEvents)) { // 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; + + spinningEvents[spinningEventIndex] = nowNt; + + // If we are using only rising edges, we never write in to the odd-index slots that + // would be used by falling edges + spinningEventIndex += engineConfiguration->useOnlyRisingEdgeForTrigger ? 2 : 1; } void PrimaryTriggerDecoder::updateInstantRpm( diff --git a/firmware/controllers/trigger/trigger_decoder.h b/firmware/controllers/trigger/trigger_decoder.h index 9548e65582..4aeb554f4b 100644 --- a/firmware/controllers/trigger/trigger_decoder.h +++ b/firmware/controllers/trigger/trigger_decoder.h @@ -184,10 +184,6 @@ private: Timer m_timeSinceDecodeError; }; -// 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 statistics in the trigger initialization instance */ @@ -213,8 +209,10 @@ public: uint32_t timeOfLastEvent[PWM_PHASE_MAX_COUNT]; int spinningEventIndex = 0; + + // we might need up to one full trigger cycle of events - which on 60-2 means storage for ~120 // todo: change the implementation to reuse 'timeOfLastEvent' - uint32_t spinningEvents[PRE_SYNC_EVENTS]; + uint32_t spinningEvents[120]; /** * instant RPM calculated at this trigger wheel tooth */ diff --git a/unit_tests/tests/trigger/test_fasterEngineSpinningUp.cpp b/unit_tests/tests/trigger/test_fasterEngineSpinningUp.cpp index 0c9105c822..9b3539fc29 100644 --- a/unit_tests/tests/trigger/test_fasterEngineSpinningUp.cpp +++ b/unit_tests/tests/trigger/test_fasterEngineSpinningUp.cpp @@ -119,8 +119,34 @@ static void doTestFasterEngineSpinningUp60_2(int startUpDelayMs, int rpm1, int e EXPECT_EQ(expectedRpm, round(Sensor::getOrZero(SensorType::Rpm))) << "test RPM: with " << std::to_string(startUpDelayMs) << " startUpDelayMs"; } +static void doTestFasterEngineSpinningUp60_2_onlyRise(int startUpDelayMs, int rpm1, int expectedRpm) { + EngineTestHelper eth(TEST_ENGINE); + // turn on FasterEngineSpinUp mode + engineConfiguration->isFasterEngineSpinUpEnabled = true; + engineConfiguration->useOnlyRisingEdgeForTrigger = true; + + setupSimpleTestEngineWithMaf(ð, IM_SEQUENTIAL, TT_TOOTHED_WHEEL_60_2); + eth.moveTimeForwardMs(startUpDelayMs); + + // fire 30 tooth rise/fall signals + eth.fireTriggerEvents2(30 /* count */, 1 /*ms*/); + // now fire missed tooth rise/fall + eth.fireRise(5 /*ms*/); + EXPECT_EQ(rpm1, round(Sensor::getOrZero(SensorType::Rpm))); + + eth.fireFall(1); + eth.fireTriggerEvents2(30, 1); + + // After some more regular teeth, instant RPM is still correct + EXPECT_EQ(rpm1, round(Sensor::getOrZero(SensorType::Rpm))); +} + TEST(cranking, testFasterEngineSpinningUp60_2) { doTestFasterEngineSpinningUp60_2(0, 1000, 1000); doTestFasterEngineSpinningUp60_2(100, 1000, 1000); doTestFasterEngineSpinningUp60_2(1000, 1000, 1000); + + doTestFasterEngineSpinningUp60_2_onlyRise(0, 1000, 1000); + doTestFasterEngineSpinningUp60_2_onlyRise(100, 1000, 1000); + doTestFasterEngineSpinningUp60_2_onlyRise(1000, 1000, 1000); }